Skip to content

Auto -Spend - "Next" arrow is disabled when user reaches 50 of 50 report #101453

Description

@jponikarchuk

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.4.81-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from BrowserStack: https://test-management.browserstack.com/projects/2219752/test-runs/74236221/folder/54000978/51874675/1766787876?sort=%2Bidentifier
Email or phone of affected tester (no customers): applausetester+testaccountforreports@applause.expensifail.com
Issue reported by: Applause Internal Team
Bug source: Regression TC Execution
Device used: Windows 11 Home/Chrome
App Component: Search

Action Performed:

Preconditions:

  1. Open https://staging.new.expensify.com/
  2. Go to Spend > Reports
  3. Click on the first report
  4. Click the "Next" navigation arrow repeatedly to cycle through the reports until you reach the 50th report
  5. Click the "Next" arrow again to navigate to report number 51

Expected Result:

Verify it is possible to click the "Next" arrow again to navigate to report number 51

Actual Result:

"Next" arrow is disabled and unable to click

Workaround:

Unknown

Platforms:

  • Android: App
  • Android: mWeb Chrome
  • iOS: App
  • iOS: mWeb Safari
  • iOS: mWeb Chrome
  • Windows: Chrome
  • MacOS: Chrome / Safari

Screenshots/Videos

Video.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @grgia

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Sep 17, 2026
  2. changed the title [-]Auto -Spend- "Next" arrow is disabled when user reaches 50 of 50 report[/-] [+]Auto -Spend - "Next" arrow is disabled when user reaches 50 of 50 report[/+] on Sep 17, 2026
  3. MelvinBot commented on Sep 17, 2026

    @MelvinBot
    Contributor

    Proposal

    What is the root cause of that problem?

    The arrows and the search results read two different lists, and only one of them grows when page 2 loads.

    MoneyRequestReportNavigation walks sortedReportIDs from the app-level Search context, and it prefers that list whenever it is non-empty and no page is in flight — MoneyRequestReportNavigation.tsx:127-129. The only writer of that list is useSaveSortedReportIDs.ts:17-29, called from the Search list screen at Search/index.tsx:811.

    Opening a report puts that Search screen behind a right-hand modal, where FreezeWrapper freezes it. Its effects stop running, so sortedReportIDs is stuck at the 50 IDs captured before you opened the first report — 50 being CONST.SEARCH.RESULTS_PAGE_SIZE.

    The arrow does fetch page 2 (:207-223) and those reports do land in the Onyx snapshot — but the list that can see them (useSearchSections) is only mounted while loading (:242). The moment the page arrives, loading flips false and the component reverts to the frozen 50.

    So at report 50, :149 disables the button, and the label reads "50 of 50" because the count is re-synced from that same frozen list.

    Two secondary defects that make this worse

    1. The tail wraps instead of advancing. :225 computes (currentIndex + 1) % effectiveAllReports.length. There is no "wait for the next page" path — so with >100 matching reports the button stays enabled but throws you back to report 1. Report 51 is unreachable either way.

    2. hasMoreResults can be erased permanently. saveLastSearchParams uses Onyx.set, not merge. Search.ts:1244-1251 writes that key before the request without hasMoreResults, wiping it. It is restored only in .then() and only when both response.search.offset and prevReportsLength are truthy (:1287-1315); the .catch() restores nothing. One failed request and the flag stays undefined, which permanently disables the arrow with no retry — unlike the list view, which re-arms via wantedOffsetRef (Search/index.tsx:883-890).

    Note the Search list's own infinite scroll never writes this key — shouldUpdateLastSearchParams defaults to false and only the arrow passes true. So the arrows' entire pagination ability rests on a single snapshot read taken at the instant you clicked the report row (Search/index.tsx:697-705).

    What changes do you think we should make in order to solve the problem?

    1. Make the snapshot the source of truth for the arrow list. Stop preferring the frozen Search-context list. Either mount the useSearchSections list unconditionally in MoneyRequestReportNavigation, or prefer whichever list is longer. The context list is a render-time cache of a screen that is frozen for the entire time the arrows are on screen, so it can never be authoritative.

    2. Don't wrap at the tail. Replace the modulo at :225 with a bounds check: if there is no next item and a page is in flight, hold position (show a loading state on the arrow) and advance once the list grows.

    3. Read hasMoreResults and offset straight off the snapshot, the way fetchMoreResults already does (Search/index.tsx:836-880), instead of the separately-persisted copy in REPORT_NAVIGATION_LAST_SEARCH_QUERY. That removes the whole wipe-and-restore class of bug.

    4. If that duplicated key must stay, change saveLastSearchParams to merge and restore the flag in .catch() too.

    Fix 1 alone unblocks report 51. Fixes 2-4 stop it regressing in the failure and >100-report cases.

    Add a regression test: today MoneyRequestReportNavigation.test.tsx asserts which list is picked, but never that the list grows after a page lands — which is exactly the untested gap.

    What alternative solutions did you explore? (Optional)

    • Raising RESULTS_PAGE_SIZE — moves the wall to 100 instead of removing it.
    • Un-freezing the Search screen while the report modal is open — fixes the symptom but costs re-renders of the full search list on every arrow click, and leaves defects 2-4 in place.
    • Backend not reporting more results — considered and set aside: the Search list's own infinite scroll does load past 50 for the same query, so the server is reporting hasMoreResults: true.
    Confidence and what I did not verify

    High confidence on the mechanism, the code paths, and the disabled condition — two independent investigations converged on the same lines, and I re-read them directly.

    Not verified:

    • I did not reproduce this live. It needs an account with 51+ matching expense reports, which the test account available to me does not have.
    • Whether the user-visible symptom is disabled (51-100 reports) or wraps to report 1 (>100 reports) depends on the server's hasMoreResults after the offset-50 request. The report says disabled, which fits 51-100.
    • I could not run git blame to pin an introducing PR (shallow checkout), so I am not attributing this to a specific merge.

    Next Steps for Contributor+ team:
    To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/101453) to create a draft PR.
    To refine: @MelvinBot <your feedback>
    To reject: Explain why you are rejecting Melvin's proposal.


    view run

  4. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Sep 17, 2026
  5. melvin-bot commented on Sep 17, 2026

    @melvin-bot

    Triggered auto assignment to Contributor-plus team member for initial proposal review - @marufsharifi (External)

  6. melvin-bot commented on Sep 17, 2026

    @melvin-bot

    Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

  7. marufsharifi commented on Sep 22, 2026

    @marufsharifi
    Contributor

    🆘 C+ Help Wanted

  8. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Sep 22, 2026
  9. Abdulloh0109 commented on Sep 22, 2026

    @Abdulloh0109
    Contributor

    Proposal

    What is the root cause of that problem?

    The Next arrow and "50 of 50" counter are rendered by MoneyRequestReportNavigationContent → PrevNextButtons / the count Text (MoneyRequestReportNavigation.tsx#L243-L253). Both read effectiveAllReports, and that list caps at the first 50 IDs.

    The component has two sources for the list. The fast path is the Search-context list contextReports; the slow path is standaloneReports, lifted from a child that mounts useSearchSections and reads the live snapshot. Which one wins is decided by one line (:127):

    const shouldUseContextReports = contextReports.length > 0 && !isSearchLoading;
    

    Walk the repro. On report ~38 the arrow correctly fetches page 2 (:209-222). While it's in flight isSearchLoading is true, so the standalone child mounts and — because useSearchSections builds its list straight from currentSearchResults.data, which pagination merges into (useSearchSections.ts#L47-L74) — standaloneReports correctly grows to the full set (say 60). The moment the page lands, isSearchLoading flips false, shouldUseContextReports flips back to true, and allReports reverts to contextReports. That context list is frozen at the 50 IDs captured before the report opened (the Search list screen behind the RHP no longer updates it), so the 10 reports the arrow just paged in are thrown away.

    At report 50 that leaves currentIndex === effectiveAllReports.length - 1 with hasMoreResults now false (page 2 was the last page for a 51–100 account), so the disabled condition fires (:149), and the label is "50 of 50" because previousLengthOfResults was persisted as the stale prevReportsLength (50) on the page-2 response (Search.ts#L1295-L1299). The list the arrow can see never grows past page 1, even though the snapshot did.

    So !isSearchLoading is the wrong gate: it's a transient signal, and once it clears we hand authority back to a list that pagination has already made stale.

    What changes do you think we should make in order to solve the problem?

    Layer: the source-selection in the content component (hook/render), not the data or the API.

    Latch the source on an explicit "have we paginated" signal instead of the transient loading flag. lastSearchQuery.offset starts at 0 for a page-1 result set (Search/index.tsx#L165 → saved at the report row click, #L699-L705) and only ever grows once a page is pulled. So at :127:

    // offset > 0 means the result set was extended past page 1; the frozen context list is now stale.
    const hasPaginated = (lastSearchQuery?.offset ?? 0) > 0;
    const shouldUseContextReports = contextReports.length > 0 && !isSearchLoading && !hasPaginated;
    

    Once you've paged, shouldUseContextReports stays false, so the standalone child stays mounted (:242) and keeps standaloneReports in sync with the snapshot as further pages land. Report 51 becomes reachable, and the counter self-corrects because the recount effect notices effectiveAllReports.length changed and rewrites previousLengthOfResults (:168) — so "50 of 50" turns into "50 of 60" and only disables at the true last report.

    This is race-free: it doesn't depend on whether the page-2 response lands before or after isSearchLoading toggles. The frozen list is structurally removed from the pipeline the instant pagination begins, rather than being preferred again on the next render. It also preserves the #86238 fast-path optimization untouched for the common case (≤50 reports never paginate, so offset stays 0 and the heavy subscriptions never mount).

    Verification: I traced the full save/restore sequence at the issue SHA but could not run jest in this environment (the App node_modules aren't provisioned here), so I'm giving the concrete regression instead of a fabricated number. Add to tests/unit/components/MoneyRequestReportNavigation.test.tsx, whose helper mirrors line 127: a case where the context list is frozen at 50, the snapshot list is 60, not loading, and offset = 50. Against main's selection it resolves to the 50-item context list (allReports length 50, report 55 absent) — fails; with the !hasPaginated gate it resolves to the 60-item standalone list (report 55 reachable) — passes. The four existing path-selection/cache tests are unaffected: they run with no pagination (offset undefined → hasPaginated false), so the fast path they assert is unchanged.

    What alternative solutions did you explore? (Optional)

    Selecting whichever list is longer is the other obvious fix, but it reverses the existing "uses fast path" assertion at test #L91-L99 (a longer standalone list would win on the pure fast path) and leans on a stale retained value rather than an explicit signal — the offset latch keeps both the test and the perf gate intact.

    Growing the Search-context sortedReportIDs while the RHP is open — rejected: the list screen is frozen behind the modal on purpose, and un-freezing it re-renders the whole search list on every arrow click for no benefit the snapshot path doesn't already give us.

    One thing my fix deliberately doesn't touch: the % effectiveAllReports.length wrap at :225 can still bounce you to report 1 on a 100+ account where hasMoreResults stays true at the tail. That's a distinct bounds-check bug on a different code path from this "disabled at 50" report, so I'd keep it out of scope here rather than fold two fixes into one.

  10. x-dev90 commented on Sep 22, 2026

    @x-dev90
    Contributor

    🚨 Edited by proposal-police: This proposal was edited at 2026-10-09 11:16:11 UTC.

    Proposal

    Please re-state the problem that we are trying to solve.

    When navigating through paginated expense reports in Search, the report Prev/Next controls can become unreliable at page boundaries.

    Examples include:

    • The Next button becomes disabled at 50 of 50, although more reports exist.
    • A rapid Next click at the loaded tail is dropped instead of advancing after the next page arrives.
    • Controls disappear after moving to a later report because the active report is absent from a transient 50-item source.
    • An unfocused Search auto-refetch can replace an expanded snapshot with page-one data and discard later loaded pages.

    What is the root cause of that problem?

    Several independent asynchronous flows update the same search state.

    1. Pagination metadata was replaced during a page request

    saveLastSearchParams() uses Onyx.set(). A pagination request supplied only partial navigation data, which replaced fields retained from the previous response, including hasMoreResults and previousLengthOfResults.

    While the request was in flight, the navigation component could read:

    lastSearchQuery.hasMoreResults === undefined;

    This made the Next button look terminal even though the previous page had confirmed more results exist.

    1. A paginated report can temporarily read an incomplete list

    sortedReportIDs is the lightweight Search-context list. It is suitable for the first page, but it can temporarily contain only the first 50 IDs during page transitions.
    If the active report is, for example, report 51, choosing that 50-item list makes:

    allReports.indexOf(reportID) === -1;

    Since the controls require a valid current index, the counter and arrows disappear.

    1. Next at a page boundary had no durable intent

    At the tail of the currently loaded list, the next report ID does not yet exist locally. A click must be retained until the next page arrives. Without a scoped pending intent, the click is lost; without query scoping, a delayed response could navigate the user in a different search.

    1. Concurrent page requests can race

    Rapid navigation near a prefetch threshold can request multiple pagination offsets for the same query before the earlier request settles. Responses can then arrive out of order and overwrite report-navigation metadata.

    1. An unfocused auto-refetch can discard loaded pages

    useSearchAutoRefetch can issue an offset-zero refresh while an RHP report is open. An offset-zero response replaces the search snapshot collection, which can discard previously loaded page-two-and-beyond records.

    What changes do you think we should make in order to solve the problem?

    1. Merge incremental report-navigation state instead of replacing it
    Add mergeLastSearchParams() in ReportNavigation.ts:

    function mergeLastSearchParams(value: Partial<LastSearchParams>) {
        Onyx.merge(ONYXKEYS.REPORT_NAVIGATION_LAST_SEARCH_QUERY, value);
    }

    Keep saveLastSearchParams() with Onyx.set() for creating a new Search session, where replacing the old query state remains correct.
    Use mergeLastSearchParams() for pagination request and response updates in Search.ts. This preserves hasMoreResults and previousLengthOfResults while a subsequent page is loading.

    2. Preserve standard offset-zero recount behavior
    The response handler must preserve the existing distinction between an initial Search response and report-navigation pagination:

    const isReportNavigationPagination = shouldUpdateLastSearchParams && offset !== undefined && offset > 0;

    For a successful offset-zero response:

    • Count returned report entries for previousLengthOfResults.
    • Set allowPostSearchRecount: true.

    For report-navigation pagination:

    • Preserve the supplied prevReportsLength.
    • Set allowPostSearchRecount: false.
    const reportCount =
        offset === 0
            ? Object.keys(response?.data ?? {}).filter((key) => key.startsWith(ONYXKEYS.COLLECTION.REPORT)).length
            : undefined;
    
    mergeLastSearchParams({
        queryJSON,
        offset,
        ...(hasMoreResults !== undefined && {hasMoreResults}),
        ...(prevReportsLength !== undefined && {previousLengthOfResults: prevReportsLength}),
        ...(reportCount !== undefined && prevReportsLength === undefined && {previousLengthOfResults: reportCount}),
        allowPostSearchRecount: !isReportNavigationPagination,
        searchKey,
    });

    The pre-request update must keep allowPostSearchRecount: false, so stale data cannot trigger a recount before the request succeeds.

    3. Serialize report-navigation pagination per query
    Extend the existing in-flight Search request registry with request metadata:

    type InFlightSearchRequest = {
        queryHash: number;
        isReportNavigationPagination: boolean;
        // Existing fields
    };

    Before starting a report-navigation pagination request, prevent another pagination request for the same query hash from starting:

    const hasInFlightReportNavigationPagination = [...inFlightSearchRequests.values()].some(
        (request) => request.isReportNavigationPagination && request.queryHash === queryJSON.hash,
    );
    
    if (isReportNavigationPagination && hasInFlightReportNavigationPagination) {
        return;
    }

    Use the existing request cleanup path in both request completion and write-queue failure handling. This avoids a second module-level cache and prevents stale locks.

    4. Select a list that contains the active report
    Continue using the lightweight Search-context list only when it covers the loaded offset:

    const shouldUseContextReports = contextReports.length > (lastSearchQuery?.offset ?? 0) && !isSearchLoading;

    When the preferred source does not contain the active report, select another available source that does:

    const preferredReports = shouldUseContextReports ? contextReports : standaloneReports;
    let allReports = preferredReports;
    
    if (!preferredReports.includes(reportID)) {
        if (standaloneReports.includes(reportID)) {
            allReports = standaloneReports;
        } else if (contextReports.includes(reportID)) {
            allReports = contextReports;
        } else if (lastValidReports?.includes(reportID)) {
            allReports = lastValidReports;
        }
    }

    This prevents a temporary empty standalone list or first-page context list from making currentIndex become -1.

    5. Queue Next navigation at a boundary, scoped to the current search and report
    Replace the boolean pending state with a query-scoped intent:

    type PendingNextNavigation = {
        queryHash: number;
        reportID: string;
    };

    At the loaded tail, retain the click only if more results are expected:

    setPendingNextNavigation({queryHash, reportID});

    When the list expands, consume the intent only if both the active query and active report still match:

    if (pendingNextNavigation.queryHash !== queryHash || pendingNextNavigation.reportID !== reportID) {
        setPendingNextNavigation(null);
        return;
    }

    Cancel the intent on request failure, terminal results, or Previous navigation.
    This prevents lost Next clicks while ensuring an old response cannot advance a new search session.

    6. Defer unfocused refreshes for expanded search snapshots
    In useSearchAutoRefetch, do not issue an offset-zero refresh while Search is unfocused and the snapshot already contains later pages:

    const hasLoadedAdditionalPages = (searchResults?.search?.offset ?? 0) > 0;
    
    if (!isFocused && (!hasAGenuinelyNewID || hasLoadedAdditionalPages)) {
        hasPendingSearchRef.current = true;
        return;
    }

    The deferred refresh runs when Search is focused again. This preserves page-two-and-beyond snapshot data while the user navigates reports in the RHP.

    What alternative solutions did you explore? (Optional)

    Always use useSearchSections()
    This avoids source handoff, but it mounts expensive subscriptions for every first-page report view and regresses the existing lightweight fast path.

    Use a separate module-level pagination map
    A separate mutable map can serialize requests, but it duplicates the existing in-flight request lifecycle and creates unnecessary cleanup/test-leak risk. Extending the existing request registry keeps ownership and cleanup in one place.

    Only defer non-new-item auto-refetches
    This still allows a genuinely new transaction created while an RHP report is open to trigger an offset-zero refresh and truncate an expanded snapshot. Expanded snapshots must defer all unfocused refreshes.

    Test plan

    1. Open an expense-report Search with more than 100 reports.
    2. Navigate slowly and rapidly across the 50 → 51 and 100 → 101 boundaries.
    3. Verify the Next click at a boundary advances automatically when the next page arrives.
    4. Verify the counter and Prev/Next controls remain visible throughout navigation.
    5. Navigate backward across a page boundary and verify no delayed Next intent changes the route.
    6. Create or modify an expense while a later-page report is open in the RHP, then confirm loaded pages remain available for navigation.
    7. Return to the Search page and verify the deferred refresh is applied.
    8. Verify a standard offset-zero Search response still enables post-search recount behavior.
  11. mukhrr commented on Sep 22, 2026

    @mukhrr
    Contributor

    Proposal

    What is the root cause of that problem?

    I found that the navigation switches between two report lists. contextReports comes from the Search screen, while standaloneReports is rebuilt from the live Onyx snapshot. Once loading finishes, shouldUseContextReports becomes true again and the navigation discards the newly paginated standalone list.

    // Lightweight subscriptions only: the current search query and its loading flag. These never mount
    // the heavy useSearchSections subscription set, so the fast context path stays cheap.
    const [lastSearchQuery] = useOnyx(ONYXKEYS.REPORT_NAVIGATION_LAST_SEARCH_QUERY);
    const [isSearchLoading = false] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${lastSearchQuery?.queryJSON?.hash}`, {selector: searchLoadingSelector});
    // Fast path: use the pre-computed IDs from the search context when they are usable and no page is in
    // flight. Otherwise fall back to the standalone list, which is produced by the child below that mounts
    // the heavy subscriptions only on this slow path. Because this is a value swap inside a single, stable
    // component, toggling isSearchLoading (e.g. the search refresh triggered by submitting a report) no
    // longer unmounts the component and wipes the lastValidReports cache below.
    const shouldUseContextReports = contextReports.length > 0 && !isSearchLoading;
    const [standaloneReports, setStandaloneReports] = useState<Array<string | undefined>>([]);
    const allReports = shouldUseContextReports ? contextReports : standaloneReports;

    The context list is populated by an effect in the Search screen:

    function useSaveSortedReportIDs(type: SearchDataTypes, items: Array<{reportID?: string | undefined}>) {
    const {setSortedReportIDs} = useSearchResultsActions();
    useEffect(() => {
    // Only expense-report searches produce report-level IDs suitable for navigation arrows.
    // For all other types (expense, invoice, etc.) the items are transaction-level and share
    // reportIDs, so we clear the context to force the fallback computation in navigation.
    if (type !== CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT) {
    setSortedReportIDs([]);
    return;
    }
    const reportIDs = items.map((item) => item.reportID);
    setSortedReportIDs(reportIDs);
    }, [type, items, setSortedReportIDs]);

    That screen is frozen while the report RHP is open, so the effect does not copy page 2 into the context. The snapshot-derived component is mounted only during loading and immediately unmounted afterward:

    return (
    <>
    {/* Slow path only: mount the heavy subscriptions and lift the computed list up. Rendered even
    when the arrows are hidden, since standaloneReports is what decides whether to show them. */}
    {!shouldUseContextReports && <MoneyRequestReportNavigationStandalone onReportsChange={setStandaloneReports} />}
    {shouldDisplayNavigationArrows && (
    <View style={[styles.flexRow, styles.alignItemsCenter, styles.gap2]}>
    {!shouldDisplayNarrowVersion && <Text style={styles.mutedTextLabel}>{`${currentIndex + 1} of ${allReportsCount}`}</Text>}
    <PrevNextButtons
    isPrevButtonDisabled={hidePrevButton}
    isNextButtonDisabled={hideNextButton}

    As a result, page 2 can be fetched successfully, but effectiveAllReports returns to the original 50 context IDs. At report 50, that stale list makes the next button disabled:

    const effectiveAllReports = liveCurrentIndex === -1 && lastValidReports ? lastValidReports : allReports;
    const currentIndex = effectiveAllReports.indexOf(reportID);
    const allReportsCount = lastSearchQuery?.previousLengthOfResults ?? 0;
    const hideNextButton = !lastSearchQuery?.hasMoreResults && currentIndex === effectiveAllReports.length - 1;
    const hidePrevButton = currentIndex === 0;
    const shouldDisplayNavigationArrows = effectiveAllReports.length > 1 && currentIndex !== -1 && !!lastSearchQuery?.queryJSON;

    The existing test also codifies the faulty transition by expecting the shorter context list whenever loading is false, even when the snapshot list contains more reports:

    describe('path selection', () => {
    it('uses fast path (context list) when context has IDs and not loading', () => {
    mockSortedReportIDs = ['1', '2'];
    mockUseSearchSections.mockReturnValue({allReports: ['1', '2', '3'], isSearchLoading: false, lastSearchQuery: undefined});
    const {result} = renderHook(() => useNavigationSource('1'));
    expect(result.current.source).toBe('fast');
    expect(result.current.allReports).toEqual(['1', '2']);
    });

    What changes do you think we should make in order to solve the problem?

    I think we should latch MoneyRequestReportNavigationContent onto the standalone source after it first enters that path. This keeps useSearchSections mounted after pagination, allowing later snapshot additions and removals to continue updating the carousel. The lightweight context path remains unchanged for report views that never paginate.

    For example, the source selection would follow this small change:

    +const hasUsedStandalonePath = useRef(false);
    +hasUsedStandalonePath.current ||= isSearchLoading;
    -const shouldUseContextReports = contextReports.length > 0 && !isSearchLoading;
    +const shouldUseContextReports =
    +    contextReports.length > 0 && !isSearchLoading && !hasUsedStandalonePath.current;

    I would add a test that starts with 50 context reports, enters the loading state with an expanded standalone list, then finishes loading while the context remains at 50. It should continue using the standalone list and enable navigation to report 51. I would also verify that removing a report from the snapshot updates this list after pagination.

    This avoids a regression in the normal fast path: useSearchSections and its heavier subscriptions still remain unmounted when no pagination or fallback has occurred. Once the live source is needed, it stays subscribed rather than retaining a stale one-time copy.

    What alternative solutions did you explore? (Optional)

    Always mounting useSearchSections would fix pagination, but it would remove the intentional lightweight path and subscribe every report view to the heavier Search data. Choosing whichever list is longer would fix the first append, but the standalone child would remain unmounted after loading, so its cached list would not receive later deletions or other same-length changes.

  12. abbasifaizan70 commented on Sep 22, 2026

    @abbasifaizan70
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    On Spend > Reports, opening a report and using the header's "Next" arrow to page through the search results works fine until the 50th report. At that point "Next" becomes disabled and the user cannot advance to report #51, even though more reports exist in the underlying search results.

    What is the root cause of that problem?

    The report header's carousel arrows are rendered by MoneyRequestReportNavigationContent. Its "Next" disabled state is not a simple index >= length check — it also depends on whether more results are known to exist on the server:

    const allReportsCount = lastSearchQuery?.previousLengthOfResults ?? 0;
    const hideNextButton = !lastSearchQuery?.hasMoreResults && currentIndex === effectiveAllReports.length - 1;

    lastSearchQuery.hasMoreResults is read from the REPORT_NAVIGATION_LAST_SEARCH_QUERY Onyx key. goToNextReport is supposed to keep this key accurate by prefetching the next page of results once the user is 75% of the way through the currently-loaded page (50 items, CONST.SEARCH.RESULTS_PAGE_SIZE):

    const goToNextReport = () => {
    if (currentIndex === -1 || effectiveAllReports.length === 0 || !lastSearchQuery?.queryJSON) {
    return;
    }
    const threshold = Math.min(effectiveAllReports.length * 0.75, effectiveAllReports.length - 2);
    if (currentIndex + 1 >= threshold && lastSearchQuery?.hasMoreResults) {
    const newOffset = (lastSearchQuery.offset ?? 0) + CONST.SEARCH.RESULTS_PAGE_SIZE;
    const queryJSON = lastSearchQuery.queryJSON;
    requestAnimationFrame(() => {
    search({
    queryJSON,
    offset: newOffset,
    prevReportsLength: effectiveAllReports.length,
    shouldCalculateTotals: false,
    searchKey: lastSearchQuery.searchKey,
    isLoading: isSearchLoading,
    shouldUpdateLastSearchParams: true,
    });
    });
    }
    const nextIndex = (currentIndex + 1) % effectiveAllReports.length;
    goToReportId(effectiveAllReports.at(nextIndex));
    };

    That prefetch calls search() with shouldUpdateLastSearchParams: true. This is the only call site in the whole codebase that passes shouldUpdateLastSearchParams: true — every other caller (useInsightData, useRecentlyAddedData, useYourSpendData) passes false. Inside search(), before the request is even sent, this flag triggers an immediate "optimistic" save:

    App/src/libs/actions/Search.ts

    Lines 1244 to 1251 in 920d627

    if (shouldUpdateLastSearchParams) {
    saveLastSearchParams({
    queryJSON,
    offset,
    allowPostSearchRecount: false,
    searchKey,
    });
    }

    Note this object only contains queryJSON, offset, allowPostSearchRecount, and searchKey — it omits hasMoreResults and previousLengthOfResults. saveLastSearchParams writes this with Onyx.set, a full overwrite (not a merge):

    function saveLastSearchParams(value: LastSearchParams) {
    Onyx.set(ONYXKEYS.REPORT_NAVIGATION_LAST_SEARCH_QUERY, value);
    }

    So the instant the prefetch for page 2 is dispatched, hasMoreResults is wiped from Onyx to undefined (falsy). It is only restored once the network response actually resolves, inside the .then() handler:

    App/src/libs/actions/Search.ts

    Lines 1287 to 1314 in 920d627

    if (shouldUpdateLastSearchParams) {
    const reports = Object.keys(response?.data ?? {})
    .filter((key) => key.startsWith(ONYXKEYS.COLLECTION.REPORT))
    .map((key) => key.replace(ONYXKEYS.COLLECTION.REPORT, ''));
    if (response?.search?.offset) {
    // Indicates that search results are extended from the Report view (with navigation between reports),
    // using previous results to enable correct counter behavior.
    if (prevReportsLength) {
    saveLastSearchParams({
    queryJSON,
    offset,
    hasMoreResults: !!response?.search?.hasMoreResults,
    previousLengthOfResults: prevReportsLength,
    allowPostSearchRecount: false,
    searchKey,
    });
    }
    } else {
    // Applies to all searches from the Search View
    saveLastSearchParams({
    queryJSON,
    offset,
    hasMoreResults: !!response?.search?.hasMoreResults,
    previousLengthOfResults: reports.length,
    allowPostSearchRecount: true,
    searchKey,
    });
    }

    If that response is slow, gets rejected (see the .catch a few lines below, which applies failure/finally Onyx updates for the snapshot but never restores hasMoreResults on REPORT_NAVIGATION_LAST_SEARCH_QUERY), or otherwise doesn't land before the user reaches the last item in the currently-loaded 50, lastSearchQuery.hasMoreResults is still falsy at that render. Combined with currentIndex === effectiveAllReports.length - 1 being true at report #50, hideNextButton evaluates to true — the arrow disables even though the backend genuinely has more reports. Because nothing ever re-triggers a fresh save of hasMoreResults after a failed/interrupted prefetch, this can remain stuck disabled indefinitely, which matches the reported "Workaround: Unknown."

    What changes do you think we should make in order to solve the problem?

    The premature save at Search.ts lines 1244-1251 is unnecessary: it exists solely to bump offset ahead of the request, but since it is exclusively used by this one prefetch call site, and the authoritative, complete save (with the real hasMoreResults, offset, and previousLengthOfResults) already happens in the .then() handler at lines 1287-1314, this pre-request write only serves to destructively clear pagination state that the UI depends on while the fetch is in flight. Removing it closes the whole race: lastSearchQuery.hasMoreResults simply keeps its last known-good value until the new page's response legitimately updates it, so "Next" stays correctly enabled while page 2 loads, and if the request fails, the button remains usable and the next click will naturally retry the fetch (the request's dedupeKey is cleared in the existing .finally() block regardless of success/failure).

    App/src/libs/actions/Search.ts

    Lines 1244 to 1251 in 920d627

    if (shouldUpdateLastSearchParams) {
    saveLastSearchParams({
    queryJSON,
    offset,
    allowPostSearchRecount: false,
    searchKey,
    });
    }

    function search({
        queryJSON,
        searchKey,
        offset,
        shouldCalculateTotals = false,
        prevReportsLength,
        isLoading,
        shouldUpdateLastSearchParams = false,
        skipWaitForWrites = false,
        shouldSaveRecentSearch = false,
    }: {
        ...

    Remove the if (shouldUpdateLastSearchParams) { saveLastSearchParams({...}); } block entirely (the four lines shown above at 1245-1250), leaving the rest of search() unchanged. The parameter shouldUpdateLastSearchParams is still used further down to gate the post-response save at lines 1287-1314, so it stays meaningful — only the redundant, data-dropping pre-request write is deleted.

    This keeps the fix minimal and confined to the one code path that actually causes the regression, without touching saveLastSearchParams's Onyx.set semantics (which other callers, e.g. Search/index.tsx and MoneyRequestReportNavigation.tsx's own effects, rely on for full-object replacement when starting a genuinely new query).

  13. 43 remaining items

  14. x-dev90 commented on Oct 2, 2026

    @x-dev90
  15. github-actions commented on Oct 2, 2026

    @github-actions
    Contributor

    ⚠️ @x-dev90 Thanks for your interest. To be considered for this job, please post a proposal following the proposal template (note the mandatory sections). Comments claiming the job without one are not reviewed.

  16. melvin-bot commented on Oct 5, 2026

    @melvin-bot

    @grgia Huh... This is 4 days overdue. Who can take care of this?

  17. grgia commented on Oct 6, 2026

    @grgia
    Contributor

    @marufsharifi @yusufdeveloper2903 @x-dev90 I'm looking into this, thank you

  18. grgia commented on Oct 6, 2026

    @grgia
    Contributor

    @marufsharifi it seems the the frozen-list claim is incorrect. This makes me think @yusufdeveloper2903's is the first correct proposal. What do you think about that?

  19. marufsharifi commented on Oct 7, 2026

    @marufsharifi
    Contributor

    Thanks! I’ll check it again and provide an update here.

  20. marufsharifi commented on Oct 9, 2026

    @marufsharifi
    Contributor

    I will update here in a few hours. don't overdue.

  21. marufsharifi commented on Oct 9, 2026

    @marufsharifi
    Contributor

    64. @marufsharifi it seems the the frozen-list claim is incorrect. This makes me think @yusufdeveloper2903's is the first correct proposal. What do you think about that?

    @grgia, thanks for catching this, and I apologize for missing it. After reviewing both proposals again, I found that neither seems to work as expected.

    @yusufdeveloper2903, I still see the Next button remaining disabled for about a minute. Could you please take another look? Thanks!

    Yousufbug.mp4

    @x-dev90, Unfortunately your proposal doesn't seem to work as expected either. When I click Next, the numbers become inconsistent. Could you please take another look? Thanks!

    XDev-bug.mp4
  22. x-dev90 commented on Oct 9, 2026

    @x-dev90
    Contributor
  23. yusufdeveloper2903 commented on Oct 9, 2026

    @yusufdeveloper2903
    Contributor

    @marufsharifi thanks for testing it, you're right. I reproduced the one-minute wait and updated the proposal.

    Cause: the page-2 search() goes through waitForWrites, so it sits behind every OpenReport queued by the Next presses. At report 50 Next stays disabled until the queue drains.

    Fix: add skipWaitForWrites: true to the carousel's page request. The useSearchAutoRefetch deferral is still needed, otherwise the page-1 refetch wipes page 2.

    With both changes (each OpenReport delayed by 1.5s to match your video) it reaches "50 of 100" with Next enabled and goes on to 70. Results for each change alone are in the proposal.

    Before (main): stops at "50 of 50", Next disabled.

    Image

    After (both changes, Next clicked quickly, OpenReport delayed by 1.5s): reaches "70 of 100" without stopping.

    both-veryfast-slow.mp4

    Updated branch: yusufdeveloper2903/App@6a1b74c...proposal/101453-v2

  24. yusufdeveloper2903 commented on Oct 9, 2026

    @yusufdeveloper2903
    Contributor

    a short summary of where my proposal stands:

    What was missing: my fix only covered the page-1 refetch. It missed that the page-2 request waits behind the queued OpenReport writes, which caused the one-minute wait in your video.

    What changed: one line, skipWaitForWrites: true on the carousel's page request. The rest of the fix is the same.

    What was already correct: the root cause. The Search list behind the RHP refetches page 1 and replaces the snapshot, so page 2 is lost. I posted this on Sep 29 (comment) and showed on Sep 30 that the selected proposal still ended at "50 of 50" because of it. The same useSearchAutoRefetch deferral was added to the other proposal on Oct 9.

    The whole fix is 2 small changes: 5 lines in useSearchAutoRefetch and 1 line in MoneyRequestReportNavigation.

  25. melvin-bot commented on Oct 9, 2026

    @melvin-bot

    @grgia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributorHelp WantedApply this label when an issue is open to proposals by contributorsOverdue

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions