diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 1b34c38a666c..c0e35a56cd97 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -195,6 +195,7 @@ import { getReportActionMessageText, getReportActionText, getRetractedMessage, + getSortedReportActions, getTravelUpdateMessage, getWorkspaceCurrencyUpdateMessage, getWorkspaceFrequencyUpdateMessage, @@ -4422,6 +4423,13 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals // eslint-disable-next-line @typescript-eslint/no-deprecated const policy = getPolicy(report?.policyID); + // If the current user took control, then they are the final approver and we don't have a next approver + // If someone else took control or rerouted, they are the next approver + const bypassApproverAccountID = getBypassApproverAccountIDIfTakenControl(report); + if (bypassApproverAccountID) { + return bypassApproverAccountID === currentUserAccountID && !isUnapproved ? undefined : bypassApproverAccountID; + } + const approvalChain = getApprovalChain(policy, report); const submitToAccountID = getSubmitToAccountID(policy, report); @@ -4430,16 +4438,19 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals return currentUserAccountID; } - return report?.managerID; + return report?.managerID ?? submitToAccountID; } if (approvalChain.length === 0) { return submitToAccountID; } - const nextApproverEmail = approvalChain.length === 1 ? approvalChain.at(0) : approvalChain.at(approvalChain.indexOf(currentUserEmail ?? '') + 1); + const currentUserIndex = approvalChain.indexOf(currentUserEmail ?? ''); + const nextApproverEmail = currentUserIndex === -1 ? approvalChain.at(0) : approvalChain.at(currentUserIndex + 1); + if (!nextApproverEmail) { - return submitToAccountID; + // If there's no next approver in the chain, return undefined to indicate the current user is the final approver + return undefined; } return getAccountIDsByLogins([nextApproverEmail]).at(0); @@ -11855,6 +11866,42 @@ function isWorkspaceEligibleForReportChange(submitterEmail: string | undefined, return isPaidGroupPolicyPolicyUtils(newPolicy) && !!newPolicy.role; } +/** + * Checks if someone took control of the report and if that take control is still valid + * A take control is invalidated if there's a SUBMITTED action after it + */ +function getBypassApproverAccountIDIfTakenControl(expenseReport: OnyxEntry): number | null { + if (!expenseReport?.reportID) { + return null; + } + + if (!isProcessingReport(expenseReport)) { + return null; + } + + const reportActions = getAllReportActions(expenseReport.reportID); + if (!reportActions) { + return null; + } + + // Sort actions by created timestamp to get chronological order + const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true); + + // Look through actions in reverse chronological order (newest first) + // If we find a SUBMITTED action, there's no valid take control since any take control would be older + for (const action of sortedActions) { + if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED)) { + return null; + } + + if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { + return action.actorAccountID ?? null; + } + } + + return null; +} + function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry): string[] { const approvalChain: string[] = []; const fullApprovalChain: string[] = []; diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 12cea19fcb9e..bc9e19f71194 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10192,13 +10192,6 @@ function getIOUReportActionToApproveOrPay(chatReport: OnyxEntry, policy: OnyxEntry, @@ -10226,22 +10219,19 @@ function approveMoneyRequest( } const optimisticApprovedReportAction = buildOptimisticApprovedReportAction(total, expenseReport.currency ?? '', expenseReport.reportID); - // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 - // eslint-disable-next-line @typescript-eslint/no-deprecated - const approvalChain = getApprovalChain(getPolicy(expenseReport.policyID), expenseReport); - - const predictedNextStatus = isLastApprover(approvalChain) ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; - const predictedNextState = isLastApprover(approvalChain) ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; - const managerID = isLastApprover(approvalChain) ? expenseReport.managerID : getNextApproverAccountID(expenseReport); + const nextApproverAccountID = getNextApproverAccountID(expenseReport); + const predictedNextStatus = !nextApproverAccountID ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; + const predictedNextState = !nextApproverAccountID ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; + const managerID = !nextApproverAccountID ? expenseReport.managerID : nextApproverAccountID; const optimisticNextStep = buildNextStepNew({ report: expenseReport, - predictedNextStatus: CONST.REPORT.STATUS_NUM.OPEN, policy, currentUserAccountIDParam, currentUserEmailParam, hasViolations, isASAPSubmitBetaEnabled, + predictedNextStatus, }); const chatReport = getReportOrDraftReport(expenseReport.chatReportID); @@ -10878,6 +10868,7 @@ function submitReport( currentUserEmailParam, hasViolations, isASAPSubmitBetaEnabled, + isUnapprove: true, }); const approvalChain = getApprovalChain(policy, expenseReport); const managerID = getAccountIDsByLogins(approvalChain).at(0); diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 4cadfe27de41..c604ab761c24 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -13,6 +13,7 @@ import useReportWithTransactionsAndViolations from '@hooks/useReportWithTransact import type {PerDiemExpenseTransactionParams, RequestMoneyParticipantParams} from '@libs/actions/IOU'; import { addSplitExpenseField, + approveMoneyRequest, calculateDiffAmount, canApproveIOU, canCancelPayment, @@ -9442,4 +9443,389 @@ describe('actions/IOU', () => { expect(iouReportID).toBe(expenseReport.reportID); }); }); + describe('approveMoneyRequest with take control', () => { + const adminAccountID = 1; + const managerAccountID = 2; + const employeeAccountID = 3; + const seniorManagerAccountID = 4; + const adminEmail = 'admin@test.com'; + const managerEmail = 'manager@test.com'; + const employeeEmail = 'employee@test.com'; + const seniorManagerEmail = 'seniormanager@test.com'; + + let expenseReport: Report; + let policy: Policy; + + beforeEach(async () => { + await Onyx.clear(); + + // Set up personal details + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [adminAccountID]: { + accountID: adminAccountID, + login: adminEmail, + displayName: 'Admin User', + }, + [seniorManagerAccountID]: { + accountID: seniorManagerAccountID, + login: seniorManagerEmail, + displayName: 'Senior Manager User', + }, + [managerAccountID]: { + accountID: managerAccountID, + login: managerEmail, + displayName: 'Manager User', + }, + [employeeAccountID]: { + accountID: employeeAccountID, + login: employeeEmail, + displayName: 'Employee User', + }, + }); + + // Set up session as admin (who will approve) + await Onyx.set(ONYXKEYS.SESSION, { + email: adminEmail, + accountID: adminAccountID, + }); + + // Create policy with approval hierarchy + policy = { + id: '1', + name: 'Test Policy', + role: CONST.POLICY.ROLE.ADMIN, + owner: adminEmail, + ownerAccountID: adminAccountID, + outputCurrency: CONST.CURRENCY.USD, + isPolicyExpenseChatEnabled: true, + type: CONST.POLICY.TYPE.CORPORATE, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_MANUAL, + employeeList: { + [employeeEmail]: { + email: employeeEmail, + role: CONST.POLICY.ROLE.USER, + submitsTo: managerEmail, + }, + [managerEmail]: { + email: managerEmail, + role: CONST.POLICY.ROLE.USER, + forwardsTo: seniorManagerEmail, + }, + [seniorManagerEmail]: { + email: seniorManagerEmail, + role: CONST.POLICY.ROLE.USER, + forwardsTo: adminEmail, + }, + [adminEmail]: { + email: adminEmail, + role: CONST.POLICY.ROLE.ADMIN, + forwardsTo: '', + }, + }, + }; + + // Create expense report + expenseReport = { + reportID: '123', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: employeeAccountID, + managerID: managerAccountID, + policyID: policy.id, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + total: 1000, + currency: 'USD', + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, policy); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport); + }); + + afterEach(async () => { + await Onyx.clear(); + }); + + it('should set report to approved when admin takes control and approves', async () => { + // Admin takes control + const takeControlAction = { + reportActionID: 'takeControl1', + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: adminAccountID, + created: '2023-01-01T10:00:00.000Z', + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { + [takeControlAction.reportActionID]: takeControlAction, + }); + + // Admin approves the report + approveMoneyRequest(expenseReport, policy, adminAccountID, adminEmail, false, false); + await waitForBatchedUpdates(); + + // Should be approved since admin took control and is the last approver + const updatedReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`); + expect(updatedReport?.stateNum).toBe(CONST.REPORT.STATE_NUM.APPROVED); + expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.APPROVED); + }); + + it('should invalidate take control when report is resubmitted after take control', async () => { + // Admin takes control first + const takeControlAction = { + reportActionID: 'takeControl3', + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: adminAccountID, + created: '2023-01-01T10:00:00.000Z', + }; + + // Employee resubmits after take control (invalidates it) + const submittedAction = { + reportActionID: 'submitted1', + actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED, + actorAccountID: employeeAccountID, + created: '2023-01-01T11:00:00.000Z', // After take control + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { + [takeControlAction.reportActionID]: takeControlAction, + [submittedAction.reportActionID]: submittedAction, + }); + + // Set session as manager (normal approver) + await Onyx.set(ONYXKEYS.SESSION, { + email: managerEmail, + accountID: managerAccountID, + }); + + // Manager approves the report + approveMoneyRequest(expenseReport, policy, managerAccountID, managerEmail, false, false); + await waitForBatchedUpdates(); + + // Should be submitted to senior manager (normal flow) since take control was invalidated + const updatedReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`); + expect(updatedReport?.stateNum).toBe(CONST.REPORT.STATE_NUM.SUBMITTED); + expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.SUBMITTED); + + // Get the optimistic next step + const nextStep = await getOnyxValue(`${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`); + + // The next step message should be defined + expect(nextStep?.message).toBeDefined(); + + // Since take control was invalidated by resubmission, the normal approval chain applies + // The next step should indicate waiting for the senior manager to approve + const fullMessage = nextStep?.message?.map((part) => part.text).join(''); + expect(fullMessage).toBe('Waiting for Senior Manager User to approve %expenses.'); + }); + + it('should mention an admin to pay expenses in optimistic next step message when admin takes control and approves', async () => { + // Admin takes control + const takeControlAction = { + reportActionID: 'takeControl2', + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: adminAccountID, + created: '2023-01-01T10:00:00.000Z', + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { + [takeControlAction.reportActionID]: takeControlAction, + }); + + // Admin approves the report + approveMoneyRequest(expenseReport, policy, adminAccountID, adminEmail, false, false); + await waitForBatchedUpdates(); + + // Get the optimistic next step + const nextStep = await getOnyxValue(`${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`); + + // The next step message should be defined + expect(nextStep?.message).toBeDefined(); + + // Since the report is fully approved when admin takes control and approves, + // the next step should be about payment, which should mention the admin + // The message should equal "Waiting for Admin User to pay" + const fullMessage = nextStep?.message?.map((part) => part.text).join(''); + expect(fullMessage).toBe('Waiting for an admin to pay %expenses.'); + }); + }); + + describe('approveMoneyRequest with normal approval chain', () => { + const adminAccountID = 1; + const managerAccountID = 2; + const employeeAccountID = 3; + const adminEmail = 'admin@test.com'; + const managerEmail = 'manager@test.com'; + const employeeEmail = 'employee@test.com'; + + let expenseReport: Report; + let policy: Policy; + + beforeEach(async () => { + await Onyx.clear(); + + // Set up personal details + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [adminAccountID]: { + accountID: adminAccountID, + login: adminEmail, + displayName: 'Admin User', + }, + [managerAccountID]: { + accountID: managerAccountID, + login: managerEmail, + displayName: 'Manager User', + }, + [employeeAccountID]: { + accountID: employeeAccountID, + login: employeeEmail, + displayName: 'Employee User', + }, + }); + + // Create policy with approval hierarchy + policy = { + id: '1', + name: 'Test Policy', + role: CONST.POLICY.ROLE.ADMIN, + owner: adminEmail, + outputCurrency: CONST.CURRENCY.USD, + isPolicyExpenseChatEnabled: true, + type: CONST.POLICY.TYPE.CORPORATE, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + employeeList: { + [employeeEmail]: { + email: employeeEmail, + role: CONST.POLICY.ROLE.USER, + submitsTo: managerEmail, + }, + [managerEmail]: { + email: managerEmail, + role: CONST.POLICY.ROLE.USER, + submitsTo: adminEmail, + forwardsTo: adminEmail, + }, + [adminEmail]: { + email: adminEmail, + role: CONST.POLICY.ROLE.ADMIN, + submitsTo: '', + forwardsTo: '', + }, + }, + }; + + // Create expense report + expenseReport = { + reportID: '123', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: employeeAccountID, + managerID: managerAccountID, + policyID: policy.id, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + total: 1000, + currency: 'USD', + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, policy); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport); + }); + + afterEach(async () => { + await Onyx.clear(); + }); + + it('should follow normal approval chain when manager approves without take control', async () => { + // Set session as manager (first approver in the chain) + await Onyx.set(ONYXKEYS.SESSION, { + email: managerEmail, + accountID: managerAccountID, + }); + + // Manager approves the report (no take control actions) + approveMoneyRequest(expenseReport, policy, managerAccountID, managerEmail, false, false); + await waitForBatchedUpdates(); + + // Should be submitted to admin (next in approval chain) since manager is not the final approver + const updatedReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`); + expect(updatedReport?.stateNum).toBe(CONST.REPORT.STATE_NUM.SUBMITTED); + expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.SUBMITTED); + expect(updatedReport?.managerID).toBe(adminAccountID); // Should be forwarded to admin + }); + + it('should handle multi-step approval chain correctly', async () => { + // First, manager approves + await Onyx.set(ONYXKEYS.SESSION, { + email: managerEmail, + accountID: managerAccountID, + }); + + approveMoneyRequest(expenseReport, policy, managerAccountID, managerEmail, false, false); + await waitForBatchedUpdates(); + + // Should be submitted to admin + let updatedReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`); + expect(updatedReport?.stateNum).toBe(CONST.REPORT.STATE_NUM.SUBMITTED); + expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.SUBMITTED); + expect(updatedReport?.managerID).toBe(adminAccountID); + + // Then, admin approves + await Onyx.set(ONYXKEYS.SESSION, { + email: adminEmail, + accountID: adminAccountID, + }); + + approveMoneyRequest(updatedReport, policy, adminAccountID, adminEmail, false, false); + await waitForBatchedUpdates(); + + // Should be fully approved + updatedReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`); + expect(updatedReport?.stateNum).toBe(CONST.REPORT.STATE_NUM.APPROVED); + expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.APPROVED); + }); + + it('should fully approve report when single approver approves', async () => { + // Create a policy with only one approver in the chain + const singleApproverPolicy: Policy = { + ...policy, + employeeList: { + [employeeEmail]: { + email: employeeEmail, + role: CONST.POLICY.ROLE.USER, + submitsTo: managerEmail, + }, + [managerEmail]: { + email: managerEmail, + role: CONST.POLICY.ROLE.ADMIN, + submitsTo: '', + forwardsTo: '', + }, + }, + }; + + // Create expense report with manager as the only approver + const singleApproverReport: Report = { + ...expenseReport, + reportID: '456', + managerID: managerAccountID, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${singleApproverPolicy.id}`, singleApproverPolicy); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${singleApproverReport.reportID}`, singleApproverReport); + + // Set session as the single approver (manager) + await Onyx.set(ONYXKEYS.SESSION, { + email: managerEmail, + accountID: managerAccountID, + }); + + // Manager approves the report + approveMoneyRequest(singleApproverReport, singleApproverPolicy, managerAccountID, managerEmail, false, false); + await waitForBatchedUpdates(); + + // Should be fully approved since manager is the final approver in the chain + const updatedReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${singleApproverReport.reportID}`); + expect(updatedReport?.stateNum).toBe(CONST.REPORT.STATE_NUM.APPROVED); + expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.APPROVED); + }); + }); }); diff --git a/tests/unit/NextStepUtilsTest.ts b/tests/unit/NextStepUtilsTest.ts index 5eb649deb945..78f6665ed391 100644 --- a/tests/unit/NextStepUtilsTest.ts +++ b/tests/unit/NextStepUtilsTest.ts @@ -615,7 +615,7 @@ describe('libs/NextStepUtils', () => { }).then(() => { // TODO: Replace onyx.connect with useOnyx hook (https://github.com/Expensify/App/issues/66365) // eslint-disable-next-line @typescript-eslint/no-deprecated - const result = buildNextStep(report, CONST.REPORT.STATUS_NUM.SUBMITTED); + const result = buildNextStep(report, CONST.REPORT.STATUS_NUM.SUBMITTED, undefined, true, undefined); expect(result).toMatchObject(optimisticNextStep); });