feat: add explicit terminal state to search snapshot

src/CONST/index.ts

@@ -6566,6 +6566,13 @@ const CONST = {
             TRIP: 'trip',
             CHAT: 'chat',
         },
+        // Terminal lifecycle state of a search snapshot's most recent request. Written by the search action lifecycle
+        // (loading on request start, loaded/error on resolve) so the snapshot always has an explicit, mutually exclusive state.
+        SNAPSHOT_STATE: {
+            LOADING: 'loading',
+            LOADED: 'loaded',
+            ERROR: 'error',
+        },
         ACTION_FILTERS: {
             SUBMIT: 'submit',
             APPROVE: 'approve',

src/libs/actions/Search.ts

@@ -663,13 +663,22 @@ function getOnyxLoadingData(
     shouldCalculateTotals?: boolean,
 ): OnyxData<typeof ONYXKEYS.COLLECTION.SNAPSHOT> {
     const shouldClearTotals = isSearchAPI && shouldCalculateTotals === false && offset === 0;
+
+    // `search.state` tracks the lifecycle of a real search request (identified by its queryJSON): it starts as
+    // `loading` optimistically and is resolved to `loaded`/`error` by successData/failureData. handlePreventSearchAPI
+    // reuses this helper as a UI-only loading toggle with no query and no success/failure step, so it must stay out of
+    // the state machine — otherwise it would strand `state: loading` with no terminal write to clear it.
+    const isSearchRequest = isSearchAPI && !!queryJSON;
+    const type = queryJSON?.type;
+
     const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.SNAPSHOT>> = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
             key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`,
             value: {
                 search: {
                     ...(isSearchAPI && {isLoading: true}),
+                    ...(isSearchRequest && {state: CONST.SEARCH.SNAPSHOT_STATE.LOADING}),
                     ...(offset !== undefined ? {offset} : {}),
                     ...(shouldClearTotals ? {count: null, total: null, currency: null} : {}),
                 },
@@ -684,6 +693,26 @@ function getOnyxLoadingData(
         },
     ];
 
+    // successData writes the terminal `loaded` state on any jsonCode 200 resolve. It also stamps `type` so
+    // responses that do carry data stay consistent with the anti-stale isSearchDataLoaded check (which compares
+    // type/hash). On a success response without data, isSearchDataLoaded still resolves to false via its own data/errors gate;
+    // `state` is what marks that case as done once a future PR wires the read side to it. `isLoading` isn't set here
+    // because finallyData always runs right after and already clears it for isSearchAPI. Empty for the non-search
+    // callers of this helper so they don't pay for a meaningless `{search: {}}` merge on the SNAPSHOT key.
+    const successData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.SNAPSHOT>> = isSearchRequest
+        ? [
+              {
+                  onyxMethod: Onyx.METHOD.MERGE,
+                  key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`,
+                  value: {
+                      search: {state: CONST.SEARCH.SNAPSHOT_STATE.LOADED, type},
+                  },
+              },
+          ]
+        : [];
+
+    // finallyData runs after successData/failureData regardless of jsonCode, so it deliberately does NOT write `state`:
+    // doing so would clobber the `error` terminal set by failureData. The terminal state is owned by success/failure.
     const finallyData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.SNAPSHOT>> = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
@@ -703,15 +732,16 @@ function getOnyxLoadingData(
             value: {
                 ...(isOffline ? {} : {data: null}),
                 search: {
-                    type: queryJSON?.type,
+                    type,
                     ...(isSearchAPI && {isLoading: false}),
+                    ...(isSearchRequest && {state: CONST.SEARCH.SNAPSHOT_STATE.ERROR}),
                 },
                 errors: getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'),
             },
         },
     ];
 
