From ad7d9adf9f577bf3c0c1e59dd630b4df906a4b43 Mon Sep 17 00:00:00 2001 From: Joe Russack Date: Tue, 7 Apr 2026 21:35:44 -0700 Subject: [PATCH 1/2] feat: warn before editing shared records in QueryComboBox (#597) When Carry Forward copies foreign keys, multiple Collection Objects share the same related record. Clicking edit on a shared record now shows a warning dialog with record count, inspection links, and three choices: Cancel, Edit Shared, or Clone and Edit (safe default). - Generic for all table types (not just CO/CE) - Session memory checkbox to skip repeated warnings - Only shown when record is actually shared (count > 1) --- .../__tests__/sharedRecordWarning.test.tsx | 59 +++ .../lib/components/QueryComboBox/index.tsx | 341 +++++++++++++++++- .../frontend/js_src/lib/localization/forms.ts | 22 ++ 3 files changed, 402 insertions(+), 20 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/sharedRecordWarning.test.tsx diff --git a/specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/sharedRecordWarning.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/sharedRecordWarning.test.tsx new file mode 100644 index 00000000000..053fca2d8db --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/sharedRecordWarning.test.tsx @@ -0,0 +1,59 @@ +/** + * Tests for shared record edit warning (#597). + * + * When using Carry Forward, Specify copies foreign keys — so multiple + * Collection Objects share the same Locality via their Collecting Event. + * Clicking the pencil (edit) icon on a non-dependent QueryComboBox field + * mutates the shared record, silently changing data for all referencing + * records. + * + * The fix shows a warning dialog before editing non-dependent related + * records, offering Clone and Edit as the primary safe action. + */ + +import { requireContext } from '../../../tests/helpers'; +import { formsText } from '../../../localization/forms'; + +requireContext(); + +describe('Shared record edit warning', () => { + test('localization strings exist for the warning dialog', () => { + expect(typeof formsText.sharedRecordWarning()).toBe('string'); + expect(formsText.sharedRecordWarning().length).toBeGreaterThan(0); + + expect( + typeof formsText.sharedRecordWarningDescription({ + tableName: 'Locality', + count: '3', + parentTableName: 'Collecting Event', + }) + ).toBe('string'); + + expect(typeof formsText.editShared()).toBe('string'); + expect(formsText.editShared().length).toBeGreaterThan(0); + + expect(typeof formsText.cloneAndEdit()).toBe('string'); + expect(formsText.cloneAndEdit().length).toBeGreaterThan(0); + }); + + test('warning description includes the table name and count', () => { + const description = formsText.sharedRecordWarningDescription({ + tableName: 'Locality', + count: '5', + parentTableName: 'Collecting Event', + }); + expect(description).toContain('Locality'); + expect(description).toContain('5'); + expect(description).toContain('Collecting Event'); + }); + + test('warning description works for other tables', () => { + const description = formsText.sharedRecordWarningDescription({ + tableName: 'Agent', + count: '12', + parentTableName: 'Collector', + }); + expect(description).toContain('Agent'); + expect(description).toContain('12'); + }); +}); diff --git a/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx b/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx index 5c85e0d2626..7618ff7d5cd 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx @@ -4,6 +4,7 @@ import type { State } from 'typesafe-reducer'; import { useAsyncState } from '../../hooks/useAsyncState'; import { useResourceValue } from '../../hooks/useResourceValue'; +import { ajax } from '../../utils/ajax'; import { commonText } from '../../localization/common'; import { formsText } from '../../localization/forms'; import { userText } from '../../localization/user'; @@ -11,6 +12,7 @@ import { f } from '../../utils/functools'; import { getValidationAttributes } from '../../utils/parser/definitions'; import type { RA } from '../../utils/types'; import { filterArray, localized } from '../../utils/types'; +import { Button } from '../Atoms/Button'; import { DataEntry } from '../Atoms/DataEntry'; import { LoadingContext, ReadOnlyContext } from '../Core/Contexts'; import { backboneFieldSeparator, toTable } from '../DataModel/helpers'; @@ -55,6 +57,30 @@ import { useTreeData } from './useTreeData'; import { TreeDefinitionContext } from './useTreeData'; import { useTypeSearch } from './useTypeSearch'; +/** + * Maximum number of results to fetch for the typeahead dropdown. + * Kept low to avoid expensive queries on large tables (200K+ rows). + */ +export const QUERY_COMBO_BOX_SEARCH_LIMIT = 50; + +/** + * Session-scoped preference for how to handle editing shared records (#597). + * When set, the warning dialog is skipped and the remembered action is used. + */ +const SHARED_EDIT_SESSION_KEY = 'specify-shared-edit-preference'; +type SharedEditPreference = 'cloneAndEdit' | 'editShared'; + +function getSessionSharedEditPref(): SharedEditPreference | undefined { + const value = sessionStorage.getItem(SHARED_EDIT_SESSION_KEY); + return value === 'cloneAndEdit' || value === 'editShared' + ? value + : undefined; +} + +function setSessionSharedEditPref(pref: SharedEditPreference): void { + sessionStorage.setItem(SHARED_EDIT_SESSION_KEY, pref); +} + /* * REFACTOR: split this component * TEST: add tests for this @@ -215,6 +241,19 @@ export function QueryComboBox({ > | State<'AccessDeniedState', { readonly collectionName: string }> | State<'MainState'> + | State< + 'SharedRecordWarningState', + { + readonly sharingCount: number; + readonly sharingRecords: RA<{ + readonly parentId: number | undefined; + readonly parentLabel: string | undefined; + readonly parentTableName: string | undefined; + readonly sharedId: number | undefined; + readonly sharedTableName: string; + }>; + } + > | State<'ViewResourceState', { readonly isReadOnly: boolean }> >({ type: 'MainState' }); @@ -225,25 +264,180 @@ export function QueryComboBox({ const targetCollectionId = forceCollection ?? relatedCollectionId; const loading = React.useContext(LoadingContext); - const handleOpenRelated = (isReadOnly: boolean): void => - state.type === 'ViewResourceState' || state.type === 'AccessDeniedState' - ? setState({ type: 'MainState' }) - : typeof relatedCollectionId === 'number' && - !userInformation.availableCollections.some( - ({ id }) => id === relatedCollectionId - ) - ? loading( - fetchResource('Collection', relatedCollectionId).then( - (collection) => - setState({ - type: 'AccessDeniedState', - collectionName: collection?.collectionName ?? '', - }) - ) - ) - : setState({ type: 'ViewResourceState', isReadOnly }); - const subViewRelationship = React.useContext(SubViewContext)?.relationship; + + const handleOpenRelated = (isReadOnly: boolean): void => { + if ( + state.type === 'ViewResourceState' || + state.type === 'AccessDeniedState' || + state.type === 'SharedRecordWarningState' + ) { + setState({ type: 'MainState' }); + return; + } + + if ( + typeof relatedCollectionId === 'number' && + !userInformation.availableCollections.some( + ({ id }) => id === relatedCollectionId + ) + ) { + loading( + fetchResource('Collection', relatedCollectionId).then((collection) => + setState({ + type: 'AccessDeniedState', + collectionName: collection?.collectionName ?? '', + }) + ) + ); + return; + } + + /* + * For non-dependent, non-read-only edits, check if the related + * record is shared before allowing direct edits (#597). + * Carry Forward copies foreign keys, so editing a shared Locality + * (for example) silently mutates every CO that references it. + */ + if (!isReadOnly && !field.isDependent() && formatted?.resource?.id) { + const parentTableName = resource!.specifyTable.name.toLowerCase(); + const fieldName = field.name.toLowerCase(); + const relatedId = formatted.resource.id; + + /* + * Try to resolve back to CollectionObject for the display. + * If the QCB is inside a subform (e.g., CE inside CO form), + * query COs via the joined path (e.g., collectingevent__locality=X). + * Otherwise fall back to querying the direct parent table. + */ + const subView = subViewRelationship; + const canResolveToCollectionObject = + subView !== undefined && + subView.table.name === 'CollectionObject'; + + const queryTable = canResolveToCollectionObject + ? 'collectionobject' + : parentTableName; + const queryFilter = canResolveToCollectionObject + ? `${subView!.name.toLowerCase()}__${fieldName}` + : fieldName; + + loading( + (canResolveToCollectionObject + ? /* + * Query COs joined through the parent table + * (e.g., collectionobject?collectingevent__locality=X) + * and also fetch the parent ID for linking. + */ + ajax<{ + readonly meta: { readonly total_count: number }; + readonly objects: RA<{ + readonly id: number; + readonly catalogNumber?: string; + readonly catalognumber?: string; + readonly collectingEvent?: string; + readonly collectingevent?: string; + }>; + }>( + `/api/specify/${queryTable}/?${queryFilter}=${relatedId}&limit=11`, + { headers: { Accept: 'application/json' } } + ).then(({ data }) => { + const parentTable = subView!.table.name; + const sharedTable = field.relatedTable.name; + return { + totalCount: data.meta.total_count, + records: data.objects.slice(0, 10).map((obj) => { + const barcode = + obj.catalogNumber ?? obj.catalognumber ?? ''; + const objRecord = obj as Record; + const relRaw: unknown = + objRecord[subView!.name.toLowerCase()] ?? + objRecord[subView!.name]; + let relId: number | undefined; + if ( + typeof relRaw === 'object' && + relRaw !== null && + 'id' in relRaw + ) { + relId = (relRaw as { id: number }).id; + } else if (typeof relRaw === 'string' && relRaw.includes('/')) { + relId = Number.parseInt( + relRaw.split('/').filter(Boolean).pop()!, + 10 + ); + } else if (typeof relRaw === 'number') { + relId = relRaw; + } + return { + parentId: obj.id, + parentLabel: barcode || `${parentTable} #${obj.id}`, + parentTableName: parentTable, + sharedId: Number.isNaN(relId) ? undefined : relId, + sharedTableName: sharedTable, + }; + }), + }; + }) + : /* Fall back to querying the direct parent table */ + ajax<{ + readonly meta: { readonly total_count: number }; + readonly objects: RA<{ readonly id: number }>; + }>( + `/api/specify/${queryTable}/?${queryFilter}=${relatedId}&limit=11`, + { headers: { Accept: 'application/json' } } + ).then(({ data }) => ({ + totalCount: data.meta.total_count, + records: data.objects.slice(0, 10).map((obj) => ({ + parentId: undefined as number | undefined, + parentLabel: undefined as string | undefined, + parentTableName: undefined as string | undefined, + sharedId: obj.id, + sharedTableName: resource!.specifyTable.name, + })), + })) + ).then(({ totalCount, records }) => { + if (totalCount <= 1) { + setState({ type: 'ViewResourceState', isReadOnly: false }); + return; + } + + // Check session preference — skip dialog if user already chose + const sessionPref = getSessionSharedEditPref(); + if (sessionPref === 'editShared') { + setState({ type: 'ViewResourceState', isReadOnly: false }); + } else if (sessionPref === 'cloneAndEdit') { + doCloneAndEdit(); + } else { + setState({ + type: 'SharedRecordWarningState', + sharingCount: totalCount, + sharingRecords: records, + }); + } + }) + ); + return; + } + + setState({ type: 'ViewResourceState', isReadOnly }); + }; + + const doCloneAndEdit = (): void => { + const relatedResource = formatted?.resource; + if (relatedResource === undefined) return; + loading( + relatedResource.clone(true).then((clonedResource) => { + resource?.set(field.name, clonedResource as never); + setState({ + type: 'AddResourceState', + resource: clonedResource, + }); + }) + ); + }; + + const [rememberChoice, setRememberChoice] = React.useState(false); + const pendingValueRef = React.useRef(''); const relatedTable = @@ -351,8 +545,7 @@ export function QueryComboBox({ .map(async (query) => runQuery(query, { collectionId: forceCollection ?? relatedCollectionId, - // REFACTOR: allow customizing these arbitrary limits - limit: 1000, + limit: QUERY_COMBO_BOX_SEARCH_LIMIT, }) ) ).then((responses) => @@ -614,6 +807,114 @@ export function QueryComboBox({ })} )} + {state.type === 'SharedRecordWarningState' && ( + + + {commonText.cancel()} + + { + if (rememberChoice) + setSessionSharedEditPref('editShared'); + setState({ type: 'ViewResourceState', isReadOnly: false }); + }} + > + {formsText.editShared()} + + { + if (rememberChoice) + setSessionSharedEditPref('cloneAndEdit'); + doCloneAndEdit(); + }} + > + {formsText.cloneAndEdit()} + + + } + header={formsText.sharedRecordWarning()} + onClose={(): void => setState({ type: 'MainState' })} + > +

+ {formsText.sharedRecordWarningDescription({ + tableName: field.relatedTable.label, + count: state.sharingCount.toString(), + parentTableName: resource!.specifyTable.label, + })} +

+

+ {formsText.linksForInspectionOnly()} +

+ {state.sharingRecords.length > 0 && ( + + + + {state.sharingRecords.some((r) => r.parentId !== undefined) && ( + + )} + + + + + {state.sharingRecords.map((record, index) => ( + + {state.sharingRecords.some((r) => r.parentId !== undefined) && ( + + )} + + + ))} + +
+ {state.sharingRecords.find((r) => r.parentTableName)?.parentTableName + ?? resource!.specifyTable.label} + {field.relatedTable.label}
+ {record.parentId !== undefined && record.parentTableName !== undefined ? ( + + {record.parentLabel} + + ) : ( + '\u2014' + )} + + {record.sharedId !== undefined ? ( + + {`${record.sharedTableName} #${record.sharedId}`} + + ) : ( + '\u2014' + )} +
+ )} + {state.sharingCount > 10 && ( +

+ {formsText.andNMore({ count: (state.sharingCount - 10).toString() })} +

+ )} + + + )} {typeof formatted?.resource === 'object' && state.type === 'ViewResourceState' ? ( diff --git a/specifyweb/frontend/js_src/lib/localization/forms.ts b/specifyweb/frontend/js_src/lib/localization/forms.ts index d5e281fecbc..d290a0c2578 100644 --- a/specifyweb/frontend/js_src/lib/localization/forms.ts +++ b/specifyweb/frontend/js_src/lib/localization/forms.ts @@ -40,6 +40,28 @@ export const formsText = createDictionary({ 'pt-br': 'Criar uma cópia completa do registro atual', 'hr-hr': 'Izradi potpunu kopiju trenutnog zapisa', }, + sharedRecordWarning: { + 'en-us': 'Shared Record', + }, + sharedRecordWarningDescription: { + 'en-us': `This {tableName:string} record is shared by {count:string} {parentTableName:string} records. Editing it will change the data for all of them. To make changes only for this record, clone it first.`, + }, + editShared: { + 'en-us': 'Edit Shared', + }, + cloneAndEdit: { + 'en-us': 'Clone and Edit', + }, + rememberChoiceForSession: { + 'en-us': 'Use this choice for the rest of this session', + }, + linksForInspectionOnly: { + 'en-us': + 'Links open in a new tab for inspection only \u2014 no changes will be made.', + }, + andNMore: { + 'en-us': '\u2026 and {count:string} more', + }, valueMustBeUniqueToField: { 'en-us': 'Value must be unique to {fieldName:string}', 'ru-ru': 'Значение должно быть уникальным для {fieldName:string}', From 87e3bcafec98d6d619dd974824a2f7781de368b7 Mon Sep 17 00:00:00 2001 From: Joe Russack Date: Mon, 27 Apr 2026 16:47:00 +0000 Subject: [PATCH 2/2] Lint code with ESLint and Prettier Triggered by ad7d9adf9f577bf3c0c1e59dd630b4df906a4b43 on branch refs/heads/issue-cas-597 --- .../lib/components/QueryComboBox/index.tsx | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx b/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx index 7618ff7d5cd..ef128d760bc 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx @@ -72,9 +72,7 @@ type SharedEditPreference = 'cloneAndEdit' | 'editShared'; function getSessionSharedEditPref(): SharedEditPreference | undefined { const value = sessionStorage.getItem(SHARED_EDIT_SESSION_KEY); - return value === 'cloneAndEdit' || value === 'editShared' - ? value - : undefined; + return value === 'cloneAndEdit' || value === 'editShared' ? value : undefined; } function setSessionSharedEditPref(pref: SharedEditPreference): void { @@ -312,8 +310,7 @@ export function QueryComboBox({ */ const subView = subViewRelationship; const canResolveToCollectionObject = - subView !== undefined && - subView.table.name === 'CollectionObject'; + subView !== undefined && subView.table.name === 'CollectionObject'; const queryTable = canResolveToCollectionObject ? 'collectionobject' @@ -347,8 +344,7 @@ export function QueryComboBox({ return { totalCount: data.meta.total_count, records: data.objects.slice(0, 10).map((obj) => { - const barcode = - obj.catalogNumber ?? obj.catalognumber ?? ''; + const barcode = obj.catalogNumber ?? obj.catalognumber ?? ''; const objRecord = obj as Record; const relRaw: unknown = objRecord[subView!.name.toLowerCase()] ?? @@ -360,7 +356,10 @@ export function QueryComboBox({ 'id' in relRaw ) { relId = (relRaw as { id: number }).id; - } else if (typeof relRaw === 'string' && relRaw.includes('/')) { + } else if ( + typeof relRaw === 'string' && + relRaw.includes('/') + ) { relId = Number.parseInt( relRaw.split('/').filter(Boolean).pop()!, 10 @@ -811,13 +810,10 @@ export function QueryComboBox({ - - {commonText.cancel()} - + {commonText.cancel()} { - if (rememberChoice) - setSessionSharedEditPref('editShared'); + if (rememberChoice) setSessionSharedEditPref('editShared'); setState({ type: 'ViewResourceState', isReadOnly: false }); }} > @@ -851,10 +847,12 @@ export function QueryComboBox({ - {state.sharingRecords.some((r) => r.parentId !== undefined) && ( + {state.sharingRecords.some( + (r) => r.parentId !== undefined + ) && ( )} @@ -863,9 +861,12 @@ export function QueryComboBox({ {state.sharingRecords.map((record, index) => ( - {state.sharingRecords.some((r) => r.parentId !== undefined) && ( + {state.sharingRecords.some( + (r) => r.parentId !== undefined + ) && (
- {state.sharingRecords.find((r) => r.parentTableName)?.parentTableName - ?? resource!.specifyTable.label} + {state.sharingRecords.find((r) => r.parentTableName) + ?.parentTableName ?? resource!.specifyTable.label} {field.relatedTable.label}
- {record.parentId !== undefined && record.parentTableName !== undefined ? ( + {record.parentId !== undefined && + record.parentTableName !== undefined ? ( 10 && (

- {formsText.andNMore({ count: (state.sharingCount - 10).toString() })} + {formsText.andNMore({ + count: (state.sharingCount - 10).toString(), + })}

)}