MBS-12356: Correctly select + clean up Tidal store pages #2515
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mwiencek
approved these changes
May 27, 2022
|
@brainzbot, retest this please |
yvanzo
reviewed
May 30, 2022
There was a problem hiding this comment.
Fixing the auto-selected relationship type looks good, but the mixed changes with stores should probably be discussed separately as it isn’t clear it is either needed or an improvement.
Even though Tidal is operating in 61 countries (source: Tidal), the website remains available under a .com domain only and it seems that the two letter-code has no influence on content, it can even be replaced with random letters such as xz, to follow the example given in test: https://store.tidal.com/xz/artist/160. I initially thought it was related to the website UI language (see comment to MBS-10621) but it doesn’t seem to be the case anymore/either.
|
Different stores charge in different currencies, at the very least (https://store.tidal.com/us/album/108715503 is in USD but https://store.tidal.com/ee/album/108715503 is in EUR). Also, this isn't changing any current links, since store ones weren't usable anyway, only the new ones :) |
|
|
Huh, wtf. Now it's showing me my local store, but when I left that comment it did show it differently. Now that's confusing. |
|
Well then, rewrote this to remove the country code altogether (which seems to also work and just uses the user's own store) :) |
The Tidal store is not a streaming page, but a download one. We were automatically dropping the store bit and turning it into a streaming link (not sure if the two always exist, but I wouldn't be too confident, so we might have broken some links). Since it's a legitimate download store, we should deal with it as such. While Tidal has a country code for the store, it seems that the purchases happen on the user's local store; removing the country code from a link turns it into a redirect to the user's local store, so this just removes the code.
yvanzo
approved these changes
Jun 13, 2022
reosarevok
added a commit
that referenced
this pull request
Jun 13, 2022
* master: Update POT files using the production database Update translations from Transifex MBS-12395: Report for videos in mediums that shouldn't support video (#2562) MBS-12356: Correctly select + clean up Tidal store pages (#2515) Add comment to ensure test data is kept Save AC redirects before swapping AC uses Document and standardize Controller::Aliases tests Document and standardize Controller::EditAlias tests Document and standardize Controller::DeleteAlias tests Document and standardize Controller::AddAlias tests MBS-9188: Improve LinkedIn URL cleanup (#2553) MBS-12419: Block Genius.com links at release level (#2560) MBS-12417: Update Soundcloud cleanup to remove ? parameters (#2552) Avoid declaring my $tx twice in one test MBS-12351: Also trim space only disambiguations (#2510) MBS-12376: Don't show spammers on area pages (#2554) MBS-12447: Also show area-series rels on series page (#2563) MBS-12393: Change TOWER RECORDS to all-caps as per store Japanese usage (#2558) Simplify the allowed hosts shortener list MBS-12383: Block smart links: bfan.link MBS-12396: Block smart links: hyperfollow.com MBS-12401: Block smart links: hypeddit.com MBS-12350: Block smart links: bio.link MBS-12352: Block smart links: streamerlinks.com Add basic tests for the artist credit page Add IDs to sections of ArtistCreditIndex MBS-12354: Check if AC IDs are valid before passing to the DB MBS-12312: Convert edit.tt to React MBS-12312: Convert history.tt to React MBS-12312: Convert diff.tt to React MBS-12312: Convert revision.tt to React MBS-12312: Remove unused summary.tt MBS-12400: fix non-musicbrainz-schema dumps (#2541) Sync incremental JSON dumps to trille
reosarevok
added a commit
that referenced
this pull request
Jun 20, 2022
* beta: Update translations from Transifex Add custom hydration event for userscripts (#2566) Update POT files using the production database Update translations from Transifex MBS-12311: Allow adding annotations to genres (#2492) Don't ISE on non-existing latest annotation MBS-12456: Load editor for latest_annotation on annotation page MBS-12455: Show annotation info when loading empty revision MBS-12453: Don't crash on null annotation comparison Use index, not ID, to enable/disable annotation comparison Add basic genre create/edit tests Update POT files using the production database Update translations from Transifex MBS-12395: Report for videos in mediums that shouldn't support video (#2562) MBS-12356: Correctly select + clean up Tidal store pages (#2515) Add comment to ensure test data is kept Save AC redirects before swapping AC uses Document and standardize Controller::Aliases tests Document and standardize Controller::EditAlias tests Document and standardize Controller::DeleteAlias tests Document and standardize Controller::AddAlias tests MBS-9188: Improve LinkedIn URL cleanup (#2553) MBS-12419: Block Genius.com links at release level (#2560) MBS-12417: Update Soundcloud cleanup to remove ? parameters (#2552) Avoid declaring my $tx twice in one test MBS-12351: Also trim space only disambiguations (#2510) MBS-12376: Don't show spammers on area pages (#2554) MBS-12447: Also show area-series rels on series page (#2563) MBS-12393: Change TOWER RECORDS to all-caps as per store Japanese usage (#2558) Remove useless Area::Create _insert_hash MBS-10165: Use edit system for genre editing MBS-10165: Use edit system for genre adding MBS-10165: Use edit system for genre deletion MBS-10165: Basic preparations for genre edits Support genres in formatEntityTypeName Simplify the allowed hosts shortener list MBS-12383: Block smart links: bfan.link MBS-12396: Block smart links: hyperfollow.com MBS-12401: Block smart links: hypeddit.com MBS-12350: Block smart links: bio.link MBS-12352: Block smart links: streamerlinks.com Add basic tests for the artist credit page Add IDs to sections of ArtistCreditIndex MBS-12354: Check if AC IDs are valid before passing to the DB MBS-12312: Convert edit.tt to React MBS-12312: Convert history.tt to React MBS-12312: Convert diff.tt to React MBS-12312: Convert revision.tt to React MBS-12312: Remove unused summary.tt
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix MBS-12356
The Tidal store is not a streaming page, but a download one. We were automatically dropping the store bit and turning it into a streaming link (not sure if the two always exist, but I wouldn't be too confident, so we might have broken some links). Since it's a legitimate download store, we should deal with it as such.
While Tidal has a country code for the store, it seems that the purchases happen on the user's local store; removing the country code from a link turns it into a redirect to the user's local store, so this just removes the code.