From 726e3f497dd18cd344672ca41e796a89a5193f25 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Wed, 7 Oct 2026 11:06:43 +0200 Subject: [PATCH 1/3] Fix: suspend Data Views record preview during merging --- .../js_src/lib/components/DataViews/index.tsx | 9 ++++ .../lib/components/QueryBuilder/ToForms.tsx | 6 ++- .../__tests__/QueryFormView.test.tsx | 53 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx diff --git a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx index d3e5697e304..8dfbe188f5e 100644 --- a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { useParams } from 'react-router-dom'; +import { useSearchParameter } from '../../hooks/navigation'; import { commonText } from '../../localization/common'; import { dataViewsText } from '../../localization/dataViews'; import { useResponsiveSplitView } from '../../hooks/useResponsiveSplitView'; @@ -10,6 +11,7 @@ import { DataEntry } from '../Atoms/DataEntry'; import { getTable } from '../DataModel/tables'; import type { Tables } from '../DataModel/types'; import { raise } from '../Errors/Crash'; +import { mergingQueryParameter } from '../Merging/queryString'; import { Dialog } from '../Molecules/Dialog'; import { TableIcon } from '../Molecules/TableIcon'; import { hasPermission } from '../Permissions/helpers'; @@ -28,6 +30,7 @@ import { } from '../QueryBuilder/SplitView'; import { QueryFormView } from '../QueryBuilder/ToForms'; import { NotFoundView } from '../Router/NotFoundView'; +import { OverlayLocation } from '../Router/Router'; import type { DataViewQueriesFile } from './queries'; import { getDataViewQueryDefinition, @@ -95,6 +98,11 @@ function LoadedDataViewFromTable({ readonly queries: DataViewQueriesFile; readonly reloadQueries: () => void; }): JSX.Element | null { + const overlayLocation = React.useContext(OverlayLocation); + const [mergingRecords] = useSearchParameter( + mergingQueryParameter, + overlayLocation + ); const table = getTable(tableName); const [selectedIds, setSelectedIds] = React.useState>( [] @@ -320,6 +328,7 @@ function LoadedDataViewFromTable({ onDelete, }) => ( ; @@ -123,7 +126,8 @@ export function QueryFormView({ readonly onSlide: (index: number) => void; }): JSX.Element | null { const ids = useSelectedResults(results, selectedRows, true, totalCount); - if (!hasFetchableRecordIds(results) || ids.length === 0) return null; + if (suspended || !hasFetchableRecordIds(results) || ids.length === 0) + return null; return (
diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx new file mode 100644 index 00000000000..b44ae42fcf2 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx @@ -0,0 +1,53 @@ +import { render, screen } from '@testing-library/react'; +import React from 'react'; + +import { commonText } from '../../../localization/common'; +import { requireContext } from '../../../tests/helpers'; +import { tables } from '../../DataModel/tables'; +import { RecordSelectorFromIds } from '../../FormSliders/RecordSelectorFromIds'; +import { QueryFormView } from '../ToForms'; + +requireContext(); + +jest.mock('../../FormSliders/RecordSelectorFromIds', () => ({ + RecordSelectorFromIds: jest.fn(() =>
), +})); + +test('unmounts the record preview during merging and restores surviving records', () => { + const props = { + table: tables.Agent, + title: commonText.view(), + results: [[38665], [38666]], + selectedRows: new Set([38665, 38666]), + selectedIndex: 1, + totalCount: 2, + onFetchMore: undefined, + onDelete: jest.fn(), + onClose: jest.fn(), + onSaved: jest.fn(), + onSlide: jest.fn(), + }; + const { rerender } = render(); + expect(screen.getByTestId('record-preview')).toBeInTheDocument(); + + jest.mocked(RecordSelectorFromIds).mockClear(); + rerender(); + expect(screen.queryByTestId('record-preview')).not.toBeInTheDocument(); + expect(RecordSelectorFromIds).not.toHaveBeenCalled(); + + rerender( + + ); + expect(screen.getByTestId('record-preview')).toBeInTheDocument(); + expect(RecordSelectorFromIds).toHaveBeenLastCalledWith( + expect.objectContaining({ ids: [38665] }), + expect.anything() + ); +}); From 5ac440b01e541d1ed40a56746250dddb9b14f1f6 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Wed, 7 Oct 2026 12:57:06 +0200 Subject: [PATCH 2/3] Fix: Protect unsaved preview edits before unmounting the form. --- .../FormSliders/RecordSelectorFromIds.tsx | 17 +++- .../lib/components/QueryBuilder/ToForms.tsx | 32 +++++-- .../__tests__/QueryFormView.test.tsx | 58 +++++++++--- .../SuspendedRecordSelector.test.tsx | 91 +++++++++++++++++++ 4 files changed, 176 insertions(+), 22 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx index 590c359e932..8c177399a1b 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx @@ -29,6 +29,7 @@ import { useRecordSelector } from './RecordSelector'; * IDs */ export function RecordSelectorFromIds({ + suspended = false, ids, newResource, onSlide: handleSlide, @@ -58,6 +59,8 @@ export function RecordSelectorFromIds({ * sets or query results with thousands of items) */ readonly ids: RA; + // Keep the current form and resource intact while a merge overlay is open. + readonly suspended?: boolean; readonly newResource: SpecifyResource | undefined; readonly title: LocalizedString | undefined; readonly headerButtons?: JSX.Element; @@ -86,6 +89,7 @@ export function RecordSelectorFromIds({ ); React.useEffect(() => { + if (suspended) return; setRecords((records) => ids.map((id) => { if (id === undefined) return undefined; @@ -96,11 +100,14 @@ export function RecordSelectorFromIds({ ); }) ); - }, [ids, table]); + }, [ids, table, suspended]); - const [rawIndex, setIndex] = useTriggerState( + const defaultIndexRef = React.useRef( Math.max(0, defaultIndex ?? ids.length - 1) ); + if (!suspended) + defaultIndexRef.current = Math.max(0, defaultIndex ?? ids.length - 1); + const [rawIndex, setIndex] = useTriggerState(defaultIndexRef.current); const index = typeof newResource === 'object' ? totalCount - 1 @@ -117,7 +124,7 @@ export function RecordSelectorFromIds({ const { dialogs, slider, - resource, + resource: selectedResource, onAdd: handleAdding, onRemove: handleRemove, isLoading, @@ -150,6 +157,7 @@ export function RecordSelectorFromIds({ } : undefined, onSlide: (index, replace, callback): void => { + if (suspended) return; function doSlide(): void { setIndex(index); handleSlide?.(index, replace); @@ -169,6 +177,9 @@ export function RecordSelectorFromIds({ else doSlide(); }, }); + const resourceRef = React.useRef(selectedResource); + if (!suspended) resourceRef.current = selectedResource; + const resource = resourceRef.current; const addLabel = isInRecordSet ? formsText.addToRecordSet({ diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx index 2311ddc38bd..8cb8e024e3c 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx @@ -126,22 +126,39 @@ export function QueryFormView({ readonly onSlide: (index: number) => void; }): JSX.Element | null { const ids = useSelectedResults(results, selectedRows, true, totalCount); - if (suspended || !hasFetchableRecordIds(results) || ids.length === 0) - return null; + const previewRef = React.useRef({ + ids, + selectedIndex, + totalCount: selectedRows.size === 0 ? totalCount : selectedRows.size, + hasRecords: !suspended && hasFetchableRecordIds(results) && ids.length > 0, + }); + if (!suspended) + previewRef.current = { + ids, + selectedIndex, + totalCount: selectedRows.size === 0 ? totalCount : selectedRows.size, + hasRecords: hasFetchableRecordIds(results) && ids.length > 0, + }; + const preview = previewRef.current; + if (!preview.hasRecords) return null; return ( -
+
{ await handleFetchMore(index); @@ -162,6 +179,7 @@ export function QueryFormView({ } onSaved={handleSaved} onSlide={(index): void => { + if (suspended) return; handleSlide(index); if (selectedRows.size === 0 && results[index] === undefined) void handleFetchMore?.(index); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx index b44ae42fcf2..d442bf3c1f6 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import React from 'react'; import { commonText } from '../../../localization/common'; @@ -9,11 +9,23 @@ import { QueryFormView } from '../ToForms'; requireContext(); -jest.mock('../../FormSliders/RecordSelectorFromIds', () => ({ - RecordSelectorFromIds: jest.fn(() =>
), -})); +jest.mock('../../FormSliders/RecordSelectorFromIds', () => { + const actualReact = jest.requireActual('react'); + return { + RecordSelectorFromIds: jest.fn(function Preview() { + const [value, setValue] = actualReact.useState(''); + return ( + setValue(event.target.value)} + /> + ); + }), + }; +}); -test('unmounts the record preview during merging and restores surviving records', () => { +test('preserves unsaved preview edits when merging is cancelled', () => { const props = { table: tables.Agent, title: commonText.view(), @@ -21,19 +33,41 @@ test('unmounts the record preview during merging and restores surviving records' selectedRows: new Set([38665, 38666]), selectedIndex: 1, totalCount: 2, - onFetchMore: undefined, + onFetchMore: jest.fn(), onDelete: jest.fn(), onClose: jest.fn(), onSaved: jest.fn(), onSlide: jest.fn(), }; const { rerender } = render(); - expect(screen.getByTestId('record-preview')).toBeInTheDocument(); + const input = screen.getByRole('textbox', { name: 'Record preview' }); + fireEvent.change(input, { target: { value: 'Unsaved agent name' } }); + + rerender( + + ); + expect(input).toBeInTheDocument(); + expect(input).toHaveValue('Unsaved agent name'); + expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); + expect(RecordSelectorFromIds).toHaveBeenLastCalledWith( + expect.objectContaining({ + ids: [38665, 38666], + defaultIndex: 1, + suspended: true, + onFetch: undefined, + }), + expect.anything() + ); - jest.mocked(RecordSelectorFromIds).mockClear(); - rerender(); - expect(screen.queryByTestId('record-preview')).not.toBeInTheDocument(); - expect(RecordSelectorFromIds).not.toHaveBeenCalled(); + rerender(); + expect(screen.getByRole('textbox')).toBe(input); + expect(input).toHaveValue('Unsaved agent name'); rerender( ); - expect(screen.getByTestId('record-preview')).toBeInTheDocument(); + expect(screen.getByRole('textbox')).toBeInTheDocument(); expect(RecordSelectorFromIds).toHaveBeenLastCalledWith( expect.objectContaining({ ids: [38665] }), expect.anything() diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx new file mode 100644 index 00000000000..95fdb830197 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx @@ -0,0 +1,91 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; + +import { commonText } from '../../../localization/common'; +import { requireContext } from '../../../tests/helpers'; +import { tables } from '../../DataModel/tables'; +import type { SpecifyResource } from '../../DataModel/legacyTypes'; +import type { Agent } from '../../DataModel/types'; +import { RecordSelectorFromIds } from '../../FormSliders/RecordSelectorFromIds'; +import type { RecordSelectorProps } from '../../FormSliders/RecordSelector'; + +requireContext(); + +const mockLoadRecord = jest.fn | undefined]>(); + +jest.mock('../../Forms/ResourceView', () => { + const actualReact = jest.requireActual('react'); + return { + ResourceView: jest.fn(function Preview({ + resource, + }: { + readonly resource: SpecifyResource | undefined; + }) { + const [value, setValue] = actualReact.useState(''); + actualReact.useEffect(() => { + mockLoadRecord(resource); + }, [resource]); + return ( + { + resource?.set('remarks', event.target.value); + setValue(event.target.value); + }} + /> + ); + }), + }; +}); + +jest.mock('../../FormSliders/RecordSelector', () => ({ + useRecordSelector: ({ records, index }: RecordSelectorProps) => ({ + resource: records[index], + dialogs: null, + slider: null, + isLoading: false, + }), +})); + +test('keeps the edited resource and form mounted without loading another record while suspended', () => { + const props = { + ids: [38665, 38666], + defaultIndex: 1, + table: tables.Agent, + title: commonText.view(), + dialog: false, + isDependent: false, + newResource: undefined, + onAdd: undefined, + onClone: undefined, + onDelete: undefined, + onSlide: undefined, + onClose: jest.fn(), + onSaved: jest.fn(), + } as const; + const { rerender } = render(); + const input = screen.getByRole('textbox'); + fireEvent.change(input, { target: { value: 'Unsaved remarks' } }); + const resource = mockLoadRecord.mock.calls[0][0]!; + expect(resource.get('remarks')).toBe('Unsaved remarks'); + expect(resource.needsSaved).toBe(true); + expect(mockLoadRecord).toHaveBeenCalledTimes(1); + + rerender( + + ); + expect(input).toBeInTheDocument(); + expect(input).toHaveValue('Unsaved remarks'); + expect(mockLoadRecord).toHaveBeenCalledTimes(1); + + rerender(); + expect(screen.getByRole('textbox')).toBe(input); + expect(resource.get('remarks')).toBe('Unsaved remarks'); + expect(mockLoadRecord).toHaveBeenCalledTimes(1); +}); From d4080abd4277cd83d8b371583415328a407a0692 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Wed, 7 Oct 2026 13:45:50 +0200 Subject: [PATCH 3/3] Feat: Prevent from merging from Data Views --- .../js_src/lib/components/Core/Contexts.tsx | 3 + .../__tests__/RecordMerging.test.tsx | 49 ++++++ .../js_src/lib/components/DataViews/index.tsx | 36 ++--- .../js_src/lib/components/FormMeta/index.tsx | 4 +- .../FormSliders/RecordSelectorFromIds.tsx | 17 +-- .../lib/components/QueryBuilder/Results.tsx | 4 +- .../lib/components/QueryBuilder/ToForms.tsx | 34 +---- .../__tests__/QueryFormView.test.tsx | 87 ----------- .../RecordMergingAvailability.test.tsx | 140 ++++++++++++++++++ .../SuspendedRecordSelector.test.tsx | 91 ------------ 10 files changed, 217 insertions(+), 248 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/DataViews/__tests__/RecordMerging.test.tsx delete mode 100644 specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx create mode 100644 specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/RecordMergingAvailability.test.tsx delete mode 100644 specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx diff --git a/specifyweb/frontend/js_src/lib/components/Core/Contexts.tsx b/specifyweb/frontend/js_src/lib/components/Core/Contexts.tsx index a1dfd528931..e3a63715e13 100644 --- a/specifyweb/frontend/js_src/lib/components/Core/Contexts.tsx +++ b/specifyweb/frontend/js_src/lib/components/Core/Contexts.tsx @@ -213,6 +213,9 @@ ErrorContext.displayName = 'ErrorContext'; export const ReadOnlyContext = React.createContext(false); ReadOnlyContext.displayName = 'ReadOnlyContext'; +export const RecordMergingContext = React.createContext(true); +RecordMergingContext.displayName = 'RecordMergingContext'; + /** If true, form is rendered in a search dialog - required fields are not enforced */ export const SearchDialogContext = React.createContext(false); SearchDialogContext.displayName = 'SearchDialogContext'; diff --git a/specifyweb/frontend/js_src/lib/components/DataViews/__tests__/RecordMerging.test.tsx b/specifyweb/frontend/js_src/lib/components/DataViews/__tests__/RecordMerging.test.tsx new file mode 100644 index 00000000000..3047c5f01ee --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DataViews/__tests__/RecordMerging.test.tsx @@ -0,0 +1,49 @@ +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; + +import { requireContext } from '../../../tests/helpers'; +import { TableDataView } from '..'; + +requireContext(); + +jest.mock('../queries', () => ({ + ...jest.requireActual('../queries'), + useDataViewQueries: () => [{ version: 1, queries: {} }, jest.fn()], +})); +jest.mock('../../Permissions/helpers', () => ({ + ...jest.requireActual('../../Permissions/helpers'), + hasPermission: () => true, +})); +jest.mock('../../Permissions/PermissionDenied', () => ({ + ProtectedTable: ({ children }: { readonly children: React.ReactNode }) => + children, +})); +jest.mock('../../QueryBuilder/ResultsWrapper', () => { + const actualReact = jest.requireActual('react'); + const { RecordMergingContext } = jest.requireActual('../../Core/Contexts'); + return { + ...jest.requireActual('../../QueryBuilder/ResultsWrapper'), + QueryResultsWrapper: () => ( +
+ {actualReact.useContext(RecordMergingContext) + ? 'Merging enabled' + : 'Merging disabled'} +
+ ), + }; +}); + +test('disables merging throughout Data Views', () => { + render( + + + } + /> + + + ); + expect(screen.getByText('Merging disabled')).toBeInTheDocument(); +}); diff --git a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx index 8dfbe188f5e..15fb780cca5 100644 --- a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx @@ -1,17 +1,16 @@ import React from 'react'; import { useParams } from 'react-router-dom'; -import { useSearchParameter } from '../../hooks/navigation'; import { commonText } from '../../localization/common'; import { dataViewsText } from '../../localization/dataViews'; import { useResponsiveSplitView } from '../../hooks/useResponsiveSplitView'; import { H2 } from '../Atoms'; import { Button } from '../Atoms/Button'; import { DataEntry } from '../Atoms/DataEntry'; +import { RecordMergingContext } from '../Core/Contexts'; import { getTable } from '../DataModel/tables'; import type { Tables } from '../DataModel/types'; import { raise } from '../Errors/Crash'; -import { mergingQueryParameter } from '../Merging/queryString'; import { Dialog } from '../Molecules/Dialog'; import { TableIcon } from '../Molecules/TableIcon'; import { hasPermission } from '../Permissions/helpers'; @@ -30,7 +29,6 @@ import { } from '../QueryBuilder/SplitView'; import { QueryFormView } from '../QueryBuilder/ToForms'; import { NotFoundView } from '../Router/NotFoundView'; -import { OverlayLocation } from '../Router/Router'; import type { DataViewQueriesFile } from './queries'; import { getDataViewQueryDefinition, @@ -48,13 +46,15 @@ export function TableDataView(): JSX.Element { return table === undefined ? ( ) : ( - - {hasPermission('/querybuilder/query', 'execute') ? ( - - ) : ( - - )} - + + + {hasPermission('/querybuilder/query', 'execute') ? ( + + ) : ( + + )} + + ); } @@ -98,11 +98,6 @@ function LoadedDataViewFromTable({ readonly queries: DataViewQueriesFile; readonly reloadQueries: () => void; }): JSX.Element | null { - const overlayLocation = React.useContext(OverlayLocation); - const [mergingRecords] = useSearchParameter( - mergingQueryParameter, - overlayLocation - ); const table = getTable(tableName); const [selectedIds, setSelectedIds] = React.useState>( [] @@ -189,15 +184,6 @@ function LoadedDataViewFromTable({ restoreScrollTopRef.current = resultsScrollRef.current.scrollTop; setRefreshToken((token) => token + 1); }, []); - const handleMerged = React.useCallback((): void => { - /* - * Merging removes the selected records. Clear the preview before the - * refreshed results arrive so it does not try to load deleted records. - */ - setSelectedIds([]); - setSelectedIndex(0); - handleRefresh(); - }, [handleRefresh]); const handleCloseQueryEditor = (): void => setQueryData(undefined); const handleOpenQueryEditor = (): void => { setIsQueryDirty(false); @@ -284,7 +270,6 @@ function LoadedDataViewFromTable({ setSelectedIds([]); setSelectedIndex(0); }} - onMerged={handleMerged} onSortChange={(newFields): void => { setRuntimeFields(unParseQueryFields(table.name, newFields)); setQueryRunCount((count) => count + 1); @@ -328,7 +313,6 @@ function LoadedDataViewFromTable({ onDelete, }) => ( void; }): JSX.Element { const subView = React.useContext(SubViewContext); - const canMergeTable = canMerge(resource.specifyTable); + const canMergeTable = + React.useContext(RecordMergingContext) && canMerge(resource.specifyTable); return ( ({ - suspended = false, ids, newResource, onSlide: handleSlide, @@ -59,8 +58,6 @@ export function RecordSelectorFromIds({ * sets or query results with thousands of items) */ readonly ids: RA; - // Keep the current form and resource intact while a merge overlay is open. - readonly suspended?: boolean; readonly newResource: SpecifyResource | undefined; readonly title: LocalizedString | undefined; readonly headerButtons?: JSX.Element; @@ -89,7 +86,6 @@ export function RecordSelectorFromIds({ ); React.useEffect(() => { - if (suspended) return; setRecords((records) => ids.map((id) => { if (id === undefined) return undefined; @@ -100,14 +96,11 @@ export function RecordSelectorFromIds({ ); }) ); - }, [ids, table, suspended]); + }, [ids, table]); - const defaultIndexRef = React.useRef( + const [rawIndex, setIndex] = useTriggerState( Math.max(0, defaultIndex ?? ids.length - 1) ); - if (!suspended) - defaultIndexRef.current = Math.max(0, defaultIndex ?? ids.length - 1); - const [rawIndex, setIndex] = useTriggerState(defaultIndexRef.current); const index = typeof newResource === 'object' ? totalCount - 1 @@ -124,7 +117,7 @@ export function RecordSelectorFromIds({ const { dialogs, slider, - resource: selectedResource, + resource, onAdd: handleAdding, onRemove: handleRemove, isLoading, @@ -157,7 +150,6 @@ export function RecordSelectorFromIds({ } : undefined, onSlide: (index, replace, callback): void => { - if (suspended) return; function doSlide(): void { setIndex(index); handleSlide?.(index, replace); @@ -177,9 +169,6 @@ export function RecordSelectorFromIds({ else doSlide(); }, }); - const resourceRef = React.useRef(selectedResource); - if (!suspended) resourceRef.current = selectedResource; - const resource = resourceRef.current; const addLabel = isInRecordSet ? formsText.addToRecordSet({ diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx index d7ff7f516cc..6eac838f511 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx @@ -10,6 +10,7 @@ import { f } from '../../utils/functools'; import { type GetSet, type RA } from '../../utils/types'; import { Container, H3 } from '../Atoms'; import { Button } from '../Atoms/Button'; +import { RecordMergingContext } from '../Core/Contexts'; import type { SpecifyResource } from '../DataModel/legacyTypes'; import { schema } from '../DataModel/schema'; import type { SpecifyTable } from '../DataModel/specifyTable'; @@ -235,7 +236,8 @@ export function QueryResults(props: QueryResultsProps): JSX.Element { setTotalCount, ]); - const canMergeTable = canMerge(table); + const canMergeTable = + React.useContext(RecordMergingContext) && canMerge(table); const visibleColumns = React.useMemo( () => diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx index 8cb8e024e3c..f5dccbeca63 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ToForms.tsx @@ -96,7 +96,6 @@ export function QueryToForms({ } export function QueryFormView({ - suspended = false, table, title, results, @@ -109,8 +108,6 @@ export function QueryFormView({ onSaved: handleSaved, onSlide: handleSlide, }: { - // Merge tasks can delete previewed records before query results are refreshed. - readonly suspended?: boolean; readonly table: SpecifyTable; readonly title: LocalizedString; readonly results: RA; @@ -126,39 +123,21 @@ export function QueryFormView({ readonly onSlide: (index: number) => void; }): JSX.Element | null { const ids = useSelectedResults(results, selectedRows, true, totalCount); - const previewRef = React.useRef({ - ids, - selectedIndex, - totalCount: selectedRows.size === 0 ? totalCount : selectedRows.size, - hasRecords: !suspended && hasFetchableRecordIds(results) && ids.length > 0, - }); - if (!suspended) - previewRef.current = { - ids, - selectedIndex, - totalCount: selectedRows.size === 0 ? totalCount : selectedRows.size, - hasRecords: hasFetchableRecordIds(results) && ids.length > 0, - }; - const preview = previewRef.current; - if (!preview.hasRecords) return null; + if (!hasFetchableRecordIds(results) || ids.length === 0) return null; return ( -
+
{ await handleFetchMore(index); @@ -179,7 +158,6 @@ export function QueryFormView({ } onSaved={handleSaved} onSlide={(index): void => { - if (suspended) return; handleSlide(index); if (selectedRows.size === 0 && results[index] === undefined) void handleFetchMore?.(index); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx deleted file mode 100644 index d442bf3c1f6..00000000000 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/QueryFormView.test.tsx +++ /dev/null @@ -1,87 +0,0 @@ -import { fireEvent, render, screen } from '@testing-library/react'; -import React from 'react'; - -import { commonText } from '../../../localization/common'; -import { requireContext } from '../../../tests/helpers'; -import { tables } from '../../DataModel/tables'; -import { RecordSelectorFromIds } from '../../FormSliders/RecordSelectorFromIds'; -import { QueryFormView } from '../ToForms'; - -requireContext(); - -jest.mock('../../FormSliders/RecordSelectorFromIds', () => { - const actualReact = jest.requireActual('react'); - return { - RecordSelectorFromIds: jest.fn(function Preview() { - const [value, setValue] = actualReact.useState(''); - return ( - setValue(event.target.value)} - /> - ); - }), - }; -}); - -test('preserves unsaved preview edits when merging is cancelled', () => { - const props = { - table: tables.Agent, - title: commonText.view(), - results: [[38665], [38666]], - selectedRows: new Set([38665, 38666]), - selectedIndex: 1, - totalCount: 2, - onFetchMore: jest.fn(), - onDelete: jest.fn(), - onClose: jest.fn(), - onSaved: jest.fn(), - onSlide: jest.fn(), - }; - const { rerender } = render(); - const input = screen.getByRole('textbox', { name: 'Record preview' }); - fireEvent.change(input, { target: { value: 'Unsaved agent name' } }); - - rerender( - - ); - expect(input).toBeInTheDocument(); - expect(input).toHaveValue('Unsaved agent name'); - expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); - expect(RecordSelectorFromIds).toHaveBeenLastCalledWith( - expect.objectContaining({ - ids: [38665, 38666], - defaultIndex: 1, - suspended: true, - onFetch: undefined, - }), - expect.anything() - ); - - rerender(); - expect(screen.getByRole('textbox')).toBe(input); - expect(input).toHaveValue('Unsaved agent name'); - - rerender( - - ); - expect(screen.getByRole('textbox')).toBeInTheDocument(); - expect(RecordSelectorFromIds).toHaveBeenLastCalledWith( - expect.objectContaining({ ids: [38665] }), - expect.anything() - ); -}); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/RecordMergingAvailability.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/RecordMergingAvailability.test.tsx new file mode 100644 index 00000000000..55d11d8569d --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/RecordMergingAvailability.test.tsx @@ -0,0 +1,140 @@ +import { + act, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/react'; +import React from 'react'; + +import { requireContext } from '../../../tests/helpers'; +import { RecordMergingContext } from '../../Core/Contexts'; +import { tables } from '../../DataModel/tables'; +import { defaultDataViewQuery } from '../../DataViews/queries'; +import { FormMeta } from '../../FormMeta'; +import { formsText } from '../../../localization/forms'; +import { UnloadProtectsContext } from '../../Router/UnloadProtect'; +import { parseQueryFields, queryFieldsToFieldSpecs } from '../helpers'; +import { QueryResults } from '../Results'; + +requireContext(); + +jest.mock('../../Permissions/helpers', () => ({ + ...jest.requireActual('../../Permissions/helpers'), + hasPermission: () => true, + hasTablePermission: () => true, + hasToolPermission: () => false, +})); +jest.mock('../../Permissions/PermissionDenied', () => ({ + ProtectedTool: () => null, + ProtectedAction: () => null, +})); +jest.mock('../../Merging', () => ({ + RecordMergingLink: () => , +})); +jest.mock('../ToForms', () => ({ QueryToForms: () => null })); +jest.mock('../ToMap', () => ({ QueryToMap: () => null })); +jest.mock('../ResultsTable', () => ({ QueryResultsTable: () => null })); +jest.mock('../../FormMeta/MergeRecord', () => ({ + MergeRecord: () => , +})); +jest.mock('../../FormMeta/AutoNumbering', () => ({ + AutoNumbering: () => null, +})); +jest.mock('../../FormMeta/CarryForward', () => ({ + ...jest.requireActual('../../FormMeta/CarryForward'), + CarryForwardConfig: () => null, +})); +jest.mock('../../FormMeta/Clone', () => ({ + CloneConfig: () => null, + AddButtonConfig: () => null, +})); +jest.mock('../../FormMeta/Definition', () => ({ Definition: () => null })); +jest.mock('../../FormMeta/EditHistory', () => ({ EditHistory: () => null })); +jest.mock('../../FormMeta/PickListUsages', () => ({ + PickListUsages: () => null, +})); +jest.mock('../../FormMeta/QueryTreeUsages', () => ({ + QueryTreeUsages: () => null, +})); +jest.mock('../../FormMeta/ReadOnlyMode', () => ({ ReadOnlyMode: () => null })); +jest.mock('../../FormMeta/ShareRecord', () => ({ ShareRecord: () => null })); +jest.mock('../../FormCommands', () => ({ GenerateLabel: () => null })); +jest.mock('../../FormFields/Checkbox', () => ({ PrintOnSave: () => null })); + +test('hides merging in Data Views without changing Query Builder availability', async () => { + const fields = parseQueryFields(defaultDataViewQuery('Agent').fields); + const fieldSpecs = queryFieldsToFieldSpecs('Agent', fields).map( + ([, fieldSpec]) => fieldSpec + ); + const results = ( + [[1], [2]]} + fetchCount={undefined} + totalCount={2} + fieldSpecs={fieldSpecs} + displayedFields={fields} + allFields={fields} + initialData={[[1], [2]]} + selectedRows={[new Set([1, 2]), jest.fn()]} + onReRun={jest.fn()} + createRecordSet={undefined} + extraButtons={undefined} + /> + ); + const { rerender } = render(results); + await waitFor(() => + expect( + screen.getByRole('button', { name: 'Merge selected records' }) + ).toBeInTheDocument() + ); + + await act(async () => + rerender( + + {results} + + ) + ); + expect( + screen.queryByRole('button', { name: 'Merge selected records' }) + ).not.toBeInTheDocument(); +}); + +test('hides the preview form merge action when merging is disabled', async () => { + const formMeta = ( + + ); + const { rerender } = render(formMeta, { + wrapper: ({ children }) => ( + + {children} + + ), + }); + await act(async () => + fireEvent.click(screen.getByRole('button', { name: formsText.formMeta() })) + ); + expect( + screen.getByRole('button', { name: 'Merge this record' }) + ).toBeInTheDocument(); + await act(async () => + rerender( + + {formMeta} + + ) + ); + await act(async () => + fireEvent.click(screen.getByRole('button', { name: formsText.formMeta() })) + ); + expect( + screen.queryByRole('button', { name: 'Merge this record' }) + ).not.toBeInTheDocument(); +}); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx deleted file mode 100644 index 95fdb830197..00000000000 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SuspendedRecordSelector.test.tsx +++ /dev/null @@ -1,91 +0,0 @@ -import { fireEvent, render, screen } from '@testing-library/react'; -import React from 'react'; - -import { commonText } from '../../../localization/common'; -import { requireContext } from '../../../tests/helpers'; -import { tables } from '../../DataModel/tables'; -import type { SpecifyResource } from '../../DataModel/legacyTypes'; -import type { Agent } from '../../DataModel/types'; -import { RecordSelectorFromIds } from '../../FormSliders/RecordSelectorFromIds'; -import type { RecordSelectorProps } from '../../FormSliders/RecordSelector'; - -requireContext(); - -const mockLoadRecord = jest.fn | undefined]>(); - -jest.mock('../../Forms/ResourceView', () => { - const actualReact = jest.requireActual('react'); - return { - ResourceView: jest.fn(function Preview({ - resource, - }: { - readonly resource: SpecifyResource | undefined; - }) { - const [value, setValue] = actualReact.useState(''); - actualReact.useEffect(() => { - mockLoadRecord(resource); - }, [resource]); - return ( - { - resource?.set('remarks', event.target.value); - setValue(event.target.value); - }} - /> - ); - }), - }; -}); - -jest.mock('../../FormSliders/RecordSelector', () => ({ - useRecordSelector: ({ records, index }: RecordSelectorProps) => ({ - resource: records[index], - dialogs: null, - slider: null, - isLoading: false, - }), -})); - -test('keeps the edited resource and form mounted without loading another record while suspended', () => { - const props = { - ids: [38665, 38666], - defaultIndex: 1, - table: tables.Agent, - title: commonText.view(), - dialog: false, - isDependent: false, - newResource: undefined, - onAdd: undefined, - onClone: undefined, - onDelete: undefined, - onSlide: undefined, - onClose: jest.fn(), - onSaved: jest.fn(), - } as const; - const { rerender } = render(); - const input = screen.getByRole('textbox'); - fireEvent.change(input, { target: { value: 'Unsaved remarks' } }); - const resource = mockLoadRecord.mock.calls[0][0]!; - expect(resource.get('remarks')).toBe('Unsaved remarks'); - expect(resource.needsSaved).toBe(true); - expect(mockLoadRecord).toHaveBeenCalledTimes(1); - - rerender( - - ); - expect(input).toBeInTheDocument(); - expect(input).toHaveValue('Unsaved remarks'); - expect(mockLoadRecord).toHaveBeenCalledTimes(1); - - rerender(); - expect(screen.getByRole('textbox')).toBe(input); - expect(resource.get('remarks')).toBe('Unsaved remarks'); - expect(mockLoadRecord).toHaveBeenCalledTimes(1); -});