Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

Added 3 more api-requests for folder management #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added 3 more api-requests for folder management #37

wants to merge 1 commit into from

Conversation

psyray
Copy link

@psyray psyray commented Apr 9, 2016

Hi,

I added 3 API request that I use in my project to get Folders of a user collection, Folder properties of a given folder and Releases contained in a given folder

Enjoy

Hi,

I added 3 API request that I use in my project to get Folders of a user collection, Folder properties of a given folder and Releases contained in a given folder
@ricbra
Copy link
Owner

ricbra commented Apr 11, 2016

Hi @psyray

Thanks for your PR. Could you also add unit tests for these new calls? Take a look at the other tests for an example of how to do this.

@psyray
Copy link
Author

psyray commented Apr 11, 2016

Hi,

To use these API requests, user need to be authenticated by OAuth.
To enable Oauth, HWI Oauth Bundle is needed, and an application must be registered in Discogs.

How can I manage this connection with your bundle ?
Maybe we could bypass the auth by creating an account with a public collection, and then we could have access to folders.
You are the owner of this repo, so I let you create it ;)

@ricbra
Copy link
Owner

ricbra commented Apr 12, 2016

Hi @psyray

In my applications I use HWIOauthBundle for the OAuth authentication (hence I even wrote the Discogs provider for the HWIOauthBundle 😉 ). I don't want any OAuth stuff in the DiscogsBundle because it has nothing to do with Discogs. It is a separate concern. Just fetch the OAuth tokens and stuff from OauthBundle and pass it in your service configuration for the Discogs client using a factory.

For the unit tests required for this PR, you should somehow log the real responses you receive from Discogs. These need to be used as mock responses for the tests. Of course, replace sensitive content in these responses with placeholders.

I hope this answers your question.

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

Successfully merging this pull request may close these issues.

2 participants