Repository: joalla/discogs_client

Issue:
Support for instance remove from collection folder
@captain-cronch discovered that the remove_release() method we have

https://github.com/joalla/discogs_client/blob/b73595dbf1c4f4677401b5062486613a0c8a08ad/discogs_client/models.py#L760:764

is misleading - it removes from the collection entirely instead of only from a collection folder.

Further investigation reveals that the provided endpoint of the Discogs API is slightly confusing already, but after reading carefully it's somewhat documented:

https://www.discogs.com/developers#page:user-collection,header:user-collection-delete-instance-from-folder


Currently python3-discogs-client supports sending a DELETE request but not a POST request to that endpoint.

Find below, the original message including a well done implementation suggestion already.

I'd like to open discussion of that suggestion and ask for further ideas especially on how the method should be named and how we should document it.

## Original post (slightly edited)

I used the following using requests to accomplish what I was after:

```
url = f"https://api.discogs.com/users/{username}/collection/folders/{folder_id}/releases/{release_id}/instances/{instance_id}"

data = {'folder_id': target_folder_id}

response = requests.post(url, auth=auth, json=data)
```

I looked through models.py and believe there's not currently a function that accomplishes this (perhaps I'm not passing optional arguments somewhere though). In the class CollectionFolder, a function called move_release would be useful and probably look something like this:

```
def move_release(self, instance, target_folder_id):
    if not isinstance(instance, CollectionItemInstance):
        raise TypeError('instance must be of type CollectionItemInstance')
    instance_url = self.fetch('resource_url') + '/releases/{0}/instances/{1}'.format(instance.id, instance.instance_id)
    data = {'folder_id': target_folder_id}
    self.client._post(instance_url, data)
```

As mentioned earlier, I wanted to remove a release from a folder, but to accomplish that it must be moved to a different folder (such as the uncategorized folder). This function also allows the passing of a different target_folder_id and would result in an existing instance being moved from its current folder to the target folder.

_Originally posted by @captain-cronch in https://github.com/joalla/discogs_client/discussions/160#discussioncomment-11181204_

Hints from issue comments:
@JOJ0: @captain-cronch I'd like to encourage you, since your suggestion seems pretty good to me already, to open a draft PR and we work from there? That would help us a lot! We certainly will assist with setting up a dev environment, answer questions, and so on.

Solution patch:
diff --git a/.github/workflows/discogs_client-build.yml b/.github/workflows/discogs_client-build.yml
index d6b9185..c1233a1 100644
--- a/.github/workflows/discogs_client-build.yml
+++ b/.github/workflows/discogs_client-build.yml
@@ -11,7 +11,7 @@ jobs:
     runs-on: ubuntu-20.04
     strategy:
       matrix:
-        python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
+        python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python-version }}
diff --git a/discogs_client/models.py b/discogs_client/models.py
index 74a2c6f..437ce31 100644
--- a/discogs_client/models.py
+++ b/discogs_client/models.py
@@ -745,7 +745,7 @@ class CollectionFolder(PrimaryAPIObject):
     count = SimpleField()  #:
 
     def __init__(self, client, dict_):
-        super(CollectionFolder, self).__init__(client, dict_)
+        super().__init__(client, dict_)
 
     @property
     def releases(self):
@@ -754,14 +754,34 @@ class CollectionFolder(PrimaryAPIObject):
 
     def add_release(self, release):
         release_id = release.id if isinstance(release, Release) else release
-        add_release_url = self.fetch('resource_url') + '/releases/{}'.format(release_id)
-        self.client._post(add_release_url, None)
+        resource_url = self.fetch('resource_url')
+        self.client._post(f"{resource_url}/releases/{release_id}", None)
 
     def remove_release(self, instance):
+        """Remove a collection item entirely.
+        """
         if not isinstance(instance, CollectionItemInstance):
             raise TypeError('instance must be of type CollectionItemInstance')
-        instance_url = self.fetch('resource_url') + '/releases/{0}/instances/{1}'.format(instance.id, instance.instance_id)
-        self.client._delete(instance_url)
+        resource_url = self.fetch('resource_url')
+        self.client._delete(f"{resource_url}/releases/{instance.id}/instances/{instance.instance_id}")
+
+    def move_release(self, instance, target_folder_id):
+        """Move a collection item to another folder.
+
+        Moving to folder id 1 moves to the "Uncategorized" folder.
+        """
+        if not isinstance(instance, CollectionItemInstance):
+            raise TypeError('instance must be of type CollectionItemInstance')
+        resource_url = self.fetch('resource_url')
+        self.client._post(
+            f"{resource_url}/releases/{instance.id}/instances/{instance.instance_id}",
+            {"folder_id": target_folder_id},
+        )
+
+    def uncategorize_release(self, instance):
+        """Move a collection item to the "Uncategorized" folder.
+        """
+        self.move_release(instance, 1)
 
     def __repr__(self):
         return '<CollectionFolder {0!r} {1!r}>'.format(self.id, self.name)
