[CP Staging] fix: gate chatReportID destination heuristic to per-diem only by MobileMage · Pull Request #92660 · Expensify/App

MobileMage · 2026-06-04T11:08:53Z

Explanation of Change

PR #91266 added a heuristic in IOURequestStepConfirmation.tsx that redirected destinationReportID to report.chatReportID whenever report was a money-request report not currently topmost. The intent was to handle per diem launched from a report preview's Add expense (#91093), where backToReport is set on the parent route but dropped by the per-diem step screens (STEP_DESTINATION, STEP_TIME, STEP_SUBRATE don't carry it). The heuristic was not scoped, so other flows hit the same branch.

In the deploy-blocker repro the user starts at the workspace chat, runs Manual, and on the confirm page changes the Report field to A:

backToReport is undefined (Manual from the workspace chat does not set it).
report becomes Report A, so isMRReport is true.
The workspace chat is still the topmost report behind the RHP, so getTopmostReportId() !== report.reportID is true.
The new branch resolves destinationReportID to report.chatReportID — Report A's parent workspace chat — instead of Report A itself.

The pre-insert and submit-dismiss flows then reveal that chat instead of Report A. Picking Create report in step 7 hits the same branch because the new report is also a money-request report.

This PR gates the heuristic on isPerDiemRequest (already declared at line 144 of the same file), so the change is a single added condition. After the change:

Manual / Scan / Distance fall back to report?.reportID exactly as they did before PR fix: per diem from report preview lands on workspace chat #91266 — the explicitly selected Report A opens.
Per diem from a report preview ([Due for payment 2026-06-12] [$250] Per diem - Expense report opens after creating distance expense from report preview #91093) still enters the chatReportID branch, so the original fix is preserved.

The matching backToReport ?? activeReportID change in useExpenseSubmission.ts lives inside submitPerDiemExpense and never runs for Manual / Scan / Distance, so it does not need to change for this fix.

Fixed Issues

$ #92641
PROPOSAL: #92641 (comment)

Tests

Precondition: a workspace chat the user can post in, and the + action available in the composer.

Sign in.
Open the workspace chat.
Create two empty reports and rename them to A and B.
In the chat composer click + > Create expense > Manual.
Enter an amount > Next.
On the confirm page, enter a merchant and click the Report field.
Select report A.
Click Create expense.
Verify report A opens after submission (the app does not stay on the workspace chat).
Repeat steps 4–8 with Scan and Distance. Verify each lands on Report A.
Repeat steps 4–8 but in step 7 select Create report instead of report A. Verify the newly created report opens.
Regression check for [Due for payment 2026-06-12] [$250] Per diem - Expense report opens after creating distance expense from report preview #91093: from the workspace chat, create an empty report, on its preview tap Add expense > Create expense > Per diem, complete the flow. Verify the app stays on the workspace chat (does not navigate into the new expense report).

Verify that no errors appear in the JS console

Offline tests

Same as Tests with the network disabled before each submission.

Go offline.
Repeat steps 4–9 above with Manual.
Verify the expense is created optimistically, Report A is opened, and the new expense appears in Report A.
Reconnect and verify the expense persists with no errors.
Repeat for Scan, Distance, Per diem from preview (step 12 in Tests) to confirm offline parity.

QA Steps

Same as Tests.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Kapture.2026-06-04.at.13.47.48.mp4

PR 91266 added a heuristic in IOURequestStepConfirmation that redirected to report.chatReportID whenever report was a money-request report not currently topmost. That solved per-diem from a report preview (issue 91093) but over-fired for Manual/Scan/Distance: when a user changes the Report field to A on the confirm page, report becomes A and the heuristic sends them to A's parent chat instead of A (deploy blocker 92641). Gating the heuristic on isPerDiemRequest keeps the 91093 fix and restores pre-PR behavior for the other expense types.

codecov · 2026-06-04T17:09:07Z

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines	Coverage Δ
...es/iou/request/step/IOURequestStepConfirmation.tsx	`63.98% <0.00%> (-0.33%)`	⬇️
... and 159 files with indirect coverage changes

melvin-bot · 2026-06-04T17:32:06Z

@brunovjk Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot · 2026-06-04T17:32:11Z

@marcaaron Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

joekaufmanexpensify

Fixing DB 👍

github-actions · 2026-06-05T06:46:57Z

🚧 @lakchote has triggered a test Expensify/App build. You can view the workflow run here.

mkhutornyi · 2026-06-05T07:23:28Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified there are no new alerts related to the canBeMissing param for useOnyx
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android.mov

Android: mWeb Chrome

iOS: HybridApp

ios.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

before:

before.mov

after:

web.mov

regression test:

from.money.request.report.mov

original.issue.mov

lakchote

LGTM

MelvinBot · 2026-06-05T07:56:32Z

Crash analysis — confirmed a `main` regression, not caused by this PR

What's happening: Some useEffect/useLayoutEffect in the report render path returns true instead of a cleanup function (or nothing). React stores whatever an effect returns as its "destroy" callback, so on the next cleanup it literally calls true() → true is not a function → the red‑screen render crash. The two LogBox entries in your screenshots are the same bug, one step apart:

Console error (on mount): useEffect must not return anything besides a function, which is used for clean‑up. You returned: true
Render error (on cleanup): true is not a function at ReactFabric-dev.js:17227 (destroy()), with the crashing fiber reported as ReportScreen.

This PR is not the cause — confirmed. It only adds one isPerDiemRequest gate to a destination heuristic in IOURequestStepConfirmation.tsx; it touches no effect/cleanup code and nothing in the report render path. Your note ("not caused by this PR but app crashes on main") is correct.

Why I couldn't pin the exact line in the App repo (and where to look next)

The crash fiber is ReportScreen, so the offending effect is owned by ReportScreen's fiber — i.e. it lives in ReportScreen itself or in a hook it calls (custom-hook effects attach to the calling component's fiber). I traced the whole chain and found no effect that returns a boolean:

