Only allow report manager to reject expense#73120
Conversation
|
@allroundexperts 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] |
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 73 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
@allroundexperts will you be able to review this PR today? |
|
Yes |
|
@bernhardoj Please fix the lint issues. Also, there are conflicts. |
|
It's Just merged main, no conflicts. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-10-23.at.2.27.51.AM.movAndroid: mWeb ChromeScreen.Recording.2025-10-23.at.2.17.00.AM.moviOS: HybridAppiOS: mWeb SafariScreen.Recording.2025-10-23.at.2.10.13.AM.movMacOS: Chrome / SafariScreen.Recording.2025-10-23.at.1.58.50.AM.movMacOS: DesktopScreen.Recording.2025-10-23.at.2.08.46.AM.mov |
| type: CONST.REPORT.TYPE.EXPENSE, | ||
| managerID: 1, | ||
| }; | ||
| const policy: Policy = { |
There was a problem hiding this comment.
This is causing a lint error. Please select another name.
allroundexperts
left a comment
There was a problem hiding this comment.
Works good. Please fix the lint error in the test before merging @bernhardoj
|
Fixed |
|
✋ 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/lakchote in version: 9.2.37-1 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.2.37-8 🚀
|
| const isCurrentUserManager = report?.managerID === currentUserAccountID; | ||
|
|
||
| const userCanReject = (isReportApprover && isCurrentUserManager) || isReportPayer; | ||
| const userCanReject = isReportApprover && isCurrentUserManager; |
There was a problem hiding this comment.
Report manager should be able to reject expense even if approver changed to another person. (context: #79599 (comment))
As a solution, isReportApprover && condition should have been removed.
Explanation of Change
Fixed Issues
$ #72465
PROPOSAL: #72465 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Prerequisite:
As User A, invite User B and C to the workspace
Set User B as approver
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop