Show 'you' instead of 'an admin' in next step when current user is payer#74583
Show 'you' instead of 'an admin' in next step when current user is payer#74583fahimj wants to merge 1 commit intoExpensify:mainfrom
Conversation
|
@parasharrajat 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❌ Patch coverage is
... and 114 files with indirect coverage changes 🚀 New features to boost your workflow:
|
JmillsExpensify
left a comment
There was a problem hiding this comment.
The text changes in this PR look good from a product perspective!
Code Review CompleteReview Summary: No code quality or performance violations found. Analysis Results:
Key Observations:
The implementation correctly addresses the issue by personalizing the next step message when the current user is a payer, improving user experience. |
|
Going to review the issue first. I am skeptical about these changes. We might be starting a chain of issues. My motive would be limit the changes to our issue. |
|
Just FYI, I checked commit |
|
Thanks for waiting on this. I will check it shortly... |
|
@fahimj Can you please update this PR with the latest code from main? we now have 2 methods for next step. |
…factor to fix eslint nested ternary error
|
@parasharrajat I think this PR is not needed anymore, since this PR for this same issue has been merged. Should we close this issue? |
|
Aha, I see. Thanks for the update. |
|
@fahimj You can close this PR. |
Explanation of Change
Fixed the next step message for approved expense reports to show personalized text ("you") instead of generic text ("an admin") when the current user is a payer.
Previously, when an admin approved a report and no specific reimbuser was configured in the policy, the next step message would always show "Waiting for an admin to pay expenses" even when the current user was an admin who could pay.
This fix updates
NextStepUtils.tsto check if the current user is a payer (using the existingisPayer()function) when generating the APPROVED status next step message, matching the pattern already used in the PROCESSING status case. Now it correctly shows:Fixed Issues
$ #74503
PROPOSAL:
Tests
Test case 1: Admin approves report (manual reimbursement, no specific reimbuser)
Test case 2: Non-payer views approved report
Test case 4: Automated test
npm test -- --testNamePattern="should mention an admin to pay expenses in optimistic next step message when admin takes control and approves"Offline tests
QA Steps
Same as Tests
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