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

MBS-13922: Add URL cleanup and validation for Naver Vibe #3463

Merged
merged 2 commits into from
Feb 12, 2025

Conversation

Maxr1998
Copy link
Contributor

@Maxr1998 Maxr1998 commented Feb 2, 2025

Problem

Currently, URL relationships for Naver Vibe aren't validated nor cleaned up to a standard format.

Solution

Implement URL cleanup code for Naver Vibe, ensure that links are only added to their respective allowed entities, and rewrite legacy Naver Music links to the new Vibe format.

Testing

Tested locally by adding various types of Naver Vibe links to artists and releases. Additionally, unit tests have been written.
Currently, the unit tests fail due to a timeout, I'm not sure what is causing that, since it also says that all tests passed.

@reosarevok
Copy link
Member

All tests passed until the one that didn't :) (and that crashed it). Will look into why.

@Maxr1998
Copy link
Contributor Author

Maxr1998 commented Feb 3, 2025

Thanks, I unfortunately couldn't find a cause for it yet..

@reosarevok
Copy link
Member

The regexes in the cleanup were a bit weird, and it was not matching your music.naver URL. AFAICT the intent is just "catch the number, ignore everything after it" so I changed it up a bit to match how we do those things in other cleanups. If there's a deeper meaning we need to keep, let me know!

@Maxr1998
Copy link
Contributor Author

Maxr1998 commented Feb 11, 2025

My original intent was to enforce that the URLs end a certain way after the ID number, for example &.. after a query parameter or with a slash. But this isn't really necessary, and your suggestion is more general, so we can merge it like this. And thanks for fixing the tests!

@reosarevok reosarevok merged commit 3057e90 into metabrainz:master Feb 12, 2025
1 of 2 checks passed
@Maxr1998 Maxr1998 deleted the vibe-url-cleanup branch February 12, 2025 08:27
reosarevok added a commit that referenced this pull request Feb 18, 2025
* master:
  Update POT files using the production database
  Merge pull request #3472 from reosarevok/MBS-13929
  Merge pull request #3477 from reosarevok/MBS-13939
  Merge pull request #3478 from reosarevok/MBS-13938
  Merge pull request #3473 from reosarevok/MBS-13930
  Add ordering to `_find_authors_or_other_artists` roles
  Use proper query variable bindings in `Data::Edit::find_by_collection`
  MBS-13914: Validate vote arguments for voter edit search (#3464)
  MBS-13763: Add past year stats to the editors statistic page (#3465)
  MBS-13928: Use jaxsta.com instead of jaxsta.io (#3471)
  add --no-same-owner option to tar (#3456)
  MBS-13925: Instrument name erroneously displayed in link type autocomplete (#3467)
  Avoid spreading `key` props in `useTable` hook (#3476)
  More direct `get_authorship_relationship_gids` implementation
  Rename "misc_artists" to "other_artists"
  MBS-8328: Split the role filter for artist works
  MBS-13917: Rename work "writers" to "authors"
  Get authorship rel gids from the DB
  Rename "composition" to "authorship"
  Add test for loading different subsets of work artists
  Drop unneeded work artist credit test SQL
  MBS-8328: Add the non-writer artists back as own column
  MBS-8328: Limit work writers to writing rels
  MBS-11916: Report for recordings marked both karaoke and instrumental (#2504)
  Merge pull request #3468 from derat/feat_error
  MBS-13920 (II): Detect feat. artists in titles when seeding (#3469)
  Use `qq` to avoid escaping "
  MBS-6502: Surround catno search with quotes
  MBS-13922: Add URL cleanup and validation for Naver Vibe (#3463)
  Move most filter find_by to right Data file
  MBS-8500: Add "not by me" filter for collection edit lists
  MBS-13597: Add release filter to label index pages
  Avoid warning when selected_artist_credit_id is missing
reosarevok added a commit that referenced this pull request Feb 24, 2025
* beta:
  Translated using Weblate (Chinese (Simplified Han script))
  Translated using Weblate (Italian)
  Update translation files
  MBS-13917: Add new properties to avoid crashes (#3481)
  Update POT files using the production database
  Merge pull request #3472 from reosarevok/MBS-13929
  Merge pull request #3477 from reosarevok/MBS-13939
  Merge pull request #3478 from reosarevok/MBS-13938
  Merge pull request #3473 from reosarevok/MBS-13930
  Add ordering to `_find_authors_or_other_artists` roles
  Use proper query variable bindings in `Data::Edit::find_by_collection`
  MBS-13914: Validate vote arguments for voter edit search (#3464)
  MBS-13763: Add past year stats to the editors statistic page (#3465)
  MBS-13928: Use jaxsta.com instead of jaxsta.io (#3471)
  add --no-same-owner option to tar (#3456)
  MBS-13925: Instrument name erroneously displayed in link type autocomplete (#3467)
  Avoid spreading `key` props in `useTable` hook (#3476)
  Translated using Weblate (Chinese (Simplified Han script))
  Translated using Weblate (Chinese (Simplified Han script))
  Translated using Weblate (Russian)
  Translated using Weblate (Chinese (Simplified Han script))
  Translated using Weblate (Polish)
  Translated using Weblate (Italian)
  More direct `get_authorship_relationship_gids` implementation
  Rename "misc_artists" to "other_artists"
  MBS-8328: Split the role filter for artist works
  MBS-13917: Rename work "writers" to "authors"
  Get authorship rel gids from the DB
  Rename "composition" to "authorship"
  Add test for loading different subsets of work artists
  Drop unneeded work artist credit test SQL
  MBS-8328: Add the non-writer artists back as own column
  MBS-8328: Limit work writers to writing rels
  MBS-11916: Report for recordings marked both karaoke and instrumental (#2504)
  Merge pull request #3468 from derat/feat_error
  MBS-13920 (II): Detect feat. artists in titles when seeding (#3469)
  Use `qq` to avoid escaping "
  MBS-6502: Surround catno search with quotes
  MBS-13922: Add URL cleanup and validation for Naver Vibe (#3463)
  Move most filter find_by to right Data file
  MBS-8500: Add "not by me" filter for collection edit lists
  MBS-13597: Add release filter to label index pages
  Avoid warning when selected_artist_credit_id is missing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants