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

Add community details to Release #105

Merged
merged 3 commits into from
Aug 9, 2022
Merged

Conversation

AnssiAhola
Copy link
Contributor

Adds community related fields from GET Release API

closes #104

Usage

release = client.release(15343679)
community_details = release.community

print(community_details)
>>> <CommunityDetails want/have: 34/74>
print (community_details.rating)
>>> <Rating avg: 4.5>
print(community_details.want)
>>> 34
print(community_details.have)
>>> 74
print(community_details.status)
>>> Accepted
print(community_details.data_quality)
>>> Needs Vote
print(community_details.contributors)
>>> [<User 1053297 'ManikRecs'>, <User 1241908 'neurosis76'>, <User 8867757 'silyn1'>, <User 3410038 'evasstra'>]
print(community_details.submitter)
>>> <User 1053297 'ManikRecs'>

@AnssiAhola AnssiAhola requested a review from JOJ0 August 4, 2022 14:32
@AnssiAhola
Copy link
Contributor Author

@JOJ0 Let me know if there's something I should fix/change before merging this.
Long overdue 😅

@JOJ0
Copy link
Contributor

JOJ0 commented Aug 4, 2022

Thanks for getting this rolling again Anssi! I think your idea of summarising under a community object is good. It took me a while though to find out which api endpoints are actually used for the object. We should document this object and where its data is fetched from, to have answers at hand for the kind of question: Are you supporting discogs api endpoint X?

I dont have time for testing atm but think this is save to merge.

If you feel like quickly putting your example above plus api endpoints to a docs chapter, work away. Paul or me might polish it later on :-) Just so we have the necessities in.

@AnssiAhola
Copy link
Contributor Author

Something like this?
supported_api_points.md

Here are the API points currently supported by the client

Basically showing same things that are in the official docs (that we support anyways)

@JOJ0
Copy link
Contributor

JOJ0 commented Aug 6, 2022

No, I think you misunderstood or I didn't explain well what I ment. I was talking about adding a docs chapter about this feature and then put the relation to what API's this particular feature is using right in that chapter.

Your approach of having a general list of "that's what we support" is definitely good for the library user to have it in one place but I am not sure if I want to go down that road. I did that for another project and for that one it's manageable but I am sure it's outdated again already (https://github.com/JOJ0/synadm#implementation-status--commands-list). We would have to watch that constantly or rely on others to keep it updated with every PR. We can think about it yes, but I'm not sure yet...

@JOJ0
Copy link
Contributor

JOJ0 commented Aug 6, 2022

But hey Anssi, maybe I did misunderstand: Is all this actually fetched from the release endpoint only?

This JSON tells me it is: #101 (comment)

@JOJ0
Copy link
Contributor

JOJ0 commented Aug 6, 2022

Yesterday when fiddling around on my mobile and try to read throught api docs and your code I thought this feature uses several of these "sub-endpoints" to fetch things: https://www.discogs.com/developers#page:database,header:database-community-release-rating

But that's not the case I think.

Is that just an alternative way of fetching ev. have's and want' I'm wondering? And by default it's contained in the plain release endpoints result already?

@AnssiAhola
Copy link
Contributor Author

Yes this data is actually part of the Release API response JSON
https://www.discogs.com/developers#page:database,header:database-release-get

and actually has more info than the community release rating endpoint you linked

I'm not sure if this info was added recently to the endpoint or why wasn't it added to the Release object already 🤷

@JOJ0
Copy link
Contributor

JOJ0 commented Aug 9, 2022

Let's merge it. Even played around a little with. I think it's great!

@AnssiAhola AnssiAhola merged commit 07edefc into master Aug 9, 2022
@AnssiAhola AnssiAhola deleted the add_release_community_info branch August 9, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Add community related details to Release
2 participants