Skip to content

Commit

Permalink
MBS-12801: Warn users about "Disc n" titles on release merge
Browse files Browse the repository at this point in the history
This is a basic warning, which is probably enough. We could also disable
submission unless they confirm, by erroring at the Perl level, but this
is less intrusive and hopefully does the trick still, since it appears
just below the name row as soon as the user enters the triggering name.
  • Loading branch information
reosarevok committed Dec 5, 2023
1 parent f5731d5 commit be82964
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 1 deletion.
47 changes: 47 additions & 0 deletions root/static/scripts/edit/components/ReleaseMergeStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {ExpandedArtistCreditList}
import Warning from '../../common/components/Warning.js';
import formatTrackLength
from '../../common/utility/formatTrackLength.js';
import isUselessMediumTitle from '../utility/isUselessMediumTitle.js';

import FormRowSelect from './FormRowSelect.js';

Expand All @@ -40,6 +41,28 @@ const mergeStrategyOptions = {
],
};

const UselessMediumTitleWarning = ({name}: {name: string}) => (
<tr>
<td colSpan="4">
<span
className="error"
style={{margin: '0 12px 0 6px'}}
>
{exp.l(
`“{matched_text}” seems to indicate medium ordering rather than
a medium title. If this is the case, please set the appropriate
medium position instead of adding a title
(see {release_style|the guidelines}).`,
{
matched_text: name,
release_style: '/doc/Style/Release#Medium_title',
},
)}
</span>
</td>
</tr>
);

const ReleaseMergeStrategy = ({
badRecordingMerges,
form,
Expand All @@ -57,6 +80,24 @@ const ReleaseMergeStrategy = ({
}

const mediumsMap = form.field.medium_positions.field.map;

const [mediumTitles, setMediumTitles] =
React.useState(mediumsMap.field.map(
field => field.field.name.value,
));

function updateTitles(
event: SyntheticEvent<HTMLSelectElement>,
index: number,
) {
const mediumTitle = event.currentTarget.value;
setMediumTitles(prevTitles => {
const titles = [...prevTitles];
titles[index] = mediumTitle;
return titles;
});
}

return (
<>
<FormRowSelect
Expand Down Expand Up @@ -109,6 +150,7 @@ const ReleaseMergeStrategy = ({
<input
defaultValue={mediumField.name.value}
name={mediumField.name.html_name}
onChange={event => updateTitles(event, index)}
type="text"
/>
<input
Expand Down Expand Up @@ -151,6 +193,11 @@ const ReleaseMergeStrategy = ({
)}
</th>
</tr>
{isUselessMediumTitle(mediumTitles[index]) ? (
<UselessMediumTitleWarning
name={mediumTitles[index]}
/>
) : null}
{medium.tracks ? medium.tracks.map((track, index) => (
<tr className={loopParity(index)} key={track.id}>
<td className="pos t">
Expand Down
12 changes: 12 additions & 0 deletions root/static/scripts/edit/utility/isUselessMediumTitle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* @flow strict
* Copyright (C) 2023 MetaBrainz Foundation
*
* This file is part of MusicBrainz, the open internet music database,
* and is licensed under the GPL version 2, or (at your option) any
* later version: http://www.gnu.org/licenses/gpl-2.0.txt
*/

export default function isUselessMediumTitle(title: string): boolean {
return /^(Cassette|CD|Dis[ck]|DVD|SACD|Vinyl)\s*\d+/i.test(title);
}
3 changes: 2 additions & 1 deletion root/static/scripts/release-editor/fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import request from '../common/utility/request.js';
import {fixedWidthInteger, uniqueId} from '../common/utility/strings.js';
import mbEdit from '../edit/MB/edit.js';
import * as dates from '../edit/utility/dates.js';
import isUselessMediumTitle from '../edit/utility/isUselessMediumTitle.js';
import * as validation from '../edit/validation.js';

import recordingAssociation from './recordingAssociation.js';
Expand Down Expand Up @@ -466,7 +467,7 @@ class Medium {
this.hasStrangeDigitalPackaging(),
);
this.hasUselessMediumTitle = ko.computed(function () {
return /^(Cassette|CD|Dis[ck]|DVD|SACD|Vinyl)\s*\d+/i.test(self.name());
return isUselessMediumTitle(self.name());
});
this.confirmedMediumTitle = ko.observable(this.hasUselessMediumTitle());
this.needsTrackArtists = ko.computed(function () {
Expand Down
32 changes: 32 additions & 0 deletions root/static/scripts/tests/edit/utility/isUselessMediumTitle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* @flow strict
* Copyright (C) 2023 MetaBrainz Foundation
*
* This file is part of MusicBrainz, the open internet music database,
* and is licensed under the GPL version 2, or (at your option) any
* later version: http://www.gnu.org/licenses/gpl-2.0.txt
*/

import test from 'tape';

import isUselessMediumTitle
from '../../../edit/utility/isUselessMediumTitle.js';

test('isUselessMediumTitle', function (t) {
t.plan(3);

t.ok(
!isUselessMediumTitle('The Happy Disc'),
'A normal title is not useless',
);

t.ok(
isUselessMediumTitle('DVD 42'),
'A "format plus number" title is useless',
);

t.ok(
isUselessMediumTitle('Disk1'),
'A "format plus number" title is still useless if not space-separated',
);
});
1 change: 1 addition & 0 deletions root/static/scripts/tests/index-web.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ require('./common/immutable-entities.js');
require('./Control/URLCleanup.js');
require('./CoverArt.js');
require('./edit.js');
require('./edit/utility/isUselessMediumTitle.js');
require('./edit/utility/linkPhrase.js');
require('./entity.js');
require('./GuessCase.js');
Expand Down

0 comments on commit be82964

Please sign in to comment.