hide auto approval limit violation for closed/paid/approved report#72217
Conversation
|
The failed test is unrelated. |
rojiphil
left a comment
There was a problem hiding this comment.
Thanks @nkdengineer for the quick PR.
Code changes looks good. Just one comment though. Please have a look.
Also, let us add test cases around the additional check on report status. Thanks.
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp72217-android-native-001.mp4Android: mWeb Chrome72217-mweb-chrome-001.mp4iOS: HybridApp72217-ios-hybrid-001.mp4iOS: mWeb Safari72217-mweb-safari-001.mp4MacOS: Chrome / Safari72217-web-chrome-001.mp4MacOS: Desktop72217-desktop-001.mp4 |
|
@rojiphil Added the test. |
rojiphil
left a comment
There was a problem hiding this comment.
Thanks @nkdengineer for the updates.
@puneetlath Changes work well. LGTM.
All yours. Thanks.
|
✋ 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/puneetlath in version: 9.2.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.30-6 🚀
|
|
|
||
| if (violationName === CONST.VIOLATIONS.OVER_AUTO_APPROVAL_LIMIT) { | ||
| return isPolicyAdmin(policy) && !isSubmitter; | ||
| return isPolicyAdmin(policy) && !isSubmitter && isOpenOrProcessingReport; |
There was a problem hiding this comment.
Coming from #78197 checklist: as approver the violation shouldn't show on draft report so condition just update to check processing report
Explanation of Change
hide auto approval limit violation for closed/paid/approved report
Fixed Issues
$ #70685
PROPOSAL:
Tests
Precondition: Invite a user to the workspace as the admin and set this user as the approver. Enable auto approval rule
Offline tests
Same
QA Steps
Precondition: Invite a user to the workspace as the admin and set this user as the approver. Enable auto approval rule
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-10-09.at.20.29.59.mov
Android: mWeb Chrome
Screen.Recording.2025-10-09.at.20.26.51.mov
iOS: Native
Screen.Recording.2025-10-09.at.20.33.30.mov
iOS: mWeb Safari
Screen.Recording.2025-10-09.at.20.31.59.mov
MacOS: Chrome / Safari
Screen.Recording.2025-10-09.at.20.25.40.mov
MacOS: Desktop
Screen.Recording.2025-10-09.at.20.38.54.mov