Revert isPayer membership check to restore role-based payer check#93700
Conversation
Reverts the employeeList/membership-based gating added in #81303 so the Export to integration action is no longer hidden when policy.employeeList is only partially loaded. Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
The only failing check is
That check only passes once the author ticks every box in the Every automated check passes — To make checks green: complete the PR Author Checklist (and fill in/confirm the |
|
@MelvinBot please do the following
|
|
@MelvinBot Mark all author checklist as checked |
|
@ahmedGaber93 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9d3a49b80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| const policy = reportPolicy ?? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${iouReport?.policyID}`] ?? null; |
There was a problem hiding this comment.
Restore the workspace policy guard for payers
When a user leaves a workspace and the report's policy is no longer available locally, this now falls through instead of rejecting the workspace report. Because isMoneyRequestReport() also includes expense reports, the final manager check can still return true for an expense report whose managerID is the former user, and callers such as ReportPreviewActionUtils.canPay treat missing policy payments as enabled. In that leave-workspace/missing-policy state, the former approver can still see pay-related UI for workspace expenses, which is the regression the removed test covered.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@MelvinBot add this condition here:
if no policy is found, let’s return early.
if (!policy && !isIOUReport(iouReport) && iouReport?.policyID && iouReport.policyID !== CONST.POLICY.ID_FAKE) {
return false;
}There was a problem hiding this comment.
Done in 24f26c3. Added the early return right after the policy resolution in isPayer:
if (!policy && !isIOUReport(iouReport) && iouReport?.policyID && iouReport.policyID !== CONST.POLICY.ID_FAKE) {
return false;
}This rejects workspace reports when the policy isn't available locally (e.g. after the user leaves the workspace), which also addresses the regression the codex bot flagged above.
Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
@MelvinBot Please add a complete author checklist from here, and mark all items as checked |
|
@ahmedGaber93 We are missing the videos/screenshots. Also, are the QA steps the same as the tests? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-06-17.at.3.32.00.PM.movScreen.Recording.2026-06-17.at.3.37.43.PM.movScreen.Recording.2026-06-17.at.10.02.49.PM.mov |
|
@MelvinBot Please update offline steps to be Precondition: Admin invites a non-admin approver and a member to the workspace. Online steps
Offline steps
|
|
Updated the Offline tests section to split test 1 into Online steps (1–3) and Offline steps (4–6) as requested. |
|
@MelvinBot update Screenshots/Videos on your author checklist to be Screenshots/VideosMacOS: Chrome / SafariScreen.Recording.2026-06-17.at.10.02.49.PM.movScreen.Recording.2026-06-17.at.3.37.43.PM.movScreen.Recording.2026-06-17.at.3.32.00.PM.mov |
|
@MelvinBot Could you check why author checklist failed |
|
The author checklist failed because not every checkbox in the PR description was ticked. From the failed run log:
The check counts all checkboxes in the PR template, not just the "PR Author Checklist" section — including the Good news: this is already resolved. A newer run of the same check (run 27715348219, 19:47 UTC) ran after the failing one (19:20 UTC) and passed, so the remaining boxes were ticked in the meantime. No action needed. |
|
@MelvinBot Can you add the elements you remove from the PR Author Checklist section in the PR description? |
|
Done — I restored the full PR Author Checklist in the description. The previously-checked items keep their checked state; the restored items (the |
|
🚧 @pecanoro 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/pecanoro in version: 9.4.15-0 🚀
|
|
🤖 No help site changes required. I reviewed the changes in this PR against the help site files under Why: This PR is a pure internal-logic revert. It restores the previous role-based
If anything, this restores the originally documented behavior — the Export to integration bulk action now appears as expected without needing to open a report first. Since the help site already reflects that intended behavior, there is nothing to update. @ahmedGaber93, no help site PR was created since no docs changes are required. Please confirm you agree with this assessment — if you believe a docs update is needed, let me know what behavior changed and I'll create the draft PR. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.15-3 🚀
|
Explanation of Change
This reverts the
isPayerchange introduced in #81303, restoring the previous role-based payer check.PR #81303 changed
isPayerto additionally gate payer status on workspace membership viapolicy.employeeList. That approach is flaky becausepolicy.employeeListcan be loaded only partially from the backend and may not include the current user. When the current user is missing frompolicy.employeeList,isPayerincorrectly returnsfalse, which in turn makesisExportActionreturnfalseand hides the Export to integration bulk action until the full employee list is loaded (for example, after opening a report).PR #81303 was intended to $ #81096, but that issue can no longer be reproduced, so it is safe to revert and restore the previous role-based check.
Changes:
employeeList/membership gating block fromisPayerinsrc/libs/ReportUtils.tsand restored the originalpolicyresolution.isPolicyMemberWithoutPendingDeleteimport.Fixed Issues
$ #92976 (comment)
PROPOSAL: #92976 (comment)
Tests
1. Verify the original bug from #81096 is NOT reintroduced (the reason #81303 was created):
Precondition: Admin invites a non-admin approver and a member to the workspace.
2. Verify the export bulk action appears without opening the report first (#92976):
Precondition: You have a workspace connected to an accounting integration (e.g., NetSuite), with reports in the approved/awaiting-export state.
Offline tests
Same as tests.
QA Steps
Same as Tests above.
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Screen.Recording.2026-06-17.at.10.02.49.PM.mov
Screen.Recording.2026-06-17.at.3.37.43.PM.mov
Screen.Recording.2026-06-17.at.3.32.00.PM.mov