From ae0d7b68d87bb5035a5c9b83c54fff8b7cc122d5 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Mon, 5 Oct 2026 10:30:17 +0200 Subject: [PATCH] Fix(query-builder): avoid dirtying saved queries on execution --- .../QueryBuilder/QueryBuilderResults.tsx | 5 ++- .../QueryBuilder/ResultsWrapper.tsx | 19 ++++++---- .../lib/components/QueryBuilder/Wrapped.tsx | 9 ++--- .../__tests__/useQueryExecution.test.tsx | 37 +++++-------------- .../QueryBuilder/useQueryExecution.ts | 33 +++++------------ 5 files changed, 38 insertions(+), 65 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx index 671786dd0bf..945471fc03f 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/QueryBuilderResults.tsx @@ -26,6 +26,7 @@ export function QueryBuilderResults({ state, isReadOnly, saveRequired, + isCountOnly, getQueryFieldRecords, selectedRows, setSelectedRows, @@ -49,6 +50,7 @@ export function QueryBuilderResults({ readonly state: MainState; readonly isReadOnly: boolean; readonly saveRequired: boolean; + readonly isCountOnly: boolean; readonly getQueryFieldRecords: | (() => RA>) | undefined; @@ -131,7 +133,7 @@ export function QueryBuilderResults({ saveRequired={saveRequired} /> )} - {query.countOnly ? undefined : ( + {isCountOnly ? undefined : ( ; readonly fields: RA; readonly recordSetId: number | undefined; @@ -185,6 +186,7 @@ const runQueryCount = async ( export function useQueryResultsWrapper({ table, queryRunCount, + countOnly, queryResource, fields, recordSetId, @@ -231,9 +233,18 @@ export function useQueryResultsWrapper({ limit: fetchSize, }; + const displayedFields = allFields.filter((field) => field.isDisplay); + const isCountOnly = + countOnly === undefined + ? queryResource.get('countOnly') === true || + // Run as "count only" if there are no visible fields + displayedFields.length === 0 + : countOnly || displayedFields.length === 0; + const query: SerializedResource = { ...serializeResource(queryResource), fields: unParseQueryFields(table.name, allFields), + countOnly: isCountOnly, }; setTotalCount(undefined); @@ -241,13 +252,6 @@ export function useQueryResultsWrapper({ runQueryCount(query, fetchPayload); fetchCount().then(setTotalCount).catch(raise); - const displayedFields = allFields.filter((field) => field.isDisplay); - const countOnly = queryResource.get('countOnly') === true; - const isCountOnly = - countOnly || - // Run as "count only" if there are no visible fields - displayedFields.length === 0; - const initialData = isCountOnly ? Promise.resolve(undefined) : runQuery(query, { offset: 0, ...fetchPayload }); @@ -316,6 +320,7 @@ export function useQueryResultsWrapper({ queryRunCount, recordSetId, handleMerged, + countOnly, ]); return props === undefined diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx index 46a457db422..fd52edc76dd 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Wrapped.tsx @@ -216,12 +216,8 @@ function Wrapped({ */ const getQueryFieldRecords = saveRequired ? serializeQueryFields : undefined; - // runQuery must always serialize the fields it is given, not just when saveRequired - const { runQuery, scheduleQueryRun } = useQueryExecution({ + const { isCountOnly, runQuery, scheduleQueryRun } = useQueryExecution({ query, - fields: state.fields, - getQueryFieldRecords: serializeQueryFields, - setQuery, onRun: (): void => dispatch({ type: 'RunQueryAction' }), }); @@ -606,6 +602,7 @@ function Wrapped({ { dispatch({ type: 'ChangeFieldsAction', fields }); - runQuery('regular', fields); + runQuery('regular'); }} /> diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/useQueryExecution.test.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/useQueryExecution.test.tsx index 41823058e7a..4bf97709217 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/useQueryExecution.test.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/useQueryExecution.test.tsx @@ -2,48 +2,36 @@ import { act, renderHook } from '@testing-library/react'; import { hasPermission } from '../../Permissions/helpers'; import type { SerializedResource } from '../../DataModel/helperTypes'; -import type { SpQuery, SpQueryField } from '../../DataModel/types'; -import type { QueryField } from '../helpers'; +import type { SpQuery } from '../../DataModel/types'; import { useQueryExecution } from '../useQueryExecution'; jest.mock('../../Permissions/helpers', () => ({ hasPermission: jest.fn(() => true), })); -const query = { fields: [] } as unknown as SerializedResource; -const fields = [] as const as readonly QueryField[]; -const serializedFields = [ - { fieldName: 'Name' }, -] as unknown as readonly SerializedResource[]; +const query = { + fields: [{ fieldName: 'Saved field' }], + countOnly: false, +} as unknown as SerializedResource; afterEach(() => { jest.useRealTimers(); }); -test('serializes the current fields and defers an authorized query run', () => { +test('keeps count mode out of the query resource and defers an authorized run', () => { jest.useFakeTimers(); - const setQuery = jest.fn(); const onRun = jest.fn(); - const getQueryFieldRecords = jest.fn(() => serializedFields); const { result } = renderHook(() => useQueryExecution({ query, - fields, - getQueryFieldRecords, - setQuery, onRun, }) ); act(() => result.current.runQuery('count')); - expect(hasPermission).toHaveBeenCalledWith('/querybuilder/query', 'execute'); - expect(getQueryFieldRecords).toHaveBeenCalledWith(fields); - expect(setQuery).toHaveBeenCalledWith({ - ...query, - fields: serializedFields, - countOnly: true, - }); + expect(result.current.isCountOnly).toBe(true); + expect(query.countOnly).toBe(false); expect(onRun).not.toHaveBeenCalled(); act(() => jest.runOnlyPendingTimers()); @@ -53,24 +41,17 @@ test('serializes the current fields and defers an authorized query run', () => { test('schedules a regular query run after pending input changes', () => { jest.useFakeTimers(); - const setQuery = jest.fn(); const onRun = jest.fn(); const { result } = renderHook(() => useQueryExecution({ query, - fields, - getQueryFieldRecords: undefined, - setQuery, onRun, }) ); act(() => result.current.scheduleQueryRun()); - expect(setQuery).toHaveBeenCalledWith({ - ...query, - countOnly: false, - }); + expect(result.current.isCountOnly).toBe(false); act(() => jest.runOnlyPendingTimers()); diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQueryExecution.ts b/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQueryExecution.ts index d36bc5047af..a9aae74d013 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQueryExecution.ts +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/useQueryExecution.ts @@ -2,45 +2,32 @@ import React from 'react'; import { useBooleanState } from '../../hooks/useBooleanState'; import type { SerializedResource } from '../DataModel/helperTypes'; -import type { SpQuery, SpQueryField } from '../DataModel/types'; +import type { SpQuery } from '../DataModel/types'; import { hasPermission } from '../Permissions/helpers'; -import type { RA } from '../../utils/types'; -import type { QueryField } from './helpers'; export function useQueryExecution({ query, - fields, - getQueryFieldRecords, - setQuery, onRun, }: { readonly query: SerializedResource; - readonly fields: RA; - readonly getQueryFieldRecords: - | ((fields: RA) => RA>) - | undefined; - readonly setQuery: (query: SerializedResource) => void; readonly onRun: () => void; }): { - readonly runQuery: ( - mode: 'count' | 'regular', - fields?: RA - ) => void; + readonly isCountOnly: boolean; + readonly runQuery: (mode: 'count' | 'regular') => void; readonly scheduleQueryRun: () => void; } { const [isQueryRunPending, scheduleQueryRun, clearQueryRunPending] = useBooleanState(); + const [isCountOnly, setIsCountOnly] = React.useState( + query.countOnly === true + ); const runQuery = React.useCallback( - (mode: 'count' | 'regular', queryFields: RA = fields): void => { + (mode: 'count' | 'regular'): void => { if (!hasPermission('/querybuilder/query', 'execute')) return; - setQuery({ - ...query, - fields: getQueryFieldRecords?.(queryFields) ?? query.fields, - countOnly: mode === 'count', - }); + setIsCountOnly(mode === 'count'); globalThis.setTimeout(onRun, 0); }, - [fields, getQueryFieldRecords, onRun, query, setQuery] + [onRun] ); React.useEffect(() => { @@ -49,5 +36,5 @@ export function useQueryExecution({ runQuery('regular'); }, [clearQueryRunPending, isQueryRunPending, runQuery]); - return { runQuery, scheduleQueryRun }; + return { isCountOnly, runQuery, scheduleQueryRun }; }