Stop the Search page resending a live first-page request, which flashed the footer skeleton after Select all

Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>

Author: MelvinBot

src/hooks/useSearchPageSetup.ts

@@ -2,7 +2,7 @@ import {useSearchQueryContext, useSearchResultsContext, useSearchSelectionAction
 import type {SearchQueryJSON} from '@components/Search/types';
 
 import {saveLastSearchParams} from '@libs/actions/ReportNavigation';
-import {clearPageRequestedSearch, markPageRequestedSearch, openSearch, search} from '@libs/actions/Search';
+import {clearPageRequestedSearch, isSearchRequestInFlight, markPageRequestedSearch, openSearch, search} from '@libs/actions/Search';
 import {hasPendingSearchWrite} from '@libs/pendingSearchWrite';
 import {isSearchDataLoaded, isSearchPending} from '@libs/SearchUIUtils';
 
@@ -64,6 +64,9 @@ function useSearchPageSetup(queryJSON: Readonly<SearchQueryJSON> | undefined) {
     // Hashes this page requested, so an error can be traced to the attempt that produced it.
     const requestedHashesRef = useRef<Set<number>>(new Set());
 
+    // The last query this page sent with shouldSaveRecentSearch.
+    const flaggedHashRef = useRef<number | undefined>(undefined);
+
     // Clear selected transactions when navigating to a different search query
     function clearOnHashChange() {
         if (hash === undefined) {
@@ -93,13 +96,17 @@ function useSearchPageSetup(queryJSON: Readonly<SearchQueryJSON> | undefined) {
         }
 
         // A pending initial request may be stale after reload and can be restarted through request deduplication.
-        // Pagination must not restart page one.
-        if (isSnapshotDataLoaded && !isInitialSearchPending) {
+        // Pagination must not restart page one. Once this visit has sent the query flagged, a live first-page request
+        // (select-all totals, a refresh) is programmatic: re-sending it flagged would only queue a duplicate request,
+        // which flashes the footer's loading skeleton after the totals have already arrived.
+        const isLiveRequestAlreadyFlagged = flaggedHashRef.current === hash && isSearchRequestInFlight(hash);
+        if (isSnapshotDataLoaded && (!isInitialSearchPending || isLiveRequestAlreadyFlagged)) {
             return;
         }
 
         const shouldSkipWaitForWrites = hasPendingSearchWrite();
         requestedHashesRef.current.add(hash);
+        flaggedHashRef.current = hash;
         // Claim this query's first page so Search does not request it again when it mounts behind the skeleton.
         // With data loaded Search is already mounted, so a token set here would never be read and would skip the next revisit.
         if (!isSnapshotDataLoaded) {

src/libs/actions/Search.ts

@@ -1192,6 +1192,11 @@ function clearPageRequestedSearch() {
     pageRequestedSearch = undefined;
 }
 
+/** Whether a request for this query's page was sent in this session and is still awaiting its response. */
+function isSearchRequestInFlight(hash: number, offset = 0) {
+    return inFlightSearchRequests.has(`${hash}_${offset}`);
+}
+
 let shouldPreventSearchAPI = false;
 function handlePreventSearchAPI(hash: number | undefined) {
     if (typeof hash === 'undefined') {
@@ -2611,5 +2616,6 @@ export {
     markPageRequestedSearch,
     consumePageRequestedSearch,
     clearPageRequestedSearch,
+    isSearchRequestInFlight,
 };
 export type {TransactionPreviewData};

tests/unit/hooks/useSearchPageSetup.test.ts

@@ -10,6 +10,7 @@ import type {SearchResults} from '@src/types/onyx';
 import type * as NativeNavigation from '@react-navigation/native';
 
 const mockSearch = jest.fn<void, unknown[]>();
+let mockIsSearchRequestInFlight = false;
 let mockCurrentSearchResults: SearchResults | undefined;
 let mockShouldUseLiveData = false;
 
@@ -33,6 +34,7 @@ jest.mock('@libs/actions/Search', () => ({
     search: (...args: unknown[]) => mockSearch(...args),
     markPageRequestedSearch: jest.fn(),
     clearPageRequestedSearch: jest.fn(),
+    isSearchRequestInFlight: () => mockIsSearchRequestInFlight,
 }));
 
 jest.mock('@react-navigation/native', () => ({
@@ -88,6 +90,7 @@ describe('useSearchPageSetup', () => {
     beforeEach(() => {
         mockSearch.mockClear();
         mockShouldUseLiveData = false;
+        mockIsSearchRequestInFlight = false;
     });
 
     it('retries an unresolved search when temporary search prevention clears', async () => {
@@ -118,6 +121,51 @@ describe('useSearchPageSetup', () => {
         expect(mockSearch).toHaveBeenCalledWith(expect.objectContaining({isLoading: false}));
     });
 
+    it('does not resend a live first-page request after the page already sent this query', async () => {
+        // Given the page already sent the first page of this query, and its response has loaded
+        const queryJSON = getQueryJSON();
+        const {rerender} = renderHook(
+            ({searchResults}) => {
+                mockCurrentSearchResults = searchResults;
+                useSearchPageSetup(queryJSON);
+            },
+            {initialProps: {searchResults: makeUnresolvedSearchResults(queryJSON.hash, true)}},
+        );
+        await waitFor(() => expect(mockSearch).toHaveBeenCalledTimes(1));
+        rerender({searchResults: makeCachedSearchResults(queryJSON.hash, false, CONST.SEARCH.SNAPSHOT_STATE.LOADED)});
+
+        // When Search sends its own first-page request (for example the select-all totals request)
+        mockIsSearchRequestInFlight = true;
+        rerender({searchResults: makeCachedSearchResults(queryJSON.hash, true, CONST.SEARCH.SNAPSHOT_STATE.LOADING)});
+        await Promise.resolve();
+
+        // Then the page does not send it again, because the resent request would run as a duplicate once the first
+        // one returns and show the footer's loading skeleton after the totals arrived
+        expect(mockSearch).toHaveBeenCalledTimes(1);
+    });
+
+    it('sends a cached query once when Search starts its first-page request', async () => {
+        // Given a cached query that the page did not need to request on this visit
+        const queryJSON = getQueryJSON();
+        const {rerender} = renderHook(
+            ({searchResults}) => {
+                mockCurrentSearchResults = searchResults;
+                useSearchPageSetup(queryJSON);
+            },
+            {initialProps: {searchResults: makeCachedSearchResults(queryJSON.hash, false, CONST.SEARCH.SNAPSHOT_STATE.LOADED)}},
+        );
+        await Promise.resolve();
+        expect(mockSearch).not.toHaveBeenCalled();
+
+        // When Search sends its own first-page request for it
+        mockIsSearchRequestInFlight = true;
+        rerender({searchResults: makeCachedSearchResults(queryJSON.hash, true, CONST.SEARCH.SNAPSHOT_STATE.LOADING)});
+
+        // Then the page still sends the query once, so the visit is saved to recent searches
+        await waitFor(() => expect(mockSearch).toHaveBeenCalledTimes(1));
+        expect(mockSearch).toHaveBeenCalledWith(expect.objectContaining({shouldSaveRecentSearch: true}));
+    });
+
     it('does not restart the first page while a later page is loading', async () => {
         const queryJSON = getQueryJSON();
         mockCurrentSearchResults = makeCachedSearchResults(queryJSON.hash, true, CONST.SEARCH.SNAPSHOT_STATE.LOADING, CONST.SEARCH.RESULTS_PAGE_SIZE);