Fix stale split highlights when splitting from the Spend page - #97520
Conversation
Splits saved from Search/Spend navigate back to Search and never mount the expense report list, so the pendingNewTransactionIDs flags written by buildOnyxDataForMoneyRequest were never consumed. They sat in REPORT_METADATA until the user later opened that report from the Inbox, highlighting rows that were no longer new. - Gate the highlight rail in MoneyRequestBuilder behind shouldSkipReportHighlightRail and restrict it to adds that make the report multi-transaction. - Thread the flag from SplitTransactionUpdate, set only for the branch that actually opens the expense report. - Drop the now-redundant addPendingNewTransactionIDs loop; the rail is written by the builder since Expensify#94670. - Add Onyx-level tests asserting REPORT_METADATA directly instead of the mock.
744c863 to
7b98b65
Compare
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.
|
|
@gijoe0295 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-04.at.23.20.08.movAndroid: mWeb ChromeScreen.Recording.2026-08-04.at.21.42.09.moviOS: HybridAppScreen.Recording.2026-08-04.at.21.52.08.moviOS: mWeb SafariScreen.Recording.2026-08-04.at.21.34.34.movMacOS: Chrome / SafariScreen.Recording.2026-08-04.at.00.27.05.mov |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b98b654ea
ℹ️ 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".
|
|
||
| // And the transaction that already existed in the report is not flagged - it is not new, so highlighting it | ||
| // would draw attention to a row the user has already seen | ||
| expect(pendingNewTransactionIDs?.['existing-tx-2']).toBeUndefined(); |
There was a problem hiding this comment.
This assertion does not match the path under test: existing-tx-2 is already in allTransactionsList, and this fixture changes its amount from 1000 to 500, so updateSplitTransactions treats it as changed and pushes the getMoneyRequestInformation optimistic updates. With shouldSkipReportHighlightRail false on the expense-report path, the builder still writes pendingNewTransactionIDs: {'existing-tx-2': true} for a multi-transaction report, so this new test will fail unless the production path preserves the old new-ID exclusion before writing the rail.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@gijoe0295 Checked this and it's a false positive.
|
@aswin-s |
…t-highlight-spend-page-93967
@gijoe0295 Merged latest main. |
|
@heyjennahay It's all yours ! |
|
Will review but the actions are not working now |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 mountiny 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/mountiny in version: 9.4.52-0 🚀
|
|
🤖 No help site changes required. I reviewed the changes in this PR against Why: This is a purely internal bug fix. It adds a The behavior being corrected is a transient "new expense" row highlight (a brief yellow flash) — ephemeral visual feedback that isn't documented in any help article. The user-facing capabilities themselves (splitting an expense, viewing reports on the Reports/Spend page) are unchanged in what they do or how they're used, so the existing help articles remain accurate. A search of Since no docs changes are required, there is no linked help site PR to review. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.52-11 🚀
Bundle Size Analysis (Sentry): |
5 participants
Explanation of Change
Splitting an expense from the Search/Spend page left stale "new transaction" highlight flags behind, so opening the expense report later from the Inbox highlighted rows that were no longer new.
The highlight rail (
REPORT_METADATA.pendingNewTransactionIDs) is written bybuildOnyxDataForMoneyRequestfor every creation flow, and it is only consumed and cleared when the expense report's transaction list mounts. The split-from-Spend flow navigates back to Search and never opens that report, so the flags were never consumed — they sat in Onyx until the user opened the report from the Inbox. The Search page has its own highlight mechanism (TRANSACTION_IDS_HIGHLIGHT_ON_SEARCH_ROUTE), so it does not need the rail at all.This is a follow-up to #92506 and #94139. Those PRs guarded the
addPendingNewTransactionIDsloop inSplitTransactionUpdate.ts, but #94670 (merged mid-review) moved rail population down intobuildOnyxDataForMoneyRequest, so the guarded loop was no longer the write site and the bug remained.Fixed Issues
$ #91679
PROPOSAL: #91679 (comment)
Tests
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
android.mp4
Android: mWeb Chrome
android-web.mp4
iOS: Native
iphone.mp4
iOS: mWeb Safari
iphone-web.mp4
MacOS: Chrome / Safari
Web.mp4