ReportScreen.tsx has no literal useEffect. Its hooks — useThemeStyles, useResponsiveLayout, useCurrentReportIDState, useViewportOffsetTop (native returns 0), useDeferNonEssentials, useSubmitToDestinationVisible, useFlushDeferredWriteOnFocus, useActionListContextValue, useOnyx (x2), useClearReportActionDraftsOnReportChange — all return undefined or a real cleanup function.
The three focus hooks use useFocusEffect from @react-navigation/native, which guards its return (typeof destroy === 'function') and would emit a different warning ("An effect function must not return…"), so they can't produce this React useEffect warning.
A repo‑wide scan of every plain useEffect/useLayoutEffect in src (implicit‑return and block forms) found none returning a boolean, and react-native-onyx's useOnyx is clean too.

Since it isn't in App's src, the boolean is almost certainly returned at runtime by a third‑party hook called somewhere in the report subtree (the classic shape is useEffect(() => subscribe(), []) where subscribe() now returns a boolean instead of an unsubscribe function), a build‑time transform, or the native/HybridApp layer (which lives in a separate internal repo). A regression like this most often rides in on a recent dependency bump on main (e.g. a navigation/keyboard/reanimated/onyx upgrade).

The LogBox component stacks are truncated in the screenshots, so they don't reveal the owning hook.

Suggested next steps to localize it (needs a native dev build — I can't run one):

Expand the LogBox component stack on the warning and grab the frames above <ReportScreen /> — that names the exact hook/component that owns the effect.
Check what landed on main just before this started — especially any dependency bump — since the offending effect isn't in App's own source.

Because the root cause sits outside App's src (likely a dependency or the internal native layer), this probably needs an internal engineer to take it the last mile. Happy to dig further if you can paste the full (untruncated) component stack from the native build.

OSBotify · 2026-06-05T08:02:13Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-06-05T08:03:14Z

🚧 @lakchote has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-06-05T08:09:02Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #92660.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-09cb9cb57c3-9809a05-c58032b7599362d60b008084e6060e29e85419eb/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-09cb9cb-9809a05-07098647963a4d0488de85818d64950513345898/index.html

Web 🕸️
https://92660.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

…rdiem-backtoreport-propagation [CP Staging] fix: gate chatReportID destination heuristic to per-diem only (cherry picked from commit 09cb9cb) (cherry-picked to staging by lakchote)

OSBotify · 2026-06-05T08:59:27Z

🚀 Cherry-picked to staging by https://github.com/lakchote in version: 9.3.99-8 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-06-05T09:02:35Z

Help site review — no docs changes required

I reviewed the changes in this PR against the help articles under App/docs/articles and applied HELPSITE_NAMING_CONVENTIONS.md, HELP_AUTHORING_GUIDELINES.md, and TEMPLATE.md.

What this PR changes

It is a single-line, internal navigation bugfix in src/pages/iou/request/step/IOURequestStepConfirmation.tsx. It gates a destinationReportID heuristic on isPerDiemRequest, so that after submitting a Manual / Scan / Distance expense where the user selected a specific Report on the confirmation page, the app opens that selected report instead of incorrectly opening the parent workspace chat. The per-diem-from-a-report-preview behavior is preserved. This restores the previously expected behavior (the PR fixes a deploy blocker).

Why no documentation update is needed

The change is internal navigation behavior (which report the app lands on after submission) — not a new or changed user-facing feature, setting, toggle, tab, or button.
The creation flow, steps, and UI labels documented in the help site (Create expense, Manual, Scan, Track distance, Per diem, the Report field) are all unchanged by this PR.
I searched the relevant articles, including new-expensify/reports-and-expenses/Create-an-Expense.md, Per-Diem-Expenses.md, Managing-Expenses-in-a-Report.md, and Create-and-Submit-Reports.md. None of them document the post-submission destination report or which screen opens after tapping Create expense, so there is no statement that has become inaccurate.

Conclusion: No help site changes are required, so I have not created a draft docs PR.

@MobileMage, please confirm this assessment that no App/docs/articles updates are required for this change. If you believe a specific article should be updated, let me know which one and I'll prepare a draft help site PR.

brunovjk · 2026-06-05T13:25:42Z

Sorry for the delay, thank you for resolving this.

OSBotify · 2026-06-05T17:52:50Z

🚀 Deployed to production by https://github.com/lakchote in version: 9.3.99-9 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

OSBotify · 2026-06-05T18:17:37Z

🚀 Cherry-picked to staging by https://github.com/lakchote in version: 9.4.0-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-06-05T18:19:17Z

🤖 Help site review: no changes required

I reviewed the change in this PR against the help site files under docs/articles.

What this PR does: It adds a single isPerDiemRequest condition to the destinationReportID calculation in IOURequestStepConfirmation.tsx, gating the chatReportID navigation heuristic to per-diem only. This fixes a deploy-blocker regression where Manual / Scan / Distance expenses created from a workspace chat opened the parent workspace chat instead of the explicitly selected report.

Why no docs update is needed: This is an internal navigation/routing fix. It does not introduce or change any user-facing feature, screen, copy, button label, tab name, or documented workflow. The relevant help articles (e.g. Per-Diem-Expenses.md, Add-an-expense.md) document how to create expenses and per diems; none of them describe the post-submit "which report opens" behavior that this PR corrects. The fix simply restores the pre-regression behavior, so existing documentation remains accurate.

No draft help site PR was created because no docs/articles changes are warranted.

@MobileMage, since this is a behavior-restoring bug fix with no user-facing or documented-workflow change, there's no help site PR to review. If you believe a specific article should be updated for this flow, let me know which one and I'll create the draft PR.

OSBotify · 2026-06-09T04:11:51Z

🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.0-7 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

melvin-bot Bot assigned MobileMage Jun 4, 2026

Retrigger CI

9cad588

MobileMage marked this pull request as ready for review June 4, 2026 17:31

MobileMage requested review from a team as code owners June 4, 2026 17:31

melvin-bot Bot requested review from a team, brunovjk and joekaufmanexpensify and removed request for a team June 4, 2026 17:32

melvin-bot Bot requested review from marcaaron and removed request for a team June 4, 2026 17:32

joekaufmanexpensify approved these changes Jun 4, 2026

View reviewed changes

lakchote changed the title ~~fix: gate chatReportID destination heuristic to per-diem only~~ [CP Staging] fix: gate chatReportID destination heuristic to per-diem only Jun 5, 2026

This comment has been minimized.

Sign in to view

lakchote requested review from lakchote and mkhutornyi and removed request for brunovjk and marcaaron June 5, 2026 07:02

This comment was marked as outdated.

Sign in to view

mkhutornyi approved these changes Jun 5, 2026

View reviewed changes

lakchote approved these changes Jun 5, 2026

View reviewed changes

lakchote merged commit 09cb9cb into Expensify:main Jun 5, 2026
36 of 37 checks passed

OSBotify added the CP Staging marks PRs that have been CP'd to staging label Jun 5, 2026

OSBotify mentioned this pull request Jun 5, 2026

Deploy Checklist: New Expensify 2026-06-04 #92626

Closed

100 tasks

melvin-bot Bot mentioned this pull request Jun 5, 2026

[Due for payment 2026-06-16] [Due for payment 2026-06-12] Expense - Expense report does not open after changing report and creating expense #92641

Closed

7 tasks

brunovjk mentioned this pull request Jun 8, 2026

[Due for payment 2026-06-12] [$250] Per diem - Expense report opens after creating distance expense from report preview #91093

Open

7 tasks

Conversation

MobileMage commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov Bot commented Jun 4, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

melvin-bot Bot commented Jun 4, 2026

Uh oh!

melvin-bot Bot commented Jun 4, 2026

Uh oh!

joekaufmanexpensify left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 5, 2026

Uh oh!

This comment has been minimized.

mkhutornyi commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

This comment was marked as outdated.

lakchote left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MelvinBot commented Jun 5, 2026

Crash analysis — confirmed a main regression, not caused by this PR

Uh oh!

OSBotify commented Jun 5, 2026

Uh oh!

github-actions Bot commented Jun 5, 2026

Uh oh!

github-actions Bot commented Jun 5, 2026

Uh oh!

OSBotify commented Jun 5, 2026

Uh oh!

MelvinBot commented Jun 5, 2026

Help site review — no docs changes required

Uh oh!

brunovjk commented Jun 5, 2026

Uh oh!

OSBotify commented Jun 5, 2026

Uh oh!

OSBotify commented Jun 5, 2026

Uh oh!

MelvinBot commented Jun 5, 2026

Uh oh!

OSBotify commented Jun 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

MobileMage commented Jun 4, 2026 •

edited

Loading

codecov Bot commented Jun 4, 2026 •

edited by github-actions Bot

Loading

mkhutornyi commented Jun 5, 2026 •

edited

Loading

Crash analysis — confirmed a `main` regression, not caused by this PR