David-Desmaisons / DiscogsClient

Discogs API C# Client
MIT License
42 stars 16 forks source link

Add DiscogRelease properties: lowest_price and num_for_sale #16

Closed blackboxlogic closed 4 years ago

blackboxlogic commented 4 years ago

Adding properties which existed in the json response, but were not in the model. I ran the unit tests and verified that they deserialized and the correct data was in the new properties.

blackboxlogic commented 4 years ago

Thank you for maintaining this project. Please let me know your best guess for when these changes might be released in a new version of your nuget package.

David-Desmaisons commented 4 years ago

Thanks for the PR, could you add a link to an url returning these files or to the oficial documentation menetionaing them?

blackboxlogic commented 4 years ago

Oops, I should have included that! Here is a sample result from Discogs including these two properties:

They are both present in your test cases already:

https://github.com/David-Desmaisons/DiscogsClient/blob/686fde30fee690e5f181928a41fac8a24c59a72f/DiscogsClient.Test/ReleaseDeserializationTest.cs#L13

And here's the documentation.

David-Desmaisons commented 4 years ago

Thanks!

blackboxlogic commented 4 years ago

No good deed goes unpunished. I believe I broke your project and I'm sorry and I would like to fix it. You're latest release 2.7.0 has a problem in client.GetReleaseAsync(int id); during deserialization. Problem When requesting Release ID 249504, it correctly returns a release. When requesting Release ID 2725443, it returns null. Cause This is because 2725443 has "lowest_price": null and my change defined public decimal lowest_price { get; set; } (not nullible) Solution I suggest un-publishing v2.7.0 from nuget.org I suggest changing the type of lowest_price from decimal to decimal? Let me know if you'd like to make the change or if you want me to make another PR I'm not worried about num_for_sale since it seems to return 0 (or more) even when requesting without a token.

David-Desmaisons commented 4 years ago

You can create another no problem