Skip to content

Commit 16872e5

Browse files
committed
Validate recording length in React form
This adds an isInvalidLength function that just checks whether unformatTrackLength returns false. The error message is not quite precise (the time does not *have* to be in MM:SS format) but it probably clarifies the easiest way to enter the length well enough for users, and is only confusing in edge cases (like a length that is invalid because it's longer than the max we can store).
1 parent 645db35 commit 16872e5

File tree

4 files changed

+104
-1
lines changed

4 files changed

+104
-1
lines changed
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* @flow strict
3+
* Copyright (C) 2025 MetaBrainz Foundation
4+
*
5+
* This file is part of MusicBrainz, the open internet music database,
6+
* and is licensed under the GPL version 2, or (at your option) any
7+
* later version: http://www.gnu.org/licenses/gpl-2.0.txt
8+
*/
9+
10+
import unformatTrackLength from '../../common/utility/unformatTrackLength.js';
11+
12+
export default function isInvalidLength(length: string): boolean {
13+
// If the unformatted length is false, then it's an unparseable date
14+
return unformatTrackLength(length) === false;
15+
}

root/static/scripts/recording/components/RecordingEditForm.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import {
5656
} from '../../edit/externalLinks.js';
5757
import guessFeat from '../../edit/utility/guessFeat2.js';
5858
import isInvalidEditNote from '../../edit/utility/isInvalidEditNote.js';
59+
import isInvalidLength from '../../edit/utility/isInvalidLength.js';
5960
import {
6061
applyAllPendingErrors,
6162
hasSubfieldErrors,
@@ -70,6 +71,7 @@ type ActionT =
7071
| {+type: 'show-all-pending-errors'}
7172
| {+type: 'toggle-bubble', +bubble: string}
7273
| {+type: 'update-edit-note', +editNote: string}
74+
| {+type: 'update-length', +length: string}
7375
| {+type: 'update-name', +action: NameActionT}
7476
| {+type: 'update-artist-credit', +action: ArtistCreditActionT};
7577
/* eslint-enable ft-flow/sort-keys */
@@ -147,6 +149,17 @@ function reducer(state: StateT, action: ActionT): StateT {
147149
value: editNote,
148150
});
149151
}
152+
{type: 'update-length', const length} => {
153+
const errors = isInvalidLength(length)
154+
? [l('Not a valid time. Must be in the format MM:SS')]
155+
: [];
156+
157+
newStateCtx.set('form', 'field', 'length', {
158+
...newStateCtx.get('form', 'field', 'length').read(),
159+
errors,
160+
value: length,
161+
});
162+
}
150163
{type: 'update-name', const action} => {
151164
const nameStateCtx = mutate({
152165
field: state.form.field.name,
@@ -247,6 +260,15 @@ component RecordingEditForm(
247260
dispatch({type: 'guess-feat'});
248261
}
249262

263+
const handleLengthChange = React.useCallback((
264+
event: SyntheticEvent<HTMLInputElement>,
265+
) => {
266+
dispatch({
267+
length: event.currentTarget.value,
268+
type: 'update-length',
269+
});
270+
}, [dispatch]);
271+
250272
function handleLengthFocus() {
251273
dispatch({bubble: 'length', type: 'toggle-bubble'});
252274
}
@@ -336,9 +358,9 @@ component RecordingEditForm(
336358
<FormRowTextLong
337359
field={state.form.field.length}
338360
label={addColonText(l('Length'))}
361+
onChange={handleLengthChange}
339362
onFocus={handleLengthFocus}
340363
rowRef={lengthFieldRef}
341-
uncontrolled
342364
/>
343365
) : (
344366
<FormRow>

root/static/scripts/tests/index-web.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ require('./utility/isDateEmpty.js');
5757
require('./utility/isDisabledLink.js');
5858
require('./utility/isFutureDate.js');
5959
require('./utility/isGuid.js');
60+
require('./utility/isInvalidLength.js');
6061
require('./utility/isObjectEmpty.js');
6162
require('./utility/isShortenedUrl.js');
6263
require('./utility/isSpecialPurpose.js');
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* @flow strict
3+
* Copyright (C) 2025 MetaBrainz Foundation
4+
*
5+
* This file is part of MusicBrainz, the open internet music database,
6+
* and is licensed under the GPL version 2, or (at your option) any
7+
* later version: http://www.gnu.org/licenses/gpl-2.0.txt
8+
*/
9+
10+
import test from 'tape';
11+
12+
import isInvalidLength from '../../edit/utility/isInvalidLength.js';
13+
14+
test('isInvalidLength', function (t) {
15+
t.plan(12);
16+
17+
t.ok(
18+
!isInvalidLength(''),
19+
'No length is not invalid',
20+
);
21+
t.ok(
22+
!isInvalidLength('?:??'),
23+
'?:?? is empty/unknown, but not invalid',
24+
);
25+
t.ok(
26+
!isInvalidLength('23 ms'),
27+
'Miliseconds length is valid',
28+
);
29+
t.ok(
30+
!isInvalidLength('00:23'),
31+
'MM:SS is valid',
32+
);
33+
t.ok(
34+
!isInvalidLength('00:57'),
35+
':SS is valid',
36+
);
37+
t.ok(
38+
!isInvalidLength('85:23'),
39+
'MM:SS is valid with more than 60 minutes',
40+
);
41+
t.ok(
42+
!isInvalidLength('1:00:57'),
43+
'HH:MM:SS is valid',
44+
);
45+
t.ok(
46+
isInvalidLength('foo'),
47+
'A random string is not valid',
48+
);
49+
t.ok(
50+
isInvalidLength('10:80'),
51+
'MM:SS is not valid with more than 60 seconds',
52+
);
53+
t.ok(
54+
isInvalidLength(':80'),
55+
':SS is not valid with more than 60 seconds',
56+
);
57+
t.ok(
58+
isInvalidLength('1:75:10'),
59+
'HH:MM:SS is not valid with more than 60 minutes',
60+
);
61+
t.ok(
62+
isInvalidLength('1000:35:10'),
63+
'Times longer than the max number of ms we can store are invalid',
64+
);
65+
});

0 commit comments

Comments
 (0)