-    return {optimisticData, finallyData, failureData};
+    return {optimisticData, successData, finallyData, failureData};
 }
 
 function saveSearch({queryJSON, newName}: {queryJSON: Readonly<SearchQueryJSON>; newName?: string}) {
@@ -908,7 +938,7 @@ function search({
     }
     inFlightSearchRequests.add(dedupeKey);
 
-    const {optimisticData, finallyData, failureData} = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, isOffline, true, shouldCalculateTotals);
+    const {optimisticData, successData, finallyData, failureData} = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, isOffline, true, shouldCalculateTotals);
     const {flatFilters, limit, ...queryJSONWithoutFlatFilters} = queryJSON;
     const backendQueryJSON = shouldUseBackendDateSortFallback(queryJSON.sortBy)
         ? {
@@ -940,7 +970,7 @@ function search({
     }
 
     const startRequest = () =>
-        makeRequestWithSideEffects(READ_COMMANDS.SEARCH, {hash: queryJSON.hash, jsonQuery}, {optimisticData, finallyData, failureData})
+        makeRequestWithSideEffects(READ_COMMANDS.SEARCH, {hash: queryJSON.hash, jsonQuery}, {optimisticData, successData, finallyData, failureData})
             .then((result) => {
                 if (shouldUpdateLastSearchParams) {
                     const response = result?.onyxData?.[0]?.value as OnyxSearchResponse;
@@ -973,6 +1003,14 @@ function search({
 
                 return result?.jsonCode;
             })
+            .catch((error) => {
+                // A network-level rejection (no HTTP response at all, e.g. offline/timeout) never reaches
+                // SaveResponseInOnyx, so nothing else applies failureData for it. Apply it here so the snapshot
+                // still reaches a terminal `error` state instead of being stranded in `loading`, then re-throw so
+                // this still rejects for any caller relying on that.
+                Onyx.update(failureData ?? []);
+                throw error;
+            })
             .finally(() => {
                 inFlightSearchRequests.delete(dedupeKey);
             });

src/types/onyx/SearchResults.ts

@@ -59,6 +59,18 @@ type SearchResultsInfo = {
     /** Whether the search results are currently loading */
     isLoading: boolean;
 
+    /** Explicit terminal lifecycle state of the most recent search request for this snapshot.
+     * Optional because snapshots persisted before this field existed (and snapshots written by
+     * non-search actions) may not carry it.
+     *
+     * Nothing reads this field yet. The existing isLoading/type/status-based loading and error gates
+     * migrate to read it in a follow-up PR.
+     *
+     * Residual limitation: if the app is killed or reloaded mid-request, no cleanup runs, so `loading` can
+     * still be stranded on disk. The future read side must treat a `loading` state with no in-flight request
+     * as stale. */
+    state?: ValueOf<typeof CONST.SEARCH.SNAPSHOT_STATE>;
+
     /** The number of results */
     count?: number;
 

tests/unit/Search/searchSnapshotStateTest.ts

@@ -0,0 +1,142 @@
+import {search} from '@libs/actions/Search';
+import {makeRequestWithSideEffects, waitForWrites} from '@libs/API';
+import {buildSearchQueryJSON} from '@libs/SearchQueryUtils';
+
+import CONST from '@src/CONST';
+import ONYXKEYS from '@src/ONYXKEYS';
+
+import type {OnyxUpdate} from 'react-native-onyx';
+
+import Onyx from 'react-native-onyx';
+
+import getOnyxValue from '../../utils/getOnyxValue';
+import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates';
+
+jest.mock('@libs/API', () => ({
+    makeRequestWithSideEffects: jest.fn(),
+    waitForWrites: jest.fn(),
+    write: jest.fn(),
+    read: jest.fn(),
+}));
+
+// A jsonCode that is not 200, so the request resolves down the failure branch of applyHTTPSOnyxUpdates.
+const FAILURE_JSON_CODE = 500;
+
+function getQueryJSON() {
+    const queryJSON = buildSearchQueryJSON('');
+    if (!queryJSON) {
+        throw new Error('Query JSON should be defined for test setup');
+    }
+
+    return queryJSON;
+}
+
+/** Read the {optimisticData, successData, failureData, finallyData} that search() handed to the API layer. */
+function getCapturedSearchOnyxData(): NonNullable<Parameters<typeof makeRequestWithSideEffects>[2]> {
+    const lastCall = jest.mocked(makeRequestWithSideEffects).mock.calls.at(-1);
+    if (!lastCall) {
+        throw new Error('makeRequestWithSideEffects was not called');
+    }
+    return lastCall[2] ?? {};
+}
+
+/**
+ * Replays a captured search request the way applyHTTPSOnyxUpdates does: optimisticData first, then any server
+ * onyxData, then successData (jsonCode 200) or failureData (non-200), and finallyData last regardless of jsonCode.
+ * This lets us assert the terminal snapshot state that the real API application order would produce.
+ */
+async function simulateResolvedRequest({jsonCode, serverOnyxData}: {jsonCode: number; serverOnyxData?: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.SNAPSHOT>>}) {
+    const {optimisticData, successData, failureData, finallyData} = getCapturedSearchOnyxData();
+
+    await Onyx.update(optimisticData ?? []);
+    if (serverOnyxData) {
+        await Onyx.update(serverOnyxData);
+    }
+    if (jsonCode === CONST.JSON_CODE.SUCCESS) {
+        await Onyx.update(successData ?? []);
+    } else {
+        await Onyx.update(failureData ?? []);
+    }
+    await Onyx.update(finallyData ?? []);
+    await waitForBatchedUpdates();
+}
+
+describe('search snapshot terminal state', () => {
+    beforeAll(() => {
+        Onyx.init({keys: ONYXKEYS});
+    });
+
+    beforeEach(async () => {
+        jest.clearAllMocks();
+        jest.mocked(waitForWrites).mockResolvedValue(undefined);
+        jest.mocked(makeRequestWithSideEffects).mockResolvedValue(undefined);
+        await Onyx.clear();
+    });
+
+    it('marks the snapshot loading optimistically while a request is in flight', async () => {
+        const queryJSON = getQueryJSON();
+
+        await search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, isLoading: false});
+        const {optimisticData} = getCapturedSearchOnyxData();
+        await Onyx.update(optimisticData ?? []);
+        await waitForBatchedUpdates();
+
+        const snapshot = await getOnyxValue(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` as const);
+        expect(snapshot?.search?.state).toBe(CONST.SEARCH.SNAPSHOT_STATE.LOADING);
+    });
+
+    it('resolves the snapshot to loaded on a successful response', async () => {
+        const queryJSON = getQueryJSON();
+
+        await search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, isLoading: false});
+        await simulateResolvedRequest({
+            jsonCode: CONST.JSON_CODE.SUCCESS,
+            serverOnyxData: [
+                {
+                    onyxMethod: Onyx.METHOD.MERGE,
+                    key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}`,
+                    value: {search: {hasResults: true}},
+                },
+            ],
+        });
+
+        const snapshot = await getOnyxValue(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` as const);
+        expect(snapshot?.search?.state).toBe(CONST.SEARCH.SNAPSHOT_STATE.LOADED);
+        expect(snapshot?.search?.hasResults).toBe(true);
+    });
+
+    it('resolves the snapshot to error on a failed response', async () => {
+        const queryJSON = getQueryJSON();
+
+        await search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, isLoading: false});
+        await simulateResolvedRequest({jsonCode: FAILURE_JSON_CODE});
+
+        const snapshot = await getOnyxValue(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` as const);
+        // finallyData runs after failureData; the error state must survive it.
+        expect(snapshot?.search?.state).toBe(CONST.SEARCH.SNAPSHOT_STATE.ERROR);
+    });
+
+    it('reaches a terminal state when a successful response resolves without any snapshot data', async () => {
+        const queryJSON = getQueryJSON();
+
+        await search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, isLoading: false});
+        // The bug scenario: a 200 response that writes no server onyxData at all.
+        await simulateResolvedRequest({jsonCode: CONST.JSON_CODE.SUCCESS});
+
+        const snapshot = await getOnyxValue(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` as const);
+        expect(snapshot?.search?.state).toBe(CONST.SEARCH.SNAPSHOT_STATE.LOADED);
+    });
+
+    it('resolves the snapshot to error when the request promise rejects, instead of staying loading', async () => {
+        const queryJSON = getQueryJSON();
+        jest.mocked(makeRequestWithSideEffects).mockRejectedValueOnce(new Error('Network request failed'));
+
+        // The failure class this field exists to eliminate: no HTTP response at all (offline/timeout), so
+        // nothing in the API layer ever applies failureData for it unless search() catches the rejection itself.
+        await expect(search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, isLoading: false})).rejects.toThrow();
+        await waitForBatchedUpdates();
+
+        const snapshot = await getOnyxValue(`${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` as const);
+        expect(snapshot?.search?.state).toBe(CONST.SEARCH.SNAPSHOT_STATE.ERROR);
+    });
+});