diff --git a/docs/source/conf.py b/docs/source/conf.py
index 6f3c6a4..8ee20ab 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -80,7 +80,8 @@ html_css_files = ["p3dc.css"]
 # MyST extenstion configuration
 myst_heading_anchors = 7
 myst_enable_extensions = [
-    "substitution"
+    "substitution",
+    "colon_fence",
 ]
 myst_substitutions = {
   "class": "I'm a **substitution**"
diff --git a/docs/source/fetching_data.md b/docs/source/fetching_data.md
index 348fe89..d6c61c8 100644
--- a/docs/source/fetching_data.md
+++ b/docs/source/fetching_data.md
@@ -231,7 +231,7 @@ me.collection_folders[2].add_release(17392219)
 _{meth}`.add_release` also accepts {class}`.Release` objects_
 
 
-### Removing a Release from a Collection Folder
+### Removing a Release from the Collection
 
 Removing a single release instance identified by its index:
 
@@ -242,6 +242,10 @@ releases = folder.releases
 folder.remove_release(releases[0])
 ```
 
+:::{caution}
+The {meth}`.remove_release` method deletes from the collection entirely. To remove an instance from a folder only, use the {meth}`.uncategorize_release` method.
+:::
+
 To filter out which instance to remove we could also use the attributes of the {class}`.Release` object attached to the {class}`.CollectionItemInstance`:
 
 ```python
@@ -251,16 +255,34 @@ for instance in folder.releases:
         folder.remove_release(instance)
 ```
 
-Another approach to removing instances from collection folders if we know a release ID already would be to make use of the {meth}`.collection_items` method. This way we could delete all the instances from all its containing folders:
+### Removing a Release from a Folder
+
+To remove a release from a collection folder, we need to know the release ID already and  make use of the {meth}`.collection_items` method. This way we could remove all the instances from all its containing folders (uncategorize them):
 
 ```python
 release_instances = me.collection_items(22155985)
 for instance in release_instances:
     folder = me.collection_folders[instance.folder_id]
-    folder.remove_release(instance)
+    folder.uncategorize_release(instance)
+```
+
+### Moving a Release to a Different Folder
+
+To move a release from a collection folder to another one, again we need to know the release ID already and make use of the {meth}`.collection_items` method. We also need to know the ID of the target folder we want to move to (use {attr}`.collection_folders`). We move all the instances from all its containing folders to a specified target folder:
+
+```python
+release_instances = me.collection_items(22155985)
+target_folder = 1
+for folder in me.collection_folders:
+    if folder.name == "My Target Folder":
+        target_folder = folder.id
+
+for instance in release_instances:
+    folder = me.collection_folders[instance.folder_id]
+    folder.move_release(instance, target_folder)
 ```
 
-_{meth}`.remove_release` only accepts {class}`.CollectionItemInstance` objects_
+_{meth}`.remove_release`, {meth}`.move_release` and {meth}`.uncategorize_release` only accept {class}`.CollectionItemInstance` objects_
 
 
 ## Using {meth}`~discogs_client.models.PrimaryAPIObject.fetch` to get other data


Test patch:
diff --git a/discogs_client/tests/test_models.py b/discogs_client/tests/test_models.py
index 20c64e6..cf1bb5c 100644
--- a/discogs_client/tests/test_models.py
+++ b/discogs_client/tests/test_models.py
@@ -1,5 +1,5 @@
 import unittest
-from discogs_client.models import Artist, Release, ListItem, CollectionValue
+from discogs_client.models import Artist, Release, ListItem, CollectionValue, CollectionItemInstance
 from discogs_client.tests import DiscogsClientTestCase
 from discogs_client.exceptions import HTTPError
 
@@ -265,7 +265,6 @@ class ModelsTestCase(DiscogsClientTestCase):
         self.assertEqual(method, 'GET')
         self.assertEqual(url, '/marketplace/listings/150899904')
 
-
     def test_collection(self):
         """Collection folders can be manipulated"""
         # Fetch the users collection folders from the filesystem
@@ -314,6 +313,41 @@ class ModelsTestCase(DiscogsClientTestCase):
         self.assertEqual(method, 'POST')
         self.assertEqual(url, '/users/example/collection/folders/1/releases/1')
 
+    def test_collection_move_release(self):
+        """Collection items can be moved to another folder"""
+        # Fetch the users collection folders from the filesystem
+        u = self.d.user("example")
+        self.assertEqual(u.collection_folders[2].id, 2)
+
+        # Mock expected responses for move_release test using "MemoryFetcher"
+        self.m._fetcher.fetcher.responses = {
+            "/users/example/collection/folders": (b'''
+                {"folders": [
+                    {"resource_url": "/users/example/collection/folders/0", "id": 0, "name": "All"},
+                    {"resource_url": "/users/example/collection/folders/1", "id": 1, "name": "Uncategorized folder"},
+                    {"resource_url": "/users/example/collection/folders/2", "id": 2, "name": "Collection folder 2"}
+                ]}
+            ''', 200),
+            # Mock the response of the POST request to the instance resource URL
+            "/users/example/collection/folders/1/releases/123456/instances/123": (b"", 204),
+        }
+
+        # Bind the user to the MemoryFetcher
+        u.client = self.m
+
+        # Mock a collection item instance
+        instance = CollectionItemInstance(
+            client=u.client,
+            dict_={"id": 123456, "instance_id": 123}
+        )
+        # Perform the move
+        u.collection_folders[1].move_release(instance, 2)
+
+        # Verify
+        method, url, _, _ = self.m._fetcher.last_request
+        self.assertEqual(method, "POST")
+        self.assertEqual(url, "/users/example/collection/folders/1/releases/123456/instances/123")
+
     def test_delete_object(self):
         """Can request DELETE on an APIObject"""
         u = self.d.user('example')