Fix merge expense transaction thread navigation - #96633
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@rojiphil, could you please take a look at this. thanks. |
|
@rojiphil, Gentle bump. thanks. |
|
@rojiphil, Friendly bump. thanks. |
|
@marufsharifi Picking this up today |
|
Thanks @marufsharifi for the update. The core approach used here looks like a good improvement over the original PR. The offline issue also seems to have been fixed. Will take a closer look at the code sometime today. 96633-web-chrome-001.mp4 |
|
@rojiphil, Quick bump. thanks. |
|
@rojiphil, quick bump. thanks. |
|
@rojiphil, Friendly bump. thanks. |
|
bumped in slack. |
|
@codex review |
|
@MelvinBot review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d066c8dfb4
ℹ️ 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".
Review — looks solid ✅Approach is sound and matches the reported bug (#83257). The fix has three coordinated parts, all of which check out:
Verification
A couple of non-blocking things worth a look: Observations / questions
Nice test coverage on the new branches (unreported→self-DM attach, and the "IOU action has no This is an automated review. Reply with |
|
@rojiphil, addressed feedback, all yours. Thanks! |
|
@marufsharifi A duplicate expense is created in self DM when merged with an unreported expense. Please have a look. Thanks 96633-issue-004.mp4 |
@rojiphil addressed this case too. could you check once again, thanks |
|
@rojiphil, Gentle bump. thanks. |
|
@rojiphil, Friendly bump. thanks. |

Explanation of Change
Merge expense now follows the same destination behavior across individual and bulk merge flows.
Before this update, merging expenses from
search/spendcould open the wrong place or leave the RHP loading, especially when the target workspace expense did not already have a transaction thread and the destination was anunreported/self-DMexpense.Now, the app creates the missing optimistic transaction thread when needed, carries that thread through confirmation, and attaches offline optimistic merge data to the correct destination report. This makes post-merge navigation consistent and predictable for
workspace,self-DM,split, and offline merge cases.Fixed Issues
$ #83257
PROPOSAL:
Tests
Flow 1: Workspace expense with self-DM expense
Flow 2: Workspace manual expense with split expense
Flow 3: Offline workspace expense with self-DM expense
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Flow:1
flow01.mov
Flow:2
flow02.mov
Flow:3
flow03.mov
Android: mWeb Chrome
Flow:1
flow1.mp4
Flow:2
flow2.mp4
Flow:3
flow3.mp4
iOS: Native
Flow:1
flow001.mov
Flow:2
flow002.mov
Flow:3
flow003.mov
iOS: mWeb Safari
Flow:1
flow0001.mov
Flow:2
flow0002.mov
Flow:3
Screen.Recording.2026-07-21.at.8.17.39.PM.mov
MacOS: Chrome / Safari
Flow:1
flow1.mov
Flow:2
flow2.mov
Flow:3
flow3.mov