fix create report becomes unresponsive after deleting workspace#74010
Conversation
|
@ikevin127 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] |
|
LGTM 👍 Thank you for your hard work! |
…port-unresponsive-after-workspace-deleted
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
Reviewer Checklist
Screenshots/Videos |
|
@huult Based on the code changes, I found some regressions, the flawed logic is: ✅ Non-Per Diem: no change - After verifying testing step number 10:
❌ We have the following issues:
2. If we proceed to create 2 or more new workspaces and enable Per Diem for each, then return to the selfDM per diem transaction, the Report field cannot be clicked or edited (should be since we have multiple per diem enabled workspaces, right ?). Important: on iOS: mWeb Safari seems that Reports field is still enabled even with 2 per diem enabled workspaces, and the UI looks like issue 1 from above.
♻️ From this I think both |
|
@ikevin127 I’ll take a look soon |
|
I will check and fix it tomorrow morning |
…port-unresponsive-after-workspace-deleted
|
@ikevin127 Could you take a look at this video after I updated the new behavior? We have logic to check for an upgrade App/src/components/ReportActionItem/MoneyRequestView.tsx Lines 924 to 933 in 6649879 Screen.Recording.2025-11-06.at.14.23.23.movAfter we upgraded, we have policyForMovingExpensesID and shouldSelectPolicy, but they belong to the transaction draft. Inside IOURequestStepReport, when the action is EDIT, we use the actual transaction. So, we can’t navigate to the report step — as shown in the video above. Instead, it returns and shows the money request details again. What do you think about this behavior? Or after the upgrade, should we use the transaction draft to create the report instead? |
…port-unresponsive-after-workspace-deleted
This comment was marked as outdated.
This comment was marked as outdated.
…port-unresponsive-after-workspace-deleted
heyjennahay
left a comment
There was a problem hiding this comment.
No concern with product change
…port-unresponsive-after-workspace-deleted
…port-unresponsive-after-workspace-deleted
…port-unresponsive-after-workspace-deleted
|
@huult Just checked, looks good now in terms of the new expectation that was agreed upon in the issue:
but there's still 1 issue left from my previous review:
Once ☝️ is fixed, let me know and I'll take another look. ℹ️ Already present on staging: the only other issue I found is that even with a Per diem enabled WS, a selfDM Per diem expense cannot be transfered to a Per diem enabled WS for some reason, the only thing that can be done in the Reports RHP is to create an empty report on that Per diem enabled WS. When report is already created and we select the expense report in Reports RHP for transfer, nothing happens and Per diem expense remains in self DM - we do get a "moved this expense to your personal space" report action though. |
…port-unresponsive-after-workspace-deleted
…port-unresponsive-after-workspace-deleted
|
Yes, I’m checking and will fix it. |
Screen.Recording.2025-11-21.at.12.42.12.mov@ikevin127 I've updated the code. please review again. thank you! |
…port-unresponsive-after-workspace-deleted
|
♻️ Checking... |
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM - thanks for addressing all issues!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.2.64-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.64-5 🚀
|







Details
Fixed Issues
$ #73312
PROPOSAL: #73312 (comment)
Tests
Same QA step
Offline tests
QA Steps
Preconditions:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
Screen.Recording.2025-11-01.at.12.49.57.mp4
Android: mWeb Chrome
Screen.Recording.2025-11-01.at.12.53.12.mp4
iOS: Native
Screen.Recording.2025-11-01.at.12.56.51.mp4
iOS: mWeb Safari
Screen.Recording.2025-11-01.at.12.59.38.mp4
MacOS: Chrome / Safari
Screen.Recording.2025-11-01.at.12.41.26.mp4
MacOS: Desktop
Screen.Recording.2025-11-01.at.12.45.21.mp4