diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 61f784d8f2283..8647279b04697 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -14,7 +14,7 @@ import type PolicyEmployee from '@src/types/onyx/PolicyEmployee'; import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; import {isBankAccountPartiallySetup} from './BankAccountUtils'; import {convertToDisplayString} from './CurrencyUtils'; -import {getDefaultApprover} from './PolicyUtils'; +import {getDefaultApprover, isExpensifyTeam} from './PolicyUtils'; const INITIAL_APPROVAL_WORKFLOW: ApprovalWorkflowOnyx = { members: [], @@ -103,6 +103,9 @@ type PolicyConversionParams = { /** Locale comparison function */ localeCompare: LocaleContextProps['localeCompare']; + + /** Current user's login email, used to determine if Expensify team members should be shown */ + currentUserLogin?: string; }; type PolicyConversionResult = { @@ -116,11 +119,35 @@ type PolicyConversionResult = { usedApproverEmails: string[]; }; +/** + * Find the first non-Expensify team member in the approval chain. + * Used to skip internal Expensify approvers when displaying workflows to customers. + * Returns undefined if no non-Expensify approver is found in the chain. + */ +function findFirstNonExpensifyApprover(employees: PolicyEmployeeList, startEmail: string): string | undefined { + let email: string | undefined = startEmail; + const visited = new Set(); + + while (email && !visited.has(email)) { + if (!isExpensifyTeam(email)) { + return email; + } + visited.add(email); + email = employees[email]?.forwardsTo; + } + + return undefined; +} + /** Convert a list of policy employees to a list of approval workflows */ -function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, firstApprover, localeCompare}: PolicyConversionParams): PolicyConversionResult { +function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, firstApprover, localeCompare, currentUserLogin}: PolicyConversionParams): PolicyConversionResult { const employees = policy?.employeeList ?? {}; const defaultApprover = getDefaultApprover(policy); const approvalWorkflows: Record = {}; + const policyOwner = policy?.owner; + + // Determine if we should filter out Expensify team members (only for non-Expensify customers) + const shouldFilterOutExpensifyTeam = !!policyOwner && !!currentUserLogin && !isExpensifyTeam(policyOwner) && !isExpensifyTeam(currentUserLogin); // Keep track of used approver emails to display hints in the UI const usedApproverEmails = new Set(); @@ -133,6 +160,11 @@ function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, fir continue; } + // Filter out Expensify team members from appearing as workflow members + if (shouldFilterOutExpensifyTeam && isExpensifyTeam(email)) { + continue; + } + const member = buildMemberFromEmployee(employee, personalDetailsByEmail, email); if (pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { @@ -143,9 +175,19 @@ function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, fir continue; } - if (!approvalWorkflows[submitsTo]) { - const approvers = calculateApprovers({employees, firstEmail: submitsTo, personalDetailsByEmail}); - if (submitsTo !== firstApprover) { + // If submitsTo is an Expensify team member, find the first non-Expensify approver in the chain + const effectiveSubmitsTo = shouldFilterOutExpensifyTeam ? (findFirstNonExpensifyApprover(employees, submitsTo) ?? submitsTo) : submitsTo; + + if (!employees[effectiveSubmitsTo]) { + continue; + } + + if (!approvalWorkflows[effectiveSubmitsTo]) { + let approvers = calculateApprovers({employees, firstEmail: effectiveSubmitsTo, personalDetailsByEmail}); + if (shouldFilterOutExpensifyTeam) { + approvers = approvers.filter((approver) => !isExpensifyTeam(approver.email)); + } + if (effectiveSubmitsTo !== firstApprover) { for (const approver of approvers) { usedApproverEmails.add(approver.email); } @@ -156,20 +198,20 @@ function convertPolicyEmployeesToApprovalWorkflows({policy, personalDetails, fir // should not affect the workflow's display state const workflowPendingAction = pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? pendingAction : undefined; - approvalWorkflows[submitsTo] = { + approvalWorkflows[effectiveSubmitsTo] = { members: [], approvers, - isDefault: defaultApprover === submitsTo, + isDefault: defaultApprover === effectiveSubmitsTo, pendingAction: workflowPendingAction, }; } - approvalWorkflows[submitsTo].members.push(member); + approvalWorkflows[effectiveSubmitsTo].members.push(member); // Only propagate ADD/UPDATE pending actions to the workflow, not DELETE // When a member is being deleted from the workspace, their DELETE pending action // should not affect the workflow's display state (e.g., strikethrough styling) if (pendingAction && pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { - approvalWorkflows[submitsTo].pendingAction = pendingAction; + approvalWorkflows[effectiveSubmitsTo].pendingAction = pendingAction; } } diff --git a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx index 2453e6a2ad552..f76fbde08d948 100644 --- a/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx +++ b/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx @@ -119,6 +119,7 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM policy, personalDetails: personalDetails ?? {}, localeCompare, + currentUserLogin: currentUserPersonalDetails?.login, }); useEffect(() => { diff --git a/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx b/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx index e9ffc40bf3ea9..bb22ee209ee8a 100644 --- a/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx +++ b/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx @@ -17,6 +17,7 @@ import Section from '@components/Section'; import Text from '@components/Text'; import useCardFeeds from '@hooks/useCardFeeds'; import useConfirmModal from '@hooks/useConfirmModal'; +import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import {useMemoizedLazyExpensifyIcons, useMemoizedLazyIllustrations} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; @@ -94,10 +95,12 @@ function WorkspaceWorkflowsPage({policy, route}: WorkspaceWorkflowsPageProps) { const isSmartLimitEnabled = isSmartLimitEnabledUtil(workspaceCards); const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); const [reimbursementAccount] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT); + const currentUserPersonalDetails = useCurrentUserPersonalDetails(); const {approvalWorkflows, availableMembers, usedApproverEmails} = convertPolicyEmployeesToApprovalWorkflows({ policy, personalDetails: personalDetails ?? {}, localeCompare, + currentUserLogin: currentUserPersonalDetails?.login, }); const hasValidExistingAccounts = getEligibleExistingBusinessBankAccounts(bankAccountList, policy?.outputCurrency, true).length > 0; diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx index 78a0c2a4cce13..aa5b23e090b0a 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx @@ -10,6 +10,7 @@ import {ModalActions} from '@components/Modal/Global/ModalContext'; import ScreenWrapper from '@components/ScreenWrapper'; import useBottomSafeSafeAreaPaddingStyle from '@hooks/useBottomSafeSafeAreaPaddingStyle'; import useConfirmModal from '@hooks/useConfirmModal'; +import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -37,6 +38,7 @@ function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true const {translate, localeCompare} = useLocalize(); const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); const [approvalWorkflow] = useOnyx(ONYXKEYS.APPROVAL_WORKFLOW); + const currentUserPersonalDetails = useCurrentUserPersonalDetails(); const [initialApprovalWorkflow, setInitialApprovalWorkflow] = useState(); const formRef = useRef(null); const {showConfirmModal} = useConfirmModal(); @@ -87,6 +89,7 @@ function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true personalDetails, firstApprover, localeCompare, + currentUserLogin: currentUserPersonalDetails?.login, }); return { diff --git a/tests/unit/WorkflowUtilsTest.ts b/tests/unit/WorkflowUtilsTest.ts index 8c32b0cc0a307..a4da85de76e07 100644 --- a/tests/unit/WorkflowUtilsTest.ts +++ b/tests/unit/WorkflowUtilsTest.ts @@ -542,6 +542,191 @@ describe('WorkflowUtils', () => { const memberEmails = availableMembers.map((m) => m.email).sort(); expect(memberEmails).toEqual(['alice@example.com', 'bob@example.com']); }); + + it('Should filter out Expensify team members for non-Expensify customers', () => { + const employees: PolicyEmployeeList = { + 'alice@example.com': { + email: 'alice@example.com', + submitsTo: 'alice@example.com', + }, + 'guide@expensify.com': { + email: 'guide@expensify.com', + submitsTo: 'alice@example.com', + }, + 'concierge@team.expensify.com': { + email: 'concierge@team.expensify.com', + submitsTo: 'alice@example.com', + }, + }; + const policy: Partial = { + ...createMockPolicy(employees, 'alice@example.com'), + owner: 'alice@example.com', + }; + const personalDetailsForTest: PersonalDetailsList = { + 'alice@example.com': {accountID: 1, login: 'alice@example.com', displayName: 'Alice'}, + 'guide@expensify.com': {accountID: 2, login: 'guide@expensify.com', displayName: 'Guide'}, + 'concierge@team.expensify.com': {accountID: 3, login: 'concierge@team.expensify.com', displayName: 'Concierge'}, + }; + + const {approvalWorkflows, availableMembers} = convertPolicyEmployeesToApprovalWorkflows({ + policy: policy as Policy, + personalDetails: personalDetailsForTest, + localeCompare, + currentUserLogin: 'alice@example.com', + }); + + const allEmails = availableMembers.map((m) => m.email); + expect(allEmails).not.toContain('guide@expensify.com'); + expect(allEmails).not.toContain('concierge@team.expensify.com'); + expect(allEmails).toContain('alice@example.com'); + + const workflowMemberEmails = approvalWorkflows.flatMap((w) => w.members.map((m) => m.email)); + expect(workflowMemberEmails).not.toContain('guide@expensify.com'); + expect(workflowMemberEmails).not.toContain('concierge@team.expensify.com'); + }); + + it('Should not filter out Expensify team members when the policy owner is Expensify team', () => { + const employees: PolicyEmployeeList = { + 'admin@expensify.com': { + email: 'admin@expensify.com', + submitsTo: 'admin@expensify.com', + }, + 'guide@expensify.com': { + email: 'guide@expensify.com', + submitsTo: 'admin@expensify.com', + }, + }; + const policy: Partial = { + ...createMockPolicy(employees, 'admin@expensify.com'), + owner: 'admin@expensify.com', + }; + const personalDetailsForTest: PersonalDetailsList = { + 'admin@expensify.com': {accountID: 1, login: 'admin@expensify.com', displayName: 'Admin'}, + 'guide@expensify.com': {accountID: 2, login: 'guide@expensify.com', displayName: 'Guide'}, + }; + + const {availableMembers} = convertPolicyEmployeesToApprovalWorkflows({ + policy: policy as Policy, + personalDetails: personalDetailsForTest, + localeCompare, + currentUserLogin: 'admin@expensify.com', + }); + + const allEmails = availableMembers.map((m) => m.email); + expect(allEmails).toContain('guide@expensify.com'); + expect(allEmails).toContain('admin@expensify.com'); + }); + + it('Should not filter out Expensify team members when currentUserLogin is not provided', () => { + const employees: PolicyEmployeeList = { + 'alice@example.com': { + email: 'alice@example.com', + submitsTo: 'alice@example.com', + }, + 'guide@expensify.com': { + email: 'guide@expensify.com', + submitsTo: 'alice@example.com', + }, + }; + const policy: Partial = { + ...createMockPolicy(employees, 'alice@example.com'), + owner: 'alice@example.com', + }; + const personalDetailsForTest: PersonalDetailsList = { + 'alice@example.com': {accountID: 1, login: 'alice@example.com', displayName: 'Alice'}, + 'guide@expensify.com': {accountID: 2, login: 'guide@expensify.com', displayName: 'Guide'}, + }; + + const {availableMembers} = convertPolicyEmployeesToApprovalWorkflows({ + policy: policy as Policy, + personalDetails: personalDetailsForTest, + localeCompare, + }); + + const allEmails = availableMembers.map((m) => m.email); + expect(allEmails).toContain('guide@expensify.com'); + }); + + it('Should redirect submitsTo through Expensify team members to the first non-Expensify approver', () => { + const employees: PolicyEmployeeList = { + 'alice@example.com': { + email: 'alice@example.com', + submitsTo: 'guide@expensify.com', + }, + 'guide@expensify.com': { + email: 'guide@expensify.com', + forwardsTo: 'bob@example.com', + submitsTo: 'bob@example.com', + }, + 'bob@example.com': { + email: 'bob@example.com', + submitsTo: 'bob@example.com', + }, + }; + const policy: Partial = { + ...createMockPolicy(employees, 'bob@example.com'), + owner: 'alice@example.com', + }; + const personalDetailsForTest: PersonalDetailsList = { + 'alice@example.com': {accountID: 1, login: 'alice@example.com', displayName: 'Alice'}, + 'guide@expensify.com': {accountID: 2, login: 'guide@expensify.com', displayName: 'Guide'}, + 'bob@example.com': {accountID: 3, login: 'bob@example.com', displayName: 'Bob'}, + }; + + const {approvalWorkflows} = convertPolicyEmployeesToApprovalWorkflows({ + policy: policy as Policy, + personalDetails: personalDetailsForTest, + localeCompare, + currentUserLogin: 'alice@example.com', + }); + + // Alice should submit to Bob (skipping the Expensify guide) + const aliceWorkflow = approvalWorkflows.find((w) => w.members.some((m) => m.email === 'alice@example.com')); + expect(aliceWorkflow?.approvers.at(0)?.email).toBe('bob@example.com'); + }); + + it('Should filter out Expensify team members from approver chains', () => { + const employees: PolicyEmployeeList = { + 'alice@example.com': { + email: 'alice@example.com', + submitsTo: 'bob@example.com', + }, + 'bob@example.com': { + email: 'bob@example.com', + forwardsTo: 'guide@expensify.com', + submitsTo: 'bob@example.com', + }, + 'guide@expensify.com': { + email: 'guide@expensify.com', + forwardsTo: 'carol@example.com', + submitsTo: 'bob@example.com', + }, + 'carol@example.com': { + email: 'carol@example.com', + submitsTo: 'bob@example.com', + }, + }; + const policy: Partial = { + ...createMockPolicy(employees, 'bob@example.com'), + owner: 'alice@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'}, + 'guide@expensify.com': {accountID: 3, login: 'guide@expensify.com', displayName: 'Guide'}, + 'carol@example.com': {accountID: 4, login: 'carol@example.com', displayName: 'Carol'}, + }; + + const {approvalWorkflows} = convertPolicyEmployeesToApprovalWorkflows({ + policy: policy as Policy, + personalDetails: personalDetailsForTest, + localeCompare, + currentUserLogin: 'alice@example.com', + }); + + const approverEmails = approvalWorkflows.flatMap((w) => w.approvers.map((a) => a.email)); + expect(approverEmails).not.toContain('guide@expensify.com'); + }); }); describe('mergeWorkflowMembersWithAvailableMembers', () => {