Remove submitter from Change Workspace gate on submitted expense reports#90796
Conversation
The canEditReportPolicy function allowed submitters to change workspace on submitted (processing) expense reports when isAwaitingFirstLevelApproval was true. The backend rejects this action, causing an optimistic rollback and generic error message. Only managers and admins should be able to change workspace on submitted reports. Co-authored-by: cretadn22 <cretadn22@users.noreply.github.com>
|
@MelvinBot Please complete the checklist and open the PR |
|
@cretadn22 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] |
|
Done! I've completed the PR author checklist and marked this PR as ready for review. What I filled in:
|
|
|
||
| if (isSubmitted) { | ||
| return (isSubmitter && isAwaitingFirstLevelApproval(report)) || isManager || isAdmin; | ||
| return isManager || isAdmin; |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
After this change, the isSubmitted branch (return isManager || isAdmin;) is now identical to the fallthrough on line 4778 (return isManager || isAdmin;). The if (isSubmitted) block is redundant and can be removed entirely.
Suggested fix -- remove the dead branch:
if (isExpenseType) {
if (isOpen) {
return isSubmitter || isAdmin;
}
return isManager || isAdmin;
}Reviewed at: 1945435 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Done — removed the redundant isSubmitted branch. The fallthrough now handles all non-open expense reports.
|
@joekaufmanexpensify Please let me know if this revised behavior looks correct to you
Previously, in Step 5, users who were also the submitters could still switch workspaces while the expense was awaiting initial approval |
|
@MelvinBot Please check all the boxes in the author checklist. |
The isSubmitted block returned the same value as the fallthrough, making it dead code after the earlier change. Co-authored-by: cretadn22 <cretadn22@users.noreply.github.com>
|
@NicolasBonet 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-05-19.at.21.46.23.movAndroid: mWeb ChromeScreen.Recording.2026-05-19.at.21.23.02.moviOS: HybridAppScreen.Recording.2026-05-19.at.21.44.49.moviOS: mWeb SafariScreen.Recording.2026-05-19.at.21.21.25.movMacOS: Chrome / SafariScreen.Recording.2026-05-19.at.21.18.40.mov |
|
🚧 @NicolasBonet has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ 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/NicolasBonet in version: 9.3.78-0 🚀
Bundle Size Analysis (Sentry): |
|
Yes, help site changes are required. The article Expense and Report Actions ( What changed: The two "Change workspace" table rows now split submitter permissions from approver/admin permissions:
Draft PR with the update: #91131 |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.78-1 🚀
|
Explanation of Change
The
canEditReportPolicyfunction inReportUtils.ts:4774allowed submitters to change workspace on submitted (processing) expense reports whenisAwaitingFirstLevelApprovalwas true. However, the backend rejects thisChangeReportPolicyAPI call for submitters, causing an optimistic rollback and a generic "Oops... something went wrong" error.This removes the
isSubmitter && isAwaitingFirstLevelApproval(report)clause from theisSubmittedbranch so only managers and admins can change workspace on submitted expense reports. TheisOpenbranch is unchanged — submitters can still move their own unsubmitted drafts.Fixed Issues
$ #88530
PROPOSAL: #88530 (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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
AI Tests
ConvertGpsPointsTo2DArray.ts)