Skip to content

Commit

Permalink
MBS-12622: Show if URL rel has pending edits in links editor
Browse files Browse the repository at this point in the history
We now show a warning icon on the (non-URL) relationship editor
if a relationship that can be changed has (previous) pending edits.

This adds the same warning 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 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.
  • Loading branch information
reosarevok committed Mar 15, 2023
1 parent 183beaf commit 8be0a2e
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* @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<typeof React.Fragment> | null => {
const hasPendingEdits = relationship.editsPending;
const openEditsLink = getOpenEditsLink(relationship);

return hasPendingEdits && nonEmpty(openEditsLink) ? (
<>
{' '}
<Tooltip
content={exp.l(
'This relationship has {edit_search|pending edits}.',
{edit_search: openEditsLink},
)}
target={
<img
alt={l('This relationship has pending edits.')}
className="info"
height={16}
src={openEditsForRelIconUrl}
style={{verticalAlign: 'middle'}}
/>
}
/>
</>
) : null;
};

export default RelationshipPendingEditsWarning;
42 changes: 34 additions & 8 deletions root/static/scripts/edit/externalLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -79,6 +81,18 @@ type LinkTypeOptionT = {
export type LinkStateT = $ReadOnly<{
...DatePeriodRoleT,
+deleted: boolean,
+editsPending: boolean,
+entity0?:
| CoreEntityT
| {
+entityType: CoreEntityTypeT,
+id?: void,
+isNewEntity?: true,
+name?: string,
+orderingTypeID?: number,
+relationships?: void,
},
+entity1?: CoreEntityT,
+pendingTypes?: $ReadOnlyArray<number>,
+rawUrl: string,
// New relationships will use a unique string ID like "new-1".
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -1228,6 +1242,7 @@ const ExternalLinkRelationship =
{link.url && !link.error && !hasUrlError
? <TypeDescription type={link.type} url={link.url} />
: null}
<RelationshipPendingEditsWarning relationship={link} />
{hasDate ? (
<span className="date-period">
{' '}
Expand Down Expand Up @@ -1541,8 +1556,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,
Expand Down Expand Up @@ -1599,15 +1617,18 @@ const isVideoAttribute =
(attr: LinkAttrT) => attr.type.gid === VIDEO_ATTRIBUTE_GID;

export function parseRelationships(
relationships?: $ReadOnlyArray<RelationshipT | {
+id: null,
+linkTypeID?: number,
+target: {
+entityType: 'url',
+name: string,
sourceData?:
| CoreEntityT
| {
+entityType: CoreEntityTypeT,
+id?: void,
+isNewEntity?: true,
+name?: string,
+orderingTypeID?: number,
+relationships?: void,
},
}>,
): Array<LinkStateT> {
const relationships = sourceData?.relationships;
if (!relationships) {
return [];
}
Expand All @@ -1617,8 +1638,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,
Expand Down Expand Up @@ -1736,6 +1760,8 @@ type InitialOptionsT = {
+entityType: CoreEntityTypeT,
+id?: void,
+isNewEntity?: true,
+name?: string,
+orderingTypeID?: number,
+relationships?: void,
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -83,9 +82,7 @@ const RelationshipItem = (React.memo<PropsT>(({
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);
Expand Down Expand Up @@ -304,26 +301,7 @@ const RelationshipItem = (React.memo<PropsT>(({
/>
</>
) : null}
{relHasPendingEdits && nonEmpty(openEditsLink) ? (
<>
{' '}
<Tooltip
content={exp.l(
'This relationship has {edit_search|pending edits}.',
{edit_search: openEditsLink},
)}
target={
<img
alt={l('This relationship has pending edits.')}
className="info"
height={16}
src={openEditsForRelIconUrl}
style={{verticalAlign: 'middle'}}
/>
}
/>
</>
) : null}
<RelationshipPendingEditsWarning relationship={relationship} />
{datesAndAttributes}
</span>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -8,15 +8,28 @@
*/

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;
Expand All @@ -26,11 +39,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' +
Expand Down

0 comments on commit 8be0a2e

Please sign in to comment.