kellnerd / harmony

Music Metadata Aggregator and MusicBrainz Importer
MIT License
40 stars 6 forks source link

Allow to use standard provider when linking to Harmony with URL parameter #17

Closed phw closed 3 months ago

phw commented 3 months ago

For integration with external tools it would be convenient to be able to easily link to Harmony with a URL or GTIN. Links like this should be supported:

With URL: https://harmony.pulsewidth.org.uk/release?url=https%3A%2F%2Fgoatgirl.bandcamp.com%2Falbum%2Fbelow-the-waste

With GTIN: https://harmony.pulsewidth.org.uk/release?gtin=191402047554

There are two separate behaviors:

  1. Linking with URL works and triggers a lookup, but all default providers are disabled. To enable a provider an explicit parameter for this provider has to be passed, e.g. deezer=. This is inconvenient for third-party tools, as they need to hardcode all possible providers. It would be better if the default providers would be used instead.
  2. The GTIN link behaves differently. It does not automatically trigger a lookup. But instead it just shows the form and has the default providers set properly.

The second case is more convenient and probably intentional. Having case 1. with url parameter behave the same would likely solve the issue.

Not sure whether both cases should trigger an automatic lookup.

kellnerd commented 3 months ago

You've got a point that this behavior feels inconsequent. Let me explain my intention.

The behavior of GTIN URLs is intentional, I want people to select the providers they want to lookup instead of sending potentially pointless requests to all providers. That becomes even more of an issue the more provider implementations we have. For power users which want this behavior without having to specify all the providers in the URL, I have added the category=all query parameter to enable this, but the regular user should still do their selection.

With the lookup URL containing an explicit provider URL, the situation is a bit different, this can be both someone seeding the form or an explicit lookup via the form, there is no way to tell currently. I feel the behavior is justified as it is doing exactly what was requested and it only queries one provider (unless someone specifies more providers or category=all). Since everything is cached, it is only a very minor performance impact to do such a lookup twice, first only for the given provider URL and then with all the checked providers. And opposed to the case when there is a gtin= parameter, we know with certainty that this request will not be pointless.

That said, we could an explicit query parameter to differentiate between someone seeding the lookup form and someone submitting it. I am just not 100% sure what the expected behavior of the url= parameter should be, so I am open to discuss this. I initially decided to use the implicit behavior of the form because that was the least effort solution and also seemed reasonable to me.

phw commented 3 months ago

This actually makes a lot of sense, and I think we can close this issue as answered. I have seen your initial approaches to the category idea in the code, this looks like the way to go.

This works well and totally solves my concern:

https://harmony.pulsewidth.org.uk/release?url=https%3A%2F%2Fgoatgirl.bandcamp.com%2Falbum%2Fbelow-the-waste&category=all

One thing here is that there could be a special category=default as an alternative to all which would enable providers as initially set when loading Harmony, so you can do:

https://harmony.pulsewidth.org.uk/release?url=https%3A%2F%2Fgoatgirl.bandcamp.com%2Falbum%2Fbelow-the-waste&category=default

At the moment this would cause Beatport to be disabled.

kellnerd commented 3 months ago

One thing here is that there could be a special category=default as an alternative to all which would enable providers as initially set when loading Harmony

Good idea, I will keep this issue as a request for that parameter value, should be easy to implement.

P.S. It would also be nice to have a category=user which automatically uses the user's currently selected providers, but these are currently only persisted to localStorage and not available to the server. We would need cookies to achieve that behavior.

kellnerd commented 3 months ago

I've added category=default in https://github.com/kellnerd/harmony/commit/69ff14dd44a2e755af67d940e353b9e734bb188b. Additionally Harmony is now skipping providers which do not support GTIN lookups early to reduce the amount of "GTIN lookups are not supported" error messages caused by category=all.