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

fix(providers): warn if GTIN lookup found multiple results for Spotify / Tidal #29

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

phw
Copy link
Collaborator

@phw phw commented Jun 11, 2024

For Spotify and Tidal show a warning with a list of URLs if a GTIN lookup found more than one match.

Adapt the existing logic already implemented in iTunes and generalize it by adding a ReleaseLookup.warnMultipleResults helper.

Notes:

  • Deezer seems to be technically limited to one result
  • I don't know about Beatport. Maybe something similar should be done there

This should resolve #25

@phw
Copy link
Collaborator Author

phw commented Jun 11, 2024

Some thoughts about this in general: In the long term it would be good if there could be some handling of multiple results. Something like this is already mentioned in the roadmap (#6) "Allow providers to return multiple releases, i.e. different variants (e.g. for Bandcamp)".

While testing I had one release that failed the lookup for iTunes due to mismatched track length. However, iTunes found multiple results and one that would have matched the tracklists from other providers was among them. I unfortunately could not find the example again in my notes, but if I do I'll post it.

For the barcode collision example 635669065024 it would also help as currently all of iTunes, Tidal and Spotify return one random result of the following two albums:

https://music.apple.com/gb/album/barcheinu/1526351623
https://music.apple.com/gb/album/ybc-iii-shabichi/376544473

No matter which one you originally wanted to lookup you most likely end up with a mixed result on Harmony.

@phw
Copy link
Collaborator Author

phw commented Jun 11, 2024

@kellnerd More an experiment, but what do you think about a45f324 ?

@kellnerd kellnerd added bug Something isn't working provider Metadata provider labels Jun 11, 2024
Copy link
Owner

@kellnerd kellnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that was fast!

providers/Spotify/mod.ts Outdated Show resolved Hide resolved
providers/Tidal/mod.ts Outdated Show resolved Hide resolved
providers/base.ts Outdated Show resolved Hide resolved
@kellnerd kellnerd merged commit 3ac4077 into kellnerd:main Jun 11, 2024
2 checks passed
@phw phw deleted the warn-multi-results branch June 11, 2024 13:18
@kellnerd
Copy link
Owner

While testing I had one release that failed the lookup for iTunes due to mismatched track length. However, iTunes found multiple results and one that would have matched the tracklists from other providers was among them. I unfortunately could not find the example again in my notes, but if I do I'll post it.

I have found the following example in my notes, taken from the server logs:
iTunes returns two totally unrelated releases by different artists: https://harmony.pulsewidth.org.uk/release?gtin=781676664221&itunes=&region=GB&ts=1717878974
Has a different number of tracks also, so it is not your example, but interesting nevertheless.

@atj
Copy link
Collaborator

atj commented Jun 11, 2024

  • I don't know about Beatport. Maybe something similar should be done there

Just to clarify, the Beatport provider explicitly returns the first release from the search results with a matching barcode:

const release = result.state.data.releases.data.find((r) => r.upc === gtin);

I don't believe I've ever encountered a situation where a GTIN search has returned multiple releases, but the code will either return a single release or undefined regardless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working provider Metadata provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Barcode collision
3 participants