From 7ca3d2b1d080495037684bf2c566a81825a8bdf8 Mon Sep 17 00:00:00 2001 From: Nabi Ebrahimi Date: Thu, 20 Nov 2025 14:15:56 +0430 Subject: [PATCH 1/7] fix(workflows): preserve Approver field visibility and correct 'Expense from' behavior when approver is removed --- src/pages/workspace/WorkspaceMembersPage.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index 083ebd00d234f..f976c89ecf075 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -208,26 +208,29 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers if (hasApprovers) { const ownerEmail = ownerDetails.login; - for (const login of selectedEmployees) { + selectedEmployees.forEach((login) => { + if (!isApprover(policy, login)) { + return; + } const accountID = policyMemberEmailsToAccountIDs[login]; const removedApprover = personalDetails?.[accountID]; if (!removedApprover?.login || !ownerEmail) { - continue; + return; } const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ approvalWorkflows, removedApprover, ownerDetails, }); - for (const workflow of updatedWorkflows) { + updatedWorkflows.forEach((workflow) => { if (workflow?.removeApprovalWorkflow) { const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; removeApprovalWorkflowAction(updatedWorkflow, policy); } else { updateApprovalWorkflow(workflow, [], [], policy); } - } - } + }); + }); } setRemoveMembersConfirmModalVisible(false); From da0824c81490b808f53ad1669a4ce75c39f96de2 Mon Sep 17 00:00:00 2001 From: Nabi Ebrahimi Date: Thu, 20 Nov 2025 14:46:01 +0430 Subject: [PATCH 2/7] replaced foreach by for of --- src/pages/workspace/WorkspaceMembersPage.tsx | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index f976c89ecf075..f682944cb9261 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -207,33 +207,39 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const hasApprovers = selectedEmployees.some((email) => isApprover(policy, email)); if (hasApprovers) { - const ownerEmail = ownerDetails.login; - selectedEmployees.forEach((login) => { + const ownerEmail = ownerDetails?.login; + + for (const login of selectedEmployees) { if (!isApprover(policy, login)) { - return; + continue; } + const accountID = policyMemberEmailsToAccountIDs[login]; const removedApprover = personalDetails?.[accountID]; + if (!removedApprover?.login || !ownerEmail) { - return; + continue; } + const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ approvalWorkflows, removedApprover, ownerDetails, }); - updatedWorkflows.forEach((workflow) => { + + for (const workflow of updatedWorkflows) { if (workflow?.removeApprovalWorkflow) { const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; removeApprovalWorkflowAction(updatedWorkflow, policy); } else { updateApprovalWorkflow(workflow, [], [], policy); } - }); - }); + } + } } setRemoveMembersConfirmModalVisible(false); + // eslint-disable-next-line @typescript-eslint/no-deprecated InteractionManager.runAfterInteractions(() => { setSelectedEmployees([]); From 9cfebe64a5297fd49a6125136c03cf83f089622c Mon Sep 17 00:00:00 2001 From: Nabi Ebrahimi Date: Tue, 2 Dec 2025 11:50:56 +0430 Subject: [PATCH 3/7] extracted main part of removeUsers to a external function and write unit test for it --- src/libs/WorkflowUtils.ts | 50 +++++++++++++ src/pages/workspace/WorkspaceMembersPage.tsx | 40 +++-------- tests/unit/WorkflowUtilsTest.ts | 75 ++++++++++++++++++++ 3 files changed, 134 insertions(+), 31 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 77417d79335ef..2b58fcf90a429 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -10,6 +10,8 @@ import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; import type PersonalDetails from '@src/types/onyx/PersonalDetails'; import type Policy from '@src/types/onyx/Policy'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; +import {isApprover} from './actions/Policy/Member'; +import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from './actions/Workflow'; import {isBankAccountPartiallySetup} from './BankAccountUtils'; import {getDefaultApprover} from './PolicyUtils'; @@ -446,6 +448,53 @@ function getEligibleExistingBusinessBankAccounts(bankAccountList: BankAccountLis }); } +type RemoveApproveWorkflowsParams = { + policy: OnyxEntry; + selectedEmployees: string[]; + policyMemberEmailsToAccountIDs: Record; + personalDetails: OnyxEntry; + ownerDetails: PersonalDetails; + approvalWorkflows: ApprovalWorkflow[]; +}; + +/** + * Runs the logic that handles workflow updates for each approver being removed. + * Extracted for unit testing. + */ +function removeApproveWorkflows(args: RemoveApproveWorkflowsParams) { + const {policy, selectedEmployees, policyMemberEmailsToAccountIDs, personalDetails, ownerDetails, approvalWorkflows} = args; + + const ownerEmail = ownerDetails?.login; + + for (const login of selectedEmployees) { + if (!isApprover(policy, login)) { + continue; + } + + const accountID = policyMemberEmailsToAccountIDs[login]; + const removedApprover = personalDetails?.[accountID]; + + if (!removedApprover?.login || !ownerEmail) { + continue; + } + + const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ + approvalWorkflows, + removedApprover, + ownerDetails, + }); + + for (const workflow of updatedWorkflows) { + if (workflow?.removeApprovalWorkflow) { + const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; + removeApprovalWorkflowAction(updatedWorkflow, policy); + } else { + updateApprovalWorkflow(workflow, [], [], policy); + } + } + } +} + export { calculateApprovers, convertPolicyEmployeesToApprovalWorkflows, @@ -453,4 +502,5 @@ export { getEligibleExistingBusinessBankAccounts, INITIAL_APPROVAL_WORKFLOW, updateWorkflowDataOnApproverRemoval, + removeApproveWorkflows, }; diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index b2a43e6461f77..4667b9b273ce5 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -43,7 +43,6 @@ import { removeMembers, updateWorkspaceMembersRole, } from '@libs/actions/Policy/Member'; -import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from '@libs/actions/Workflow'; import {canUseTouchScreen} from '@libs/DeviceCapabilities'; import {getLatestErrorMessageField} from '@libs/ErrorUtils'; import Log from '@libs/Log'; @@ -55,7 +54,7 @@ import {getDisplayNameOrDefault, getPersonalDetailsByIDs} from '@libs/PersonalDe import {getMemberAccountIDsForWorkspace, isControlPolicy, isDeletedPolicyEmployee, isExpensifyTeam, isPaidGroupPolicy, isPolicyAdmin as isPolicyAdminUtils} from '@libs/PolicyUtils'; import {getDisplayNameForParticipant} from '@libs/ReportUtils'; import tokenizedSearch from '@libs/tokenizedSearch'; -import {convertPolicyEmployeesToApprovalWorkflows, updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils'; +import {convertPolicyEmployeesToApprovalWorkflows, removeApproveWorkflows} from '@libs/WorkflowUtils'; import {close} from '@userActions/Modal'; import {dismissAddedWithPrimaryLoginMessages} from '@userActions/Policy/Policy'; import CONST from '@src/CONST'; @@ -209,35 +208,14 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const hasApprovers = selectedEmployees.some((email) => isApprover(policy, email)); if (hasApprovers) { - const ownerEmail = ownerDetails?.login; - - for (const login of selectedEmployees) { - if (!isApprover(policy, login)) { - continue; - } - - const accountID = policyMemberEmailsToAccountIDs[login]; - const removedApprover = personalDetails?.[accountID]; - - if (!removedApprover?.login || !ownerEmail) { - continue; - } - - const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ - approvalWorkflows, - removedApprover, - ownerDetails, - }); - - for (const workflow of updatedWorkflows) { - if (workflow?.removeApprovalWorkflow) { - const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; - removeApprovalWorkflowAction(updatedWorkflow, policy); - } else { - updateApprovalWorkflow(workflow, [], [], policy); - } - } - } + removeApproveWorkflows({ + policy, + selectedEmployees, + policyMemberEmailsToAccountIDs, + personalDetails, + ownerDetails, + approvalWorkflows, + }); } setRemoveMembersConfirmModalVisible(false); diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 4a245f08e11b6..3cfc682e545d8 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -1,5 +1,8 @@ /* eslint-disable @typescript-eslint/naming-convention */ +import type {OnyxEntry} from 'react-native-onyx'; +import CONST from '@src/CONST'; import * as WorkflowUtils from '@src/libs/WorkflowUtils'; +import type {Policy} from '@src/types/onyx'; import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; @@ -642,4 +645,76 @@ describe('WorkflowUtils', () => { expect(updateWorkflowDataOnApproverRemoval).toEqual([approvalWorkflow1, {...approvalWorkflow2, removeApprovalWorkflow: true}]); }); }); + + describe('removeApproveWorkflows', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + // Mock the actions + const mockUpdateApprovalWorkflow = jest.fn(); + const mockRemoveApprovalWorkflowAction = jest.fn(); + + // Replace the real imports with mocks + jest.spyOn(require('@libs/actions/Workflow'), 'updateApprovalWorkflow').mockImplementation(mockUpdateApprovalWorkflow); + jest.spyOn(require('@libs/actions/Workflow'), 'removeApprovalWorkflow').mockImplementation(mockRemoveApprovalWorkflowAction); + + it('should NOT call updateApprovalWorkflow or removeApprovalWorkflowAction when selected employee is NOT an approver', () => { + const policy: OnyxEntry = { + id: 'POLICY123', + name: 'Test Policy', + role: CONST.POLICY.ROLE.ADMIN, + type: CONST.POLICY.TYPE.TEAM, + owner: 'owner@example.com', + outputCurrency: 'USD', + isPolicyExpenseChatEnabled: true, + employeeList: { + 'alice@example.com': {email: 'alice@example.com', submitsTo: 'bob@example.com'}, + 'bob@example.com': {email: 'bob@example.com', submitsTo: 'owner@example.com'}, + 'carol@example.com': {email: 'carol@example.com', submitsTo: 'bob@example.com'}, // not an approver + }, + approver: 'owner@example.com', + }; + + const approvalWorkflows: ApprovalWorkflow[] = [ + { + members: [{email: 'carol@example.com', displayName: 'Carol', avatar: ''}], + approvers: [{email: 'bob@example.com', displayName: 'Bob', avatar: '', forwardsTo: 'owner@example.com'}], + isDefault: false, + }, + { + members: [], + approvers: [{email: 'owner@example.com', displayName: 'Owner', avatar: ''}], + isDefault: true, + }, + ]; + + const localPersonalDetails = { + 1: {accountID: 1, login: 'owner@example.com', displayName: 'Owner'}, + 2: {accountID: 2, login: 'bob@example.com', displayName: 'Bob'}, + 3: {accountID: 3, login: 'carol@example.com', displayName: 'Carol'}, + }; + + const policyMemberEmailsToAccountIDs: Record = { + 'owner@example.com': 1, + 'bob@example.com': 2, + 'carol@example.com': 3, + }; + + const ownerDetails = localPersonalDetails[1]; + + WorkflowUtils.removeApproveWorkflows({ + policy, + selectedEmployees: ['carol@example.com'], // Carol is NOT an approver + policyMemberEmailsToAccountIDs, + personalDetails, + ownerDetails, + approvalWorkflows, + }); + + // Assert - both actions must NOT be called + expect(mockUpdateApprovalWorkflow).not.toHaveBeenCalled(); + expect(mockRemoveApprovalWorkflowAction).not.toHaveBeenCalled(); + }); + }); }); From 4f78e5224153a5c3fa89a41004aa6d77ace7ac51 Mon Sep 17 00:00:00 2001 From: Nabi Ebrahimi Date: Wed, 3 Dec 2025 10:00:38 +0430 Subject: [PATCH 4/7] reverted extracted codes --- src/libs/WorkflowUtils.ts | 50 ------------- src/pages/workspace/WorkspaceMembersPage.tsx | 36 +++++++--- tests/unit/WorkflowUtilsTest.ts | 75 -------------------- 3 files changed, 27 insertions(+), 134 deletions(-) diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 2b58fcf90a429..77417d79335ef 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -10,8 +10,6 @@ import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; import type PersonalDetails from '@src/types/onyx/PersonalDetails'; import type Policy from '@src/types/onyx/Policy'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; -import {isApprover} from './actions/Policy/Member'; -import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from './actions/Workflow'; import {isBankAccountPartiallySetup} from './BankAccountUtils'; import {getDefaultApprover} from './PolicyUtils'; @@ -448,53 +446,6 @@ function getEligibleExistingBusinessBankAccounts(bankAccountList: BankAccountLis }); } -type RemoveApproveWorkflowsParams = { - policy: OnyxEntry; - selectedEmployees: string[]; - policyMemberEmailsToAccountIDs: Record; - personalDetails: OnyxEntry; - ownerDetails: PersonalDetails; - approvalWorkflows: ApprovalWorkflow[]; -}; - -/** - * Runs the logic that handles workflow updates for each approver being removed. - * Extracted for unit testing. - */ -function removeApproveWorkflows(args: RemoveApproveWorkflowsParams) { - const {policy, selectedEmployees, policyMemberEmailsToAccountIDs, personalDetails, ownerDetails, approvalWorkflows} = args; - - const ownerEmail = ownerDetails?.login; - - for (const login of selectedEmployees) { - if (!isApprover(policy, login)) { - continue; - } - - const accountID = policyMemberEmailsToAccountIDs[login]; - const removedApprover = personalDetails?.[accountID]; - - if (!removedApprover?.login || !ownerEmail) { - continue; - } - - const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ - approvalWorkflows, - removedApprover, - ownerDetails, - }); - - for (const workflow of updatedWorkflows) { - if (workflow?.removeApprovalWorkflow) { - const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; - removeApprovalWorkflowAction(updatedWorkflow, policy); - } else { - updateApprovalWorkflow(workflow, [], [], policy); - } - } - } -} - export { calculateApprovers, convertPolicyEmployeesToApprovalWorkflows, @@ -502,5 +453,4 @@ export { getEligibleExistingBusinessBankAccounts, INITIAL_APPROVAL_WORKFLOW, updateWorkflowDataOnApproverRemoval, - removeApproveWorkflows, }; diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index 4667b9b273ce5..0b3e780d8f037 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -43,6 +43,7 @@ import { removeMembers, updateWorkspaceMembersRole, } from '@libs/actions/Policy/Member'; +import {removeApprovalWorkflow as removeApprovalWorkflowAction, updateApprovalWorkflow} from '@libs/actions/Workflow'; import {canUseTouchScreen} from '@libs/DeviceCapabilities'; import {getLatestErrorMessageField} from '@libs/ErrorUtils'; import Log from '@libs/Log'; @@ -54,7 +55,7 @@ import {getDisplayNameOrDefault, getPersonalDetailsByIDs} from '@libs/PersonalDe import {getMemberAccountIDsForWorkspace, isControlPolicy, isDeletedPolicyEmployee, isExpensifyTeam, isPaidGroupPolicy, isPolicyAdmin as isPolicyAdminUtils} from '@libs/PolicyUtils'; import {getDisplayNameForParticipant} from '@libs/ReportUtils'; import tokenizedSearch from '@libs/tokenizedSearch'; -import {convertPolicyEmployeesToApprovalWorkflows, removeApproveWorkflows} from '@libs/WorkflowUtils'; +import {convertPolicyEmployeesToApprovalWorkflows, updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils'; import {close} from '@userActions/Modal'; import {dismissAddedWithPrimaryLoginMessages} from '@userActions/Policy/Policy'; import CONST from '@src/CONST'; @@ -208,14 +209,31 @@ function WorkspaceMembersPage({personalDetails, route, policy}: WorkspaceMembers const hasApprovers = selectedEmployees.some((email) => isApprover(policy, email)); if (hasApprovers) { - removeApproveWorkflows({ - policy, - selectedEmployees, - policyMemberEmailsToAccountIDs, - personalDetails, - ownerDetails, - approvalWorkflows, - }); + const ownerEmail = ownerDetails.login; + for (const login of selectedEmployees) { + if (!isApprover(policy, login)) { + continue; + } + + const accountID = policyMemberEmailsToAccountIDs[login]; + const removedApprover = personalDetails?.[accountID]; + if (!removedApprover?.login || !ownerEmail) { + continue; + } + const updatedWorkflows = updateWorkflowDataOnApproverRemoval({ + approvalWorkflows, + removedApprover, + ownerDetails, + }); + for (const workflow of updatedWorkflows) { + if (workflow?.removeApprovalWorkflow) { + const {removeApprovalWorkflow, ...updatedWorkflow} = workflow; + removeApprovalWorkflowAction(updatedWorkflow, policy); + } else { + updateApprovalWorkflow(workflow, [], [], policy); + } + } + } } setRemoveMembersConfirmModalVisible(false); diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 3cfc682e545d8..4a245f08e11b6 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -1,8 +1,5 @@ /* eslint-disable @typescript-eslint/naming-convention */ -import type {OnyxEntry} from 'react-native-onyx'; -import CONST from '@src/CONST'; import * as WorkflowUtils from '@src/libs/WorkflowUtils'; -import type {Policy} from '@src/types/onyx'; import type {Approver, Member} from '@src/types/onyx/ApprovalWorkflow'; import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; @@ -645,76 +642,4 @@ describe('WorkflowUtils', () => { expect(updateWorkflowDataOnApproverRemoval).toEqual([approvalWorkflow1, {...approvalWorkflow2, removeApprovalWorkflow: true}]); }); }); - - describe('removeApproveWorkflows', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - // Mock the actions - const mockUpdateApprovalWorkflow = jest.fn(); - const mockRemoveApprovalWorkflowAction = jest.fn(); - - // Replace the real imports with mocks - jest.spyOn(require('@libs/actions/Workflow'), 'updateApprovalWorkflow').mockImplementation(mockUpdateApprovalWorkflow); - jest.spyOn(require('@libs/actions/Workflow'), 'removeApprovalWorkflow').mockImplementation(mockRemoveApprovalWorkflowAction); - - it('should NOT call updateApprovalWorkflow or removeApprovalWorkflowAction when selected employee is NOT an approver', () => { - const policy: OnyxEntry = { - id: 'POLICY123', - name: 'Test Policy', - role: CONST.POLICY.ROLE.ADMIN, - type: CONST.POLICY.TYPE.TEAM, - owner: 'owner@example.com', - outputCurrency: 'USD', - isPolicyExpenseChatEnabled: true, - employeeList: { - 'alice@example.com': {email: 'alice@example.com', submitsTo: 'bob@example.com'}, - 'bob@example.com': {email: 'bob@example.com', submitsTo: 'owner@example.com'}, - 'carol@example.com': {email: 'carol@example.com', submitsTo: 'bob@example.com'}, // not an approver - }, - approver: 'owner@example.com', - }; - - const approvalWorkflows: ApprovalWorkflow[] = [ - { - members: [{email: 'carol@example.com', displayName: 'Carol', avatar: ''}], - approvers: [{email: 'bob@example.com', displayName: 'Bob', avatar: '', forwardsTo: 'owner@example.com'}], - isDefault: false, - }, - { - members: [], - approvers: [{email: 'owner@example.com', displayName: 'Owner', avatar: ''}], - isDefault: true, - }, - ]; - - const localPersonalDetails = { - 1: {accountID: 1, login: 'owner@example.com', displayName: 'Owner'}, - 2: {accountID: 2, login: 'bob@example.com', displayName: 'Bob'}, - 3: {accountID: 3, login: 'carol@example.com', displayName: 'Carol'}, - }; - - const policyMemberEmailsToAccountIDs: Record = { - 'owner@example.com': 1, - 'bob@example.com': 2, - 'carol@example.com': 3, - }; - - const ownerDetails = localPersonalDetails[1]; - - WorkflowUtils.removeApproveWorkflows({ - policy, - selectedEmployees: ['carol@example.com'], // Carol is NOT an approver - policyMemberEmailsToAccountIDs, - personalDetails, - ownerDetails, - approvalWorkflows, - }); - - // Assert - both actions must NOT be called - expect(mockUpdateApprovalWorkflow).not.toHaveBeenCalled(); - expect(mockRemoveApprovalWorkflowAction).not.toHaveBeenCalled(); - }); - }); }); From a2cecabe734c05c0c7a16adf0ba7c81e977049cd Mon Sep 17 00:00:00 2001 From: Nabi Ebrahimi Date: Thu, 4 Dec 2025 11:27:56 +0430 Subject: [PATCH 5/7] added ui test to not call workflow actions for non approver when removeUsers --- tests/ui/WorkspaceMembersTest.tsx | 55 +++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/ui/WorkspaceMembersTest.tsx b/tests/ui/WorkspaceMembersTest.tsx index f78e192b27983..b9569abaf4f0f 100644 --- a/tests/ui/WorkspaceMembersTest.tsx +++ b/tests/ui/WorkspaceMembersTest.tsx @@ -9,7 +9,9 @@ import OnyxListItemProvider from '@components/OnyxListItemProvider'; import {CurrentReportIDContextProvider} from '@hooks/useCurrentReportID'; import * as useResponsiveLayoutModule from '@hooks/useResponsiveLayout'; import type ResponsiveLayoutResult from '@hooks/useResponsiveLayout/types'; +import * as workflow from '@libs/actions/Workflow'; import createPlatformStackNavigator from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigator'; +import * as workflowUtils from '@libs/WorkflowUtils'; import type {WorkspaceSplitNavigatorParamList} from '@navigation/types'; import WorkspaceMembersPage from '@pages/workspace/WorkspaceMembersPage'; import CONST from '@src/CONST'; @@ -24,6 +26,12 @@ jest.unmock('react-native-worklets'); jest.mock('@src/components/ConfirmedRoute.tsx'); +const updateWorkflowDataOnApproverRemovalMock = jest + .spyOn(workflowUtils, 'updateWorkflowDataOnApproverRemoval') + .mockImplementation(() => [{members: [], approvers: [], isDefault: false, removeApprovalWorkflow: true}]); + +const removeApprovalWorkflowActionMock = jest.spyOn(workflow, 'removeApprovalWorkflow').mockImplementation(() => {}); + TestHelper.setupGlobalFetchMock(); const Stack = createPlatformStackNavigator(); @@ -66,6 +74,7 @@ describe('WorkspaceMembers', () => { owner: ownerEmail, ownerAccountID, type: CONST.POLICY.TYPE.CORPORATE, + approver: adminEmail, employeeList: { [ownerEmail]: {email: ownerEmail, role: CONST.POLICY.ROLE.ADMIN}, [adminEmail]: {email: adminEmail, role: CONST.POLICY.ROLE.ADMIN}, @@ -296,4 +305,50 @@ describe('WorkspaceMembers', () => { await waitForBatchedUpdatesWithAct(); }); }); + + describe('Removing members who are approvers and non-approvers', () => { + it('should not call workflow actions when removing only non-approvers', async () => { + const {unmount} = renderPage(SCREENS.WORKSPACE.MEMBERS, {policyID: policy.id}); + await waitForBatchedUpdatesWithAct(); + + await screen.findByText(ADMIN_OPTION); + + // Select all + fireEvent.press(screen.getByTestId('selection-list-select-all-checkbox')); + + // Open dropdown + fireEvent.press(await screen.findByTestId('WorkspaceMembersPage-header-dropdown-menu-button')); + await waitForBatchedUpdatesWithAct(); + + // Click "Remove members" + const removeText = TestHelper.translateLocal('workspace.people.removeMembersTitle', {count: 3}); + const removeMenuItem = screen.getByText(removeText); + fireEvent.press(removeMenuItem, { + nativeEvent: {}, + type: 'press', + target: removeMenuItem, + currentTarget: removeMenuItem, + }); + + await waitForBatchedUpdatesWithAct(); + + // Wait until confirm modal confirm button exists + const confirmText = TestHelper.translateLocal('common.remove'); + + await waitFor(() => { + expect(screen.getByLabelText(confirmText)).toBeOnTheScreen(); + }); + + // Press confirm button + fireEvent.press(screen.getByLabelText(confirmText)); + + await waitForBatchedUpdatesWithAct(); + + expect(updateWorkflowDataOnApproverRemovalMock).toHaveBeenCalledTimes(1); + expect(removeApprovalWorkflowActionMock).toHaveBeenCalledTimes(1); + + unmount(); + await waitForBatchedUpdatesWithAct(); + }); + }); }); From 67404996d6e73a3feadfbf8e5f7659c45df580e6 Mon Sep 17 00:00:00 2001 From: Nabi Ebrahimi Date: Thu, 4 Dec 2025 12:36:40 +0430 Subject: [PATCH 6/7] fixed eslint failure --- tests/ui/WorkspaceMembersTest.tsx | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/ui/WorkspaceMembersTest.tsx b/tests/ui/WorkspaceMembersTest.tsx index b9569abaf4f0f..f1627b265b9c8 100644 --- a/tests/ui/WorkspaceMembersTest.tsx +++ b/tests/ui/WorkspaceMembersTest.tsx @@ -9,9 +9,9 @@ import OnyxListItemProvider from '@components/OnyxListItemProvider'; import {CurrentReportIDContextProvider} from '@hooks/useCurrentReportID'; import * as useResponsiveLayoutModule from '@hooks/useResponsiveLayout'; import type ResponsiveLayoutResult from '@hooks/useResponsiveLayout/types'; -import * as workflow from '@libs/actions/Workflow'; +import {removeApprovalWorkflow} from '@libs/actions/Workflow'; import createPlatformStackNavigator from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigator'; -import * as workflowUtils from '@libs/WorkflowUtils'; +import {updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils'; import type {WorkspaceSplitNavigatorParamList} from '@navigation/types'; import WorkspaceMembersPage from '@pages/workspace/WorkspaceMembersPage'; import CONST from '@src/CONST'; @@ -26,11 +26,25 @@ jest.unmock('react-native-worklets'); jest.mock('@src/components/ConfirmedRoute.tsx'); -const updateWorkflowDataOnApproverRemovalMock = jest - .spyOn(workflowUtils, 'updateWorkflowDataOnApproverRemoval') - .mockImplementation(() => [{members: [], approvers: [], isDefault: false, removeApprovalWorkflow: true}]); +jest.mock('@libs/WorkflowUtils', () => { + // eslint-disable-next-line + const actual = jest.requireActual('@libs/WorkflowUtils'); + // eslint-disable-next-line + return { + ...actual, + updateWorkflowDataOnApproverRemoval: jest.fn(() => [{members: [], approvers: [], isDefault: false, removeApprovalWorkflow: true}]), + }; +}); -const removeApprovalWorkflowActionMock = jest.spyOn(workflow, 'removeApprovalWorkflow').mockImplementation(() => {}); +jest.mock('@libs/actions/Workflow', () => { + // eslint-disable-next-line + const actual = jest.requireActual('@libs/actions/Workflow'); + // eslint-disable-next-line + return { + ...actual, + removeApprovalWorkflow: jest.fn(), + }; +}); TestHelper.setupGlobalFetchMock(); @@ -344,8 +358,8 @@ describe('WorkspaceMembers', () => { await waitForBatchedUpdatesWithAct(); - expect(updateWorkflowDataOnApproverRemovalMock).toHaveBeenCalledTimes(1); - expect(removeApprovalWorkflowActionMock).toHaveBeenCalledTimes(1); + expect(updateWorkflowDataOnApproverRemoval).toHaveBeenCalledTimes(1); + expect(removeApprovalWorkflow).toHaveBeenCalledTimes(1); unmount(); await waitForBatchedUpdatesWithAct(); From 245f71d5edeae62c694fbe5c8c143138343b13df Mon Sep 17 00:00:00 2001 From: Nabi Ebrahimi Date: Thu, 4 Dec 2025 13:56:30 +0430 Subject: [PATCH 7/7] minor change --- tests/ui/WorkspaceMembersTest.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ui/WorkspaceMembersTest.tsx b/tests/ui/WorkspaceMembersTest.tsx index f1627b265b9c8..df6f62d35f0d4 100644 --- a/tests/ui/WorkspaceMembersTest.tsx +++ b/tests/ui/WorkspaceMembersTest.tsx @@ -321,7 +321,7 @@ describe('WorkspaceMembers', () => { }); describe('Removing members who are approvers and non-approvers', () => { - it('should not call workflow actions when removing only non-approvers', async () => { + it('should call workflow actions once when removing multiple members including an approver', async () => { const {unmount} = renderPage(SCREENS.WORKSPACE.MEMBERS, {policyID: policy.id}); await waitForBatchedUpdatesWithAct(); @@ -358,6 +358,7 @@ describe('WorkspaceMembers', () => { await waitForBatchedUpdatesWithAct(); + // Verify workflow actions are only called once when an approver is removed expect(updateWorkflowDataOnApproverRemoval).toHaveBeenCalledTimes(1); expect(removeApprovalWorkflow).toHaveBeenCalledTimes(1);