Create optimistic report when approving/paying for report with hold expenses#42573
Conversation
…ith-holds # Conflicts: # src/libs/actions/IOU.ts
This reverts commit a49af7b.
|
Done |
|
Oh, I prepared changes for the old report value 😅 |
|
I assume we're fine with not changing them then |
|
@war-in new PR welcome 😄 |
|
That's only 10 lines of code but I can create it 👍 |
|
✋ 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 @ZhenjaHorbach could you jump in and verify the PR? 🙏 |
Yeah |
|
🚀 Deployed to staging by https://github.com/robertjchen in version: 9.0.13-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.0.13-4 🚀
|
|
Two issues were created coming from this PR:
|
| chatReport.reportID, | ||
| chatReport.policyID ?? iouReport.policyID ?? '', | ||
| recipient.accountID ?? 1, | ||
| (firstHoldTransaction?.amount ?? 0) * -1, |
There was a problem hiding this comment.
This argument is for the total. We should have summed up the hold transactions. (Coming from #48760)
| chatReport.reportID, | ||
| chatReport.policyID ?? iouReport.policyID ?? '', | ||
| recipient.accountID ?? 1, | ||
| (firstHoldTransaction?.amount ?? 0) * -1, |
There was a problem hiding this comment.
When we edit the amount, the new amount is stored in modifiedAmount. This caused #49269
| const {holdReportActions, holdTransactions} = getHoldReportActionsAndTransactions(iouReport.reportID); | ||
| const firstHoldTransaction = holdTransactions[0]; | ||
|
|
||
| const optimisticExpenseReport = ReportUtils.buildOptimisticExpenseReport( |
There was a problem hiding this comment.
We should create the optimistic data that matches with the data from our BE.
This caused #49754 more detailed:
#49754 (comment)
There was a problem hiding this comment.
We should cover a case where it's an IOU report. Otherwise, it will build a wrong optimistic report, causing LHN to display a wrong message. More details here #49274 (comment)
There was a problem hiding this comment.
Here's another case (when approve/pay the report partially) of inconsistent logic between optimistic data and BE.
More details: #76780 (comment)
| }; | ||
| }); | ||
|
|
||
| const optimisticData: OnyxUpdate[] = [ |
There was a problem hiding this comment.
Coming from #52696, We need to reset pending action in all places if we use it in optimistic data.
| reportActionID, | ||
| originalMessage: { | ||
| ...originalMessage, | ||
| IOUReportID: optimisticExpenseReport.reportID, |
There was a problem hiding this comment.
We should update the lastVisibleActionCreated optimistically. Ref: #54655
14 participants
Details
Fixed Issues
$ #41652
PROPOSAL:
Tests
Force offlineinTroubleshooting(to better see what happens when slow backend replies).Force offline.Offline tests
QA Steps
Force offlineinTroubleshooting(to better see what happens when slow backend replies).Force offline.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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.native.mov
iOS: mWeb Safari
ios.web.mov
MacOS: Chrome / Safari
Screen.Recording.2024-07-24.at.16.15.15.mov
MacOS: Desktop
Desktop.video.mov