diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index a6109820873e..61f784d8f228 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -574,6 +574,15 @@ function getOpenConnectedToPolicyBusinessBankAccounts(bankAccountList: BankAccou }); } +/** + * Combine workflow members with available members, deduplicating by email. + */ +function mergeWorkflowMembersWithAvailableMembers(workflowMembers: Member[], allAvailableMembers: Member[]): Member[] { + const memberEmails = new Set(workflowMembers.map((m) => m.email)); + const additionalMembers = allAvailableMembers.filter((m) => !memberEmails.has(m.email)); + return [...workflowMembers, ...additionalMembers]; +} + export { calculateApprovers, convertPolicyEmployeesToApprovalWorkflows, @@ -582,5 +591,6 @@ export { getEligibleExistingBusinessBankAccounts, getOpenConnectedToPolicyBusinessBankAccounts, INITIAL_APPROVAL_WORKFLOW, + mergeWorkflowMembersWithAvailableMembers, updateWorkflowDataOnApproverRemoval, }; diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx index 5a353813b27a..78a0c2a4cce1 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx @@ -17,7 +17,7 @@ import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {WorkspaceSplitNavigatorParamList} from '@libs/Navigation/types'; import {goBackFromInvalidPolicy, isPendingDeletePolicy, isPolicyAdmin} from '@libs/PolicyUtils'; -import {convertPolicyEmployeesToApprovalWorkflows} from '@libs/WorkflowUtils'; +import {convertPolicyEmployeesToApprovalWorkflows, mergeWorkflowMembersWithAvailableMembers} from '@libs/WorkflowUtils'; import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper'; import withPolicyAndFullscreenLoading from '@pages/workspace/withPolicyAndFullscreenLoading'; import type {WithPolicyAndFullscreenLoadingProps} from '@pages/workspace/withPolicyAndFullscreenLoading'; @@ -117,7 +117,7 @@ function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true setApprovalWorkflow({ ...currentApprovalWorkflow, - availableMembers: [...currentApprovalWorkflow.members, ...defaultWorkflowMembers], + availableMembers: mergeWorkflowMembersWithAvailableMembers(currentApprovalWorkflow.members, defaultWorkflowMembers), usedApproverEmails, action: CONST.APPROVAL_WORKFLOW.ACTION.EDIT, errors: null, diff --git a/tests/ui/WorkspaceWorkflowsApprovalsEditPageTest.tsx b/tests/ui/WorkspaceWorkflowsApprovalsEditPageTest.tsx new file mode 100644 index 000000000000..ef462e550190 --- /dev/null +++ b/tests/ui/WorkspaceWorkflowsApprovalsEditPageTest.tsx @@ -0,0 +1,134 @@ +import {act, render} from '@testing-library/react-native'; +import React from 'react'; +import Onyx from 'react-native-onyx'; +import ComposeProviders from '@components/ComposeProviders'; +import {LocaleContextProvider} from '@components/LocaleContextProvider'; +import OnyxListItemProvider from '@components/OnyxListItemProvider'; +import WorkspaceWorkflowsApprovalsEditPage from '@pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage'; +import {setApprovalWorkflow} from '@userActions/Workflow'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Policy} from '@src/types/onyx'; +import type {Member} from '@src/types/onyx/ApprovalWorkflow'; +import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; +import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; +import {buildPersonalDetails} from '../utils/TestHelper'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +const POLICY_ID = 'workflow-approvals-edit-test-policy'; +const ALICE_EMAIL = 'alice@example.com'; +const ALICE_ACCOUNT_ID = 1; + +jest.mock('@react-navigation/native', () => { + // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment + const actualNav = jest.requireActual('@react-navigation/native'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return { + ...actualNav, + useIsFocused: () => true, + usePreventRemove: jest.fn(), + }; +}); + +jest.mock('@libs/Navigation/Navigation', () => ({ + goBack: jest.fn(), + dismissModal: jest.fn(), +})); + +function buildPolicy(): Policy { + const employeeList: PolicyEmployeeList = { + [ALICE_EMAIL]: { + email: ALICE_EMAIL, + submitsTo: ALICE_EMAIL, + forwardsTo: undefined, + }, + }; + return { + id: POLICY_ID, + name: 'Test Workspace', + type: CONST.POLICY.TYPE.CORPORATE, + role: CONST.POLICY.ROLE.ADMIN, + owner: ALICE_EMAIL, + employeeList, + approver: ALICE_EMAIL, + areWorkflowsEnabled: true, + isPolicyExpenseChatEnabled: true, + outputCurrency: 'USD', + avatarURL: '', + lastModified: new Date().toISOString(), + pendingAction: null, + errors: {}, + } as Policy; +} + +function buildPersonalDetailsList(): PersonalDetailsList { + return { + [ALICE_ACCOUNT_ID]: buildPersonalDetails(ALICE_EMAIL, ALICE_ACCOUNT_ID, 'alice'), + }; +} + +const mockRoute = { + key: 'test-route', + name: 'Workspace_Approvals_Edit', + params: { + policyID: POLICY_ID, + firstApproverEmail: ALICE_EMAIL, + }, +}; + +const renderEditPage = () => + render( + + + , + ); + +describe('WorkspaceWorkflowsApprovalsEditPage', () => { + beforeAll(async () => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + jest.spyOn(require('@userActions/Workflow'), 'setApprovalWorkflow'); + await act(async () => { + await Onyx.clear(); + await Onyx.set(ONYXKEYS.HAS_LOADED_APP, true); + await Onyx.set(ONYXKEYS.IS_LOADING_REPORT_DATA, false); + + const policy = buildPolicy(); + const personalDetails = buildPersonalDetailsList(); + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails); + await Onyx.merge(ONYXKEYS.SESSION, {email: ALICE_EMAIL, accountID: ALICE_ACCOUNT_ID}); + await waitForBatchedUpdatesWithAct(); + }); + }); + + afterEach(async () => { + jest.restoreAllMocks(); + await act(async () => { + await Onyx.clear(); + await waitForBatchedUpdatesWithAct(); + }); + }); + + it('should pass deduplicated availableMembers to setApprovalWorkflow for self-approval workflow', async () => { + renderEditPage(); + await waitForBatchedUpdatesWithAct(); + + expect(setApprovalWorkflow).toHaveBeenCalled(); + const mockCalls = (setApprovalWorkflow as jest.Mock).mock.calls; + const firstCall = mockCalls.at(0); + const callArg = firstCall?.at(0); + const availableMembers: Member[] = callArg?.availableMembers ?? []; + const emails = availableMembers.map((m) => m.email); + const uniqueEmails = [...new Set(emails)]; + + expect(emails).toHaveLength(uniqueEmails.length); + expect(emails).toContain(ALICE_EMAIL); + }); +}); diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index f2b665593545..8c32b0cc0a30 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -6,6 +6,7 @@ import { convertPolicyEmployeesToApprovalWorkflows, getApprovalLimitDescription, getOpenConnectedToPolicyBusinessBankAccounts, + mergeWorkflowMembersWithAvailableMembers, updateWorkflowDataOnApproverRemoval, } from '@src/libs/WorkflowUtils'; import type {Policy} from '@src/types/onyx'; @@ -543,6 +544,44 @@ describe('WorkflowUtils', () => { }); }); + describe('mergeWorkflowMembersWithAvailableMembers', () => { + it('Should deduplicate members when workflow members overlap with available members', () => { + const workflowMembers = [buildMember(1)]; + const allAvailableMembers = [buildMember(1), buildMember(2), buildMember(3)]; + const result = mergeWorkflowMembersWithAvailableMembers(workflowMembers, allAvailableMembers); + + expect(result).toHaveLength(3); + expect(result.map((m) => m.email)).toEqual(['1@example.com', '2@example.com', '3@example.com']); + }); + + it('Should not duplicate when editing self-approval workflow (user A submits to user A)', () => { + const userA = buildMember(1); + const workflowMembers = [userA]; + const allAvailableMembers = [userA]; + const result = mergeWorkflowMembersWithAvailableMembers(workflowMembers, allAvailableMembers); + + expect(result).toHaveLength(1); + expect(result.at(0)?.email).toBe('1@example.com'); + }); + + it('Should preserve workflow member order and append additional members', () => { + const workflowMembers = [buildMember(2), buildMember(3)]; + const allAvailableMembers = [buildMember(1), buildMember(2), buildMember(3), buildMember(4)]; + const result = mergeWorkflowMembersWithAvailableMembers(workflowMembers, allAvailableMembers); + + expect(result.map((m) => m.email)).toEqual(['2@example.com', '3@example.com', '1@example.com', '4@example.com']); + }); + + it('Should return workflow members only when all available are already in workflow', () => { + const workflowMembers = [buildMember(1), buildMember(2)]; + const allAvailableMembers = [buildMember(1), buildMember(2)]; + const result = mergeWorkflowMembersWithAvailableMembers(workflowMembers, allAvailableMembers); + + expect(result).toHaveLength(2); + expect(result.map((m) => m.email)).toEqual(['1@example.com', '2@example.com']); + }); + }); + describe('convertApprovalWorkflowToPolicyEmployees', () => { it('Should return an updated employee list for a simple default workflow', () => { const approvalWorkflow: ApprovalWorkflow = {