Skip to content

Commit 29b3034

Browse files
committed
External links editor refactor
This was the very first React component written for MBS, all the way back in 2015, and requires significant refactoring in order to follow the Rules of React [1] and work properly with React v19 and new tools like react-compiler. Fixes MBS-11889 Fixes MBS-11521 [1] https://react.dev/reference/rules
1 parent 074882e commit 29b3034

36 files changed

Lines changed: 2776 additions & 2104 deletions

lib/MusicBrainz/Server/Controller/ReleaseEditor.pm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ sub _init_release_editor
3939
my @medium_formats = $c->model('MediumFormat')->get_all;
4040
my $discid_formats = [ grep { $_ } map { $_->has_discids ? ($_->id) : () } @medium_formats ];
4141
my %medium_format_dates = map { $_->id => $_->year } @medium_formats;
42+
my $release = $c->stash->{release};
4243

4344
$c->stash(
4445
template => 'release/edit/layout.tt',
@@ -48,7 +49,7 @@ sub _init_release_editor
4849
statuses => select_options_tree($c, 'ReleaseStatus'),
4950
languages => build_grouped_options($c, language_options($c)),
5051
scripts => build_grouped_options($c, script_options($c)),
51-
source_entity => to_json_object($c->stash->{release}),
52+
source_entity => to_json_object($release) // {entityType => 'release', isNewEntity => \1},
5253
packagings => select_options_tree($c, 'ReleasePackaging'),
5354
countries => select_options($c, 'CountryArea'),
5455
formats => select_options_tree($c, 'MediumFormat'),

root/components/forms.tt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,5 +273,5 @@
273273
[%- END -%]
274274

275275
[%- MACRO external_links_editor BLOCK -%]
276-
<div id="external-links-editor-container"></div>
276+
[%- React.embed(c, 'static/scripts/external-links-editor/components/StandaloneExternalLinksEditor') -%]
277277
[%- END -%]

root/server/components.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@ export default {
550550
'static/scripts/edit/components/GuessCaseIcon': (): Promise<mixed> => import('../static/scripts/edit/components/GuessCaseIcon.js'),
551551
'static/scripts/edit/components/HydratedDateRangeFieldset': (): Promise<mixed> => import('../static/scripts/edit/components/HydratedDateRangeFieldset.js'),
552552
'static/scripts/edit/components/InformationIcon': (): Promise<mixed> => import('../static/scripts/edit/components/InformationIcon.js'),
553+
'static/scripts/external-links-editor/components/StandaloneExternalLinksEditor': (): Promise<mixed> => import('../static/scripts/external-links-editor/components/StandaloneExternalLinksEditor.js'),
553554
'static/scripts/recording/RecordingName': (): Promise<mixed> => import('../static/scripts/recording/RecordingName.js'),
554555
'static/scripts/relationship-editor/components/RelationshipEditorWrapper': (): Promise<mixed> => import('../static/scripts/relationship-editor/components/RelationshipEditorWrapper.js'),
555556
'static/scripts/release-editor/components/EditNoteTab': (): Promise<mixed> => import('../static/scripts/release-editor/components/EditNoteTab.js'),

root/static/scripts/edit/MB/edit.js

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import linkedEntities from '../../common/linkedEntities.mjs';
1515
import {compactMap, sortByNumber} from '../../common/utility/arrays.js';
1616
import clean from '../../common/utility/clean.js';
1717
import deepEqual from '../../common/utility/deepEqual.js';
18+
import isDatabaseRowId from '../../common/utility/isDatabaseRowId.js';
1819
import request from '../../common/utility/request.js';
1920

2021
const edit = {TYPES};
@@ -86,33 +87,28 @@ var fields = edit.fields = {
8687
return {names};
8788
},
8889

89-
externalLinkRelationship(link, source) {
90+
externalLinkRelationship(release, urlEntity, relationship) {
91+
const relationshipId = number(relationship.id);
9092
var editData = {
91-
id: number(link.relationship),
92-
linkTypeID: number(link.type),
93-
attributes: [],
93+
id: isDatabaseRowId(relationshipId) ? relationshipId : null,
94+
linkTypeID: number(relationship.linkTypeID),
95+
attributes: relationship.video
96+
? [{type: {gid: VIDEO_ATTRIBUTE_GID}}]
97+
: [],
9498
entities: [
95-
this.relationshipEntity(source),
96-
{entityType: 'url', name: string(link.url)},
99+
this.relationshipEntity(release),
100+
this.relationshipEntity(urlEntity),
97101
],
98102
};
99103

100-
if (source.entityType > 'url') {
101-
editData.entities.reverse();
102-
}
103-
104-
if (link.video) {
105-
editData.attributes = [{type: {gid: VIDEO_ATTRIBUTE_GID}}];
106-
}
107-
108-
editData.begin_date = fields.partialDate(link.begin_date);
109-
editData.end_date = fields.partialDate(link.end_date);
104+
editData.begin_date = fields.partialDate(relationship.beginDate);
105+
editData.end_date = fields.partialDate(relationship.endDate);
110106

111107
if (editData.end_date &&
112108
Object.values(editData.end_date).some(nonEmpty)) {
113109
editData.ended = true;
114110
} else {
115-
editData.ended = Boolean(value(link.ended));
111+
editData.ended = Boolean(value(relationship.ended));
116112
}
117113

118114
return editData;

root/static/scripts/edit/URLCleanup.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7771,7 +7771,7 @@ export class Checker {
77717771
selectedTypes: $ReadOnlyArray<string>,
77727772
allowedTypes: $ReadOnlyArray<RelationshipTypeT> | null,
77737773
): ValidationResult {
7774-
if (!allowedTypes) {
7774+
if (allowedTypes == null || allowedTypes.length === 0) {
77757775
return {result: true};
77767776
}
77777777
// Only a single type is selected

root/static/scripts/edit/components/RelationshipPendingEditsWarning.js

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,18 @@
99

1010
import openEditsForRelIconUrl
1111
from '../../../images/icons/open_edits_for_rel.svg';
12-
import type {
13-
LinkRelationshipT,
14-
} from '../../external-links-editor/types.js';
15-
import type {
16-
RelationshipStateT,
17-
} from '../../relationship-editor/types.js';
18-
import getOpenEditsLink
19-
from '../../relationship-editor/utility/getOpenEditsLink.js';
12+
import getOpenEditsLink, {
13+
type RelationshipEntityPropertiesT,
14+
} from '../../relationship-editor/utility/getOpenEditsLink.js';
2015

2116
import Tooltip from './Tooltip.js';
2217

2318
component RelationshipPendingEditsWarning(
24-
relationship: LinkRelationshipT | RelationshipStateT
19+
relationship: $ReadOnly<{
20+
...RelationshipEntityPropertiesT,
21+
+editsPending: boolean,
22+
...
23+
}>
2524
) {
2625
const hasPendingEdits = relationship.editsPending;
2726
const openEditsLink = getOpenEditsLink(relationship);

root/static/scripts/edit/components/RemoveButton.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@
88
*/
99

1010
component RemoveButton(
11-
dataIndex?: number,
12-
onClick: (event: SyntheticEvent<HTMLButtonElement>) => void,
11+
onClick: (event: SyntheticMouseEvent<HTMLButtonElement>) => void,
1312
title: string,
1413
) {
1514
return (
1615
<button
1716
className="nobutton icon remove-item"
18-
data-index={dataIndex}
1917
onClick={onClick}
2018
title={title}
2119
type="button"

root/static/scripts/event/components/EventEditForm.js

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,16 @@ import {
4040
applyAllPendingErrors,
4141
hasSubfieldErrors,
4242
} from '../../edit/utility/subfieldErrors.js';
43-
import ExternalLinksEditor, {
44-
_ExternalLinksEditor,
45-
prepareExternalLinksHtmlFormSubmission,
46-
} from '../../external-links-editor/components/ExternalLinksEditor.js';
43+
import useExternalLinksEditor
44+
from '../../external-links-editor/hooks/useExternalLinksEditor.js';
45+
import {
46+
createInitialState as createExternalLinksEditorState,
47+
reducer as externalLinksEditorReducer,
48+
} from '../../external-links-editor/state.js';
49+
import type {
50+
LinksEditorActionT,
51+
LinksEditorStateT,
52+
} from '../../external-links-editor/types.js';
4753
import {
4854
NonHydratedRelationshipEditorWrapper as RelationshipEditorWrapper,
4955
} from '../../relationship-editor/components/RelationshipEditorWrapper.js';
@@ -55,18 +61,27 @@ type ActionT =
5561
| {+type: 'show-all-pending-errors'}
5662
| {+type: 'toggle-type-bubble'}
5763
| {+type: 'update-date-range', +action: DateRangeFieldsetActionT}
64+
| {+type: 'update-external-links-editor', +action: LinksEditorActionT}
5865
| {+type: 'update-name', +action: NameActionT};
5966
/* eslint-enable ft-flow/sort-keys */
6067

6168
type StateT = {
69+
+externalLinksEditor: LinksEditorStateT,
6270
+form: EventFormT,
6371
+guessCaseOptions: GuessCaseOptionsStateT,
6472
+isGuessCaseOptionsOpen: boolean,
6573
+showTypeBubble: boolean,
6674
};
6775

68-
function createInitialState(form: EventFormT) {
76+
function createInitialState({
77+
$c,
78+
form,
79+
}: {
80+
+$c: SanitizedCatalystContextT,
81+
+form: EventFormT,
82+
}) {
6983
return {
84+
externalLinksEditor: createExternalLinksEditorState($c),
7085
form,
7186
guessCaseOptions: createGuessCaseOptionsState(),
7287
isGuessCaseOptionsOpen: false,
@@ -111,6 +126,12 @@ function reducer(state: StateT, action: ActionT): StateT {
111126
.set('guessCaseOptions', nameState.guessCaseOptions)
112127
.set('isGuessCaseOptionsOpen', nameState.isGuessCaseOptionsOpen);
113128
}
129+
{type: 'update-external-links-editor', const action} => {
130+
newStateCtx.set(
131+
'externalLinksEditor',
132+
externalLinksEditorReducer(state.externalLinksEditor, action),
133+
);
134+
}
114135
{type: 'toggle-type-bubble'} => {
115136
newStateCtx.set('showTypeBubble', true);
116137
}
@@ -153,7 +174,7 @@ component EventEditForm(
153174

154175
const [state, dispatch] = React.useReducer(
155176
reducer,
156-
initialForm,
177+
{$c, form: initialForm},
157178
createInitialState,
158179
);
159180

@@ -183,11 +204,12 @@ component EventEditForm(
183204
dispatch({action, type: 'update-date-range'});
184205
}, [dispatch]);
185206

186-
const hasErrors = hasSubfieldErrors(state.form);
207+
const externalLinksEditor = useExternalLinksEditor(state, dispatch);
187208

188-
const eventEntity: EventT = getSourceEntityData($c, 'event');
209+
const hasErrors = hasSubfieldErrors(state.form) ||
210+
externalLinksEditor.hasErrors;
189211

190-
const externalLinksEditorRef = React.createRef<_ExternalLinksEditor>();
212+
const eventEntity: EventT = getSourceEntityData($c, 'event');
191213

192214
// Ensure errors are shown if the user tries to submit with Enter
193215
const handleKeyDown = (event: SyntheticKeyboardEvent<HTMLFormElement>) => {
@@ -201,11 +223,6 @@ component EventEditForm(
201223
dispatch({type: 'show-all-pending-errors'});
202224
event.preventDefault();
203225
}
204-
invariant(externalLinksEditorRef.current);
205-
prepareExternalLinksHtmlFormSubmission(
206-
'edit-event',
207-
externalLinksEditorRef.current,
208-
);
209226
};
210227

211228
const typeSelectRef = React.useRef<HTMLDivElement | null>(null);
@@ -299,14 +316,7 @@ component EventEditForm(
299316
seededRelationships={$c.stash.seeded_relationships}
300317
/>
301318

302-
<fieldset>
303-
<legend>{l('External links')}</legend>
304-
<ExternalLinksEditor
305-
isNewEntity={!eventEntity.id}
306-
ref={externalLinksEditorRef}
307-
sourceData={eventEntity}
308-
/>
309-
</fieldset>
319+
{externalLinksEditor.element}
310320

311321
<EnterEditNote field={state.form.field.edit_note} />
312322
<EnterEdit disabled={hasErrors} form={state.form} />

0 commit comments

Comments
 (0)