fix: Report RHP is empty after merging expenses offline#87522
fix: Report RHP is empty after merging expenses offline#87522paulnjs wants to merge 2 commits intoExpensify:mainfrom
Conversation
|
@ZhenjaHorbach 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: 81a152853b
ℹ️ 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".
| }); | ||
|
|
||
| const oldIOUAction = getIOUActionForReportID(mergeTransaction.reportID, mergeTransaction.sourceTransactionID); | ||
| const oldIOUAction = getIOUActionForReportID(targetTransaction.reportID, targetTransaction.transactionID); |
There was a problem hiding this comment.
Use source report action when mutating source report actions
In the cross-report branch, oldIOUAction is now fetched from the target report, but the subsequent oldIOUAction mutations still write to REPORT_ACTIONS${mergeTransaction.reportID} (the source report). On a failed MERGE_TRANSACTION request, the rollback path re-inserts that target action into the source report, which can leave duplicated/misplaced expense actions in local state after offline retries or server-side merge rejection.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| const oldIOUAction = getIOUActionForReportID(mergeTransaction.reportID, mergeTransaction.sourceTransactionID); | ||
| const oldIOUAction = getIOUActionForReportID(targetTransaction.reportID, targetTransaction.transactionID); |
There was a problem hiding this comment.
Restore thread parent report ID during merge rollback
After this change, oldIOUAction may point to the target transaction thread, and the optimistic update moves that thread's parentReportID to mergeTransaction.reportID. However, rollback only restores parentReportActionID, not parentReportID, so if the merge API call fails the thread can remain attached to the wrong report and produce incorrect RHP/thread navigation state.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Product scope LGTM
Explanation of Change
Fixed Issues
$#86809
PROPOSAL:#86809 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
android.mov
Android: mWeb Chrome
android_web.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios_web.mov
MacOS: Chrome / Safari
web.mov