diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 07f525b403fb..a6109820873e 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -10,6 +10,7 @@ import type ApprovalWorkflow from '@src/types/onyx/ApprovalWorkflow'; 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 PolicyEmployee from '@src/types/onyx/PolicyEmployee'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; import {isBankAccountPartiallySetup} from './BankAccountUtils'; import {convertToDisplayString} from './CurrencyUtils'; @@ -80,6 +81,16 @@ function calculateApprovers({employees, firstEmail, personalDetailsByEmail}: Get return approvers; } +/** Build a Member from a policy employee using personal details for avatar/displayName */ +function buildMemberFromEmployee(employee: PolicyEmployee, personalDetailsByEmail: PersonalDetailsList, email: string): Member { + return { + email, + avatar: personalDetailsByEmail[email]?.avatar, + displayName: personalDetailsByEmail[email]?.displayName ?? email, + pendingFields: employee.pendingFields, + }; +} + type PolicyConversionParams = { /** Policy data containing employees and approver information */ policy: OnyxEntry; @@ -114,20 +125,23 @@ function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, fir // Keep track of used approver emails to display hints in the UI const usedApproverEmails = new Set(); const personalDetailsByEmail = lodashMapKeys(personalDetails, (value, key) => value?.login ?? key); + const availableMembers: Member[] = []; - // Add each employee to the appropriate workflow for (const employee of Object.values(employees)) { const {email, submitsTo, pendingAction} = employee; - if (!email || !submitsTo || !employees[submitsTo] || employee.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { + if (!email) { continue; } - const member: Member = { - email, - avatar: personalDetailsByEmail[email]?.avatar, - displayName: personalDetailsByEmail[email]?.displayName ?? email, - pendingFields: employee.pendingFields, - }; + const member = buildMemberFromEmployee(employee, personalDetailsByEmail, email); + + if (pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { + availableMembers.push(member); + } + + if (!submitsTo || !employees[submitsTo]) { + continue; + } if (!approvalWorkflows[submitsTo]) { const approvers = calculateApprovers({employees, firstEmail: submitsTo, personalDetailsByEmail}); @@ -182,8 +196,10 @@ function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, fir }); } - const availableMembers = - policy?.approvalMode === CONST.POLICY.APPROVAL_MODE.BASIC ? sortedApprovalWorkflows?.flatMap((workflow) => workflow.members) : (sortedApprovalWorkflows.at(0)?.members ?? []); + // availableMembers built in loop above: all employees with email, excluding pending delete. + // Includes members with orphaned submitsTo/forwardsTo so admins can fix chains from Expenses From picker. + // See https://github.com/Expensify/Expensify/issues/598876 + availableMembers.sort((a, b) => localeCompare(a.displayName ?? a.email, b.displayName ?? b.email)); return {approvalWorkflows: sortedApprovalWorkflows, usedApproverEmails: [...usedApproverEmails], availableMembers}; } diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index faa33b3d9650..f2b665593545 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -469,6 +469,78 @@ describe('WorkflowUtils', () => { } expect(approvalWorkflows).toEqual([defaultWorkflow, secondWorkflow]); }); + + it('Should include all workspace members in availableMembers', () => { + // Simulates ADVANCED approval mode: Alex and Hannah submit to Carolyn (custom workflow), + // others submit to Hannah (default). Previously availableMembers only had default workflow + // members, excluding Alex and Hannah. + const employees: PolicyEmployeeList = { + 'alex@htc.us': { + email: 'alex@htc.us', + submitsTo: 'carolyn@htc.us', + }, + 'hannahw@htc.us': { + email: 'hannahw@htc.us', + submitsTo: 'carolyn@htc.us', + }, + 'carolyn@htc.us': { + email: 'carolyn@htc.us', + submitsTo: 'carolyn@htc.us', + }, + 'gio@htc.us': { + email: 'gio@htc.us', + submitsTo: 'hannahw@htc.us', + }, + }; + const policy = createMockPolicy(employees, 'hannahw@htc.us'); + (policy as Policy).approvalMode = CONST.POLICY.APPROVAL_MODE.ADVANCED; + const personalDetailsForTest: PersonalDetailsList = { + 'alex@htc.us': {accountID: 1, login: 'alex@htc.us', displayName: 'Alex Walker'}, + 'hannahw@htc.us': {accountID: 2, login: 'hannahw@htc.us', displayName: 'Hannah Walker'}, + 'carolyn@htc.us': {accountID: 3, login: 'carolyn@htc.us', displayName: 'Carolyn Smith'}, + 'gio@htc.us': {accountID: 4, login: 'gio@htc.us', displayName: 'Gio'}, + }; + + const {availableMembers} = convertPolicyEmployeesToApprovalWorkflows({ + policy, + personalDetails: personalDetailsForTest, + localeCompare, + }); + + const memberEmails = availableMembers.map((m) => m.email).sort(); + expect(memberEmails).toEqual(['alex@htc.us', 'carolyn@htc.us', 'gio@htc.us', 'hannahw@htc.us']); + }); + + it('Should include members with orphaned submitsTo in availableMembers', () => { + // Member with submitsTo pointing to non-member (not in employeeList) won't appear in any + // workflow, but should still appear in availableMembers so admins can fix the chain. + const employees: PolicyEmployeeList = { + 'alice@example.com': { + email: 'alice@example.com', + submitsTo: 'nonexistent@example.com', + }, + 'bob@example.com': { + email: 'bob@example.com', + submitsTo: 'bob@example.com', + }, + }; + const policy = createMockPolicy(employees, 'bob@example.com'); + const personalDetailsForTest: PersonalDetailsList = { + 'alice@example.com': {accountID: 1, login: 'alice@example.com', displayName: 'Alice'}, + 'bob@example.com': {accountID: 2, login: 'bob@example.com', displayName: 'Bob'}, + }; + + const {approvalWorkflows, availableMembers} = convertPolicyEmployeesToApprovalWorkflows({ + policy, + personalDetails: personalDetailsForTest, + localeCompare, + }); + + expect(approvalWorkflows).toHaveLength(1); + expect(approvalWorkflows.at(0)?.members).toHaveLength(1); + const memberEmails = availableMembers.map((m) => m.email).sort(); + expect(memberEmails).toEqual(['alice@example.com', 'bob@example.com']); + }); }); describe('convertApprovalWorkflowToPolicyEmployees', () => {