Fix the Search footer flickering to a loading skeleton after Select all

src/libs/actions/Search.ts

@@ -782,6 +782,7 @@ function getOnyxLoadingData(
     offset?: number,
     isSearchAPI = false,
     shouldCalculateTotals?: boolean,
+    shouldShowLoading = true,
 ): OnyxData<typeof ONYXKEYS.COLLECTION.SNAPSHOT> {
     const shouldClearTotals = isSearchAPI && shouldCalculateTotals === false && offset === 0;
 
@@ -798,7 +799,7 @@ function getOnyxLoadingData(
             key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`,
             value: {
                 search: {
-                    ...(isSearchAPI && {isLoading: true}),
+                    ...(isSearchAPI && shouldShowLoading && {isLoading: true}),
                     ...(isSearchRequest && {state: CONST.SEARCH.SNAPSHOT_STATE.LOADING}),
                     ...(offset !== undefined ? {offset} : {}),
                     ...(shouldClearTotals ? {count: null, reportCount: null, total: null, currency: null} : {}),
@@ -1163,6 +1164,7 @@ type InFlightSearchRequest = {
     pendingShouldCalculateTotals?: boolean;
     pendingShouldSaveRecentSearch?: boolean;
     pendingUpgradeRequest?: () => Promise<string | number | undefined> | undefined;
+    didSucceed?: boolean;
 };
 
 // Tracks in-flight search requests by hash+offset to prevent duplicate API calls when both page-level
@@ -1246,6 +1248,7 @@ function search({
     shouldUpdateLastSearchParams = false,
     skipWaitForWrites = false,
     shouldSaveRecentSearch = false,
+    shouldShowLoading = true,
 }: {
     queryJSON: Readonly<SearchQueryJSON>;
     searchKey: SearchKey | undefined;
@@ -1269,6 +1272,8 @@ function search({
      * with an incomplete filter set and has to be re-fired once that data lands.
      */
     skipWaitForWrites?: boolean;
+    /** When false, the request leaves `search.isLoading` unset, so the current results stay on screen without a loading state. */
+    shouldShowLoading?: boolean;
 }): Promise<string | number | undefined> | undefined {
     if (isLoading || shouldPreventSearchAPI) {
         return;
@@ -1300,14 +1305,17 @@ function search({
                     shouldUpdateLastSearchParams,
                     skipWaitForWrites,
                     shouldSaveRecentSearch: inFlightRequest.pendingShouldSaveRecentSearch,
+                    // A re-fire that only adds the save flag returns the same results the finished request just showed,
+                    // so it must not show loading again (the footer would flash its skeleton over the totals).
+                    shouldShowLoading: !inFlightRequest.didSucceed || inFlightRequest.pendingShouldCalculateTotals !== inFlightRequest.shouldCalculateTotals,
                 });
         }
         return;
     }
     const inFlightRequestState: InFlightSearchRequest = {shouldCalculateTotals, shouldSaveRecentSearch};
     inFlightSearchRequests.set(dedupeKey, inFlightRequestState);
 
-    const onyxLoadingData = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, true, shouldCalculateTotals);
+    const onyxLoadingData = getOnyxLoadingData(queryJSON.hash, queryJSON, offset, true, shouldCalculateTotals, shouldShowLoading);
     const {backendQueryJSON, limit, exactMatchFilterKeys} = getBackendQueryJSON(queryJSON);
     const query = {
         ...backendQueryJSON,
@@ -1350,6 +1358,7 @@ function search({
         makeRequestWithSideEffects(READ_COMMANDS.SEARCH, {hash: queryJSON.hash, jsonQuery}, {optimisticData, finallyData, failureData})
             .then((result) => {
                 const response = result?.onyxData?.[0]?.value as OnyxSearchResponse;
+                inFlightRequestState.didSucceed = result?.jsonCode === CONST.JSON_CODE.SUCCESS;
 
                 // The UI treats a successful response with no snapshot data as an empty result, so record it for diagnosis.
                 if (result?.jsonCode === CONST.JSON_CODE.SUCCESS && response?.data === undefined) {

tests/unit/Search/searchSaveRecentSearchFlagTest.ts

@@ -36,6 +36,17 @@ function getLastRequestJsonQuery(): unknown {
     return parsedJsonQuery;
 }
 
+function getLastRequestOptimisticIsLoading(): unknown {
+    const optimisticData = mockedMakeRequestWithSideEffects.mock.calls.at(-1)?.[2]?.optimisticData ?? [];
+    for (const update of optimisticData) {
+        const value: unknown = update.value;
+        if (value && typeof value === 'object' && 'search' in value && value.search && typeof value.search === 'object' && 'isLoading' in value.search) {
+            return value.search.isLoading;
+        }
+    }
+    return undefined;
+}
+
 // The backend only saves a query to the recent searches NVP when the payload declares it was
 // user-submitted, so these tests pin down exactly when the flag is (and is not) serialized.
 describe('search shouldSaveRecentSearch flag', () => {
@@ -202,6 +213,77 @@ describe('search shouldSaveRecentSearch flag', () => {
         expect(getLastRequestJsonQuery()).toEqual(expect.objectContaining({shouldCalculateTotals: true, shouldSaveRecentSearch: true}));
     });
 
+    it('does not show loading again when a save-only re-fire follows a successful totals request', async () => {
+        // Given a totals request without the flag that is still waiting for its response, like the select-all totals request
+        const queryJSON = getQueryJSON('type:expense merchant:tram');
+        let resolveFirstRequest: () => void = () => {};
+        const firstRequestPromise = new Promise<{jsonCode: number}>((resolve) => {
+            resolveFirstRequest = () => resolve({jsonCode: CONST.JSON_CODE.SUCCESS});
+        });
+        mockedMakeRequestWithSideEffects.mockImplementationOnce(() => firstRequestPromise);
+
+        const firstSearch = search({
+            queryJSON,
+            searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES,
+            offset: 0,
+            shouldCalculateTotals: true,
+            isLoading: false,
+        });
+
+        // When the Search page sends the same query with the flag, and the first request then succeeds
+        search({
+            queryJSON,
+            searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES,
+            offset: 0,
+            isLoading: false,
+            shouldSaveRecentSearch: true,
+        });
+        resolveFirstRequest();
+        await firstSearch;
+        await Promise.resolve();
+
+        // Then the flagged re-fire is still sent, but it doesn't set isLoading, because the totals it returns are already
+        // on screen and a loading state would flash the footer skeleton over them
+        expect(mockedMakeRequestWithSideEffects.mock.calls).toHaveLength(2);
+        expect(getLastRequestJsonQuery()).toEqual(expect.objectContaining({shouldCalculateTotals: true, shouldSaveRecentSearch: true}));
+        expect(getLastRequestOptimisticIsLoading()).toBeUndefined();
+    });
+
+    it('shows loading on a re-fire that adds totals the finished request did not calculate', async () => {
+        // Given a search without totals that is still waiting for its response
+        const queryJSON = getQueryJSON('type:expense merchant:bus');
+        let resolveFirstRequest: () => void = () => {};
+        const firstRequestPromise = new Promise<{jsonCode: number}>((resolve) => {
+            resolveFirstRequest = () => resolve({jsonCode: CONST.JSON_CODE.SUCCESS});
+        });
+        mockedMakeRequestWithSideEffects.mockImplementationOnce(() => firstRequestPromise);
+
+        const firstSearch = search({
+            queryJSON,
+            searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES,
+            offset: 0,
+            shouldCalculateTotals: false,
+            isLoading: false,
+        });
+
+        // When a flagged totals request collides with it, and the first request then succeeds
+        search({
+            queryJSON,
+            searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES,
+            offset: 0,
+            shouldCalculateTotals: true,
+            isLoading: false,
+            shouldSaveRecentSearch: true,
+        });
+        resolveFirstRequest();
+        await firstSearch;
+        await Promise.resolve();
+
+        // Then the re-fire still shows loading, because the totals it brings aren't on screen yet
+        expect(mockedMakeRequestWithSideEffects.mock.calls).toHaveLength(2);
+        expect(getLastRequestOptimisticIsLoading()).toBe(true);
+    });
+
     // The original bug was a wiring problem: programmatic callers looked identical to user submits.
     // Guard the wiring statically so a future caller cannot re-flag a programmatic path unnoticed.
     describe('call-site wiring', () => {