diff --git a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx index b733637e142..d3e5697e304 100644 --- a/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DataViews/index.tsx @@ -10,7 +10,6 @@ import { DataEntry } from '../Atoms/DataEntry'; import { getTable } from '../DataModel/tables'; import type { Tables } from '../DataModel/types'; import { raise } from '../Errors/Crash'; -import { RecordSelectorFromIds } from '../FormSliders/RecordSelectorFromIds'; import { Dialog } from '../Molecules/Dialog'; import { TableIcon } from '../Molecules/TableIcon'; import { hasPermission } from '../Permissions/helpers'; @@ -23,11 +22,11 @@ import { parseQueryFields, unParseQueryFields } from '../QueryBuilder/helpers'; import { queryIdField } from '../QueryBuilder/Results'; import { QueryResultsWrapper } from '../QueryBuilder/ResultsWrapper'; import { - SplitView, SplitViewOrientationButton, SplitViewToggleButton, useSplitViewOrientation, } from '../QueryBuilder/SplitView'; +import { QueryFormView } from '../QueryBuilder/ToForms'; import { NotFoundView } from '../Router/NotFoundView'; import type { DataViewQueriesFile } from './queries'; import { @@ -103,7 +102,6 @@ function LoadedDataViewFromTable({ const selectedIdsRef = React.useRef(selectedIds); selectedIdsRef.current = selectedIds; const resultOrderRef = React.useRef>([]); - const hasSeenNonEmptyResultsRef = React.useRef(false); const [selectedIndex, setSelectedIndex] = React.useState(0); const selectedIndexRef = React.useRef(selectedIndex); selectedIndexRef.current = selectedIndex; @@ -153,20 +151,9 @@ function LoadedDataViewFromTable({ const id = getNumericResultId(row?.[queryIdField]); return id === undefined ? [] : [id]; }); - const isInitialResults = - !hasSeenNonEmptyResultsRef.current && orderedIds.length > 0; - if (orderedIds.length > 0) hasSeenNonEmptyResultsRef.current = true; resultOrderRef.current = orderedIds; - if (selectedIdsRef.current.length === 0) { - if (!isInitialResults) return; - const firstId = orderedIds[0]; - if (firstId !== undefined) { - setSelectedIds([firstId]); - setSelectedIndex(0); - } - return; - } + if (selectedIdsRef.current.length === 0) return; const positions = new Map( orderedIds.map((id, index) => [id, index] as const) @@ -322,38 +309,36 @@ function LoadedDataViewFromTable({ table={table} onResults={handleResults} refreshToken={refreshToken} - restoreScrollTopRef={restoreScrollTopRef} - scrollRef={resultsScrollRef} - /> - ); - const form = ( -
- {selectedIds.length === 0 ? ( -

{commonText.select()}

- ) : ( - ( + { setSelectedIds([]); setSelectedIndex(0); }} - onDelete={undefined} + onDelete={onDelete} + onFetchMore={onFetchMore} onSaved={handleRefresh} - onSlide={(index): void => setSelectedIndex(index)} + onSlide={setSelectedIndex} /> )} -
+ restoreScrollTopRef={restoreScrollTopRef} + scrollRef={resultsScrollRef} + /> ); return ( @@ -381,15 +366,7 @@ function LoadedDataViewFromTable({ className="flex h-full max-h-full min-h-0 min-w-0 flex-1 overflow-hidden" ref={splitViewRef} > - + {results} ); diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx index 334c8d000da..590c359e932 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromIds.tsx @@ -85,20 +85,17 @@ export function RecordSelectorFromIds({ ids.map((id) => (id === undefined ? undefined : new table.Resource({ id }))) ); - const previousIds = React.useRef(ids); - React.useEffect(() => { setRecords((records) => - ids.map((id, index) => { + ids.map((id) => { if (id === undefined) return undefined; - else if (records[index]?.id === id) return records[index]; - else return new table.Resource({ id }); + else + return ( + records.find((record) => record?.id === id) ?? + new table.Resource({ id }) + ); }) ); - - return (): void => { - previousIds.current = ids; - }; }, [ids, table]); const [rawIndex, setIndex] = useTriggerState( diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx index 945471fc03f..7073d0cf538 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx @@ -1,13 +1,11 @@ import React from 'react'; -import { commonText } from '../../localization/common'; import { type RA, localized } from '../../utils/types'; import { BatchEditFromQuery } from '../BatchEdit'; import type { SerializedResource } from '../DataModel/helperTypes'; import type { SpecifyResource } from '../DataModel/legacyTypes'; import type { SpecifyTable } from '../DataModel/specifyTable'; import type { RecordSet, SpQuery, SpQueryField } from '../DataModel/types'; -import { RecordSelectorFromIds } from '../FormSliders/RecordSelectorFromIds'; import { hasPermission } from '../Permissions/helpers'; import { datasetVariants } from '../WbUtils/datasetVariants'; import { MakeRecordSetButton } from './Components'; @@ -16,6 +14,7 @@ import type { QueryField } from './helpers'; import type { MainState } from './reducer'; import type { QueryResultRow } from './Results'; import { QueryResultsWrapper } from './ResultsWrapper'; +import { hasFetchableRecordIds, QueryFormView } from './ToForms'; export function QueryBuilderResults({ table, @@ -73,40 +72,6 @@ export function QueryBuilderResults({ readonly onSortChange: (fields: RA) => void; }): JSX.Element | null { const [refreshToken, setRefreshToken] = React.useState(0); - const selectedIds = React.useMemo( - () => Array.from(selectedRows), - [selectedRows] - ); - const recordPreview = ( -
- {selectedIds.length === 0 ? ( -

{commonText.select()}

- ) : ( - { - setSelectedRows(new Set()); - setSelectedIndex(0); - }} - onDelete={undefined} - onSaved={(): void => setRefreshToken((token) => token + 1)} - onSlide={setSelectedIndex} - /> - )} -
- ); - return hasPermission('/querybuilder/query', 'execute') ? ( ( + <> + {query.selectDistinct !== true && + !isCountOnly && + hasFetchableRecordIds(results) ? ( + { + setSelectedRows(new Set()); + setSelectedIndex(0); + }} + onDelete={onDelete} + onFetchMore={onFetchMore} + onSaved={(): void => setRefreshToken((token) => token + 1)} + onSlide={setSelectedIndex} + /> + ) : null} + + )} table={table} onReRun={handleReRun} onResults={handleResults} diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx index d673aade458..d7ff7f516cc 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx @@ -35,6 +35,7 @@ import { sortTypes } from './helpers'; import { QueryResultsTable } from './ResultsTable'; import { QueryToForms } from './ToForms'; import { QueryToMap } from './ToMap'; +import { SplitView } from './SplitView'; export type QueryResultRow = RA; @@ -91,6 +92,10 @@ export type QueryResultsProps = { readonly tableClassName?: string; readonly selectedRows: GetSet>; readonly onResults?: (results: RA) => void; + readonly renderSplitPane?: (props: QueryResultsSplitPaneProps) => JSX.Element; + readonly isSplit?: boolean; + readonly splitHorizontal?: boolean; + readonly splitPrimaryPaneMaxWidth?: string; readonly onDeleted?: (recordId: number) => void; readonly onMerged?: () => void; readonly scrollRef?: React.MutableRefObject; @@ -99,6 +104,17 @@ export type QueryResultsProps = { readonly resultsRef?: React.MutableRefObject< RA | undefined >; + readonly isLoading?: boolean; +}; + +export type QueryResultsSplitPaneProps = { + readonly results: RA; + readonly selectedRows: ReadonlySet; + readonly totalCount: number | undefined; + readonly onFetchMore: + | ((index?: number) => Promise | undefined>) + | undefined; + readonly onDelete: (id: number) => void; }; export function QueryResults(props: QueryResultsProps): JSX.Element { @@ -122,12 +138,17 @@ export function QueryResults(props: QueryResultsProps): JSX.Element { tableClassName = '', selectedRows: [selectedRows, setSelectedRows], onResults: handleResults, + renderSplitPane, + isSplit, + splitHorizontal, + splitPrimaryPaneMaxWidth, onDeleted: handleDeleted, scrollRef, restoreScrollTopRef, refreshToken, resultsRef, displayedFields, + isLoading = false, } = props; const { @@ -460,7 +481,7 @@ export function QueryResults(props: QueryResultsProps): JSX.Element { typeof loadedResults?.[0]?.[0] === 'string' && loadedResults !== undefined; const metaColumns = (showLineNumber ? 1 : 0) + 2; - return ( + const queryResults = ( @@ -686,7 +707,9 @@ export function QueryResults(props: QueryResultsProps): JSX.Element { }} /> ) : undefined} - {isFetching || (!showResults && Array.isArray(results)) ? ( + {isLoading || + isFetching || + (!showResults && Array.isArray(results)) ? (
{loadingGif}
@@ -695,6 +718,32 @@ export function QueryResults(props: QueryResultsProps): JSX.Element {
); + + return renderSplitPane === undefined ? ( + queryResults + ) : ( + + ) + } + secondaryPaneKey="split-pane" + /> + ); } function TableHeaderCell({ diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx index 1ad339483e9..0a9aa2c23c4 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx @@ -22,7 +22,7 @@ import { queryFieldsToFieldSpecs, unParseQueryFields, } from './helpers'; -import type { QueryResultRow } from './Results'; +import type { QueryResultRow, QueryResultsSplitPaneProps } from './Results'; import { QueryResults } from './Results'; import { SplitView } from './SplitView'; @@ -36,6 +36,7 @@ export function QueryResultsWrapper({ onResults: handleResults, onDeleted: handleDeleted, onReRun: handleReRun, + renderSplitPane, onMerged: handleMerged, refreshToken, splitPane, @@ -58,16 +59,21 @@ export function QueryResultsWrapper({ readonly splitPrimaryPaneMaxWidth?: string; readonly isSplit?: boolean; readonly onReRun: () => void; + readonly renderSplitPane?: (props: QueryResultsSplitPaneProps) => JSX.Element; }): JSX.Element | null { - const newProps = useQueryResultsWrapper(props); + const wrappedProps = useQueryResultsWrapper(props); - if (newProps === undefined) + if (wrappedProps === undefined) return props.queryRunCount === 0 ? null : (
{loadingGif}
); + const { isLoading, ...newProps } = wrappedProps; const queryResults = ( -
+
@@ -135,7 +146,12 @@ type ResultsProps = { type PartialProps = Omit< Parameters[0], - 'createRecordSet' | 'extraButtons' | 'model' | 'onReRun' | 'onSelected' + | 'createRecordSet' + | 'extraButtons' + | 'isLoading' + | 'model' + | 'onReRun' + | 'onSelected' >; export const runQuery = async ( @@ -200,7 +216,7 @@ export function useQueryResultsWrapper({ scrollRef, restoreScrollTopRef, resultsRef, -}: ResultsProps): PartialProps | undefined { +}: ResultsProps): (PartialProps & { readonly isLoading: boolean }) | undefined { /* * Need to store all props in a state so that query field edits do not affect * the query results until query is reRun @@ -209,16 +225,27 @@ export function useQueryResultsWrapper({ Omit | undefined >(undefined); + const [isLoading, setIsLoading] = React.useState(false); const [totalCount, setTotalCount] = React.useState( undefined ); const previousQueryRunCount = React.useRef(0); + const requestGeneration = React.useRef(0); + + React.useEffect( + () => (): void => { + requestGeneration.current++; + previousQueryRunCount.current = 0; + }, + [] + ); React.useEffect(() => { if (queryRunCount === previousQueryRunCount.current) return; previousQueryRunCount.current = queryRunCount; - // Display the loading GIF - setProps(undefined); + const generation = ++requestGeneration.current; + setIsLoading(true); + setTotalCount(undefined); const isDistinct = queryResource.get('selectDistinct') === true; const allFields = augmentQueryFields( @@ -247,10 +274,13 @@ export function useQueryResultsWrapper({ countOnly: isCountOnly, }; - setTotalCount(undefined); const fetchCount = async (): Promise => runQueryCount(query, fetchPayload); - fetchCount().then(setTotalCount).catch(raise); + fetchCount() + .then((count) => { + if (generation === requestGeneration.current) setTotalCount(count); + }) + .catch(raise); const initialData = isCountOnly ? Promise.resolve(undefined) @@ -265,7 +295,8 @@ export function useQueryResultsWrapper({ const queryFields = fieldSpecsAndFields.map(([field]) => field); initialData - .then((initialData) => + .then((initialData) => { + if (generation !== requestGeneration.current) return; setProps({ queryResource, containerClassName, @@ -309,9 +340,13 @@ export function useQueryResultsWrapper({ ); } : undefined, - }) - ) - .catch(raise); + }); + setIsLoading(false); + }) + .catch((error) => { + if (generation === requestGeneration.current) setIsLoading(false); + raise(error); + }); }, [ fields, table, @@ -330,5 +365,6 @@ export function useQueryResultsWrapper({ totalCount, selectedRows: [selectedRows, setSelectedRows], resultsRef, + isLoading, }; } diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx index 8f3f7a2784e..97866f65703 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/SplitView.tsx @@ -77,11 +77,16 @@ export function SplitView({ null ); const previousIsHorizontal = React.useRef(isHorizontal); + const previousIsSplit = React.useRef(isSplit); React.useLayoutEffect(() => { - if (previousIsHorizontal.current !== isHorizontal) + if ( + previousIsHorizontal.current !== isHorizontal || + (previousIsSplit.current && !isSplit) + ) splitterRef.current?.setState({ primaryPane: undefined }); previousIsHorizontal.current = isHorizontal; - }, [isHorizontal]); + previousIsSplit.current = isSplit; + }, [isHorizontal, isSplit]); return (
; + readonly selectedRows: ReadonlySet; + readonly selectedIndex: number; + readonly totalCount: number | undefined; + readonly onFetchMore: + | ((index?: number) => Promise | undefined>) + | undefined; + readonly onDelete: (id: number) => void; + readonly onClose: () => void; + readonly onSaved: () => void; + readonly onSlide: (index: number) => void; +}): JSX.Element | null { + const ids = useSelectedResults(results, selectedRows, true, totalCount); + if (!hasFetchableRecordIds(results) || ids.length === 0) return null; + + return ( +
+ { + const id = + selectedRows.size === 0 + ? results[index]?.[queryIdField] + : Array.from(selectedRows)[index]; + if (typeof id === 'number') handleDelete(id); + }} + onFetch={ + handleFetchMore === undefined + ? undefined + : async (index) => { + await handleFetchMore(index); + return undefined; + } + } + onSaved={handleSaved} + onSlide={(index): void => { + handleSlide(index); + if (selectedRows.size === 0 && results[index] === undefined) + void handleFetchMore?.(index); + }} + /> +
+ ); +} + +export function getSelectedResults( + results: RA, + selectedRows: ReadonlySet, + isOpen: boolean, + totalCount: number | undefined +): RA { + if (!isOpen) return []; + if (selectedRows.size > 0) return Array.from(selectedRows); + + const ids = results.map((row) => row?.[queryIdField] as number | undefined); + if (totalCount !== undefined) ids.length = Math.max(ids.length, totalCount); + return ids; +} + +export function hasFetchableRecordIds( + results: RA +): boolean { + return results.some((row) => { + const id = row?.[queryIdField]; + return typeof id === 'number' && Number.isFinite(id); + }); +} + function useSelectedResults( results: RA, selectedRows: ReadonlySet, @@ -101,21 +197,7 @@ function useSelectedResults( totalCount: number | undefined ): RA { return React.useMemo( - () => - isOpen - ? selectedRows.size === 0 - ? totalCount - ? ([ - ...results.map((row) => row?.[queryIdField]), - ...Array.from({ length: totalCount - results.length }).fill( - undefined - ), - ] as RA) - : (results.map((row) => row?.[queryIdField]) as RA< - number | undefined - >) - : Array.from(selectedRows) - : [], - [results, isOpen, selectedRows] + () => getSelectedResults(results, selectedRows, isOpen, totalCount), + [results, isOpen, selectedRows, totalCount] ); } diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx index fd52edc76dd..f7e26264e1d 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx @@ -284,7 +284,7 @@ function Wrapped({ toggleSplit, toggleOrientation, onResults: handleSplitViewResults, - } = useQuerySplitView(resultsRef, state.queryRunCount); + } = useQuerySplitView(state.queryRunCount); const showSeries = React.useMemo( () => diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/ResultsWrapper.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/ResultsWrapper.test.tsx new file mode 100644 index 00000000000..a7363e3c197 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/ResultsWrapper.test.tsx @@ -0,0 +1,137 @@ +import { act, renderHook, waitFor } from '@testing-library/react'; +import React from 'react'; + +import { requireContext } from '../../../tests/helpers'; +import * as ajaxModule from '../../../utils/ajax'; +import { Http } from '../../../utils/ajax/definitions'; +import { tables } from '../../DataModel/tables'; +import { + defaultDataViewQuery, + makeDataViewQuery, +} from '../../DataViews/queries'; +import { parseQueryFields } from '../helpers'; +import { useQueryResultsWrapper } from '../ResultsWrapper'; + +requireContext(); + +function makeProps() { + const definition = defaultDataViewQuery('Agent'); + return { + table: tables.Agent, + queryRunCount: 1, + queryResource: makeDataViewQuery('Agent', definition), + fields: parseQueryFields(definition.fields), + recordSetId: undefined, + forceCollection: undefined, + selectedRows: [new Set(), jest.fn()] as const, + }; +} + +function response(id: number) { + return { + data: { count: id, results: [[id]] }, + status: Http.OK, + response: new Response('', { status: Http.OK }), + }; +} + +function deferredResponse() { + let resolve!: (value: ReturnType) => void; + const promise = new Promise>((callback) => { + resolve = callback; + }); + return { promise, resolve }; +} + +afterEach(() => { + jest.restoreAllMocks(); +}); + +test('loads Data View results during Strict Mode effect replay', async () => { + jest.spyOn(ajaxModule, 'ajax').mockResolvedValue(response(3)); + const props = makeProps(); + const { result } = renderHook(() => useQueryResultsWrapper(props), { + wrapper: ({ children }) => {children}, + }); + + await waitFor(() => expect(result.current?.initialData).toEqual([[3]])); + expect(result.current?.totalCount).toBe(3); + expect(result.current?.isLoading).toBe(false); +}); + +test('does not cancel an in-flight query when fields change without a new run', async () => { + const pending = deferredResponse(); + const ajax = jest.spyOn(ajaxModule, 'ajax').mockReturnValue(pending.promise); + const props = makeProps(); + const { result, rerender } = renderHook(useQueryResultsWrapper, { + initialProps: props, + }); + + rerender({ ...props, fields: [...props.fields] }); + await act(async () => pending.resolve(response(3))); + + expect(result.current?.initialData).toEqual([[3]]); + expect(result.current?.totalCount).toBe(3); + expect(result.current?.isLoading).toBe(false); + expect(ajax).toHaveBeenCalledTimes(2); +}); + +test('ignores stale results and counts after a new query run', async () => { + const previous = deferredResponse(); + const current = deferredResponse(); + jest + .spyOn(ajaxModule, 'ajax') + .mockReturnValueOnce(previous.promise) + .mockReturnValueOnce(previous.promise) + .mockReturnValue(current.promise); + const props = makeProps(); + const { result, rerender } = renderHook(useQueryResultsWrapper, { + initialProps: props, + }); + + rerender({ ...props, queryRunCount: 2 }); + await act(async () => current.resolve(response(7))); + expect(result.current?.initialData).toEqual([[7]]); + expect(result.current?.totalCount).toBe(7); + + await act(async () => previous.resolve(response(3))); + expect(result.current?.initialData).toEqual([[7]]); + expect(result.current?.totalCount).toBe(7); + expect(result.current?.isLoading).toBe(false); +}); + +test('ignores the cancelled request after Strict Mode starts a replacement', async () => { + const previous = deferredResponse(); + const current = deferredResponse(); + jest + .spyOn(ajaxModule, 'ajax') + .mockReturnValueOnce(previous.promise) + .mockReturnValueOnce(previous.promise) + .mockReturnValue(current.promise); + const props = makeProps(); + const { result } = renderHook(() => useQueryResultsWrapper(props), { + wrapper: ({ children }) => {children}, + }); + + await act(async () => previous.resolve(response(3))); + expect(result.current).toBeUndefined(); + + await act(async () => current.resolve(response(7))); + expect(result.current?.initialData).toEqual([[7]]); + expect(result.current?.totalCount).toBe(7); + expect(result.current?.isLoading).toBe(false); +}); + +test('preserves count-only runs in Strict Mode', async () => { + const ajax = jest.spyOn(ajaxModule, 'ajax').mockResolvedValue(response(3)); + const props = { ...makeProps(), countOnly: true }; + const { result } = renderHook(() => useQueryResultsWrapper(props), { + wrapper: ({ children }) => {children}, + }); + + await waitFor(() => expect(result.current?.totalCount).toBe(3)); + expect(result.current?.initialData).toBeUndefined(); + expect(result.current?.fetchResults).toBeUndefined(); + expect(result.current?.isLoading).toBe(false); + expect(ajax).toHaveBeenCalledTimes(2); +}); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx index f3a54609a66..e67ccddb6f7 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/SplitView.test.tsx @@ -4,11 +4,18 @@ import React from 'react'; import { SplitView } from '../SplitView'; const mockSetState = jest.fn(); +const mockSplitterProps = jest.fn(); jest.mock('m-react-splitters', () => { const actualReact = jest.requireActual('react'); const MockSplitter = actualReact.forwardRef(function MockSplitter( - { children }: { readonly children?: React.ReactNode }, + { + children, + ...props + }: { + readonly children?: React.ReactNode; + readonly [key: string]: unknown; + }, ref: React.ForwardedRef<{ readonly setState: typeof mockSetState }> ): JSX.Element { actualReact.useImperativeHandle( @@ -16,15 +23,37 @@ jest.mock('m-react-splitters', () => { () => ({ setState: mockSetState }), [] ); + mockSplitterProps(props); return
{children}
; }); return { __esModule: true, default: MockSplitter }; }); +test('uses the full width for a vertical split', () => { + render( + + ); + + expect(mockSplitterProps).toHaveBeenLastCalledWith( + expect.objectContaining({ + primaryPaneHeight: '50%', + primaryPaneWidth: '100%', + primaryPaneMaxWidth: '100%', + }) + ); +}); + const primaryPane =
; const secondaryPane =
; test('clears the dragged pane size when the orientation changes', () => { + mockSetState.mockClear(); const { rerender } = render( { ); expect(mockSetState).toHaveBeenCalledWith({ primaryPane: undefined }); }); + +test('clears the dragged pane size when the split view is hidden', () => { + mockSetState.mockClear(); + const { rerender } = render( + + ); + expect(mockSetState).not.toHaveBeenCalled(); + + rerender( + + ); + expect(mockSetState).toHaveBeenCalledWith({ primaryPane: undefined }); +}); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/ToForms.test.ts b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/ToForms.test.ts new file mode 100644 index 00000000000..5abd16f022a --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/ToForms.test.ts @@ -0,0 +1,38 @@ +import { getSelectedResults, hasFetchableRecordIds } from '../ToForms'; + +test('only exposes results with fetchable record IDs to the form view', () => { + expect(hasFetchableRecordIds([[101, 'First'], undefined])).toBe(true); + expect(hasFetchableRecordIds([])).toBe(false); + expect(hasFetchableRecordIds([undefined, ['not an ID']])).toBe(false); + expect(hasFetchableRecordIds([[Number.NaN, 'Invalid ID']])).toBe(false); +}); + +test('empty selection represents all results without allocating unloaded IDs', () => { + const ids = getSelectedResults( + [ + [101, 'First'], + [102, 'Second'], + ], + new Set(), + true, + 1_000_000 + ); + + expect(ids).toHaveLength(1_000_000); + expect(ids.slice(0, 2)).toEqual([101, 102]); + expect(Object.keys(ids)).toEqual(['0', '1']); +}); + +test('non-empty selection remains limited to selected records', () => { + expect( + getSelectedResults( + [ + [101, 'First'], + [102, 'Second'], + ], + new Set([102]), + true, + 4 + ) + ).toEqual([102]); +}); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts b/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts index 93771e3a6b7..66ff6efaa8f 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQuerySplitView.ts @@ -3,15 +3,10 @@ import React from 'react'; import { useResponsiveSplitView } from '../../hooks/useResponsiveSplitView'; import type { RA } from '../../utils/types'; import { userPreferences } from '../Preferences/userPreferences'; -import { queryIdField, type QueryResultRow } from './Results'; +import type { QueryResultRow } from './Results'; import { useSplitViewOrientation } from './SplitView'; -export function useQuerySplitView( - resultsRef: React.MutableRefObject< - RA | undefined - >, - queryRunCount: number -): { +export function useQuerySplitView(queryRunCount: number): { readonly selectedRows: ReadonlySet; readonly setSelectedRows: React.Dispatch< React.SetStateAction> @@ -52,31 +47,6 @@ export function useQuerySplitView( maximumPrimaryPaneWidth, } = useResponsiveSplitView(preferredIsHorizontal); - const selectFirstResult = React.useCallback((): boolean => { - const firstId = resultsRef.current?.find( - (result) => result !== undefined - )?.[queryIdField]; - if (typeof firstId !== 'number') return false; - setSelectedRows(new Set([firstId])); - setSelectedIndex(0); - return true; - }, [resultsRef]); - - const [resultsVersion, setResultsVersion] = React.useState(0); - const notifiedRunRef = React.useRef(undefined); - const queryRunCountRef = React.useRef(queryRunCount); - queryRunCountRef.current = queryRunCount; - const onResults = React.useCallback( - (results: RA): void => { - const hasRow = results.some((result) => result !== undefined); - const currentRun = queryRunCountRef.current; - if (!hasRow || notifiedRunRef.current === currentRun) return; - notifiedRunRef.current = currentRun; - setResultsVersion((version) => version + 1); - }, - [] - ); - // Clear the parent-owned selection on a new query run, but not on // orientation changes (isSplit/isHorizontal don't affect this effect) const previousQueryRunCountRef = React.useRef(queryRunCount); @@ -87,28 +57,8 @@ export function useQuerySplitView( setSelectedIndex(0); }, [queryRunCount]); - // Track transitions so clearing selection (e.g. on close) doesn't retrigger - // a selection; only enabling split view or a fresh set of results should - const previousIsSplitRef = React.useRef(false); - const previousResultsVersionRef = React.useRef(resultsVersion); - React.useEffect(() => { - const splitJustEnabled = isSplit && !previousIsSplitRef.current; - const newResultsArrived = - resultsVersion !== previousResultsVersionRef.current; - previousIsSplitRef.current = isSplit; - previousResultsVersionRef.current = resultsVersion; - if ( - isSplit && - selectedRows.size === 0 && - (splitJustEnabled || newResultsArrived) - ) - selectFirstResult(); - }, [isSplit, resultsVersion, selectedRows.size, selectFirstResult]); - const toggleSplit = (): void => { - const nextIsSplit = !isSplit; - setIsSplit(nextIsSplit); - if (nextIsSplit && selectedRows.size === 0) selectFirstResult(); + setIsSplit((split) => !split); }; return { selectedRows, @@ -122,6 +72,6 @@ export function useQuerySplitView( isHorizontal, toggleSplit, toggleOrientation, - onResults, + onResults: (): void => undefined, }; } diff --git a/specifyweb/frontend/js_src/lib/hooks/__tests__/usePaginatedCollection.test.tsx b/specifyweb/frontend/js_src/lib/hooks/__tests__/usePaginatedCollection.test.tsx index 59547e3b0be..2c1b7ea4796 100644 --- a/specifyweb/frontend/js_src/lib/hooks/__tests__/usePaginatedCollection.test.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/__tests__/usePaginatedCollection.test.tsx @@ -17,6 +17,72 @@ test('recognizes a complete initial result set', () => { expect(result.current.canFetchMore).toBe(false); }); +test('replaces the current result page when initial records change', () => { + const { result, rerender } = renderHook( + ({ initialRecords, totalCount }) => + usePaginatedCollection({ + initialRecords, + totalCount, + fetchMore: jest.fn(async () => []), + }), + { + initialProps: { + initialRecords: [1, 2], + totalCount: 2, + }, + } + ); + + rerender({ + initialRecords: [3, 4], + totalCount: 4, + }); + + expect(result.current.results[0]).toEqual([3, 4]); + expect(result.current.canFetchMore).toBe(true); +}); + +test('ignores page requests from the previous result set', async () => { + let resolveFetch: ((rows: number[]) => void) | undefined; + const fetchMore = jest.fn( + () => + new Promise((resolve) => { + resolveFetch = resolve; + }) + ); + const { result, rerender } = renderHook( + ({ initialRecords, totalCount }) => + usePaginatedCollection({ + initialRecords, + totalCount, + fetchMore, + fetchSize: 2, + }), + { + initialProps: { + initialRecords: [1, 2], + totalCount: 4, + }, + } + ); + + let request: Promise | undefined> | undefined; + act(() => { + request = result.current.onFetchMore(); + }); + rerender({ + initialRecords: [3, 4], + totalCount: 2, + }); + + await act(async () => { + resolveFetch?.([5, 6]); + await request; + }); + + expect(result.current.results[0]).toEqual([3, 4]); +}); + test('appends the next page of results', async () => { const fetchMore = jest.fn(async (offset: number) => offset === 2 ? [3, 4] : [] diff --git a/specifyweb/frontend/js_src/lib/hooks/usePaginatedCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/usePaginatedCollection.tsx index cc9bffbdd28..58267d0b2f5 100644 --- a/specifyweb/frontend/js_src/lib/hooks/usePaginatedCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/usePaginatedCollection.tsx @@ -38,11 +38,23 @@ export function usePaginatedCollection({ const fetchersRef = React.useRef | undefined>>>( {} ); + const collectionGeneration = React.useRef(0); - const getSetTotalCount = useTriggerState( + const [totalCount, setTotalCount] = useTriggerState( initialTotalCount ); - const [totalCount] = getSetTotalCount; + const previousInitialRecords = React.useRef(initialRecords); + React.useLayoutEffect(() => { + if (previousInitialRecords.current === initialRecords) { + setTotalCount(initialTotalCount); + return; + } + previousInitialRecords.current = initialRecords; + collectionGeneration.current++; + fetchersRef.current = {}; + handleSetResults(initialRecords); + setTotalCount(initialTotalCount); + }, [initialRecords, initialTotalCount, handleSetResults, setTotalCount]); const canFetchMore = !Array.isArray(results) || totalCount === undefined || @@ -53,6 +65,7 @@ export function usePaginatedCollection({ async (index: number = 0): Promise | undefined> => { const currentResults = resultsRef.current ?? []; if (rawHandleFetchMore == undefined) return undefined; + const generation = collectionGeneration.current; // Prevent concurrent fetching in different places fetchersRef.current[index] ??= rawHandleFetchMore(index) .then(async (newResults) => { @@ -66,6 +79,8 @@ export function usePaginatedCollection({ ) ); + if (generation !== collectionGeneration.current) return undefined; + // Results might have changed while fetching const newCurrentResults = resultsRef.current ?? currentResults; @@ -90,10 +105,11 @@ export function usePaginatedCollection({ return newResults; }) .catch((error) => { - fetchersRef.current = removeKey( - fetchersRef.current, - index.toString() - ); + if (generation === collectionGeneration.current) + fetchersRef.current = removeKey( + fetchersRef.current, + index.toString() + ); raise(error); return undefined; }); @@ -162,7 +178,7 @@ export function usePaginatedCollection({ return { results: [results, handleSetResults] as const, onFetchMore: handleFetchMore, - totalCount: getSetTotalCount, + totalCount: [totalCount, setTotalCount] as const, canFetchMore, }; }