Fix: Workspace - Approver user briefly reappears in member list after removal#81733
Fix: Workspace - Approver user briefly reappears in member list after removal#81733rlinoz merged 10 commits intoExpensify:mainfrom
Conversation
src/libs/actions/Workflow.ts
Outdated
| employeeList: Object.fromEntries( | ||
| Object.keys(updatedEmployees).map((key) => [ | ||
| key, | ||
| previousEmployeeList[key]?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? {pendingFields: null} : {pendingAction: null, pendingFields: null}, |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The conditional logic for clearing pendingAction based on DELETE status is duplicated across three functions (createApprovalWorkflow, updateApprovalWorkflow, and removeApprovalWorkflow).
Extract this into a reusable helper function:
function getPendingActionClearValue(previousEmployeeList: Record<string, Employee>, key: string) {
return previousEmployeeList[key]?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE
? {pendingFields: null}
: {pendingAction: null, pendingFields: null};
}Then use it in all three locations:
employeeList: Object.fromEntries(
Object.keys(updatedEmployees).map((key) => [
key,
getPendingActionClearValue(previousEmployeeList, key),
]),
)Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
| @@ -187,25 +190,28 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM | |||
| return; | |||
| } | |||
|
|
|||
| // Remove the member first so they disappear immediately from the list | |||
| removeMemberAndCloseModal(); | |||
There was a problem hiding this comment.
I think we don't need to swap the order as the order looks correct.
There was a problem hiding this comment.
Ah You are right. I fixed
src/libs/actions/Workflow.ts
Outdated
| @@ -39,7 +39,7 @@ function createApprovalWorkflow({approvalWorkflow, policy, addExpenseApprovalsTa | |||
| return; | |||
| } | |||
|
|
|||
| const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value, pendingAction: null}])); | |||
| const previousEmployeeList = Object.fromEntries(Object.entries(policy.employeeList ?? {}).map(([key, value]) => [key, {...value}])); | |||
There was a problem hiding this comment.
Do we really need to set pendingAction separately here?
| if (workflow?.removeApprovalWorkflow) { | ||
| const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| InteractionManager.runAfterInteractions(() => { |
There was a problem hiding this comment.
I don't think we need these changes as we are now preserving the pendingAction.
There was a problem hiding this comment.
If we don’t change this part, it will automatically go back.
|
|
||
| // Remove the member and close the modal | ||
| // Remove the member first so they disappear immediately from the list |
There was a problem hiding this comment.
| // Remove the member first so they disappear immediately from the list | |
| // Remove the member and close the modal |
No need to update this comment
|
@annaweber830 Isn't the below change is enough to fix the issue? pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? {pendingFields: null} : {pendingAction: null, pendingFields: null} |
Hi @Pujan92, this change isn't enough to fix the issue. |
In that case, can you revert the other changes you made? I will review and test this tomorrow. |
Hi @Pujan92 I reverted my change. |
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-02-10.at.19.00.23.mov@annaweber830 The issue isn't solved. I think we can't solve it this way bcoz pendingAction won't be available as this data( App/src/libs/actions/Workflow.ts Line 106 in b2679d9 Let's use the same pattern of delaying(InteractionManager) remove member here that is used in App/src/pages/workspace/WorkspaceMembersPage.tsx Lines 261 to 265 in b2679d9 Let me know if that makes sense. |
711aeef to
c26358d
Compare
Hi @Pujan92 This solution didn't solve issue. I updated my code. Please check. |
Ah, in native it isn't fixed. I will test new changes. |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Agree with the product behavior outlined in the testing steps.
Hi @Pujan92 Sorry I don't understand.Do you want us to keep the sequence as: update approval workflow(s) first, then call member removal once? |
|
Yes, otherwise I ended up with the error that I shared in the screenshot. |
|
I think we can apply the following steps. Let me know if it makes sense to you.
App/src/libs/actions/Workflow.ts Line 154 in 87ce5ef
successMembersState[employeeEmail] = {
...successMembersState[employeeEmail],
pendingAction: null,
};App/src/libs/actions/Policy/Member.ts Line 402 in 87ce5ef |
|
Hi @Pujan92 Your solution didn't solve issue. |
|
Hey what is next here @Pujan92 @annaweber830 |
|
Hi @Pujan92 Please take a look this. |
|
I will try with this once |
|
@annaweber830 I found the RCA and the problem lies inside if(employee?.email && selectedMemberEmails.includes(employee?.email)) {
continue;
}App/src/libs/actions/Policy/Member.ts Lines 399 to 412 in bc3be6e |
|
Bump @annaweber830 to check comment |
This comment was marked as outdated.
This comment was marked as outdated.
src/libs/actions/Policy/Member.ts
Outdated
| if (selectedMemberEmailsWithDuplicates.includes(employeeEmail)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
@annaweber830 could you plz revert all other file changes? I think this change is enough to fix the issue.
|
Gentle Bump @annaweber830 |
|
Hey @annaweber830 can you take a look at this one, please? If you have too much on your plate now we can reassign. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApppm1.webmAndroid: mWeb ChromeiOS: HybridAppSimulator.Screen.Recording.-.iPhone.15.Pro.-.2026-03-18.at.15.34.24.moviOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-03-18.at.15.21.25.mov |
|
🚧 @rlinoz has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.3.40-0 🚀
|
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.3.40-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|

Explanation of Change
Fix: Workspace - Approver user briefly reappears in member list after removal
Fixed Issues
$ #80032
PROPOSAL: #80032 (comment)
Tests
Offline tests
Same as 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
Android: Native
Screen.Recording.2026-02-06.at.10.32.30.AM.mov
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari