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-1736: Block setting format on too early releases #1111

Merged
merged 1 commit into from Jun 10, 2020
Merged

MBS-1736: Block setting format on too early releases #1111

merged 1 commit into from Jun 10, 2020

Conversation

reosarevok
Copy link
Member

https://tickets.metabrainz.org/browse/MBS-1736

Look at the earliest release year of all release events. If the release year is earlier than the first year of use stored for the format, block the user from selecting that format for any mediums.

This currently has the issue that we don't have a format year for, for example. 7" vinyl, only for vinyl as a whole. Since I don't think we use year externally anyway and it's just used for things like this, I'd suggest we can just use the same year for all vinyls (and the CD year for specific types of CD if we can't find the year), to at least block some incorrect usage. No idea what would be a sensible year for digital media though! :)

@reosarevok reosarevok force-pushed the MBS-1736 branch 3 times, most recently from a618f13 to 753649d Compare June 20, 2019 10:09
@reosarevok reosarevok added the QoL Non-urgent quality of life improvements label Sep 27, 2019
@reosarevok reosarevok added New feature Non urgent new stuff and removed QoL Non-urgent quality of life improvements labels Feb 18, 2020
@reosarevok reosarevok added this to the Tiburón Abuelo milestone Feb 18, 2020
mwiencek
mwiencek requested changes Feb 19, 2020
View reviewed changes
lib/MusicBrainz/Server/Controller/ReleaseEditor.pm Outdated
@@ -43,6 +43,7 @@ sub _init_release_editor

my @medium_formats = $c->model('MediumFormat')->get_all;
my $discid_formats = [ grep { $_ } map { $_->has_discids ? ($_->id) : () } @medium_formats ];
my %format_dates = map { $_->id => $_->year } @medium_formats;
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to fall back to the parent/root format's year if a specific one isn't set?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I'd dare to block by inheritance - I'm sure there'll be some obscure case where that doesn't work. Honestly, the main use case for this is just blocking CDs before the 1980s, so either way would work.

Copy link
Member

Choose a reason for hiding this comment

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

It seems weird that the error basically never triggers for any of the child formats even when it obviously should, but they're probably rare enough that it doesn't matter.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems easier to set year for every children formats.

root/release/edit/tracklist.tt Outdated Show resolved Hide resolved
root/static/scripts/release-editor/fields.js Outdated Show resolved Hide resolved
root/static/scripts/release-editor/fields.js Outdated Show resolved Hide resolved
root/static/scripts/tests/release-editor/common.js Outdated Show resolved Hide resolved
mwiencek
mwiencek approved these changes May 27, 2020
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.

Left one nit-picky comment though this seems good to merge as-is.

yvanzo
yvanzo requested changes Jun 10, 2020
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.

Overall, it looks good. year should probably be set for HD-DVD and other children formats missing it.

Please prefix format date variables with medium so as to disambiguate with date formatting functions.

@reosarevok
MBS-1736: Block setting format on too early releases
07947d9 
Look at the earliest release year of all release events.
If the release year is earlier than the first year of use
stored for the format, block the user from selecting that
format for any mediums.
@yvanzo yvanzo merged commit 2e9f2e6 into metabrainz:master Jun 10, 2020
@reosarevok reosarevok deleted the MBS-1736 branch June 10, 2020 21:19
yvanzo added a commit that referenced this pull request Jun 10, 2020
@yvanzo
Merge branch 'master' into beta
97841a5 
* master:
  Update POT files using the production database
  MBS-1736: Block setting format on too early releases (#1111)
  MBS-10697 / MBS-10725 / MBS-10758 / MBS-10848: Smart links (#1544)
  MBS-6864: Drop all PUID-related code (#1551)
  Eslint fixes + typographic apostrophe
  MBS-4644: Show releases that have CAA images in lists
  MBS-10873: Load ACs for annotation edits (#1541)
  MBS-6532: Show work edits in recording / release history (#1165)
  MBS-10381: Disallow using a work language multiple times (#1559)
  MBS-10892: Show private collaborated-on collections on sidebars (#1565)
  MBS-8625: release data quality icons (#1556)
  Fix invalid <div> inside <span>
  Compile static resources in the docker images
  Remove DBDefs dependency from webpack config
  Make DBDefs JS external from build
  Add DBDefs Flow defs
  Cleanup dbdefs_to_js conversion
  Remove unused variable
  MBS-10893: Document selecting artist for CD lookup (#1563)
  MBS-10890: Do not load language for works multiple times (#1562)
  Fix error message about Instagram `/explore` URLs (#1566)
  MBS-10821: Remove orphaned recordings from collections for deletion (#1512)
  MBS-10862: Report "CatNoLooksLikeLabelCode" (#1548)
  MBS-9931: Fail gracefully when trying to remove example rel
  MBS-9340: Only allow mul and zxx as the only work language
  Set cacheDirectory: true for babel-loader
  Set preset-env target to node version on server
  Bump babel-loader to 8.1.0
  Fix multiple imports on one line
  Upgrade core-js to v3 and set useBuiltIns: 'usage'
  Fix format_editnote running on localized note URL
  MBS-10186: Check for missing CD TOC on SetTrackLengths (#1511)
  Upgrade all babel deps
  Bump mutate-cow to 4.0.2
  MBS-10891: Replace Magic MP3 Tagger link with AudioRanger (#1560)
  Escape ', ", `, and GIT_MSG in GIT_MSG (#1557)
  MBS-10885: Fix error when URL has been removed (#1553)
  Eslint fixes
  MBS-10887: Fix country/date text CSS alignment (#1554)
  Refactor 435310d: Check DBDefs instead of stash (#1555)
  Fix TypeError when changing relationship type
  MBS-10856: Trim whitespace on relationship credits (#1545)
  Fix non-sanitized staging server is beta not test
  Disable IP hashes on sanitized staging server
  MBS-10836: Fix edit / remove collection links (#1550)
  Drop FUNDING file made obsolete by '.github' repo
  MBS-10882: Remove accidental duplicate push (#1547)
  Rename AddStandaloneRecordingProps and add separate props type
  $EDIT_RECORDING_CREATE artist_credit is not optional
  Fix bracket indentation
  $EDIT_RECORDING_CREATE name is not optional
  MBS-10771: Block tagging for unverified users (#1504)
  MBS-10877: Display source URLs in the relationship editor dialog
  MBS-10867: Fix "entity not in the entities array"
  MBS-10469: Add 10 minute delay to shown CAA on front page (#1540)
  Only print recording name instead of linking recording twice
  MBS-10393: Convert Add Standalone Recording edit to React
  Add react-dom libdefs from flow-typed
  Document $c-in-JS a bit more
  Enable types-first mode in Flow
  Add Flow types to AreaWithContainmentLink.js
  Add Flow types to AreaContainmentLink.js
  Add Flow types to 404.js
  Add Flow types to Footer.js
  Pass $c to all server-rendered components
  Add Flow types to ExternalLinks.js
  Bump Flow to 0.125.1
  Fix typo preventing to store alert_mtime on beta
  Add colon instead of brackets to row header
yvanzo added a commit that referenced this pull request Jun 15, 2020
@yvanzo
Merge branch 'beta' into production
c3c6348 
* beta:
  Update translations from Transifex
  Fix warning about masked variable since c1faa8b
  Update POT files using the production database
  MBS-1736: Block setting format on too early releases (#1111)
  MBS-10697 / MBS-10725 / MBS-10758 / MBS-10848: Smart links (#1544)
  MBS-6864: Drop all PUID-related code (#1551)
  Eslint fixes + typographic apostrophe
  MBS-4644: Show releases that have CAA images in lists
  MBS-10873: Load ACs for annotation edits (#1541)
  MBS-6532: Show work edits in recording / release history (#1165)
  MBS-10381: Disallow using a work language multiple times (#1559)
  MBS-10892: Show private collaborated-on collections on sidebars (#1565)
  MBS-8625: release data quality icons (#1556)
  Fix invalid <div> inside <span>
  Compile static resources in the docker images
  Remove DBDefs dependency from webpack config
  Make DBDefs JS external from build
  Add DBDefs Flow defs
  Cleanup dbdefs_to_js conversion
  Remove unused variable
  MBS-10893: Document selecting artist for CD lookup (#1563)
  MBS-10890: Do not load language for works multiple times (#1562)
  Fix error message about Instagram `/explore` URLs (#1566)
  MBS-10821: Remove orphaned recordings from collections for deletion (#1512)
  MBS-10862: Report "CatNoLooksLikeLabelCode" (#1548)
  MBS-9931: Fail gracefully when trying to remove example rel
  MBS-9340: Only allow mul and zxx as the only work language
  Set cacheDirectory: true for babel-loader
  Set preset-env target to node version on server
  Bump babel-loader to 8.1.0
  Fix multiple imports on one line
  Upgrade core-js to v3 and set useBuiltIns: 'usage'
  Fix format_editnote running on localized note URL
  MBS-10186: Check for missing CD TOC on SetTrackLengths (#1511)
  Upgrade all babel deps
  Bump mutate-cow to 4.0.2
  MBS-10891: Replace Magic MP3 Tagger link with AudioRanger (#1560)
  Escape ', ", `, and GIT_MSG in GIT_MSG (#1557)
  MBS-10885: Fix error when URL has been removed (#1553)
  Eslint fixes
  MBS-10887: Fix country/date text CSS alignment (#1554)
  Refactor 435310d: Check DBDefs instead of stash (#1555)
  Fix TypeError when changing relationship type
  MBS-10856: Trim whitespace on relationship credits (#1545)
  Fix non-sanitized staging server is beta not test
  Disable IP hashes on sanitized staging server
  MBS-10836: Fix edit / remove collection links (#1550)
  Drop FUNDING file made obsolete by '.github' repo
  MBS-10882: Remove accidental duplicate push (#1547)
  Rename AddStandaloneRecordingProps and add separate props type
  $EDIT_RECORDING_CREATE artist_credit is not optional
  Fix bracket indentation
  $EDIT_RECORDING_CREATE name is not optional
  MBS-10771: Block tagging for unverified users (#1504)
  MBS-10877: Display source URLs in the relationship editor dialog
  MBS-10867: Fix "entity not in the entities array"
  MBS-10469: Add 10 minute delay to shown CAA on front page (#1540)
  Only print recording name instead of linking recording twice
  MBS-10393: Convert Add Standalone Recording edit to React
  Add react-dom libdefs from flow-typed
  Document $c-in-JS a bit more
  Enable types-first mode in Flow
  Add Flow types to AreaWithContainmentLink.js
  Add Flow types to AreaContainmentLink.js
  Add Flow types to 404.js
  Add Flow types to Footer.js
  Pass $c to all server-rendered components
  Add Flow types to ExternalLinks.js
  Bump Flow to 0.125.1
  Fix typo preventing to store alert_mtime on beta
  Add colon instead of brackets to row header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwiencek mwiencek mwiencek approved these changes

@yvanzo yvanzo yvanzo approved these changes

Assignees
No one assigned
Labels
New feature Non urgent new stuff
Projects
None yet
Milestone
Bébé requin
3 participants
@reosarevok @mwiencek @yvanzo