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.

Sign up for GitHub

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

Jump to bottom

MBS-12351: Also trim space only disambiguations #2510

Merged
merged 2 commits into from
Jun 10, 2022
Merged

MBS-12351: Also trim space only disambiguations #2510

merged 2 commits into from
Jun 10, 2022

Conversation

reosarevok
Copy link
Member

Fix MBS-12351

The transform was not being applied on spaces-only disambiguation fields, because FormHandler considers it empty. As such, I added validate_when_empty to enforce always running the transform on the comment field.

@reosarevok reosarevok added the Bug Bugs that should be checked/fixed soonish label May 4, 2022
@reosarevok
MBS-12351: Also trim space only disambiguations
311bb6b 
The transform was not being applied on spaces-only
disambiguation fields, because FormHandler considers it
empty. As such, I added validate_when_empty to enforce
always running the transform on the comment field.

This broke the test adding a rel, because we weren't entering
the comment field. That seemed weird to begin with (we are also
not changing name and sort name, yet we list them) so adding it here.
@reosarevok
Copy link
Member Author

@brainzbot, retest this please

mwiencek
mwiencek approved these changes May 30, 2022
View reviewed changes
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.

(fixup commit should be squashed when merging)

@reosarevok reosarevok requested a review from yvanzo May 30, 2022 17:26
yvanzo
yvanzo approved these changes Jun 10, 2022
View reviewed changes
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

LGTMBDNT. Squash 🖐️ and merge it!

@reosarevok reosarevok merged commit 1a300d3 into metabrainz:master Jun 10, 2022
@reosarevok reosarevok deleted the MBS-12351 branch June 10, 2022 16:02
reosarevok added a commit that referenced this pull request Jun 13, 2022
@reosarevok
Merge branch 'master' into beta
6ccdff6 
* 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
@reosarevok
Merge branch 'beta' into production
9a99206 
* 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
Reviewers

@yvanzo yvanzo yvanzo approved these changes

@mwiencek mwiencek mwiencek approved these changes

Assignees
No one assigned
Labels
Bug Bugs that should be checked/fixed soonish
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@reosarevok @mwiencek @yvanzo