From 6f992e198acd0f2dff0fa6d1474f7ab3368a1778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Tamargo?= Date: Thu, 23 Feb 2023 13:07:45 +0200 Subject: [PATCH 1/2] MBS-12622: Show if URL rel has pending edits in links editor We now show a special icon on the (non-URL) relationship editor if a relationship that can be changed has (previous) pending edits. This adds the same icon to the URL editor, where nothing was being shown currently. This should help avoid unnecessary URL removals when someone is already setting it as ended, for example. The externalLinks code does slightly odd things to entity/relationship data to put them into state, so flow typing here is pretty messy - unsure if it can be improved without a proper refactoring of the externalLinks component though. --- .../RelationshipPendingEditsWarning.js | 57 +++++++++++++++++++ root/static/scripts/edit/externalLinks.js | 42 +++++++++++--- .../components/RelationshipItem.js | 28 +-------- .../utility/getOpenEditsLink.js | 20 +++++-- 4 files changed, 110 insertions(+), 37 deletions(-) create mode 100644 root/static/scripts/edit/components/RelationshipPendingEditsWarning.js diff --git a/root/static/scripts/edit/components/RelationshipPendingEditsWarning.js b/root/static/scripts/edit/components/RelationshipPendingEditsWarning.js new file mode 100644 index 00000000000..034cfda824e --- /dev/null +++ b/root/static/scripts/edit/components/RelationshipPendingEditsWarning.js @@ -0,0 +1,57 @@ +/* + * @flow strict-local + * 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 * as React from 'react'; + +import openEditsForRelIconUrl + from '../../../images/icons/open_edits_for_rel.svg'; +import type { + RelationshipStateT, +} from '../../relationship-editor/types.js'; +import getOpenEditsLink + from '../../relationship-editor/utility/getOpenEditsLink.js'; +import type { + LinkRelationshipT, +} from '../externalLinks.js'; + +import Tooltip from './Tooltip.js'; + +type PropsT = { + +relationship: LinkRelationshipT | RelationshipStateT, +}; + +const RelationshipPendingEditsWarning = ({ + relationship, +}: PropsT): React$Element | null => { + const hasPendingEdits = relationship.editsPending; + const openEditsLink = getOpenEditsLink(relationship); + + return hasPendingEdits && nonEmpty(openEditsLink) ? ( + <> + {' '} + + } + /> + + ) : null; +}; + +export default RelationshipPendingEditsWarning; diff --git a/root/static/scripts/edit/externalLinks.js b/root/static/scripts/edit/externalLinks.js index 0603953a10c..53212407adb 100644 --- a/root/static/scripts/edit/externalLinks.js +++ b/root/static/scripts/edit/externalLinks.js @@ -42,6 +42,8 @@ import {isMalware} from '../url/utility/isGreyedOut.js'; import ExternalLinkAttributeDialog from './components/ExternalLinkAttributeDialog.js'; import HelpIcon from './components/HelpIcon.js'; +import RelationshipPendingEditsWarning + from './components/RelationshipPendingEditsWarning.js'; import RemoveButton from './components/RemoveButton.js'; import URLInputPopover from './components/URLInputPopover.js'; import withLoadedTypeInfo from './components/withLoadedTypeInfo.js'; @@ -79,6 +81,18 @@ type LinkTypeOptionT = { export type LinkStateT = $ReadOnly<{ ...DatePeriodRoleT, +deleted: boolean, + +editsPending: boolean, + +entity0?: + | RelatableEntityT + | { + +entityType: RelatableEntityTypeT, + +id?: void, + +isNewEntity?: true, + +name?: string, + +orderingTypeID?: number, + +relationships?: void, + }, + +entity1?: RelatableEntityT, +pendingTypes?: $ReadOnlyArray, +rawUrl: string, // New relationships will use a unique string ID like "new-1". @@ -139,7 +153,7 @@ export class _ExternalLinksEditor const sourceData = props.sourceData; const sourceType = sourceData.entityType; const entityTypes = [sourceType, 'url'].sort().join('-'); - let initialLinks = parseRelationships(sourceData.relationships); + let initialLinks = parseRelationships(sourceData); initialLinks.sort(function (a, b) { const typeA = a.type && linkedEntities.link_type[a.type]; @@ -1237,6 +1251,7 @@ const ExternalLinkRelationship = {link.url && !link.error && !hasUrlError ? : null} + {hasDate ? ( {' '} @@ -1550,8 +1565,11 @@ export const ExternalLinksEditor: const defaultLinkState: LinkStateT = { begin_date: EMPTY_PARTIAL_DATE, deleted: false, + editsPending: false, end_date: EMPTY_PARTIAL_DATE, ended: false, + entity0: undefined, + entity1: undefined, rawUrl: '', relationship: null, submitted: false, @@ -1608,15 +1626,18 @@ const isVideoAttribute = (attr: LinkAttrT) => attr.type.gid === VIDEO_ATTRIBUTE_GID; export function parseRelationships( - relationships?: $ReadOnlyArray, ): Array { + const relationships = sourceData?.relationships; if (!relationships) { return []; } @@ -1626,8 +1647,11 @@ export function parseRelationships( accum.push({ begin_date: data.begin_date || EMPTY_PARTIAL_DATE, deleted: false, + editsPending: data.editsPending, end_date: data.end_date || EMPTY_PARTIAL_DATE, ended: data.ended || false, + entity0: sourceData || undefined, + entity1: target, rawUrl: target.name, relationship: data.id, submitted: true, @@ -1749,6 +1773,8 @@ type InitialOptionsT = { +entityType: RelatableEntityTypeT, +id?: void, +isNewEntity?: true, + +name?: string, + +orderingTypeID?: number, +relationships?: void, }, }; diff --git a/root/static/scripts/relationship-editor/components/RelationshipItem.js b/root/static/scripts/relationship-editor/components/RelationshipItem.js index 64063900e2f..7dea02afda2 100644 --- a/root/static/scripts/relationship-editor/components/RelationshipItem.js +++ b/root/static/scripts/relationship-editor/components/RelationshipItem.js @@ -12,8 +12,6 @@ import * as tree from 'weight-balanced-tree'; import openEditsForEntityIconUrl from '../../../images/icons/open_edits_for_entity.svg'; -import openEditsForRelIconUrl - from '../../../images/icons/open_edits_for_rel.svg'; import ButtonPopover from '../../common/components/ButtonPopover.js'; import DescriptiveLink from '../../common/components/DescriptiveLink.js'; import {bracketedText} from '../../common/utility/bracketed.js'; @@ -29,6 +27,8 @@ import { } from '../../common/utility/isLinkTypeDirectionOrderable.js'; import relationshipDateText from '../../common/utility/relationshipDateText.js'; +import RelationshipPendingEditsWarning + from '../../edit/components/RelationshipPendingEditsWarning.js'; import Tooltip from '../../edit/components/Tooltip.js'; import { getPhraseAndExtraAttributesText, @@ -46,7 +46,6 @@ import type { RelationshipEditorActionT, } from '../types/actions.js'; import getLinkPhrase from '../utility/getLinkPhrase.js'; -import getOpenEditsLink from '../utility/getOpenEditsLink.js'; import getRelationshipKey from '../utility/getRelationshipKey.js'; import getRelationshipLinkType from '../utility/getRelationshipLinkType.js'; import getRelationshipStatusName @@ -83,9 +82,7 @@ const RelationshipItem = (React.memo(({ const [sourceCredit, targetCredit] = backward ? [relationship.entity1_credit, relationship.entity0_credit] : [relationship.entity0_credit, relationship.entity1_credit]; - const relHasPendingEdits = relationship.editsPending; const targetHasPendingEdits = Boolean(target.editsPending); - const openEditsLink = getOpenEditsLink(relationship); const isRemoved = relationship._status === REL_STATUS_REMOVE; const removeButtonId = 'remove-relationship-' + getRelationshipKey(relationship); @@ -305,26 +302,7 @@ const RelationshipItem = (React.memo(({ /> ) : null} - {relHasPendingEdits && nonEmpty(openEditsLink) ? ( - <> - {' '} - - } - /> - - ) : null} + {datesAndAttributes} diff --git a/root/static/scripts/relationship-editor/utility/getOpenEditsLink.js b/root/static/scripts/relationship-editor/utility/getOpenEditsLink.js index 9e6119780c9..842c152f807 100644 --- a/root/static/scripts/relationship-editor/utility/getOpenEditsLink.js +++ b/root/static/scripts/relationship-editor/utility/getOpenEditsLink.js @@ -1,5 +1,5 @@ /* - * @flow strict + * @flow strict-local * Copyright (C) 2022 MetaBrainz Foundation * * This file is part of MusicBrainz, the open internet music database, @@ -8,15 +8,27 @@ */ import isDatabaseRowId from '../../common/utility/isDatabaseRowId.js'; +import type { + LinkRelationshipT, +} from '../../edit/externalLinks.js'; import type { RelationshipStateT, } from '../types.js'; export default function getOpenEditsLink( - relationship: RelationshipStateT, + relationship: LinkRelationshipT | RelationshipStateT, ): string | null { const entity0 = relationship.entity0; const entity1 = relationship.entity1; + if (!entity0 || !entity1) { + return null; + } + + const entity0Name = entity0.name; + const entity1Name = entity1.name; + if (empty(entity0Name) || empty(entity1Name)) { + return null; + } if (!isDatabaseRowId(entity0.id) || !isDatabaseRowId(entity1.id)) { return null; @@ -26,11 +38,11 @@ export default function getOpenEditsLink( '/search/edits?auto_edit_filter=&order=desc&negation=0&combinator=and' + `&conditions.0.field=${encodeURIComponent(entity0.entityType)}` + '&conditions.0.operator=%3D' + - `&conditions.0.name=${encodeURIComponent(entity0.name)}` + + `&conditions.0.name=${encodeURIComponent(entity0Name)}` + `&conditions.0.args.0=${encodeURIComponent(String(entity0.id))}` + `&conditions.1.field=${encodeURIComponent(entity1.entityType)}` + '&conditions.1.operator=%3D' + - `&conditions.1.name=${encodeURIComponent(entity1.name)}` + + `&conditions.1.name=${encodeURIComponent(entity1Name)}` + `&conditions.1.args.0=${encodeURIComponent(String(entity1.id))}` + '&conditions.2.field=type' + '&conditions.2.operator=%3D&conditions.2.args=90%2C233' + From 796a3d267d00a220dba4a39eaf544f3634261965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Tamargo?= Date: Thu, 23 Feb 2023 13:44:03 +0200 Subject: [PATCH 2/2] MBS-12622: Show if URL has pending edits in links editor If a URL is being edited, we should indicate it in the links editor. This way it is possible to find out that, say, the URL you are adding or changing the relationship for is actually going to change soon, otherwise making your changes have unexpected consequences. --- .../components/EntityPendingEditsWarning.js | 50 +++++++++++++++++++ root/static/scripts/edit/externalLinks.js | 9 +++- .../components/RelationshipItem.js | 28 ++--------- 3 files changed, 61 insertions(+), 26 deletions(-) create mode 100644 root/static/scripts/edit/components/EntityPendingEditsWarning.js diff --git a/root/static/scripts/edit/components/EntityPendingEditsWarning.js b/root/static/scripts/edit/components/EntityPendingEditsWarning.js new file mode 100644 index 00000000000..bd15cb201ae --- /dev/null +++ b/root/static/scripts/edit/components/EntityPendingEditsWarning.js @@ -0,0 +1,50 @@ +/* + * @flow strict-local + * 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 * as React from 'react'; + +import openEditsForEntityIconUrl + from '../../../images/icons/open_edits_for_entity.svg'; +import entityHref from '../../common/utility/entityHref.js'; + +import Tooltip from './Tooltip.js'; + +type PropsT = { + +entity: RelatableEntityT, +}; + +const EntityPendingEditsWarning = ({ + entity, +}: PropsT): React$Element | null => { + const hasPendingEdits = Boolean(entity.editsPending); + const openEditsLink = entityHref(entity, '/open_edits'); + + return hasPendingEdits && nonEmpty(openEditsLink) ? ( + <> + {' '} + + } + /> + + ) : null; +}; + +export default EntityPendingEditsWarning; diff --git a/root/static/scripts/edit/externalLinks.js b/root/static/scripts/edit/externalLinks.js index 53212407adb..7bfecbb9c2b 100644 --- a/root/static/scripts/edit/externalLinks.js +++ b/root/static/scripts/edit/externalLinks.js @@ -39,6 +39,8 @@ import { } from '../relationship-editor/utility/prepareHtmlFormSubmission.js'; import {isMalware} from '../url/utility/isGreyedOut.js'; +import EntityPendingEditsWarning + from './components/EntityPendingEditsWarning.js'; import ExternalLinkAttributeDialog from './components/ExternalLinkAttributeDialog.js'; import HelpIcon from './components/HelpIcon.js'; @@ -937,7 +939,7 @@ export class _ExternalLinksEditor * The first element of tuple `item` is not the URL * when the URL is not submitted therefore isn't grouped. */ - const {url, rawUrl} = relationships[0]; + const {url, rawUrl, entity1} = relationships[0]; const isLastLink = index === linksByUrl.length - 1; const links = [...relationships]; const linkIndexes = []; @@ -1072,6 +1074,7 @@ export class _ExternalLinksEditor relationships={links} typeOptions={typeOptions} url={url} + urlEntity={entity1} urlMatchesType={urlMatchesType} validateLink={(link) => this.validateLink(link)} /> @@ -1315,6 +1318,7 @@ type LinkProps = { +relationships: $ReadOnlyArray, +typeOptions: $ReadOnlyArray, +url: string, + +urlEntity?: RelatableEntityT, +urlMatchesType: boolean, +validateLink: (LinkRelationshipT | LinkStateT) => ErrorT | null, }; @@ -1457,6 +1461,9 @@ export class ExternalLink extends React.Component { {props.url} )} + {props.urlEntity ? ( + + ) : null} {props.url && props.duplicate !== null ? (
(({ const [sourceCredit, targetCredit] = backward ? [relationship.entity1_credit, relationship.entity0_credit] : [relationship.entity0_credit, relationship.entity1_credit]; - const targetHasPendingEdits = Boolean(target.editsPending); const isRemoved = relationship._status === REL_STATUS_REMOVE; const removeButtonId = 'remove-relationship-' + getRelationshipKey(relationship); @@ -282,26 +279,7 @@ const RelationshipItem = (React.memo(({ }) ) : targetDisplay} - {targetHasPendingEdits ? ( - <> - {' '} - - } - /> - - ) : null} + {datesAndAttributes}