Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/libs/WorkflowUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -582,5 +591,6 @@ export {
getEligibleExistingBusinessBankAccounts,
getOpenConnectedToPolicyBusinessBankAccounts,
INITIAL_APPROVAL_WORKFLOW,
mergeWorkflowMembersWithAvailableMembers,
updateWorkflowDataOnApproverRemoval,
};
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
134 changes: 134 additions & 0 deletions tests/ui/WorkspaceWorkflowsApprovalsEditPageTest.tsx
Original file line number Diff line number Diff line change
@@ -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(
<ComposeProviders components={[OnyxListItemProvider, LocaleContextProvider]}>
<WorkspaceWorkflowsApprovalsEditPage
// @ts-expect-error - route type from navigator
route={mockRoute}
/>
</ComposeProviders>,
);

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<unknown[], [{availableMembers: Member[]}]>).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);
});
});
39 changes: 39 additions & 0 deletions tests/unit/WorkflowUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
convertPolicyEmployeesToApprovalWorkflows,
getApprovalLimitDescription,
getOpenConnectedToPolicyBusinessBankAccounts,
mergeWorkflowMembersWithAvailableMembers,
updateWorkflowDataOnApproverRemoval,
} from '@src/libs/WorkflowUtils';
import type {Policy} from '@src/types/onyx';
Expand Down Expand Up @@ -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 = {
Expand Down
Loading