[Hold Web-E #50684]Remove unnecessary call openWorkspaceMembersPage in WorkspaceCompanyCardsPage.tsx#82055
[Hold Web-E #50684]Remove unnecessary call openWorkspaceMembersPage in WorkspaceCompanyCardsPage.tsx#82055fedirjh wants to merge 11 commits intoExpensify:mainfrom
openWorkspaceMembersPage in WorkspaceCompanyCardsPage.tsx#82055Conversation
…and update related function
…ls in openPolicyCompanyCardsFeed call
|
@fedirjh I don't think the failing test is related, try merging main? |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Is this ready to review Fedi? |
…related function calls to simplify API usage.
emailList to openPolicyCompanyCardsFeed and remove extra openWorkspaceMembersPage callopenWorkspaceMembersPage in WorkspaceCompanyCardsPage.tsx
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-02-12.at.03.19.36.movAndroid: mWeb ChromeScreen.Recording.2026-02-12.at.03.15.37.moviOS: HybridAppScreen.Recording.2026-02-12.at.03.18.45.moviOS: mWeb SafariScreen.Recording.2026-02-12.at.03.14.21.movMacOS: Chrome / SafariScreen.Recording.2026-02-12.at.03.13.21.mov |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9555ec4d48
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const clientMemberEmails = Object.keys(getMemberAccountIDsForWorkspace(policy?.employeeList)); | ||
| openWorkspaceMembersPage(policyID, clientMemberEmails); | ||
| openPolicyCompanyCardsFeed(domainOrWorkspaceAccountID, policyID, bankName, translate); |
There was a problem hiding this comment.
Restore member sync before loading the card feed
Removing the openWorkspaceMembersPage call here means Company Cards no longer refreshes policy.employeeList before the assign-card flow starts; when that list is missing or stale in Onyx, AssigneeStep classifies existing members as non-members (if (!policy?.employeeList?.[assignee?.login])) and routes to InviteNewMemberStep, whose auto-advance guard also depends on policy.employeeList, so assigning to an already-member user can fail with duplicate-invite behavior instead of proceeding to assignment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We will return employeeList within https://github.com/Expensify/Auth/pull/19872 .
chrispader
left a comment
There was a problem hiding this comment.
LGTM! Let's remove the unnecessary API call 👍🏼
openWorkspaceMembersPage in WorkspaceCompanyCardsPage.tsxopenWorkspaceMembersPage in WorkspaceCompanyCardsPage.tsx
|
@fedirjh some failing checks. |
Explanation of Change
The Company Cards page previously made two separate API calls when loading a card feed:
openWorkspaceMembersPage— to sync the policy's employee list (emails + roles)openPolicyCompanyCardsFeed— to get the card feed data (domain settings + cards)This PR removes the extra
openWorkspaceMembersPagecall introduced to cover the invite user during the assign card flow. It has been observed that the inviter flow is working as expected and is updating the frontend with the neededpersonalDetailsdata for the newly added user. Therefore, the separate call for syncing the employee list is no longer required, streamlining the loading process for the card feed.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/583683
PROPOSAL: https://github.com/Expensify/Expensify/issues/583683#issuecomment-3813238448
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
CleanShot.2026-02-11.at.19.17.26.mp4
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari