Fix(query-builder): avoid dirtying saved queries on execution

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<SerializedResource<SpQueryField>>)
     | undefined;
@@ -131,7 +133,7 @@ export function QueryBuilderResults({
               saveRequired={saveRequired}
             />
           )}
-          {query.countOnly ? undefined : (
+          {isCountOnly ? undefined : (
             <QueryExportButtons
               baseTableName={state.baseTableName}
               fields={state.fields}
@@ -146,6 +148,7 @@ export function QueryBuilderResults({
       }
       fields={state.fields}
       forceCollection={forceCollection}
+      countOnly={isCountOnly}
       queryResource={queryResource}
       queryRunCount={state.queryRunCount}
       refreshToken={refreshToken}

specifyweb/frontend/js_src/lib/components/QueryBuilder/ResultsWrapper.tsx

@@ -107,6 +107,7 @@ export function QueryResultsWrapper({
 type ResultsProps = {
   readonly table: SpecifyTable;
   readonly queryRunCount: number;
+  readonly countOnly?: boolean;
   readonly queryResource: SpecifyResource<SpQuery>;
   readonly fields: RA<QueryField>;
   readonly recordSetId: number | undefined;
@@ -185,6 +186,7 @@ const runQueryCount = async (
 export function useQueryResultsWrapper({
   table,
   queryRunCount,
+  countOnly,
   queryResource,
   fields,
   recordSetId,
@@ -231,23 +233,25 @@ 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<SpQuery> = {
       ...serializeResource(queryResource),
       fields: unParseQueryFields(table.name, allFields),
+      countOnly: isCountOnly,
     };
 
     setTotalCount(undefined);
     const fetchCount = async (): Promise<number> =>
       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

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({
             <QueryBuilderResults
               forceCollection={forceCollection}
               getQueryFieldRecords={getQueryFieldRecords}
+              isCountOnly={isCountOnly}
               isHorizontal={isHorizontal}
               isReadOnly={isReadOnly}
               isSplit={isSplit}
@@ -630,7 +627,7 @@ function Wrapped({
               }}
               onSortChange={(fields): void => {
                 dispatch({ type: 'ChangeFieldsAction', fields });
-                runQuery('regular', fields);
+                runQuery('regular');
               }}
             />
           </Form>

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<SpQuery>;
-const fields = [] as const as readonly QueryField[];
-const serializedFields = [
-  { fieldName: 'Name' },
-] as unknown as readonly SerializedResource<SpQueryField>[];
+const query = {
+  fields: [{ fieldName: 'Saved field' }],
+  countOnly: false,
+} as unknown as SerializedResource<SpQuery>;
 
 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());
 

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<SpQuery>;
-  readonly fields: RA<QueryField>;
-  readonly getQueryFieldRecords:
-    | ((fields: RA<QueryField>) => RA<SerializedResource<SpQueryField>>)
-    | undefined;
-  readonly setQuery: (query: SerializedResource<SpQuery>) => void;
   readonly onRun: () => void;
 }): {
-  readonly runQuery: (
-    mode: 'count' | 'regular',
-    fields?: RA<QueryField>
-  ) => 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<QueryField> = 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 };
 }