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-13541: Accept uppercase MBIDs in JavaScript #3235

Merged
merged 1 commit into from Apr 15, 2024

Conversation

derat
Copy link
Contributor

@derat derat commented Apr 15, 2024

MBS-13541

Update MBID_REGEXP and isGuid() to additionally match UUIDs containing uppercase letters (as recommended by RFC 4122). Previously, Autocomplete 2 would fail to extract MBIDs containing uppercase letters within pasted URLs.

Problem

As discussed in https://community.metabrainz.org/t/boom-boom-not-recognized-as-a-work-when-pasting-in-add-relationship-dialog-on-add-standalone-recording/690757, the relationship editor doesn't recognize MBIDs with capital A-F within pasted URLs.

(This seems like a minor issue since the MBS always lowercases MBIDs, as far as I can tell, but URLs like https://musicbrainz.org/artist/65F4F0C5-EF9E-490C-AEE3-909E7AE6B2AB work and don't get redirected or rewritten automatically.)

Per https://datatracker.ietf.org/doc/html/rfc4122#section-3, uppercase letters should be accepted in UUIDs:

"The hexadecimal values "a" through "f" are output as lower case characters and are case insensitive on input.

Solution

This change updates the MBID_REGEXP regular expression to be case-insensitive, and also updates the MBID_ONLY_REGEXP regexp used by isGuid().

I'm still making Autocomplete2 lowercase extracted MBIDs before passing them to the /ws/js/entity endpoint.

Per @mwiencek at https://tickets.metabrainz.org/browse/MBS-13541?focusedId=71904&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-71904, this shouldn't affect database validation since MBIDs are only generated in Perl and are stored using the case-insensitive UUID PostgreSQL type.

Testing

I added a new test case for isGuid() and have manually tested that the relationship editor now recognizes MBIDs containing capital letters.

Update MBID_REGEXP and isGuid() to additionally match UUIDs
containing uppercase letters (as recommended by RFC 4122).
Previously, Autocomplete 2 would fail to extract MBIDs
containing uppercase letters within pasted URLs.
@derat
Copy link
Contributor Author

derat commented Apr 15, 2024

@mwiencek, mind taking a look?

Copy link
Member

@mwiencek mwiencek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mwiencek mwiencek merged commit 36f9503 into metabrainz:master Apr 15, 2024
3 checks passed
@derat derat deleted the uppercase_mbid branch April 15, 2024 20:49
yvanzo added a commit that referenced this pull request Apr 17, 2024
* master:
  Update POT files using the production database
  Bump musicbrainz-tests image tag after MBS-13358
  Regenerate cpanfile.snapshot files after MBS-13358
  Update Perl dependencies for internationalization
  MBS-13546: Switch to Email::Address::XS
  Update DateTime::Locale version to 1.41
  MBS-13358 (II.2): Require Perl 5.38.0 or later
  Bind install_svlogd_services.sh instead of copying
  Replace RUN chmod/chown with COPY --chmod/--chown
  Always set PERL_CARTON_PATH in Docker images
  Bind cpanfile and snapshot to install Perl modules
  Always set ownership for cpanm/Carton directories
  Always install Perl and cpanm/Carton in one RUN
  Cache cpanm downloads, builds, and tests
  Reinstall perl modules only when cpanfile changes
  Drop instruction included in install_perl_modules
  Always clean up the cpanm cache for root user
  MBS-13358 (II.1): Upgrade Perl version to 5.38.2
  Install ts from source instead of using apt-get
  Install carton from cpanm instead of using apt-get
  Drop support for Perl 5.30
  Update Flow to v0.233.0 (#3237)
  MBS-13540: Update track credits in a few more cases (#3233)
  MBS-13541: Accept uppercase MBIDs in JavaScript (#3235)
  Install xz through APT as it isn’t so new anymore
  Install APT packages using self-documented macros
  Factorize setting up cpanm & carton through macros
  Templatize Dockerfile.tests to reuse macros
  Rework installing cpanm after official docker-perl
  Document Perl deps installation through comments
  Replace the few `wget` calls with `curl` calls
  Use build ARG instruction for installed XZ version
  Remove temporary keys to install XZ utils
  Add temporary keyrings through bind mount
  Format lists of packages one by line
  Generalize options passed to curl
  Factorize copy_common_mbs_files into server_base
  Drop duplicate ARG already provided in server_base
  Cache APT directories when building Docker images
  MBS-13539: Update VK logo (#3232)
  Add context to "order" header (#3229)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants