[Due for payment 2026-05-11] [$250] Spends → Reports: modal dismisses when navigating between reports with arrows

[mountiny]

Problem

In Spends → Reports, when opening a money request report and moving between reports with the next/previous controls, the report modal can dismiss unexpectedly and return the user to the reports list instead of staying on the selected report.

Cause

On SearchMoneyRequestReportPage, the screen stays mounted while reportIDFromRoute changes. The effect that treats a missing report as “removed” can run before Onyx has finished loading the new report. In that window the report looks absent, so the code incorrectly calls Navigation.dismissModal().

Expected

Arrow navigation should switch between reports without closing the modal or leaving the report flow.

Reproduction

1. Use an account with multiple reports and expenses.
2. Go to Spends → Reports.
3. Open a report.
4. Use next/previous several times.
5. Before fix: you can be sent back to the reports list.

Fix

Fix modal dismiss on report route change in SearchMoneyRequestReportPage — guard the removal effect when the route report id just changed (e.g. usePrevious + early return) so a route transition is not mistaken for report deletion.

---

Note for PR author: #80560 linked in the PR “Fixed Issues” section is a different workstream (large-account tracking). After this issue is created, update the PR’s Fixed Issues to use this issue number instead.

Issue Owner

Current Issue Owner: @abzokhattab

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022049489947228713329
• Upwork Job ID: 2049489947228713329
• Last Price Increase: 2026-04-29

[MelvinBot]

Proposal

What is the root cause of that problem?

When navigating between reports via next/previous arrows, reportIDFromRoute changes and useOnyx re-subscribes to the new report key. Before Onyx populates that key, report is undefined. The removal-detection effect sees !report && isMoneyRequestReport(prevReport) as true and calls Navigation.dismissModal() — mistaking a route transition for a report deletion.

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

Guard the removal-detection effect with usePrevious(reportIDFromRoute). When prevReportIDFromRoute !== reportIDFromRoute, the route just changed, so we skip the removal check for that render cycle. This gives Onyx time to load the new report. Legitimate deletions (where the route hasn't changed) are unaffected.

This is exactly what PR #89150 implements — a 3-line early return plus moving an existing usePrevious declaration earlier in the component.

What alternative solutions did you explore? (Optional)

An alternative would be using useOnyx's loading state to skip the effect while the new report is loading. The usePrevious approach is simpler and doesn't require changes to the Onyx subscription.

Supporting evidence

• Bug location: SearchMoneyRequestReportPage.tsx:94-110
• Onyx subscription: SearchMoneyRequestReportPage.tsx:77
• Data flow: Arrow click → route params update → useOnyx re-subscribes → report is undefined for one render → effect fires dismissModal()

Likely related issue: Expensify/App#87784 ("RHP closes unexpectedly when navigating between expense reports using prev/next arrows") describes the same bug and is currently in Reviewing state.

---

Next Steps for Contributor+ team: Reply with @MelvinBot implement this to create a draft PR, @MelvinBot <your feedback> to refine this analysis, or explain why you are rejecting Melvin's proposal.

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022049489947228713329

[KJ21-ENG]

Proposal

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

In Spends → Reports, opening a money request report and stepping through reports with the next/previous arrows can dismiss the report modal mid-navigation and drop the user back on the reports list, instead of swapping the modal contents to the next/previous report.

Media not yet reviewed; deferred to post-eligibility verification.

What is the root cause of that problem?

SearchMoneyRequestReportPage stays mounted while the route's reportID changes — the arrow controls only update route.params.reportID, they do not unmount/remount the screen. The "report was removed" effect treats a transient empty report as a deletion, even when the emptiness is just the gap between routes:

• Route changes → reportIDFromRoute = getNonEmptyStringOnyxID(route.params?.reportID) flips at

  App/src/pages/Search/SearchMoneyRequestReportPage.tsx

  Line 72 in 4acb25b

  const reportIDFromRoute = getNonEmptyStringOnyxID(route.params?.reportID);

  .
• The Onyx subscription useOnyx(${ONYXKEYS.COLLECTION.REPORT}${reportIDFromRoute}) re-keys at

  App/src/pages/Search/SearchMoneyRequestReportPage.tsx

  Line 77 in 4acb25b

  const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportIDFromRoute}`);

  , so report is briefly undefined until the new key resolves. prevReport (

  App/src/pages/Search/SearchMoneyRequestReportPage.tsx

  Line 89 in 4acb25b

  const prevReport = usePrevious(report);

  ) still holds the previous route's money-request report.
• The dismiss effect at

  App/src/pages/Search/SearchMoneyRequestReportPage.tsx

  Lines 93 to 110 in 4acb25b

  	// Dismiss modal when the money request report is removed (e.g. deleted or merged).
  	useEffect(() => {
  	// Skip first run so we don't dismiss on mount when report may still be loading.
  	if (firstRenderRef.current) {
  	firstRenderRef.current = false;
  	return;
  	}
  	// Report is gone now but we had a money request report before → it was removed.
  	const isRemovalExpectedForReportType = !report && isMoneyRequestReport(prevReport);
  	if (isRemovalExpectedForReportType) {
  	if (!isFocused) {
  	return;
  	}
  	Navigation.dismissModal();
  	}
  	}, [report, isFocused, prevReport]);

  evaluates !report && isMoneyRequestReport(prevReport). During the route swap that condition is true && true, so Navigation.dismissModal() fires and the user is sent back to the reports list.

The firstRenderRef guard only suppresses the very first run after mount; on subsequent route swaps the screen is already mounted, the ref is already false, and there is nothing left to distinguish "report deleted" from "next/previous arrow pressed". A prevReportIDFromRoute reference does exist further down at

App/src/pages/Search/SearchMoneyRequestReportPage.tsx

Line 207 in 4acb25b

const prevReportIDFromRoute = usePrevious(reportIDFromRoute);

, but it is declared after the dismiss effect and is wired into a different concern (the isInitialMountRef reset), so the dismiss effect has no signal that the route id just changed.

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

1. Move (or add) a const prevReportIDFromRoute = usePrevious(reportIDFromRoute); so it is in scope above the dismiss effect at

   App/src/pages/Search/SearchMoneyRequestReportPage.tsx

   Lines 93 to 110 in 4acb25b

   	// Dismiss modal when the money request report is removed (e.g. deleted or merged).
   	useEffect(() => {
   	// Skip first run so we don't dismiss on mount when report may still be loading.
   	if (firstRenderRef.current) {
   	firstRenderRef.current = false;
   	return;
   	}
   	// Report is gone now but we had a money request report before → it was removed.
   	const isRemovalExpectedForReportType = !report && isMoneyRequestReport(prevReport);
   	if (isRemovalExpectedForReportType) {
   	if (!isFocused) {
   	return;
   	}
   	Navigation.dismissModal();
   	}
   	}, [report, isFocused, prevReport]);

   . The existing declaration at L207 can simply be hoisted; no second usePrevious is needed.
2. Inside that effect, add an early return when the route id just changed: if (prevReportIDFromRoute !== reportIDFromRoute) { return; }. This treats the transient !report window as a route swap, not a deletion, so Navigation.dismissModal() is no longer reached for arrow navigation. Add prevReportIDFromRoute and reportIDFromRoute to the effect's dependency array so the guard re-evaluates on the next render after the swap settles.
3. Genuine deletion still dismisses correctly: when a report the user is viewing is deleted/merged, route.params.reportID does not change, so prevReportIDFromRoute === reportIDFromRoute, the new guard does not fire, and the existing !report && isMoneyRequestReport(prevReport) path runs as before.

Scope boundaries:

• Files touched: src/pages/Search/SearchMoneyRequestReportPage.tsx only.
• Platforms affected: all (web, mWeb, iOS, Android, desktop) via the shared component.
• Not touched: usePaginatedReportActions, useTransactionsAndViolationsForReport, the isInitialMountRef/openReport effect at L209, RHP routing, or any other Navigation.dismissModal callsite.
• No API contract or backend behavior changes.

What alternative solutions did you explore? (Optional)

• Reset prevReport on route change — e.g. clear the usePrevious(report) snapshot when reportIDFromRoute changes so the isMoneyRequestReport(prevReport) arm cannot match across reports. Workable but more invasive: usePrevious has no public reset API, so it would require either a custom hook or a useRef shim. The route-id guard is narrower and has the same effect for the symptom described.
• Gate the dismiss on reportLoadingState — wait until the new report's loading state has settled before evaluating "removed". Correct in spirit but couples the dismiss heuristic to the RAM-only loading state machine and risks delaying genuine deletion dismissals; the route-id guard is simpler and preserves the deletion path unchanged.
• Unmount the screen between arrow presses — would side-step the stale prevReport entirely but defeats the performance reason the screen stays mounted (avoids re-running usePaginatedReportActions/useTransactionsAndViolationsForReport on every arrow press) and is a much larger architectural change.

[aswin-s]

🚨 Edited by proposal-police: This proposal was edited at 2026-04-29 14:27:13 UTC.

Proposal

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

When a user opens an expense report from Spends → Reports and uses the previous/next arrows in the report header, the modal closes and returns to the reports list instead of staying open and showing the adjacent report.

What is the root cause of that problem?

The bug is in src/pages/Search/SearchMoneyRequestReportPage.tsx, inside the effect that dismisses the modal when the current money-request report is removed.

That effect currently treats this condition as a real removal:

• report is now missing
• prevReport was a money-request report

That is too broad for this screen.

When the user clicks the navigation arrows, src/components/MoneyRequestReportView/MoneyRequestReportNavigation.tsx updates the route in place with:

Navigation.setParams({reportID: reportId});

The screen stays mounted, but reportIDFromRoute changes to the next report ID. At that point, useOnyx(${ONYXKEYS.COLLECTION.REPORT}${reportIDFromRoute}) re-subscribes to the new report key, and report is briefly empty while the new report hydrates. During that brief transition:

• prevReport still contains the previous money-request report
• report has no reportID yet
• the dismissal effect misclassifies the transition as “the current report was removed”

The deeper issue is that the effect never verifies that the report that disappeared is the same report the route is still pointing at.

There is already a correct analog for this pattern in src/pages/inbox/ReportNavigateAwayHandler.tsx. That file does not dismiss just because the next report value is empty. It first checks that the vanished Onyx report ID still matches the current reportIDFromRoute:

const prevOnyxReportID = prevReport?.reportID;
const wasReportRemoved = !!prevOnyxReportID && prevOnyxReportID === reportIDFromRoute && !onyxReportID;

That identity check is what prevents route transitions from being mistaken for deletions.

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

1. Update the removal-detection effect in src/pages/Search/SearchMoneyRequestReportPage.tsx so it matches the inbox pattern and only dismisses when the missing report is the same report the current route still targets.
2. In that effect, compute:
   • const onyxReportID = report?.reportID;
   • const prevOnyxReportID = prevReport?.reportID;
   • const wasReportRemoved = !!prevOnyxReportID && prevOnyxReportID === reportIDFromRoute && !onyxReportID;
3. Change the dismissal condition to require both:
   • wasReportRemoved
   • isMoneyRequestReport(prevReport)

So the effect should effectively behave like:

const onyxReportID = report?.reportID;
const prevOnyxReportID = prevReport?.reportID;
const wasReportRemoved = !!prevOnyxReportID && prevOnyxReportID === reportIDFromRoute && !onyxReportID;
const isRemovalExpectedForReportType = wasReportRemoved && isMoneyRequestReport(prevReport);

and only call Navigation.dismissModal() when that condition is true and the screen is focused.

This is more precise than guarding on prevReportIDFromRoute !== reportIDFromRoute, because it ties the dismissal to the actual Onyx report record that vanished, not just to route-param churn.

What alternative solutions did you explore? (Optional)

Using prevReportIDFromRoute !== reportIDFromRoute as an early return would stop the reported repro, but it is a weaker fix because it patches one transition case rather than correcting the actual removal test.

A loading-state guard would also work, but it is less direct than the existing inbox pattern. The strongest fix is to reuse the same report-identity check already used in ReportNavigateAwayHandler.

[emkhalid]

Proposal

What is the root cause of that problem?

SearchMoneyRequestReportPage.tsx reads the active report ID from the route and subscribes to that report with useOnyx.

App/src/pages/Search/SearchMoneyRequestReportPage.tsx

Lines 72 to 77 in c99f4df

	const reportIDFromRoute = getNonEmptyStringOnyxID(route.params?.reportID);
	const {currentSearchResults: snapshot} = useSearchStateContext();
	const firstRenderRef = useRef(true);
	const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportIDFromRoute}`);

When the user clicks the next/previous arrows in Spends -> Reports, the screen stays mounted and only route.params.reportID changes. That makes reportIDFromRoute point to the next report, and the useOnyx subscription switches to the new report key. Before Onyx has returned the new report, report can briefly be undefined.

The removal effect in SearchMoneyRequestReportPage.tsx only checks whether the current report is missing and the previous report was a money request report.

App/src/pages/Search/SearchMoneyRequestReportPage.tsx

Lines 93 to 110 in c99f4df

	// Dismiss modal when the money request report is removed (e.g. deleted or merged).
	useEffect(() => {
	// Skip first run so we don't dismiss on mount when report may still be loading.
	if (firstRenderRef.current) {
	firstRenderRef.current = false;
	return;
	}
	// Report is gone now but we had a money request report before → it was removed.
	const isRemovalExpectedForReportType = !report && isMoneyRequestReport(prevReport);
	if (isRemovalExpectedForReportType) {
	if (!isFocused) {
	return;
	}
	Navigation.dismissModal();
	}
	}, [report, isFocused, prevReport]);

That condition is too broad:

const isRemovalExpectedForReportType = !report && isMoneyRequestReport(prevReport);

During arrow navigation, prevReport is the old report and reportIDFromRoute is already the new report ID. The code does not verify that the missing report is the same report requested by the route, so it mistakes a normal route transition for report deletion and calls Navigation.dismissModal().

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

Update the removal effect in SearchMoneyRequestReportPage.tsx.

App/src/pages/Search/SearchMoneyRequestReportPage.tsx

Lines 101 to 110 in c99f4df

	// Report is gone now but we had a money request report before → it was removed.
	const isRemovalExpectedForReportType = !report && isMoneyRequestReport(prevReport);
	if (isRemovalExpectedForReportType) {
	if (!isFocused) {
	return;
	}
	Navigation.dismissModal();
	}
	}, [report, isFocused, prevReport]);

Instead of dismissing whenever any previous money request report disappears, dismiss only when the previous report's ID still matches the current route report ID. This makes the deletion check report-ID aware.

diff --git a/src/pages/Search/SearchMoneyRequestReportPage.tsx b/src/pages/Search/SearchMoneyRequestReportPage.tsx
@@
-        // Report is gone now but we had a money request report before → it was removed.
-        const isRemovalExpectedForReportType = !report && isMoneyRequestReport(prevReport);
+        // Report is gone now, and the removed report is still the report requested by the route.
+        const wasCurrentReportRemoved = !report && isMoneyRequestReport(prevReport) && prevReport.reportID === reportIDFromRoute;
 
-        if (isRemovalExpectedForReportType) {
+        if (wasCurrentReportRemoved) {
             if (!isFocused) {
                 return;
             }
             Navigation.dismissModal();
         }
-    }, [report, isFocused, prevReport]);
+    }, [report, isFocused, prevReport, reportIDFromRoute]);

With this change, arrow navigation no longer dismisses the modal because prevReport.reportID is the old report ID while reportIDFromRoute is the new report ID. A real deletion or merge still dismisses correctly because the route continues to point to the deleted report, so prevReport.reportID === reportIDFromRoute remains true.

What alternative solutions did you explore? (Optional)

I considered guarding the effect with usePrevious(reportIDFromRoute) and returning early when the route ID changes. That works, but the ID comparison above is more direct and avoids moving or adding another hook for this effect.

I considered gating the dismissal on report loading state. I rejected that because it couples deletion handling to loading state timing and can delay legitimate dismissal.

I considered resetting prevReport when the route ID changes. I rejected that because usePrevious has no reset API, so it would require extra ref logic for a narrower problem.

I considered remounting the page between arrow navigations. I rejected that because it is a broader navigation change and would lose the current performance benefit of keeping the report page mounted.

[github-actions]

⚠️ @emkhalid Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

[aswin-s]

Proposal updated — Working analog in ReportNavigateAwayHandler shows the fix should key off vanished report identity, not prev route ID.

See updated proposal.