Fix - Newly created expense is not highlighted in the expense list#93338
Conversation
|
@mkhutornyi 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fba583508
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ReviewThe fix is well-targeted: the A few things worth confirming before merge — none are blocking, but the two behavioral changes (1 and 2) deserve a careful test pass. 1. Subrate → Confirmation now passes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e51ecb79c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb1.movweb2.mov |
|
@FitseTLT There's double highlight on mobile. Is it expected? ios.movAnd please check AI review comments. |
|
No code changes since my previous review (head is still Why the double highlight happensOn platforms where
On web the component fresh-mounts, so only the fallback path runs once — which is why this only reproduces on mobile. Suggested directionGate the ConfidenceMedium — this is a code-traced hypothesis that matches the reported symptom (mobile-only, double highlight) and the PR's own stated premise that web fresh-mounts while the diff path is the normal mechanism elsewhere. I have not run it on a device to confirm the exact re-render trigger. Automated follow-up — reply with |
This is not a double highlighting. As you can see from the video after finishing the expense creation the user was navigated back to the spend page(and spend page has its own highlighting mechanism) and then it was navigated to the expense report then you saw the highlighting we fixed. Now I tried to reproduce it to debug the navigation in ios but it doesn't happen; only navigation happens if you are in inbox. Do you remember any specific step you did to reproduce it? 2026-06-15.15-11-00.mp4 |
|
Now I am not able to reproduce. Let's not block on this. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @robertjchen has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/robertjchen in version: 9.4.8-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review: no changes required. I reviewed the changes in this PR against the help site files under WhyThis PR is an internal bug fix with no user-facing feature, setting, label, or workflow change:
All modified files are code/navigation internals ( @FitseTLT, since no documentation changes are required, no help site PR was created. If you believe a user-facing behavior here should be documented, let me know and I'll draft the changes. |
|
🚀 Deployed to staging by https://github.com/robertjchen in version: 9.4.8-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.8-3 🚀
|
Explanation of Change
When a report goes from 1 to 2 transactions, MoneyRequestReportActionsList switches from a single-transaction view to the transaction list — a fresh mount with no previous state. This breaks both detection paths in useNewTransactions: the diff-based path (no usePrevious baseline) and the pendingNewTransactionIDs fallback (never written for expense report destinations). The fix registers pendingNewTransactionIDs in REPORT_METADATA before the optimistic write in TrackExpense.requestMoney, Split.createDistanceRequest, and useExpenseSubmission.submitPerDiem, gated on the current non-deleted transaction count being exactly 1. A shouldNavigate param is added to navigateAfterExpenseCreate / handleNavigateAfterExpenseCreate so the pending ID registration can be decoupled from actual navigation. MoneyRequestReportActionsList now subscribes to pendingNewTransactionIDs and passes them to useNewTransactions.
PR #73272 changed the per diem subrate→confirmation navigation to use chatReportID instead of the IOU reportID, causing activeReportID in submitPerDiem to resolve to the chat report even when the expense is created inside an IOU report. Other expense flows avoid this by forwarding backToReport through route params so the confirmation page knows the original report. The fix adds backToReport as an optional route param to STEP_DESTINATION, STEP_TIME, and STEP_SUBRATE (plus STEP_TIME_EDIT for type union compatibility), and each step forwards it to the next. This lets submitPerDiem use the IOU report's reportID as activeReportID directly, and rely on backToReport for correct post-creation navigation — matching how all other expense flows work.
Fixed Issues
$ #92401
PROPOSAL: #92401 (comment)
Tests
Offline tests
same as tests
QA Steps
same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
2026-06-12.16-15-06.mp4
Android: mWeb Chrome
2026-06-12.01-35-41.mp4
iOS: Native
2026-06-12.16-20-38.mp4
iOS: mWeb Safari
2026-06-12.01-30-50.mp4
MacOS: Chrome / Safari
2026-06-12.01-27-01.mp4