Feat: Add ability to unreport transactions v3#67048
Feat: Add ability to unreport transactions v3#67048luacmartins merged 12 commits intoExpensify:mainfrom
Conversation
|
@allgandalf 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] |
|
@luacmartins opened a PR |
|
Couldn't reproduce this one |
| if (shouldTurnOffSelectionMode) { | ||
| turnOffMobileSelectionMode(); | ||
| } |
There was a problem hiding this comment.
@waterim Why do we have different behavior here? We should either always turn off mobile selection mode or do this conditionally for all options.
There was a problem hiding this comment.
We are turning off mobile selection mode in case when user removes expense from a report and only one transaction is left.
There was a problem hiding this comment.
@waterim Why do we always turn off mobile selection while moving reports then? Can you add an explanation here for future context?
There was a problem hiding this comment.
@shubham1206agra It was a regression from this PR
@SzymczakJ Can you please help with explaining this changes?
There was a problem hiding this comment.
Sure thing! The bug that my closed PR fixed was that, when the user selects 2 out of 3 expenses in the report and then unreports them, we are redirected back to the report screen, but the selection mode is still on.
On report screen that has only one transaction the selection mode doesn't make sense, so in this case we should call turnOffMobileSelectionMode when unreporting expenses in removeFromReport callback.
|
Update: Working on a fix for an issue above |
|
@shubham1206agra update solution for 66122 |
|
@waterim we have conflicts |
…rt-transaction-v3
|
@luacmartins resolved |
|
@shubham1206agra @allgandalf let's prioritize reviewing this PR please |
…rt-transaction-v3
|
resolved conflicts again |
|
@shubham1206agra @allgandalf let's prioritize this one please |
|
@shubham1206agra are you happy with this resolution?
|
|
@allgandalf Yes |
|
are you completing the checklist or should i? |
|
@allgandalf You can |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
allgandalf
left a comment
There was a problem hiding this comment.
Hope we have no significant deploy blockers this time 🤞
|
I'll be out tomorrow and Friday, so @waterim @allgandalf please keep an eye out for blockers and revert the PR if needed. |
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.88-0 🚀
|
|
🚀 Deployed to production by https://github.com/grgia in version: 9.1.88-3 🚀
|
| if (!transactionReport || selectedTransactionIDs.length === 0) { | ||
| return; | ||
| } | ||
| changeTransactionsReport(selectedTransactionIDs, CONST.REPORT.UNREPORTED_REPORT_ID); |
There was a problem hiding this comment.
caused a bug - #67811 (comment)
Use InteractionManager.runAfterInteractions()
| @@ -4237,6 +4237,12 @@ function canEditFieldOfMoneyRequest( | |||
| } | |||
|
|
|||
| if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.REPORT) { | |||
There was a problem hiding this comment.
we should only allow this action for outstanding reports. #67583
Details
Fixed Issues
$ #60288
$ #63946
PROPOSAL: N/A
Tests
Also go through all these tickets: #63946
Also go through all these tickets: #64259 (comment)
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)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
Details
Screen.Recording.2025-05-09.at.00.29.24.mov