Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for instance remove from collection folder #161

Open
JOJ0 opened this issue Nov 11, 2024 · 1 comment
Open

Support for instance remove from collection folder #161

JOJ0 opened this issue Nov 11, 2024 · 1 comment

Comments

@JOJ0
Copy link
Contributor

JOJ0 commented Nov 11, 2024

@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 #160 (reply in thread)

@JOJ0 JOJ0 changed the title The 3rd described way appears to also delete every instance in every folder. I used the following using requests to accomplish what I was after (having already established username from authentication and other necessary variables from user input): Support for instance remove from collection folder Nov 11, 2024
@JOJ0
Copy link
Contributor Author

JOJ0 commented Nov 11, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant