Clear stale iouReportID on DM chat after IOU is moved to a new workspace#89551
Draft
wildan-m wants to merge 3 commits intoExpensify:mainfrom
Draft
Conversation
…ce via pay-with-business-account cancel flow
…stale-iou-report-id-on-dm-after-pay-to-workspace
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
In a 1:1 DM, when the recipient taps "Pay with business account" on a manual expense and cancels the bank-account setup,
createWorkspaceFromIOUPaymentinsrc/libs/actions/Policy/Policy.tsoptimistically converts the IOU report into an Expense report under a new workspace and updates the old DM chat in three ways: it nullifies the oldREPORT_PREVIEW, setshasOutstandingChildRequest: false, and appends aMOVEDaction.It never clears
chatReport.iouReportIDon the DM, so the field still points at the report that has just been moved to a different policy. When the sender then creates another expense in the same DM,getMoneyRequestInformationresolves the moved report viachatReport.iouReportID,shouldCreateNewMoneyRequestReportreuses it because it is stillOPEN, and the moved expense "comes back" to the DM preview alongside the new transaction. Opening the preview shows a header from the new workspace's policy, and the newly created expense errors out because the participants/policy no longer match.This PR extends the optimistic merge on the old DM chat to also clear
iouReportID: null, and mirrors the rollback in the failure entry by restoring the prioriouReportID. With the pointer cleared,getMoneyRequestInformationno longer resolves the moved report from the DM, so a fresh IOU report is built for the next expense and the duplicate preview / mismatched header / new-transaction error all disappear.This mirrors the precedent already set by other flows that remove an IOU report from a DM:
src/libs/actions/IOU/PayMoneyRequest.tsclearsiouReportIDon payment success.submitReportinsrc/libs/actions/IOU/index.tsclears it when a report is submitted to a workspace.DeleteMoneyRequest.tsclears it when the last transaction on a DM's IOU report is deleted.createWorkspaceFromIOUPaymentwas simply missing the equivalent step. The change is scoped entirely to the optimistic/failureonyxDatabuilt inside that function — its only caller issrc/components/KYCWall/BaseKYCWall.tsx, which is unaffected.Fixed Issues
$ #81636
PROPOSAL: #81636 (comment)
Tests
Test data setup:
Steps:
Offline tests
QA Steps
Same as tests.
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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari