fix: Search page re-fires a first page that is already refetching

src/hooks/useSearchPageSetup.ts

@@ -2,7 +2,8 @@ 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, isFirstPageSearchInFlight, markPageRequestedSearch, openSearch, search} from '@libs/actions/Search';
+import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
 import {hasPendingSearchWrite} from '@libs/pendingSearchWrite';
 import {isSearchDataLoaded, isSearchPending} from '@libs/SearchUIUtils';
 
@@ -70,6 +71,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 hash this page has shown since the user last came to Search. Its recent search was saved on arrival, so a later refetch need not save it again.
+    const shownHashRef = useRef<number | undefined>(undefined);
+
     // Clear selected transactions when navigating to a different search query
     function clearOnHashChange() {
         if (hash === undefined) {
@@ -121,6 +125,13 @@ function useSearchPageSetup(queryJSON: Readonly<SearchQueryJSON> | undefined) {
         // Pagination must not restart page one.
         // First open of a preloaded tab refreshes its cached results and saves the recent search, with no loading state over them.
         if (isSnapshotDataLoaded && !isInitialSearchPending && !isFirstOpenOfPreloadedTab) {
+            shownHashRef.current = hash;
+            return;
+        }
+
+        // A refetch started elsewhere (auto-refetch, post-action refresh) marks the shown snapshot pending, which reopens the guard above.
+        // Requesting again would only re-fire that refetch with the save flag, which this visit already sent.
+        if (!isFirstOpenOfPreloadedTab && isSnapshotDataLoaded && shownHashRef.current === hash && isFirstPageSearchInFlight(queryJSON, shouldCalculateTotals, shouldReturnTripRooms)) {
             return;
         }
 
@@ -165,6 +176,18 @@ function useSearchPageSetup(queryJSON: Readonly<SearchQueryJSON> | undefined) {
 
     useFocusEffect(dropPageRequestOnBlur);
 
+    // An RHP over Search blurs the page while the user is still on it, so only leaving Search for another fullscreen page counts.
+    function forgetShownHashOnLeave() {
+        return () => {
+            if (isSearchTopmostFullScreenRoute()) {
+                return;
+            }
+            shownHashRef.current = undefined;
+        };
+    }
+
+    useFocusEffect(forgetShownHashOnLeave);
+
     // Stable callback: useFocusEffect re-subscribes on a new identity and would fire an extra request.
     useFocusEffect(
         useCallback(() => {

src/libs/actions/Search.ts

@@ -1284,6 +1284,25 @@ type InFlightSearchRequest = {
 // request to run immediately afterward.
 const inFlightSearchRequests = new Map<string, InFlightSearchRequest>();
 
+/**
+ * True when a running first-page request already returns what the caller wants, so the caller can skip its own request.
+ * Background refreshes (auto-refetch, post-action refresh, Home) may differ in totals, footer breakdown or trip rooms.
+ * A caller that needs any of those must still fire, or search() never re-fires the running request and that data never arrives.
+ */
+function isFirstPageSearchInFlight(queryJSON: Readonly<SearchQueryJSON>, shouldCalculateTotals: boolean, shouldReturnTripRooms: boolean) {
+    const inFlightRequest = inFlightSearchRequests.get(`${queryJSON.hash}_0`);
+    if (!inFlightRequest) {
+        return false;
+    }
+
+    const isTripRoomsSearch = shouldReturnTripRooms && queryJSON.type === CONST.SEARCH.DATA_TYPES.TRIP;
+    const isMissingTotals = shouldCalculateTotals && !inFlightRequest.shouldCalculateTotals;
+    const hasDifferentFooterTotal = getFooterSelectionFromQuery(queryJSON).footerTotal !== inFlightRequest.footerTotal;
+    const hasDifferentTripRooms = isTripRoomsSearch !== inFlightRequest.shouldReturnTripRooms;
+
+    return !isMissingTotals && !hasDifferentFooterTotal && !hasDifferentTripRooms;
+}
+
 // Search mounts only after the page's request has finished, so the map above no longer holds it.
 // One slot, not a set: a new query replaces the old token, so a token left behind by an abandoned query cannot skip a later refresh of it.
 let pageRequestedSearch: {hash: number; shouldCalculateTotals: boolean} | undefined;
@@ -2864,6 +2883,7 @@ export {
     getReportActionsFromSearchSnapshot,
     resolveSearchPayPaymentMethod,
     markPageRequestedSearch,
+    isFirstPageSearchInFlight,
     consumePageRequestedSearch,
     clearPageRequestedSearch,
 };

tests/unit/Search/searchSaveRecentSearchFlagTest.ts

@@ -1,4 +1,4 @@
-import {search} from '@libs/actions/Search';
+import {isFirstPageSearchInFlight, search} from '@libs/actions/Search';
 import {makeRequestWithSideEffects, waitForWrites} from '@libs/API';
 import {buildSearchQueryJSON} from '@libs/SearchQueryUtils';
 
@@ -85,6 +85,30 @@ describe('search shouldSaveRecentSearch flag', () => {
         expect(getLastRequestJsonQuery()).toEqual(expect.objectContaining({shouldSaveRecentSearch: false}));
     });
 
+    it('reports a running first page as covering a page request unless the page needs totals it lacks', async () => {
+        // Given an unflagged first-page refetch without totals that is still waiting for its response
+        const queryJSON = getQueryJSON('type:expense merchant:costco');
+        let resolveRequest: () => void = () => {};
+        mockedMakeRequestWithSideEffects.mockImplementationOnce(
+            () =>
+                new Promise<void>((resolve) => {
+                    resolveRequest = resolve;
+                }),
+        );
+        const request = search({queryJSON, searchKey: CONST.SEARCH.SEARCH_KEYS.EXPENSES, offset: 0, isLoading: false});
+        await Promise.resolve();
+
+        // When the page asks whether it can rely on that request
+        // Then it can without totals, but not when it needs totals, because those would be dropped
+        expect(isFirstPageSearchInFlight(queryJSON, false, false)).toBe(true);
+        expect(isFirstPageSearchInFlight(queryJSON, true, false)).toBe(false);
+
+        // And once the request finishes nothing is in flight any more
+        resolveRequest();
+        await request;
+        expect(isFirstPageSearchInFlight(queryJSON, false, false)).toBe(false);
+    });
+
     it('preserves the flag on a totals request queued behind an in-flight search', async () => {
         // Given a search request that is still waiting for its response
         const queryJSON = getQueryJSON('type:expense merchant:starbucks');

tests/unit/hooks/useSearchPageSetup.test.ts

@@ -12,6 +12,10 @@ import type * as NativeNavigation from '@react-navigation/native';
 const mockSearch = jest.fn<void, unknown[]>();
 let mockCurrentSearchResults: SearchResults | undefined;
 let mockShouldUseLiveData = false;
+const mockMarkPageRequestedSearch = jest.fn<void, unknown[]>();
+let mockIsSearchTopmostFullScreenRoute = true;
+let mockFocusEffects: Array<() => void | (() => void)> = [];
+const mockIsFirstPageSearchInFlight = jest.fn<boolean, unknown[]>(() => false);
 
 jest.mock('@components/Search/SearchContext', () => ({
     useSearchQueryContext: () => ({currentSearchKey: 'expenses'}),
@@ -31,15 +35,27 @@ jest.mock('@libs/actions/ReportNavigation', () => ({
 jest.mock('@libs/actions/Search', () => ({
     openSearch: jest.fn(),
     search: (...args: unknown[]) => mockSearch(...args),
-    markPageRequestedSearch: jest.fn(),
+    markPageRequestedSearch: (...args: unknown[]) => mockMarkPageRequestedSearch(...args),
     clearPageRequestedSearch: jest.fn(),
+    isFirstPageSearchInFlight: (...args: unknown[]) => mockIsFirstPageSearchInFlight(...args),
 }));
 
+jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => () => mockIsSearchTopmostFullScreenRoute);
+
 jest.mock('@react-navigation/native', () => ({
     ...jest.requireActual<typeof NativeNavigation>('@react-navigation/native'),
-    useFocusEffect: jest.fn(),
+    useFocusEffect: (effect: () => void | (() => void)) => {
+        mockFocusEffects.push(effect);
+    },
 }));
 
+/** Runs the cleanups of the latest render's focus effects, like React Navigation does when the screen loses focus. */
+function blurScreen() {
+    for (const effect of mockFocusEffects) {
+        effect()?.();
+    }
+}
+
 function makeUnresolvedSearchResults(hash: number, isLoading: boolean): SearchResults {
     const searchResults: SearchResults = {
         data: {personalDetailsList: {}},
@@ -87,7 +103,12 @@ function getQueryJSON() {
 describe('useSearchPageSetup', () => {
     beforeEach(() => {
         mockSearch.mockClear();
+        mockMarkPageRequestedSearch.mockClear();
+        mockIsFirstPageSearchInFlight.mockClear();
+        mockIsSearchTopmostFullScreenRoute = true;
+        mockFocusEffects = [];
         mockShouldUseLiveData = false;
+        mockIsFirstPageSearchInFlight.mockReturnValue(false);
     });
 
     it('retries an unresolved search when temporary search prevention clears', async () => {
@@ -118,6 +139,103 @@ describe('useSearchPageSetup', () => {
         expect(mockSearch).toHaveBeenCalledWith(expect.objectContaining({isLoading: false}));
     });
 
+    it('does not re-request a shown first page that another caller is already refetching', async () => {
+        // Given a page showing a settled snapshot, whose recent search was saved when the user arrived
+        const queryJSON = getQueryJSON();
+        const {rerender} = renderHook(
+            ({state}) => {
+                mockCurrentSearchResults = makeCachedSearchResults(queryJSON.hash, false, state);
+                useSearchPageSetup(queryJSON);
+            },
+            {initialProps: {state: CONST.SEARCH.SNAPSHOT_STATE.LOADED as SearchResults['search']['state']}},
+        );
+        await Promise.resolve();
+        expect(mockSearch).not.toHaveBeenCalled();
+
+        // When an auto-refetch of the same first page starts and marks the snapshot pending
+        mockIsFirstPageSearchInFlight.mockReturnValue(true);
+        rerender({state: CONST.SEARCH.SNAPSHOT_STATE.LOADING});
+
+        // Then the page leaves the running request alone instead of re-firing the same query with the save flag
+        await Promise.resolve();
+        expect(mockIsFirstPageSearchInFlight).toHaveBeenCalledWith(queryJSON, false, false);
+        expect(mockSearch).not.toHaveBeenCalled();
+    });
+
+    it('keeps skipping the refetch duplicate while an RHP covers the page', async () => {
+        // Given a page showing a settled snapshot
+        const queryJSON = getQueryJSON();
+        const {rerender} = renderHook(
+            ({state}) => {
+                mockCurrentSearchResults = makeCachedSearchResults(queryJSON.hash, false, state);
+                useSearchPageSetup(queryJSON);
+            },
+            {initialProps: {state: CONST.SEARCH.SNAPSHOT_STATE.LOADED as SearchResults['search']['state']}},
+        );
+        await Promise.resolve();
+
+        // When the user opens an expense in the RHP, which blurs the page while Search stays the topmost fullscreen route,
+        // and the auto-refetch for the open query starts
+        blurScreen();
+        mockIsFirstPageSearchInFlight.mockReturnValue(true);
+        rerender({state: CONST.SEARCH.SNAPSHOT_STATE.LOADING});
+
+        // Then the page still treats the query as shown and does not send a second request
+        await Promise.resolve();
+        expect(mockSearch).not.toHaveBeenCalled();
+    });
+
+    it('saves the recent search again after the user left Search and a refresh of the query is running', async () => {
+        // Given a page showing a settled snapshot
+        const queryJSON = getQueryJSON();
+        const {rerender} = renderHook(
+            ({state}) => {
+                mockCurrentSearchResults = makeCachedSearchResults(queryJSON.hash, false, state);
+                useSearchPageSetup(queryJSON);
+            },
+            {initialProps: {state: CONST.SEARCH.SNAPSHOT_STATE.LOADED as SearchResults['search']['state']}},
+        );
+        await Promise.resolve();
+
+        // When the user leaves Search for another fullscreen page and a refresh of the same query starts
+        mockIsSearchTopmostFullScreenRoute = false;
+        blurScreen();
+        mockIsFirstPageSearchInFlight.mockReturnValue(true);
+        rerender({state: CONST.SEARCH.SNAPSHOT_STATE.LOADING});
+
+        // Then the page sends its flagged request, because the next visit has not saved the recent search yet
+        await waitFor(() => expect(mockSearch).toHaveBeenCalledTimes(1));
+        expect(mockSearch).toHaveBeenCalledWith(expect.objectContaining({shouldSaveRecentSearch: true}));
+    });
+
+    it('still saves the recent search when the user arrives while another caller refreshes the query', async () => {
+        // Given Home is refreshing the query behind one of its rows, so the cached snapshot is already pending
+        const queryJSON = getQueryJSON();
+        mockCurrentSearchResults = makeCachedSearchResults(queryJSON.hash, false, CONST.SEARCH.SNAPSHOT_STATE.LOADING);
+        mockIsFirstPageSearchInFlight.mockReturnValue(true);
+
+        // When the user opens that query on the Search page
+        renderHook(() => useSearchPageSetup(queryJSON));
+
+        // Then the page still sends its flagged request, because this visit has not saved the recent search yet
+        await waitFor(() => expect(mockSearch).toHaveBeenCalledTimes(1));
+        expect(mockSearch).toHaveBeenCalledWith(expect.objectContaining({shouldSaveRecentSearch: true}));
+    });
+
+    it('still claims an unloaded first page whose request is already running', async () => {
+        // Given the user comes back to a query whose first request has not answered yet
+        const queryJSON = getQueryJSON();
+        mockCurrentSearchResults = makeUnresolvedSearchResults(queryJSON.hash, false);
+        mockIsFirstPageSearchInFlight.mockReturnValue(true);
+
+        // When the page sets up the query again
+        renderHook(() => useSearchPageSetup(queryJSON));
+
+        // Then it still claims the page and goes through search(), so Search does not request it a second time on mount
+        await waitFor(() => expect(mockSearch).toHaveBeenCalledTimes(1));
+        expect(mockMarkPageRequestedSearch).toHaveBeenCalledWith(queryJSON.hash, false);
+    });
+
     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);

tests/unit/useSearchPageSetupTest.tsx

@@ -51,6 +51,7 @@ jest.mock('@libs/actions/Search', () => ({
     openSearch: (...args: [unknown, number | undefined]) => mockOpenSearch(...args),
     markPageRequestedSearch: (...args: unknown[]) => mockMarkPageRequestedSearch(...args),
     clearPageRequestedSearch: () => mockClearPageRequestedSearch(),
+    isFirstPageSearchInFlight: () => false,
 }));
 
 jest.mock('@libs/actions/ReportNavigation', () => ({