diff --git a/src/languages/de.ts b/src/languages/de.ts index f81370936d01..fc16cd73b66b 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -3260,6 +3260,8 @@ ${amount} für ${merchant} – ${date}`, defaultWorkflowApproverNotWorkspaceMember: 'Diese genehmigende Person ist nicht mehr Mitglied des Arbeitsbereichs. Wählen Sie eine neue genehmigende Person.', overLimitApproverNotWorkspaceMember: 'Die zusätzliche genehmigende Person für Berichte über dem Limit ist nicht mehr Mitglied des Arbeitsbereichs. Wählen Sie eine neue zusätzliche genehmigende Person.', + overLimitApproverInWorkflow: + 'Die zusätzliche Genehmigungsperson für Berichte über dem Limit ist bereits eine Genehmigungsperson in diesem Workflow. Wählen Sie eine andere zusätzliche Genehmigungsperson.', approverCircularReference: (name1: string, name2: string) => `${name1} genehmigt bereits Berichte für ${name2}. Bitte wähle eine andere approvierende Person, um einen zirkulären Workflow zu vermeiden.`, emptyContent: { diff --git a/src/languages/el.ts b/src/languages/el.ts index 576923fa7a55..dd3fb9a2ebb2 100644 --- a/src/languages/el.ts +++ b/src/languages/el.ts @@ -3319,6 +3319,7 @@ ${amount} για ${merchant} - ${date}`, approverNotWorkspaceMember: 'Αυτός ο εγκρίνων δεν είναι πλέον μέλος του χώρου εργασίας. Επιλέξτε νέο εγκρίνοντα ή διαγράψτε αυτή τη ροή εργασίας.', defaultWorkflowApproverNotWorkspaceMember: 'Αυτός ο εγκρίνων δεν είναι πλέον μέλος του χώρου εργασίας. Επιλέξτε νέο εγκρίνοντα.', overLimitApproverNotWorkspaceMember: 'Ο πρόσθετος εγκρίνων για αναφορές πάνω από το όριο δεν είναι πλέον μέλος του χώρου εργασίας. Επιλέξτε νέο πρόσθετο εγκρίνοντα.', + overLimitApproverInWorkflow: 'Ο επιπλέον εγκρίνων για αναφορές πάνω από το όριο είναι ήδη εγκρίνων σε αυτή τη ροή εργασίας. Επιλέξτε έναν διαφορετικό επιπλέον εγκρίνοντα.', approverCircularReference: (name1: string, name2: string) => `Ο/Η ${name1} ήδη εγκρίνει αναφορές προς τον/την ${name2}. Παρακαλούμε επιλέξτε διαφορετικό εγκρίνων για να αποφύγετε έναν κυκλικό κύκλο έγκρισης.`, emptyContent: { diff --git a/src/languages/en.ts b/src/languages/en.ts index c8bdaa542c82..5f981c4f0b97 100644 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -3413,6 +3413,7 @@ const translations = { approverNotWorkspaceMember: 'This approver is no longer a workspace member. Choose a new approver or delete this workflow.', defaultWorkflowApproverNotWorkspaceMember: 'This approver is no longer a workspace member. Choose a new approver.', overLimitApproverNotWorkspaceMember: 'The additional approver for reports over the limit is no longer a workspace member. Choose a new additional approver.', + overLimitApproverInWorkflow: 'The additional approver for reports over the limit is already an approver in this workflow. Choose a different additional approver.', approverCircularReference: (name1: string, name2: string) => `${name1} already approves reports to ${name2}. Please choose a different approver to avoid a circular workflow.`, emptyContent: { diff --git a/src/languages/es.ts b/src/languages/es.ts index 43a401c5de6c..538d43284fbf 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -3156,6 +3156,7 @@ ${amount} para ${merchant} - ${date}`, approverNotWorkspaceMember: 'Este aprobador ya no es miembro del espacio de trabajo. Elige un nuevo aprobador o elimina este flujo de trabajo.', defaultWorkflowApproverNotWorkspaceMember: 'Este aprobador ya no es miembro del espacio de trabajo. Elige un nuevo aprobador.', overLimitApproverNotWorkspaceMember: 'El aprobador adicional para los informes que superan el límite ya no es miembro del espacio de trabajo. Elige un nuevo aprobador adicional.', + overLimitApproverInWorkflow: 'El aprobador adicional para los informes que superan el límite ya es un aprobador en este flujo de trabajo. Elige un aprobador adicional diferente.', approverCircularReference: (name1, name2) => `${name1} ya aprueba informes a ${name2}. Por favor, elige un aprobador diferente para evitar un flujo de trabajo circular.`, emptyContent: { diff --git a/src/languages/fr.ts b/src/languages/fr.ts index 16025f958d57..7d4ba57750d8 100644 --- a/src/languages/fr.ts +++ b/src/languages/fr.ts @@ -3265,6 +3265,8 @@ ${amount} pour ${merchant} - ${date}`, defaultWorkflowApproverNotWorkspaceMember: 'Cet approbateur n’est plus membre de l’espace de travail. Choisissez un nouvel approbateur.', overLimitApproverNotWorkspaceMember: 'L’approbateur supplémentaire pour les notes de frais au-delà de la limite n’est plus membre de l’espace de travail. Choisissez un nouvel approbateur supplémentaire.', + overLimitApproverInWorkflow: + 'L’approbateur supplémentaire pour les notes de frais dépassant la limite est déjà un approbateur dans ce flux de travail. Choisissez un autre approbateur supplémentaire.', approverCircularReference: (name1: string, name2: string) => `${name1} approuve déjà les notes de frais de ${name2}. Veuillez choisir un autre approbateur pour éviter un circuit d'approbation circulaire.`, emptyContent: { diff --git a/src/languages/it.ts b/src/languages/it.ts index c69b696bf807..bd3f13b4d885 100644 --- a/src/languages/it.ts +++ b/src/languages/it.ts @@ -3245,6 +3245,7 @@ ${amount} per ${merchant} - ${date}`, approverNotWorkspaceMember: 'Questo approvatore non è più un membro dello spazio di lavoro. Scegli un nuovo approvatore oppure elimina questo workflow.', defaultWorkflowApproverNotWorkspaceMember: 'Questo approvatore non è più un membro dello spazio di lavoro. Scegli un nuovo approvatore.', overLimitApproverNotWorkspaceMember: 'L’approvatore aggiuntivo per i report oltre il limite non è più un membro dello spazio di lavoro. Scegli un nuovo approvatore aggiuntivo.', + overLimitApproverInWorkflow: 'L’approvatore aggiuntivo per i report oltre il limite è già un approvatore in questo workflow. Scegli un altro approvatore aggiuntivo.', approverCircularReference: (name1: string, name2: string) => `${name1} approva già i report per ${name2}. Scegli un altro approvatore per evitare un flusso di lavoro circolare.`, emptyContent: { diff --git a/src/languages/ja.ts b/src/languages/ja.ts index be5282d2266d..e70804a17316 100644 --- a/src/languages/ja.ts +++ b/src/languages/ja.ts @@ -3220,6 +3220,7 @@ ${date} の ${merchant} への ${amount}`, approverNotWorkspaceMember: 'この承認者はワークスペースのメンバーではなくなりました。新しい承認者を選択するか、このワークフローを削除してください。', defaultWorkflowApproverNotWorkspaceMember: 'この承認者はワークスペースのメンバーではなくなりました。新しい承認者を選択してください。', overLimitApproverNotWorkspaceMember: '上限を超えるレポートの追加承認者は、ワークスペースのメンバーではなくなりました。新しい追加承認者を選択してください。', + overLimitApproverInWorkflow: '上限を超えるレポートの追加承認者として選択されたユーザーは、このワークフロー内ですでに承認者になっています。別の追加承認者を選択してください。', approverCircularReference: (name1: string, name2: string) => `${name1} はすでに ${name2} にレポートを承認しています。ワークフローが循環しないよう、別の承認者を選択してください。`, emptyContent: { diff --git a/src/languages/nl.ts b/src/languages/nl.ts index 60520425de68..277314f40081 100644 --- a/src/languages/nl.ts +++ b/src/languages/nl.ts @@ -3248,6 +3248,7 @@ ${amount} voor ${merchant} - ${date}`, approverNotWorkspaceMember: 'Deze fiatteur is geen lid meer van de workspace. Kies een nieuwe fiatteur of verwijder deze workflow.', defaultWorkflowApproverNotWorkspaceMember: 'Deze fiatteur is geen lid meer van de workspace. Kies een nieuwe fiatteur.', overLimitApproverNotWorkspaceMember: 'De extra fiatteur voor rapporten boven de limiet is geen lid meer van de workspace. Kies een nieuwe extra fiatteur.', + overLimitApproverInWorkflow: 'De extra fiatteur voor rapporten boven de limiet is al een fiatteur in deze workflow. Kies een andere extra fiatteur.', approverCircularReference: (name1: string, name2: string) => `${name1} keurt al rapporten goed voor ${name2}. Kies een andere fiatteur om een cirkelstroom te voorkomen.`, emptyContent: { diff --git a/src/languages/pl.ts b/src/languages/pl.ts index 3ecfb0add1a4..be659ecab1cc 100644 --- a/src/languages/pl.ts +++ b/src/languages/pl.ts @@ -3281,6 +3281,7 @@ ${amount} dla ${merchant} - ${date}`, approverNotWorkspaceMember: 'Ten zatwierdzający nie jest już członkiem tego workspace’u. Wybierz nowego zatwierdzającego albo usuń ten workflow.', defaultWorkflowApproverNotWorkspaceMember: 'Ten zatwierdzający nie jest już członkiem tego workspace’u. Wybierz nowego zatwierdzającego.', overLimitApproverNotWorkspaceMember: 'Dodatkowy zatwierdzający dla raportów powyżej limitu nie jest już członkiem tego workspace’u. Wybierz nowego dodatkowego zatwierdzającego.', + overLimitApproverInWorkflow: 'Dodatkowy akceptujący dla raportów powyżej limitu jest już akceptującym w tym przepływie pracy. Wybierz innego dodatkowego akceptującego.', approverCircularReference: (name1: string, name2: string) => `${name1} już zatwierdza raporty dla ${name2}. Wybierz innego zatwierdzającego, aby uniknąć zapętlenia przepływu pracy.`, emptyContent: { diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts index fb2be3403595..24df5eac03ec 100644 --- a/src/languages/pt-BR.ts +++ b/src/languages/pt-BR.ts @@ -3238,6 +3238,7 @@ ${amount} para ${merchant} - ${date}`, approverNotWorkspaceMember: 'Esse aprovador não é mais membro do workspace. Escolha um novo aprovador ou exclua este fluxo de trabalho.', defaultWorkflowApproverNotWorkspaceMember: 'Esse aprovador não é mais membro do workspace. Escolha um novo aprovador.', overLimitApproverNotWorkspaceMember: 'O aprovador adicional para relatórios acima do limite não é mais membro do workspace. Escolha um novo aprovador adicional.', + overLimitApproverInWorkflow: 'O aprovador adicional para relatórios acima do limite já é um aprovador neste fluxo de trabalho. Escolha um aprovador adicional diferente.', approverCircularReference: (name1: string, name2: string) => `${name1} já aprova relatórios para ${name2}. Escolha um aprovador diferente para evitar um fluxo de aprovação circular.`, emptyContent: { diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts index 40a0d421f177..28c8a2d9b982 100644 --- a/src/languages/zh-hans.ts +++ b/src/languages/zh-hans.ts @@ -3145,6 +3145,7 @@ ${amount}，商户：${merchant} - 日期：${date}`, approverNotWorkspaceMember: '此审批人已不再是工作区成员。请选择新的审批人或删除此工作流程。', defaultWorkflowApproverNotWorkspaceMember: '此审批人已不再是工作区成员。请选择新的审批人。', overLimitApproverNotWorkspaceMember: '超出限额报告的额外审批人已不再是工作区成员。请选择新的额外审批人。', + overLimitApproverInWorkflow: '此限额以上报表的额外审批人已是此工作流中的审批人。请选择其他额外审批人。', approverCircularReference: (name1: string, name2: string) => `${name1} 已经将报表提交给 ${name2} 审批。请选择其他审批人以避免形成循环审批流程。`, emptyContent: { title: '没有可显示的成员', diff --git a/src/libs/actions/Workflow.ts b/src/libs/actions/Workflow.ts index 10bf8e37c5b8..ed848d2621b5 100644 --- a/src/libs/actions/Workflow.ts +++ b/src/libs/actions/Workflow.ts @@ -754,9 +754,10 @@ type ApprovalWorkflowOnyxValidated = Omit & { /** * Validates the approval workflow and sets the errors on the approval workflow * @param approvalWorkflow the approval workflow to validate + * @param isMultipleApproversBetaEnabled whether the `MULTIPLE_APPROVERS` beta is enabled, which saves the workflow as approval workflow rules * @returns true if the approval workflow is valid, false otherwise */ -function validateApprovalWorkflow(approvalWorkflow: ApprovalWorkflowOnyx): approvalWorkflow is ApprovalWorkflowOnyxValidated { +function validateApprovalWorkflow(approvalWorkflow: ApprovalWorkflowOnyx, isMultipleApproversBetaEnabled: boolean): approvalWorkflow is ApprovalWorkflowOnyxValidated { const errors: Record = {}; for (const [approverIndex, approver] of approvalWorkflow.approvers.entries()) { @@ -773,6 +774,15 @@ function validateApprovalWorkflow(approvalWorkflow: ApprovalWorkflowOnyx): appro errors[`approver-${approverIndex}`] = nonMemberApproverError; } + // Someone who already approves in this workflow approves every report anyway, so the limit can't add them + if ( + isMultipleApproversBetaEnabled && + approver?.overLimitForwardsTo && + approvalWorkflow.approvers.some((workflowApprover) => workflowApprover?.email === approver.overLimitForwardsTo) + ) { + errors[`approver-${approverIndex}`] = 'workflowsPage.overLimitApproverInWorkflow'; + } + // Validate that if overLimitForwardsTo is set, approvalLimit must also be set if (approver?.overLimitForwardsTo && (!approver?.approvalLimit || approver.approvalLimit <= 0)) { errors[`approver-${approverIndex}`] = 'workflowsApprovalLimitPage.enterAmountError'; diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx index c6923dc4b12e..bc8141d4420a 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx @@ -102,6 +102,12 @@ function WorkspaceWorkflowsApprovalsApproverPage({policy, personalDetails, isLoa if (isEmailAlreadyInApprovers && visibleSelectedApproverEmail !== email) { return null; } + + // Another step's additional approver can't also approve here, or that step's limit would add no one + const isOtherStepsAdditionalApprover = approversFromWorkflow?.some((approver, index) => approver?.overLimitForwardsTo === email && index !== approverIndex); + if (isMultipleApproversBetaEnabled && isOtherStepsAdditionalApprover && visibleSelectedApproverEmail !== email) { + return null; + } if (!isMultipleApproversBetaEnabled && !isDefault && approverIndex === 0 && defaultApprover === email) { return null; } diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsCreatePage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsCreatePage.tsx index a8a6ae89e05a..3ff2431d4d48 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsCreatePage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsCreatePage.tsx @@ -67,7 +67,7 @@ function WorkspaceWorkflowsApprovalsCreatePage({policy, isLoadingReportData = tr return; } - if (!validateApprovalWorkflow(approvalWorkflow)) { + if (!validateApprovalWorkflow(approvalWorkflow, isBetaEnabled(CONST.BETAS.MULTIPLE_APPROVERS))) { return; } diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx index 0086f7100a66..26124ed591d4 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsEditPage.tsx @@ -118,7 +118,7 @@ function WorkspaceWorkflowsApprovalsEditPage({policy, isLoadingReportData = true return; } - if (!validateApprovalWorkflow(approvalWorkflow)) { + if (!validateApprovalWorkflow(approvalWorkflow, isBetaEnabled(CONST.BETAS.MULTIPLE_APPROVERS))) { return; } diff --git a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsOverLimitApproverPage.tsx b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsOverLimitApproverPage.tsx index 30f3067b78dc..b9406ca8ce88 100644 --- a/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsOverLimitApproverPage.tsx +++ b/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsOverLimitApproverPage.tsx @@ -6,6 +6,7 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails' import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import usePermissions from '@hooks/usePermissions'; import {usePersonalDetailsByLogins} from '@hooks/usePersonalDetailByLogin'; import usePersonalDetailsByEmail from '@hooks/usePersonalDetailsByEmail'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -42,6 +43,8 @@ function WorkspaceWorkflowsApprovalsOverLimitApproverPage({policy, personalDetai const personalDetailsByEmail = usePersonalDetailsByEmail(); const employeePersonalDetails = usePersonalDetailsByLogins(Object.keys(policy?.employeeList ?? {})); const {login: currentUserLogin = ''} = useCurrentUserPersonalDetails(); + const {isBetaEnabled} = usePermissions(); + const isMultipleApproversBetaEnabled = isBetaEnabled(CONST.BETAS.MULTIPLE_APPROVERS); const expensifyIcons = useMemoizedLazyExpensifyIcons(['FallbackAvatar']); const policyID = route.params.policyID; @@ -51,6 +54,7 @@ function WorkspaceWorkflowsApprovalsOverLimitApproverPage({policy, personalDetai const employeeList = policy?.employeeList; const isDefault = approvalWorkflow?.isDefault; const membersEmail = approvalWorkflow?.members.map((member) => member.email); + const workflowApproverEmails = new Set(approvalWorkflow?.approvers?.map((approver) => approver?.email)); const currentApproverEmail = currentApprover?.email; @@ -88,6 +92,11 @@ function WorkspaceWorkflowsApprovalsOverLimitApproverPage({policy, personalDetai return null; } + // Someone who already approves in this workflow approves every report anyway, so the limit can't add them + if (isMultipleApproversBetaEnabled && workflowApproverEmails.has(email)) { + return null; + } + const accountID = Number(policyMemberEmailsToAccountIDs[email] ?? ''); if (!accountID) { return null; diff --git a/tests/actions/WorkflowTest.ts b/tests/actions/WorkflowTest.ts index 7b3840c4ed93..32c9f138425d 100644 --- a/tests/actions/WorkflowTest.ts +++ b/tests/actions/WorkflowTest.ts @@ -204,7 +204,7 @@ describe('actions/Workflow', () => { await Onyx.set(ONYXKEYS.APPROVAL_WORKFLOW, approvalWorkflow); // When the admin tries to save it - const isValid = validateApprovalWorkflow(approvalWorkflow); + const isValid = validateApprovalWorkflow(approvalWorkflow, true); await waitForBatchedUpdates(); // Then the save is blocked and the approver shows the error offering to replace them or delete the workflow @@ -223,7 +223,7 @@ describe('actions/Workflow', () => { await Onyx.set(ONYXKEYS.APPROVAL_WORKFLOW, approvalWorkflow); // When the admin tries to save it - const isValid = validateApprovalWorkflow(approvalWorkflow); + const isValid = validateApprovalWorkflow(approvalWorkflow, true); await waitForBatchedUpdates(); // Then the save is blocked, and since the default workflow can't be deleted the error only asks for a new approver @@ -242,7 +242,7 @@ describe('actions/Workflow', () => { await Onyx.set(ONYXKEYS.APPROVAL_WORKFLOW, approvalWorkflow); // When the admin tries to save it - const isValid = validateApprovalWorkflow(approvalWorkflow); + const isValid = validateApprovalWorkflow(approvalWorkflow, true); await waitForBatchedUpdates(); // Then the save is blocked and the approver shows the error asking for a new additional approver @@ -250,6 +250,66 @@ describe('actions/Workflow', () => { const workflowState = await getOnyxValue(ONYXKEYS.APPROVAL_WORKFLOW); expect(workflowState?.errors?.['approver-0']).toBe('workflowsPage.overLimitApproverNotWorkspaceMember'); }); + + it.each([ + [ + 'earlier in the workflow', + [ + {email: employee1Email, displayName: employee1Email}, + {email: employee2Email, displayName: employee2Email, approvalLimit: 5000, overLimitForwardsTo: employee1Email}, + {email: employee3Email, displayName: employee3Email}, + ], + 1, + ], + [ + 'later in the workflow', + [ + {email: employee1Email, displayName: employee1Email, approvalLimit: 5000, overLimitForwardsTo: employee3Email}, + {email: employee2Email, displayName: employee2Email}, + {email: employee3Email, displayName: employee3Email}, + ], + 0, + ], + ])('should block saving a workflow whose additional approver for reports over the limit is already an approver %s', async (_description, approvers, limitedApproverIndex) => { + // Given a workflow saved as approval workflow rules, whose reports over an approver's limit would go to someone + // who already approves in it, so the limit adds no one + const approvalWorkflow: ApprovalWorkflowOnyx = { + ...INITIAL_APPROVAL_WORKFLOW, + members: [{email: employee4Email, displayName: employee4Email}], + approvers, + }; + await Onyx.set(ONYXKEYS.APPROVAL_WORKFLOW, approvalWorkflow); + + // When the admin tries to save it + const isValid = validateApprovalWorkflow(approvalWorkflow, true); + await waitForBatchedUpdates(); + + // Then the save is blocked and the limited approver shows the error asking for a different additional approver + expect(isValid).toBe(false); + const workflowState = await getOnyxValue(ONYXKEYS.APPROVAL_WORKFLOW); + expect(workflowState?.errors?.[`approver-${limitedApproverIndex}`]).toBe('workflowsPage.overLimitApproverInWorkflow'); + }); + + it('should allow saving a workflow whose additional approver for reports over the limit is not one of its approvers', async () => { + // Given a workflow saved as approval workflow rules, whose reports over the first approver's limit also go to + // someone outside it + const approvalWorkflow: ApprovalWorkflowOnyx = { + ...INITIAL_APPROVAL_WORKFLOW, + members: [{email: employee4Email, displayName: employee4Email}], + approvers: [ + {email: employee1Email, displayName: employee1Email, approvalLimit: 5000, overLimitForwardsTo: ownerEmail}, + {email: employee2Email, displayName: employee2Email}, + ], + }; + await Onyx.set(ONYXKEYS.APPROVAL_WORKFLOW, approvalWorkflow); + + // When the admin tries to save it + const isValid = validateApprovalWorkflow(approvalWorkflow, true); + await waitForBatchedUpdates(); + + // Then it saves, since the limit adds someone who wouldn't otherwise approve the report + expect(isValid).toBe(true); + }); }); describe('setApprovalWorkflowApprover', () => { diff --git a/tests/ui/WorkspaceWorkflowsApprovalsApproverPageTest.tsx b/tests/ui/WorkspaceWorkflowsApprovalsApproverPageTest.tsx index 4f4cf3419630..150a02b5ccdf 100644 --- a/tests/ui/WorkspaceWorkflowsApprovalsApproverPageTest.tsx +++ b/tests/ui/WorkspaceWorkflowsApprovalsApproverPageTest.tsx @@ -5,6 +5,7 @@ import ApproverSelectionList from '@components/ApproverSelectionList'; import ComposeProviders from '@components/ComposeProviders'; import {LocaleContextProvider} from '@components/LocaleContextProvider'; import OnyxListItemProvider from '@components/OnyxListItemProvider'; +import PersonalDetailsByLoginProvider from '@components/PersonalDetailsByLoginProvider'; import Navigation from '@libs/Navigation/Navigation'; import {convertPolicyEmployeesToApprovalWorkflows} from '@libs/WorkflowUtils'; @@ -117,7 +118,7 @@ async function editWorkflowWithRemovedApprover() { async function renderPage(approverIndex: number) { const result = render( - + { expect(getRows().some((row) => row.login === REMOVED_EMAIL)).toBe(false); }); + it("never offers another step's additional approver for reports over the limit, but still offers a step its own", async () => { + // Given the rules-based workflow Bob > Erin > an approver who left, where Bob's reports over his limit also go to the admin + const approvalWorkflow = await getOnyxValue(ONYXKEYS.APPROVAL_WORKFLOW); + await act(async () => { + await Onyx.set(ONYXKEYS.BETAS, [CONST.BETAS.MULTIPLE_APPROVERS]); + await Onyx.merge(ONYXKEYS.APPROVAL_WORKFLOW, { + approvers: approvalWorkflow?.approvers.map((approver, index) => (index === 0 && approver ? {...approver, approvalLimit: 5000, overLimitForwardsTo: ADMIN_EMAIL} : approver)), + }); + }); + await waitForBatchedUpdatesWithAct(); + + // When the admin opens Erin's slot + const {unmount} = await renderPage(1); + + // Then the admin can't be picked, since approving there would make Bob's limit add no one + expect(getRows().some((row) => row.login === ADMIN_EMAIL)).toBe(false); + unmount(); + + // When the admin opens Bob's own slot + await renderPage(0); + + // Then the admin can be picked, since replacing Bob clears his limit + expect(getRows().some((row) => row.login === ADMIN_EMAIL)).toBe(true); + }); + it('lists a flagged approver only once when they are back on the workspace', async () => { // Given the approver who left was invited back while the edited workflow still has them flagged await act(async () => { diff --git a/tests/ui/WorkspaceWorkflowsApprovalsOverLimitApproverPageTest.tsx b/tests/ui/WorkspaceWorkflowsApprovalsOverLimitApproverPageTest.tsx new file mode 100644 index 000000000000..6af46e5a2e90 --- /dev/null +++ b/tests/ui/WorkspaceWorkflowsApprovalsOverLimitApproverPageTest.tsx @@ -0,0 +1,150 @@ +import {act, render} from '@testing-library/react-native'; + +import type {SelectionListApprover} from '@components/ApproverSelectionList'; +import ApproverSelectionList from '@components/ApproverSelectionList'; +import ComposeProviders from '@components/ComposeProviders'; +import {LocaleContextProvider} from '@components/LocaleContextProvider'; +import OnyxListItemProvider from '@components/OnyxListItemProvider'; +import PersonalDetailsByLoginProvider from '@components/PersonalDetailsByLoginProvider'; + +import {INITIAL_APPROVAL_WORKFLOW} from '@libs/WorkflowUtils'; + +import WorkspaceWorkflowsApprovalsOverLimitApproverPage from '@pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsOverLimitApproverPage'; + +import {setApprovalWorkflow} from '@userActions/Workflow'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import SCREENS from '@src/SCREENS'; +import type {Policy} from '@src/types/onyx'; +import type {PersonalDetailsList} from '@src/types/onyx/PersonalDetails'; + +import type {PropsWithChildren} from 'react'; + +import React from 'react'; +import Onyx from 'react-native-onyx'; + +import createMock from '../utils/createMock'; +import {buildPersonalDetails} from '../utils/TestHelper'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +const POLICY_ID = 'workflow-over-limit-approver-test-policy'; +const ADMIN_EMAIL = 'admin@example.com'; +const ADMIN_ACCOUNT_ID = 1; +const BOB_EMAIL = 'bob@example.com'; +const BOB_ACCOUNT_ID = 2; +const ERIN_EMAIL = 'erin@example.com'; +const ERIN_ACCOUNT_ID = 3; +const DAVE_EMAIL = 'dave@example.com'; +const DAVE_ACCOUNT_ID = 4; +const FRANK_EMAIL = 'frank@example.com'; +const FRANK_ACCOUNT_ID = 5; + +// The tests read the rows the page hands the list, so the list itself renders nothing. +jest.mock('@components/ApproverSelectionList', () => jest.fn(() => null)); +jest.mock('@pages/workspace/AccessOrNotFoundWrapper', () => jest.fn(({children}: PropsWithChildren) => children)); + +jest.mock('@react-navigation/native', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const actualNav = jest.requireActual('@react-navigation/native'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return { + ...actualNav, + useNavigationState: () => [], + }; +}); + +jest.mock('@libs/Navigation/Navigation', () => ({ + goBack: jest.fn(), + navigate: jest.fn(), +})); + +function buildPolicy(): Policy { + return createMock({ + id: POLICY_ID, + name: 'Test Workspace', + type: CONST.POLICY.TYPE.CORPORATE, + role: CONST.POLICY.ROLE.ADMIN, + owner: ADMIN_EMAIL, + approver: ADMIN_EMAIL, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + areWorkflowsEnabled: true, + employeeList: { + [ADMIN_EMAIL]: {email: ADMIN_EMAIL, role: CONST.POLICY.ROLE.ADMIN, submitsTo: ADMIN_EMAIL}, + [BOB_EMAIL]: {email: BOB_EMAIL, role: CONST.POLICY.ROLE.USER, submitsTo: ADMIN_EMAIL}, + [ERIN_EMAIL]: {email: ERIN_EMAIL, role: CONST.POLICY.ROLE.USER, submitsTo: ADMIN_EMAIL}, + [DAVE_EMAIL]: {email: DAVE_EMAIL, role: CONST.POLICY.ROLE.USER, submitsTo: ADMIN_EMAIL}, + [FRANK_EMAIL]: {email: FRANK_EMAIL, role: CONST.POLICY.ROLE.USER, submitsTo: ADMIN_EMAIL}, + }, + }); +} + +function buildPersonalDetailsList(): PersonalDetailsList { + return { + [ADMIN_ACCOUNT_ID]: buildPersonalDetails(ADMIN_EMAIL, ADMIN_ACCOUNT_ID, 'admin'), + [BOB_ACCOUNT_ID]: buildPersonalDetails(BOB_EMAIL, BOB_ACCOUNT_ID, 'bob'), + [ERIN_ACCOUNT_ID]: buildPersonalDetails(ERIN_EMAIL, ERIN_ACCOUNT_ID, 'erin'), + [DAVE_ACCOUNT_ID]: buildPersonalDetails(DAVE_EMAIL, DAVE_ACCOUNT_ID, 'dave'), + [FRANK_ACCOUNT_ID]: buildPersonalDetails(FRANK_EMAIL, FRANK_ACCOUNT_ID, 'frank'), + }; +} + +async function renderPage(approverIndex: number) { + const result = render( + + + , + ); + await waitForBatchedUpdatesWithAct(); + return result; +} + +function getRowLogins() { + const rows: SelectionListApprover[] = jest.mocked(ApproverSelectionList).mock.lastCall?.[0].allApprovers ?? []; + return rows.map((row) => row.login); +} + +describe('WorkspaceWorkflowsApprovalsOverLimitApproverPage', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + jest.mocked(ApproverSelectionList).mockClear(); + await act(async () => { + await Onyx.clear(); + await Onyx.set(ONYXKEYS.HAS_LOADED_APP, true); + await Onyx.set(ONYXKEYS.BETAS, [CONST.BETAS.MULTIPLE_APPROVERS]); + await Onyx.set(ONYXKEYS.IS_LOADING_REPORT_DATA, false); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, buildPolicy()); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, buildPersonalDetailsList()); + await Onyx.merge(ONYXKEYS.SESSION, {email: ADMIN_EMAIL, accountID: ADMIN_ACCOUNT_ID}); + setApprovalWorkflow({ + ...INITIAL_APPROVAL_WORKFLOW, + action: CONST.APPROVAL_WORKFLOW.ACTION.EDIT, + members: [{email: DAVE_EMAIL, displayName: DAVE_EMAIL}], + approvers: [ + {email: BOB_EMAIL, displayName: BOB_EMAIL, approvalLimit: 5000}, + {email: ERIN_EMAIL, displayName: ERIN_EMAIL}, + ], + }); + }); + await waitForBatchedUpdatesWithAct(); + }); + + it('never offers an approver of the workflow as the additional approver for reports over the limit', async () => { + // Given the rules-based workflow Bob > Erin, where Bob is setting a limit, and Erin already approves every report after him + + // When the admin picks who else approves Bob's reports over the limit + await renderPage(0); + + // Then neither Bob nor Erin can be picked, since the limit would add no one, while everyone else can + const logins = getRowLogins(); + expect(logins).not.toContain(BOB_EMAIL); + expect(logins).not.toContain(ERIN_EMAIL); + expect(logins).toEqual(expect.arrayContaining([ADMIN_EMAIL, DAVE_EMAIL, FRANK_EMAIL])); + }); +});