From 95d4111e3db133a84250723ac11b1b8443142c29 Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 10 Sep 2025 22:05:04 +0700 Subject: [PATCH 01/24] Implement bypass approvers logic in getApprovalChain function and add corresponding unit tests. The function now returns only the current user's email if they are the manager of the expense report, indicating that they bypassed other approvers. Unit tests cover both bypass and normal approval scenarios. --- src/libs/ReportUtils.ts | 8 ++++++ tests/unit/ReportUtilsTest.ts | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 6cb14f2b40b9..ee19efca9ccb 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11145,6 +11145,14 @@ function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry { }); }); }); + + describe('bypass approvers scenario', () => { + it('should return array containing only the current user when they are the manager (bypassed approvers)', () => { + const policyTest: Policy = { + ...createRandomPolicy(0), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.TEAM, + employeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, + }; + const expenseReport: Report = { + ...createRandomReport(0), + ownerAccountID: employeeAccountID, + managerID: currentUserAccountID, // Current user is the manager (bypassed approvers) + type: CONST.REPORT.TYPE.EXPENSE, + }; + + Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails).then(() => { + const result = [currentUserEmail]; + expect(getApprovalChain(policyTest, expenseReport)).toStrictEqual(result); + }); + }); + + it('should return normal approval chain when current user is not the manager', () => { + const policyTest: Policy = { + ...createRandomPolicy(0), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.TEAM, + employeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, + }; + const expenseReport: Report = { + ...createRandomReport(0), + ownerAccountID: employeeAccountID, + managerID: 1, // Different user is the manager (normal flow) + type: CONST.REPORT.TYPE.EXPENSE, + }; + + Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails).then(() => { + const result = ['owner@test.com']; + expect(getApprovalChain(policyTest, expenseReport)).toStrictEqual(result); + }); + }); + }); }); describe('shouldReportShowSubscript', () => { From da0718409277ef571ebb616a1bf0df20f78434d6 Mon Sep 17 00:00:00 2001 From: fahimj Date: Thu, 11 Sep 2025 23:15:52 +0700 Subject: [PATCH 02/24] update getApprovalChain function to handle bypass scenarios for managers --- src/libs/ReportUtils.ts | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index ee19efca9ccb..5f7b1c5be3db 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11145,14 +11145,6 @@ function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry, expenseReport: OnyxEntry 0 ? fullApprovalChain[0] : null; + const shouldCurrentUserBeNextApprover = nextApprover === currentUserEmailForApproval; + + if (isCurrentUserManager && !shouldCurrentUserBeNextApprover && currentUserEmailForApproval) { + // This indicates the current user bypassed the normal approval workflow + // (they became manager but wouldn't normally be the next approver) + return [currentUserEmailForApproval]; + } + + // Return the normal approval chain for all other cases return fullApprovalChain; } From ea3d8a3e110daf12348ec628c4e91cec477362fc Mon Sep 17 00:00:00 2001 From: fahimj Date: Fri, 12 Sep 2025 09:09:15 +0700 Subject: [PATCH 03/24] Revert "update getApprovalChain function to handle bypass scenarios for managers" This reverts commit da0718409277ef571ebb616a1bf0df20f78434d6. --- src/libs/ReportUtils.ts | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 5f7b1c5be3db..ee19efca9ccb 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11145,6 +11145,14 @@ function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry, expenseReport: OnyxEntry 0 ? fullApprovalChain[0] : null; - const shouldCurrentUserBeNextApprover = nextApprover === currentUserEmailForApproval; - - if (isCurrentUserManager && !shouldCurrentUserBeNextApprover && currentUserEmailForApproval) { - // This indicates the current user bypassed the normal approval workflow - // (they became manager but wouldn't normally be the next approver) - return [currentUserEmailForApproval]; - } - - // Return the normal approval chain for all other cases return fullApprovalChain; } From 3999783649d621f4c4a3f60357feab2305522da5 Mon Sep 17 00:00:00 2001 From: fahimj Date: Fri, 12 Sep 2025 09:11:48 +0700 Subject: [PATCH 04/24] Revert "Implement bypass approvers logic in getApprovalChain function and add corresponding unit tests. The function now returns only the current user's email if they are the manager of the expense report, indicating that they bypassed other approvers. Unit tests cover both bypass and normal approval scenarios." This reverts commit 95d4111e3db133a84250723ac11b1b8443142c29. --- src/libs/ReportUtils.ts | 8 ------ tests/unit/ReportUtilsTest.ts | 46 ----------------------------------- 2 files changed, 54 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index ee19efca9ccb..6cb14f2b40b9 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11145,14 +11145,6 @@ function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry { }); }); }); - - describe('bypass approvers scenario', () => { - it('should return array containing only the current user when they are the manager (bypassed approvers)', () => { - const policyTest: Policy = { - ...createRandomPolicy(0), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.TEAM, - employeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, - }; - const expenseReport: Report = { - ...createRandomReport(0), - ownerAccountID: employeeAccountID, - managerID: currentUserAccountID, // Current user is the manager (bypassed approvers) - type: CONST.REPORT.TYPE.EXPENSE, - }; - - Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails).then(() => { - const result = [currentUserEmail]; - expect(getApprovalChain(policyTest, expenseReport)).toStrictEqual(result); - }); - }); - - it('should return normal approval chain when current user is not the manager', () => { - const policyTest: Policy = { - ...createRandomPolicy(0), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.TEAM, - employeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, - }; - const expenseReport: Report = { - ...createRandomReport(0), - ownerAccountID: employeeAccountID, - managerID: 1, // Different user is the manager (normal flow) - type: CONST.REPORT.TYPE.EXPENSE, - }; - - Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails).then(() => { - const result = ['owner@test.com']; - expect(getApprovalChain(policyTest, expenseReport)).toStrictEqual(result); - }); - }); - }); }); describe('shouldReportShowSubscript', () => { From 84221b30002aaf4e3f3e92e1ef376aa6f7708ee4 Mon Sep 17 00:00:00 2001 From: fahimj Date: Fri, 12 Sep 2025 14:26:46 +0700 Subject: [PATCH 05/24] Add unit tests for take control functionality in getApprovalChain --- tests/unit/ReportUtilsTest.ts | 269 ++++++++++++++++++++++++++++++++++ 1 file changed, 269 insertions(+) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 4f112008695d..da5d74f5a7c7 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -235,6 +235,10 @@ const personalDetails: PersonalDetailsList = { accountID: 8, login: CONST.EMAIL.GUIDES_DOMAIN, }, + '9': { + accountID: 9, + login: 'manager@test.com', + }, }; const rules = { @@ -3547,6 +3551,271 @@ describe('ReportUtils', () => { }); }); }); + + describe('take control functionality', () => { + const adminAccountID = 1; // From personalDetails + const managerAccountID = 9; // From personalDetails + const adminEmail = 'admin@test.com'; + const managerEmail = 'manager@test.com'; // Use the new manager email + + // Create a multi-level employee list with approval hierarchy + const multiLevelEmployeeList: PolicyEmployeeList = { + ...employeeList, + 'employee@test.com': { + email: 'employee@test.com', + role: 'user', + submitsTo: 'manager@test.com', // Employee submits to manager + }, + 'manager@test.com': { + email: 'manager@test.com', + role: 'user', + submitsTo: 'admin@test.com', // Manager submits to admin + forwardsTo: 'admin@test.com', // Manager forwards to admin + }, + 'admin@test.com': { + email: 'admin@test.com', + role: 'admin', + submitsTo: '', // Admin is the final approver + forwardsTo: '', // Admin doesn't forward to anyone + }, + }; + + beforeEach(async () => { + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails); + }); + + afterEach(async () => { + await Onyx.clear(); + await Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID}); + }); + + it('should return admin as sole approver when admin takes control', async () => { + const policyTest: Policy = { + ...createRandomPolicy(1001), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList: multiLevelEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + + const expenseReport: Report = { + ...createRandomReport(1001), + ownerAccountID: employeeAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + // Create a TAKE_CONTROL action by admin + const takeControlAction: ReportAction = { + ...createRandomReportAction(1001), + 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, + }); + + const result = getApprovalChain(policyTest, expenseReport); + expect(result).toStrictEqual([adminEmail]); + }); + + it('should return normal approval chain when no take control action exists', async () => { + const policyTest: Policy = { + ...createRandomPolicy(1002), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList: multiLevelEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + const expenseReport: Report = { + ...createRandomReport(1002), + ownerAccountID: employeeAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + const result = getApprovalChain(policyTest, expenseReport); + expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); + }); + + it('should invalidate take control when report is resubmitted after take control', async () => { + const policyTest: Policy = { + ...createRandomPolicy(1003), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList: multiLevelEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + const expenseReport: Report = { + ...createRandomReport(1003), + ownerAccountID: employeeAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + // Create a TAKE_CONTROL action first + const takeControlAction: ReportAction = { + ...createRandomReportAction(1003), + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: adminAccountID, + created: '2023-01-01T10:00:00.000Z', + }; + + // Create a SUBMITTED action after take control (invalidates it) + const submittedAction: ReportAction = { + ...createRandomReportAction(1004), + 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, + }); + + const result = getApprovalChain(policyTest, expenseReport); + // Should return normal approval chain since take control was invalidated + expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); + }); + + it('should invalidate take control when report is submitted and closed after take control', async () => { + const policyTest: Policy = { + ...createRandomPolicy(1005), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList: multiLevelEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + const expenseReport: Report = { + ...createRandomReport(1005), + ownerAccountID: employeeAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + // Create a TAKE_CONTROL action first + const takeControlAction: ReportAction = { + ...createRandomReportAction(1005), + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: managerAccountID, + created: '2023-01-01T10:00:00.000Z', + }; + + // Create a SUBMITTED_AND_CLOSED action after take control (invalidates it) + const submittedAndClosedAction: ReportAction = { + ...createRandomReportAction(1006), + actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED_AND_CLOSED, + actorAccountID: employeeAccountID, + created: '2023-01-01T11:00:00.000Z', // After take control + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { + [takeControlAction.reportActionID]: takeControlAction, + [submittedAndClosedAction.reportActionID]: submittedAndClosedAction, + }); + + const result = getApprovalChain(policyTest, expenseReport); + // Should return normal approval chain since take control was invalidated + expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); + }); + + it('should keep take control valid when submitted action happened before take control', async () => { + const policyTest: Policy = { + ...createRandomPolicy(1007), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList: multiLevelEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + const expenseReport: Report = { + ...createRandomReport(1007), + ownerAccountID: employeeAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + // Create a SUBMITTED action first + const submittedAction: ReportAction = { + ...createRandomReportAction(1007), + actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED, + actorAccountID: employeeAccountID, + created: '2023-01-01T09:00:00.000Z', // Before take control + }; + + // Create a TAKE_CONTROL action after submission + const takeControlAction: ReportAction = { + ...createRandomReportAction(1008), + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: adminAccountID, + created: '2023-01-01T10:00:00.000Z', // After submission + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { + [submittedAction.reportActionID]: submittedAction, + [takeControlAction.reportActionID]: takeControlAction, + }); + + const result = getApprovalChain(policyTest, expenseReport); + // Should return admin as sole approver since take control is valid + expect(result).toStrictEqual([adminEmail]); + }); + + it('should not return submitter as approver even if they take control', async () => { + const policyTest: Policy = { + ...createRandomPolicy(1009), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList: multiLevelEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + const expenseReport: Report = { + ...createRandomReport(1009), + ownerAccountID: employeeAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + // Employee (submitter) tries to take control + const takeControlAction: ReportAction = { + ...createRandomReportAction(1009), + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: employeeAccountID, // Same as report owner + created: '2023-01-01T10:00:00.000Z', + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { + [takeControlAction.reportActionID]: takeControlAction, + }); + + const result = getApprovalChain(policyTest, expenseReport); + // Should return normal approval chain, not the submitter + expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); + }); + + it('should handle empty or missing report actions gracefully', async () => { + const policyTest: Policy = { + ...createRandomPolicy(1010), + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList: multiLevelEmployeeList, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + const expenseReport: Report = { + ...createRandomReport(1010), + ownerAccountID: employeeAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + // Don't set any report actions + const result = getApprovalChain(policyTest, expenseReport); + // Should return normal approval chain + expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); + }); + }); }); describe('shouldReportShowSubscript', () => { From c2a6eac551fe4f1975e25eb74cf10c7e02ae0ccc Mon Sep 17 00:00:00 2001 From: fahimj Date: Fri, 12 Sep 2025 14:32:09 +0700 Subject: [PATCH 06/24] Add getBypassApproverIfTakeControl function to validate take control actions in expense reports. Update getApprovalChain to incorporate bypass approver logic based on recent actions. --- src/libs/ReportUtils.ts | 61 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 6cb14f2b40b9..ca32d21b3372 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11135,6 +11135,61 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report return isPaidGroupPolicyPolicyUtils(newPolicy) && (isPolicyMember(newPolicy, submitterEmail) || isPolicyAdmin(newPolicy?.id, policies)); } +/** + * 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 getBypassApproverIfTakeControl(expenseReport: OnyxEntry): string | null { + if (!expenseReport?.reportID) { + return null; + } + + const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`]; + if (!reportActions) { + return null; + } + + const actions = Object.values(reportActions).filter(Boolean); + + // Sort actions by created timestamp to get chronological order + const sortedActions = actions.sort((a, b) => { + const aTime = new Date(a.created).getTime(); + const bTime = new Date(b.created).getTime(); + return aTime - bTime; + }); + + let lastTakeControlAction: ReportAction | null = null; + let lastTakeControlActorAccountID: number | null = null; + + // Look through actions chronologically to find the most recent take control + // and check if it's been invalidated by a subsequent SUBMITTED action + for (const action of sortedActions) { + if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { + lastTakeControlAction = action; + lastTakeControlActorAccountID = action.actorAccountID ?? null; + } + + // If we find a SUBMITTED or SUBMITTED_AND_CLOSED action after a take control, the take control is invalidated + if (lastTakeControlAction && (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED) || isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED_AND_CLOSED))) { + const takeControlTime = new Date(lastTakeControlAction.created).getTime(); + const submitTime = new Date(action.created).getTime(); + + if (submitTime > takeControlTime) { + lastTakeControlAction = null; + lastTakeControlActorAccountID = null; + } + } + } + + // If we have a valid take control action, return the approver email + if (lastTakeControlAction && lastTakeControlActorAccountID) { + const approverEmail = getLoginsByAccountIDs([lastTakeControlActorAccountID]).at(0); + return approverEmail ?? null; + } + + return null; +} + function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry): string[] { const approvalChain: string[] = []; const fullApprovalChain: string[] = []; @@ -11145,6 +11200,12 @@ function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry Date: Sat, 27 Sep 2025 17:11:23 +0700 Subject: [PATCH 07/24] Merge branch 'main' into fix/69249 From 51f90e4fc6c2705f51b36138b10f470e534af4e3 Mon Sep 17 00:00:00 2001 From: fahimj Date: Sat, 27 Sep 2025 21:33:54 +0700 Subject: [PATCH 08/24] Added logic to handle bypass approver in getNextApproverAccountID and refactored approveMoneyRequest to accommodate new approval flow. Updated unit tests to validate take control scenarios. --- src/libs/ReportUtils.ts | 14 +- src/libs/actions/IOU.ts | 25 ++-- tests/actions/IOUTest.ts | 192 ++++++++++++++++++++++++ tests/unit/ReportUtilsTest.ts | 265 ---------------------------------- 4 files changed, 211 insertions(+), 285 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 758081acdba0..d0bb4277c1d5 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4238,6 +4238,13 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 // eslint-disable-next-line deprecation/deprecation const policy = getPolicy(report?.policyID); + + // Check if someone took control and should be the next approver + const bypassApprover = getBypassApproverIfTakeControl(report); + if (bypassApprover) { + return getAccountIDsByLogins([bypassApprover]).at(0); + } + const approvalChain = getApprovalChain(policy, report); const submitToAccountID = getSubmitToAccountID(policy, report); @@ -11387,12 +11394,6 @@ function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry, full?: boolean) { if (!expenseReport) { return; @@ -9714,13 +9708,16 @@ function approveMoneyRequest(expenseReport: OnyxEntry, full?: // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 // eslint-disable-next-line deprecation/deprecation - 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 policy = getPolicy(expenseReport.policyID); + const nextApproverAccountID = getNextApproverAccountID(expenseReport); + const isCurrentUserLastApprover = !nextApproverAccountID || nextApproverAccountID === userAccountID; + const predictedNextStatus = isCurrentUserLastApprover ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; + const predictedNextState = isCurrentUserLastApprover ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; + const managerID = isCurrentUserLastApprover ? expenseReport.managerID : nextApproverAccountID; - const optimisticNextStep = buildNextStep(expenseReport, predictedNextStatus); + const hasViolations = hasViolationsReportUtils(expenseReport.reportID, allTransactionViolations); + const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas); + const optimisticNextStep = buildNextStepNew(expenseReport, policy, userAccountID, currentUserEmail, hasViolations, isASAPSubmitBetaEnabled, predictedNextStatus); const chatReport = getReportOrDraftReport(expenseReport.chatReportID); const optimisticReportActionsData: OnyxUpdate = { diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index e97cc1404abc..31fc27306243 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -11,6 +11,7 @@ import useReportWithTransactionsAndViolations from '@hooks/useReportWithTransact import type {PerDiemExpenseTransactionParams, RequestMoneyParticipantParams} from '@libs/actions/IOU'; import { addSplitExpenseField, + approveMoneyRequest, calculateDiffAmount, canApproveIOU, canCancelPayment, @@ -8614,4 +8615,195 @@ describe('actions/IOU', () => { ); }); }); + + describe('approveMoneyRequest with take control', () => { + 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', + }, + }); + + // 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, + outputCurrency: CONST.CURRENCY.USD, + isPolicyExpenseChatEnabled: true, + type: CONST.POLICY.TYPE.CORPORATE, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + employeeList: { + [employeeEmail]: { + email: employeeEmail, + role: 'user', + submitsTo: managerEmail, + }, + [managerEmail]: { + email: managerEmail, + role: 'user', + submitsTo: adminEmail, + forwardsTo: adminEmail, + }, + [adminEmail]: { + email: adminEmail, + 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 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); + 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); + await waitForBatchedUpdates(); + + // Should be submitted to admin (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); + }); + + it('should handle take control with SUBMITTED_AND_CLOSED action invalidation', async () => { + // Admin takes control + const takeControlAction = { + reportActionID: 'takeControl7', + actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, + actorAccountID: adminAccountID, + created: '2023-01-01T10:00:00.000Z', + }; + + // Employee submits and closes after take control (invalidates it) + const submittedAndClosedAction = { + reportActionID: 'submittedClosed1', + actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED_AND_CLOSED, + actorAccountID: employeeAccountID, + created: '2023-01-01T11:00:00.000Z', // After take control + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { + [takeControlAction.reportActionID]: takeControlAction, + [submittedAndClosedAction.reportActionID]: submittedAndClosedAction, + }); + + // Set session as manager (normal approver) + await Onyx.set(ONYXKEYS.SESSION, { + email: managerEmail, + accountID: managerAccountID, + }); + + // Manager approves the report + approveMoneyRequest(expenseReport); + await waitForBatchedUpdates(); + + // Should be submitted to admin (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); + }); + }); }); diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 6394e8794056..5f87205f443f 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -4504,271 +4504,6 @@ describe('ReportUtils', () => { }); }); }); - - describe('take control functionality', () => { - const adminAccountID = 1; // From personalDetails - const managerAccountID = 9; // From personalDetails - const adminEmail = 'admin@test.com'; - const managerEmail = 'manager@test.com'; // Use the new manager email - - // Create a multi-level employee list with approval hierarchy - const multiLevelEmployeeList: PolicyEmployeeList = { - ...employeeList, - 'employee@test.com': { - email: 'employee@test.com', - role: 'user', - submitsTo: 'manager@test.com', // Employee submits to manager - }, - 'manager@test.com': { - email: 'manager@test.com', - role: 'user', - submitsTo: 'admin@test.com', // Manager submits to admin - forwardsTo: 'admin@test.com', // Manager forwards to admin - }, - 'admin@test.com': { - email: 'admin@test.com', - role: 'admin', - submitsTo: '', // Admin is the final approver - forwardsTo: '', // Admin doesn't forward to anyone - }, - }; - - beforeEach(async () => { - await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails); - }); - - afterEach(async () => { - await Onyx.clear(); - await Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID}); - }); - - it('should return admin as sole approver when admin takes control', async () => { - const policyTest: Policy = { - ...createRandomPolicy(1001), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList: multiLevelEmployeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - - const expenseReport: Report = { - ...createRandomReport(1001), - ownerAccountID: employeeAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - // Create a TAKE_CONTROL action by admin - const takeControlAction: ReportAction = { - ...createRandomReportAction(1001), - 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, - }); - - const result = getApprovalChain(policyTest, expenseReport); - expect(result).toStrictEqual([adminEmail]); - }); - - it('should return normal approval chain when no take control action exists', async () => { - const policyTest: Policy = { - ...createRandomPolicy(1002), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList: multiLevelEmployeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - const expenseReport: Report = { - ...createRandomReport(1002), - ownerAccountID: employeeAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - const result = getApprovalChain(policyTest, expenseReport); - expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); - }); - - it('should invalidate take control when report is resubmitted after take control', async () => { - const policyTest: Policy = { - ...createRandomPolicy(1003), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList: multiLevelEmployeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - const expenseReport: Report = { - ...createRandomReport(1003), - ownerAccountID: employeeAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - // Create a TAKE_CONTROL action first - const takeControlAction: ReportAction = { - ...createRandomReportAction(1003), - actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, - actorAccountID: adminAccountID, - created: '2023-01-01T10:00:00.000Z', - }; - - // Create a SUBMITTED action after take control (invalidates it) - const submittedAction: ReportAction = { - ...createRandomReportAction(1004), - 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, - }); - - const result = getApprovalChain(policyTest, expenseReport); - // Should return normal approval chain since take control was invalidated - expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); - }); - - it('should invalidate take control when report is submitted and closed after take control', async () => { - const policyTest: Policy = { - ...createRandomPolicy(1005), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList: multiLevelEmployeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - const expenseReport: Report = { - ...createRandomReport(1005), - ownerAccountID: employeeAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - // Create a TAKE_CONTROL action first - const takeControlAction: ReportAction = { - ...createRandomReportAction(1005), - actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, - actorAccountID: managerAccountID, - created: '2023-01-01T10:00:00.000Z', - }; - - // Create a SUBMITTED_AND_CLOSED action after take control (invalidates it) - const submittedAndClosedAction: ReportAction = { - ...createRandomReportAction(1006), - actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED_AND_CLOSED, - actorAccountID: employeeAccountID, - created: '2023-01-01T11:00:00.000Z', // After take control - }; - - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { - [takeControlAction.reportActionID]: takeControlAction, - [submittedAndClosedAction.reportActionID]: submittedAndClosedAction, - }); - - const result = getApprovalChain(policyTest, expenseReport); - // Should return normal approval chain since take control was invalidated - expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); - }); - - it('should keep take control valid when submitted action happened before take control', async () => { - const policyTest: Policy = { - ...createRandomPolicy(1007), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList: multiLevelEmployeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - const expenseReport: Report = { - ...createRandomReport(1007), - ownerAccountID: employeeAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - // Create a SUBMITTED action first - const submittedAction: ReportAction = { - ...createRandomReportAction(1007), - actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED, - actorAccountID: employeeAccountID, - created: '2023-01-01T09:00:00.000Z', // Before take control - }; - - // Create a TAKE_CONTROL action after submission - const takeControlAction: ReportAction = { - ...createRandomReportAction(1008), - actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, - actorAccountID: adminAccountID, - created: '2023-01-01T10:00:00.000Z', // After submission - }; - - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { - [submittedAction.reportActionID]: submittedAction, - [takeControlAction.reportActionID]: takeControlAction, - }); - - const result = getApprovalChain(policyTest, expenseReport); - // Should return admin as sole approver since take control is valid - expect(result).toStrictEqual([adminEmail]); - }); - - it('should not return submitter as approver even if they take control', async () => { - const policyTest: Policy = { - ...createRandomPolicy(1009), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList: multiLevelEmployeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - const expenseReport: Report = { - ...createRandomReport(1009), - ownerAccountID: employeeAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - // Employee (submitter) tries to take control - const takeControlAction: ReportAction = { - ...createRandomReportAction(1009), - actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, - actorAccountID: employeeAccountID, // Same as report owner - created: '2023-01-01T10:00:00.000Z', - }; - - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { - [takeControlAction.reportActionID]: takeControlAction, - }); - - const result = getApprovalChain(policyTest, expenseReport); - // Should return normal approval chain, not the submitter - expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); - }); - - it('should handle empty or missing report actions gracefully', async () => { - const policyTest: Policy = { - ...createRandomPolicy(1010), - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList: multiLevelEmployeeList, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - const expenseReport: Report = { - ...createRandomReport(1010), - ownerAccountID: employeeAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - // Don't set any report actions - const result = getApprovalChain(policyTest, expenseReport); - // Should return normal approval chain - expect(result).toStrictEqual(['manager@test.com', 'admin@test.com']); - }); - }); }); describe('shouldReportShowSubscript', () => { From d9285e0deb7b347c30b36095647f636240463035 Mon Sep 17 00:00:00 2001 From: fahimj Date: Tue, 30 Sep 2025 07:36:10 +0700 Subject: [PATCH 09/24] Refactor getBypassApproverIfTakeControl to getBypassApproverIfTakenControl and improve action validation logic. Used existing getSortedReportActions instead of writing a new sorting code --- src/libs/ReportUtils.ts | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index d0bb4277c1d5..af35f14d01ce 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -223,6 +223,7 @@ import { isWhisperAction, shouldReportActionBeVisible, wasActionTakenByCurrentUser, + getSortedReportActions, } from './ReportActionsUtils'; import type {LastVisibleMessage} from './ReportActionsUtils'; import type {ArchivedReportsIDSet} from './SearchUIUtils'; @@ -4240,7 +4241,7 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals const policy = getPolicy(report?.policyID); // Check if someone took control and should be the next approver - const bypassApprover = getBypassApproverIfTakeControl(report); + const bypassApprover = getBypassApproverIfTakenControl(report); if (bypassApprover) { return getAccountIDsByLogins([bypassApprover]).at(0); } @@ -11333,12 +11334,17 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report * 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 getBypassApproverIfTakeControl(expenseReport: OnyxEntry): string | null { +function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): string | null { if (!expenseReport?.reportID) { return null; } - const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`]; + // Early return if report is not in processing state + if (!isProcessingReport(expenseReport)) { + return null; + } + + const reportActions = getAllReportActions(expenseReport.reportID); if (!reportActions) { return null; } @@ -11346,32 +11352,21 @@ function getBypassApproverIfTakeControl(expenseReport: OnyxEntry): strin const actions = Object.values(reportActions).filter(Boolean); // Sort actions by created timestamp to get chronological order - const sortedActions = actions.sort((a, b) => { - const aTime = new Date(a.created).getTime(); - const bTime = new Date(b.created).getTime(); - return aTime - bTime; - }); + const sortedActions = getSortedReportActions(actions, true); let lastTakeControlAction: ReportAction | null = null; let lastTakeControlActorAccountID: number | null = null; - // Look through actions chronologically to find the most recent take control - // and check if it's been invalidated by a subsequent SUBMITTED action + // 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.TAKE_CONTROL)) { + if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED)) { + // If we find a SUBMITTED action, no take control is valid since it would be older + return null; + } else if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { lastTakeControlAction = action; lastTakeControlActorAccountID = action.actorAccountID ?? null; - } - - // If we find a SUBMITTED or SUBMITTED_AND_CLOSED action after a take control, the take control is invalidated - if (lastTakeControlAction && (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED) || isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED_AND_CLOSED))) { - const takeControlTime = new Date(lastTakeControlAction.created).getTime(); - const submitTime = new Date(action.created).getTime(); - - if (submitTime > takeControlTime) { - lastTakeControlAction = null; - lastTakeControlActorAccountID = null; - } + break; // Found the most recent take control, no need to continue } } @@ -12197,7 +12192,6 @@ export { hasReportViolations, isPayAtEndExpenseReport, getApprovalChain, - getBypassApproverIfTakeControl, isIndividualInvoiceRoom, hasOutstandingChildRequest, isAuditor, From 2a096071d2c917bc8b8df410598f34d6ed7f2b51 Mon Sep 17 00:00:00 2001 From: fahimj Date: Tue, 30 Sep 2025 08:07:04 +0700 Subject: [PATCH 10/24] Updated variable naming to isCurrentUserNextApprover --- src/libs/actions/IOU.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 84463e1112d2..1e4f04997d23 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -9710,10 +9710,10 @@ function approveMoneyRequest(expenseReport: OnyxEntry, full?: // eslint-disable-next-line deprecation/deprecation const policy = getPolicy(expenseReport.policyID); const nextApproverAccountID = getNextApproverAccountID(expenseReport); - const isCurrentUserLastApprover = !nextApproverAccountID || nextApproverAccountID === userAccountID; - const predictedNextStatus = isCurrentUserLastApprover ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; - const predictedNextState = isCurrentUserLastApprover ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; - const managerID = isCurrentUserLastApprover ? expenseReport.managerID : nextApproverAccountID; + const isCurrentUserNextApprover = !nextApproverAccountID || nextApproverAccountID === userAccountID; + const predictedNextStatus = isCurrentUserNextApprover ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; + const predictedNextState = isCurrentUserNextApprover ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; + const managerID = isCurrentUserNextApprover ? expenseReport.managerID : nextApproverAccountID; const hasViolations = hasViolationsReportUtils(expenseReport.reportID, allTransactionViolations); const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas); From 9d2a8a52f38d862eecda969d104bd5e571dee529 Mon Sep 17 00:00:00 2001 From: fahimj Date: Tue, 30 Sep 2025 08:17:11 +0700 Subject: [PATCH 11/24] prettier --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index af35f14d01ce..4a5912e3e464 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -176,6 +176,7 @@ import { getReportActionMessageText, getReportActionText, getRetractedMessage, + getSortedReportActions, getTravelUpdateMessage, getWorkspaceCurrencyUpdateMessage, getWorkspaceFrequencyUpdateMessage, @@ -223,7 +224,6 @@ import { isWhisperAction, shouldReportActionBeVisible, wasActionTakenByCurrentUser, - getSortedReportActions, } from './ReportActionsUtils'; import type {LastVisibleMessage} from './ReportActionsUtils'; import type {ArchivedReportsIDSet} from './SearchUIUtils'; From 478fb9df28797953b0a54670bd733607a6e012d1 Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 1 Oct 2025 16:41:51 +0700 Subject: [PATCH 12/24] Refactor approval logic in ReportUtils to return null for final approver and add tests for multi-step approval chain in IOUTest --- src/libs/ReportUtils.ts | 3 +- tests/actions/IOUTest.ts | 135 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 9b25b0ec0b17..621c856a9679 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4273,7 +4273,8 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals const nextApproverEmail = approvalChain.length === 1 ? approvalChain.at(0) : approvalChain.at(approvalChain.indexOf(currentUserEmail ?? '') + 1); if (!nextApproverEmail) { - return submitToAccountID; + // If there's no next approver in the chain, return null to indicate the current user is the final approver + return null; } return getAccountIDsByLogins([nextApproverEmail]).at(0); diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 7168aa58f3a0..c3d6354f73a1 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -8881,4 +8881,139 @@ describe('actions/IOU', () => { expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.SUBMITTED); }); }); + + 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: 'user', + submitsTo: managerEmail, + }, + [managerEmail]: { + email: managerEmail, + role: 'user', + submitsTo: adminEmail, + forwardsTo: adminEmail, + }, + [adminEmail]: { + email: adminEmail, + 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); + 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); + 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); + 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); + }); + }); }); From af3d1b89cb1a916bfc6100f0ba35090a37fb1451 Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 1 Oct 2025 22:50:40 +0700 Subject: [PATCH 13/24] Refactor getBypassApproverIfTakenControl to return account ID instead of email and update approval logic in IOU to handle absence of next approver --- src/libs/ReportUtils.ts | 22 ++++++++-------------- src/libs/actions/IOU.ts | 8 ++++---- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 621c856a9679..61ae9e9bca27 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4252,8 +4252,8 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals // Check if someone took control and should be the next approver const bypassApprover = getBypassApproverIfTakenControl(report); - if (bypassApprover) { - return getAccountIDsByLogins([bypassApprover]).at(0); + if (bypassApprover === currentUserAccountID) { + return null; } const approvalChain = getApprovalChain(policy, report); @@ -11388,7 +11388,7 @@ function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry, report * 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 getBypassApproverIfTakenControl(expenseReport: OnyxEntry): string | null { +function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): number | null { if (!expenseReport?.reportID) { return null; } @@ -11403,10 +11403,8 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): stri return null; } - const actions = Object.values(reportActions).filter(Boolean); - // Sort actions by created timestamp to get chronological order - const sortedActions = getSortedReportActions(actions, true); + const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true) let lastTakeControlAction: ReportAction | null = null; let lastTakeControlActorAccountID: number | null = null; @@ -11417,20 +11415,16 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): stri if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED)) { // If we find a SUBMITTED action, no take control is valid since it would be older return null; - } else if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { + } + + if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { lastTakeControlAction = action; lastTakeControlActorAccountID = action.actorAccountID ?? null; break; // Found the most recent take control, no need to continue } } - // If we have a valid take control action, return the approver email - if (lastTakeControlAction && lastTakeControlActorAccountID) { - const approverEmail = getLoginsByAccountIDs([lastTakeControlActorAccountID]).at(0); - return approverEmail ?? null; - } - - return null; + return lastTakeControlActorAccountID; } function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry): string[] { diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 0f8776b894a4..98dbb6c8a52b 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -9734,10 +9734,10 @@ function approveMoneyRequest(expenseReport: OnyxEntry, full?: // eslint-disable-next-line deprecation/deprecation const policy = getPolicy(expenseReport.policyID); const nextApproverAccountID = getNextApproverAccountID(expenseReport); - const isCurrentUserNextApprover = !nextApproverAccountID || nextApproverAccountID === userAccountID; - const predictedNextStatus = isCurrentUserNextApprover ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; - const predictedNextState = isCurrentUserNextApprover ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; - const managerID = isCurrentUserNextApprover ? expenseReport.managerID : nextApproverAccountID; + const noNextApproverExist = !nextApproverAccountID; + const predictedNextStatus = noNextApproverExist ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; + const predictedNextState = noNextApproverExist ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; + const managerID = noNextApproverExist ? expenseReport.managerID : nextApproverAccountID; const hasViolations = hasViolationsReportUtils(expenseReport.reportID, allTransactionViolations); const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas); From aea04a47c71d49dffcf4a2f02315d076305de6eb Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 1 Oct 2025 22:53:02 +0700 Subject: [PATCH 14/24] prettier --- src/libs/ReportUtils.ts | 4 ++-- tests/actions/IOUTest.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 61ae9e9bca27..8d497d7d929b 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11404,7 +11404,7 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb } // Sort actions by created timestamp to get chronological order - const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true) + const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true); let lastTakeControlAction: ReportAction | null = null; let lastTakeControlActorAccountID: number | null = null; @@ -11416,7 +11416,7 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb // If we find a SUBMITTED action, no take control is valid since it would be older return null; } - + if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { lastTakeControlAction = action; lastTakeControlActorAccountID = action.actorAccountID ?? null; diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index c3d6354f73a1..b54fad57e423 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -8984,7 +8984,7 @@ describe('actions/IOU', () => { 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, { From be361f42bb9d35fa110f47116ce0582ae1348997 Mon Sep 17 00:00:00 2001 From: fahimj Date: Thu, 2 Oct 2025 05:12:01 +0700 Subject: [PATCH 15/24] Refactor getNextApproverAccountID to return undefined instead of null for final approver, keeping its return type number | undefined --- src/libs/ReportUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 8d497d7d929b..18d8a2108b9d 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4253,7 +4253,7 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals // Check if someone took control and should be the next approver const bypassApprover = getBypassApproverIfTakenControl(report); if (bypassApprover === currentUserAccountID) { - return null; + return undefined; } const approvalChain = getApprovalChain(policy, report); @@ -4273,8 +4273,8 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals const nextApproverEmail = approvalChain.length === 1 ? approvalChain.at(0) : approvalChain.at(approvalChain.indexOf(currentUserEmail ?? '') + 1); if (!nextApproverEmail) { - // If there's no next approver in the chain, return null to indicate the current user is the final approver - return null; + // 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); From 7db3f4aa8de7ff69da88e2c26b96ee70a2107b44 Mon Sep 17 00:00:00 2001 From: fahimj Date: Thu, 2 Oct 2025 05:19:23 +0700 Subject: [PATCH 16/24] Remove unused variable lastTakeControlAction from getBypassApproverIfTakenControl function in ReportUtils.ts --- src/libs/ReportUtils.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 18d8a2108b9d..33646070d571 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11405,8 +11405,7 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb // Sort actions by created timestamp to get chronological order const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true); - - let lastTakeControlAction: ReportAction | null = null; + let lastTakeControlActorAccountID: number | null = null; // Look through actions in reverse chronological order (newest first) @@ -11418,7 +11417,6 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb } if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { - lastTakeControlAction = action; lastTakeControlActorAccountID = action.actorAccountID ?? null; break; // Found the most recent take control, no need to continue } From df79e2c8e027cd638956876984bd08916686475e Mon Sep 17 00:00:00 2001 From: fahimj Date: Thu, 2 Oct 2025 05:25:53 +0700 Subject: [PATCH 17/24] prettier --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 33646070d571..35a3797e2a10 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11405,7 +11405,7 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb // Sort actions by created timestamp to get chronological order const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true); - + let lastTakeControlActorAccountID: number | null = null; // Look through actions in reverse chronological order (newest first) From 6da3f0a13df4ced34d5385e33973c03a30f0f6d7 Mon Sep 17 00:00:00 2001 From: fahimj Date: Tue, 7 Oct 2025 02:56:04 +0700 Subject: [PATCH 18/24] Refactor getBypassApproverIfTakenControl to directly return the actorAccountID of the most recent TAKE_CONTROL action, removing the unused variable. --- src/libs/ReportUtils.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 35a3797e2a10..28438bfbbab5 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -11406,8 +11406,6 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb // Sort actions by created timestamp to get chronological order const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true); - let lastTakeControlActorAccountID: number | null = null; - // 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) { @@ -11417,12 +11415,11 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb } if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) { - lastTakeControlActorAccountID = action.actorAccountID ?? null; - break; // Found the most recent take control, no need to continue + return action.actorAccountID ?? null; } } - return lastTakeControlActorAccountID; + return null; } function getApprovalChain(policy: OnyxEntry, expenseReport: OnyxEntry): string[] { From 41a6a08a1b428609c5205b146ec0037cd4feec0f Mon Sep 17 00:00:00 2001 From: fahimj Date: Tue, 7 Oct 2025 03:47:11 +0700 Subject: [PATCH 19/24] Refactor approveMoneyRequest to use an object for parameters in buildNextStepNew, --- src/libs/actions/IOU.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 14a4414b4e94..08e592aac46e 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -9852,7 +9852,15 @@ function approveMoneyRequest(expenseReport: OnyxEntry, full?: const hasViolations = hasViolationsReportUtils(expenseReport.reportID, allTransactionViolations); const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas); - const optimisticNextStep = buildNextStepNew(expenseReport, policy, userAccountID, currentUserEmail, hasViolations, isASAPSubmitBetaEnabled, predictedNextStatus); + const optimisticNextStep = buildNextStepNew({ + report: expenseReport, + policy, + currentUserAccountIDParam: userAccountID, + currentUserEmailParam: currentUserEmail, + hasViolations, + isASAPSubmitBetaEnabled, + predictedNextStatus, + }); const chatReport = getReportOrDraftReport(expenseReport.chatReportID); const optimisticReportActionsData: OnyxUpdate = { From 18d01e55f18d8309c71a7d60f67fd86474154e38 Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 15 Oct 2025 07:00:22 +0700 Subject: [PATCH 20/24] use !nextApproverAccountID directly --- src/libs/actions/IOU.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 7da777ebbd06..641873e00d7c 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10142,10 +10142,9 @@ function approveMoneyRequest(expenseReport: OnyxEntry, full?: // eslint-disable-next-line deprecation/deprecation const policy = getPolicy(expenseReport.policyID); const nextApproverAccountID = getNextApproverAccountID(expenseReport); - const noNextApproverExist = !nextApproverAccountID; - const predictedNextStatus = noNextApproverExist ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED; - const predictedNextState = noNextApproverExist ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; - const managerID = noNextApproverExist ? expenseReport.managerID : nextApproverAccountID; + 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 hasViolations = hasViolationsReportUtils(expenseReport.reportID, allTransactionViolations); const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas); From b150943f3913b5e2af9df0fd926d47ee7e975d6a Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 15 Oct 2025 07:04:51 +0700 Subject: [PATCH 21/24] update comments --- src/libs/ReportUtils.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index bfc080681eff..43f4fe4b73d4 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4297,7 +4297,7 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals // eslint-disable-next-line deprecation/deprecation const policy = getPolicy(report?.policyID); - // Check if someone took control and should be the next approver + // If the current user took control, then they are the final approver and we don't have a next approver const bypassApprover = getBypassApproverIfTakenControl(report); if (bypassApprover === currentUserAccountID) { return undefined; @@ -11448,7 +11448,6 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb return null; } - // Early return if report is not in processing state if (!isProcessingReport(expenseReport)) { return null; } @@ -11465,7 +11464,6 @@ function getBypassApproverIfTakenControl(expenseReport: OnyxEntry): numb // 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)) { - // If we find a SUBMITTED action, no take control is valid since it would be older return null; } From 60e21a7abef1db076d649d33aa8e1ec65c1df625 Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 15 Oct 2025 07:17:57 +0700 Subject: [PATCH 22/24] Refactor role assignments in IOUTest to use constants for better maintainability --- tests/actions/IOUTest.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 4fcceca6981e..ce9b5fd7c744 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -3997,7 +3997,7 @@ describe('actions/IOU', () => { taxCode: '', policy: { id: '123', - role: 'user', + role: CONST.POLICY.ROLE.USER, type: CONST.POLICY.TYPE.TEAM, name: '', owner: '', @@ -6414,7 +6414,7 @@ describe('actions/IOU', () => { taxCode: '', policy: { id: '123', - role: 'user', + role: CONST.POLICY.ROLE.USER, type: CONST.POLICY.TYPE.TEAM, name: '', owner: '', @@ -6474,7 +6474,7 @@ describe('actions/IOU', () => { taxCode: '', policy: { id: '123', - role: 'user', + role: CONST.POLICY.ROLE.USER, type: CONST.POLICY.TYPE.TEAM, name: '', owner: '', @@ -8789,18 +8789,18 @@ describe('actions/IOU', () => { employeeList: { [employeeEmail]: { email: employeeEmail, - role: 'user', + role: CONST.POLICY.ROLE.USER, submitsTo: managerEmail, }, [managerEmail]: { email: managerEmail, - role: 'user', + role: CONST.POLICY.ROLE.USER, submitsTo: adminEmail, forwardsTo: adminEmail, }, [adminEmail]: { email: adminEmail, - role: 'admin', + role: CONST.POLICY.ROLE.ADMIN, submitsTo: '', forwardsTo: '', }, From a13a819325c98c4b589835dc8cae8eb4474146ba Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 15 Oct 2025 07:22:50 +0700 Subject: [PATCH 23/24] remove redundant take control SUBMITTED_AND_CLOSED test case --- tests/actions/IOUTest.ts | 44 +++------------------------------------- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index ce9b5fd7c744..d955f663ac33 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -8888,44 +8888,6 @@ describe('actions/IOU', () => { expect(updatedReport?.stateNum).toBe(CONST.REPORT.STATE_NUM.SUBMITTED); expect(updatedReport?.statusNum).toBe(CONST.REPORT.STATUS_NUM.SUBMITTED); }); - - it('should handle take control with SUBMITTED_AND_CLOSED action invalidation', async () => { - // Admin takes control - const takeControlAction = { - reportActionID: 'takeControl7', - actionName: CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL, - actorAccountID: adminAccountID, - created: '2023-01-01T10:00:00.000Z', - }; - - // Employee submits and closes after take control (invalidates it) - const submittedAndClosedAction = { - reportActionID: 'submittedClosed1', - actionName: CONST.REPORT.ACTIONS.TYPE.SUBMITTED_AND_CLOSED, - actorAccountID: employeeAccountID, - created: '2023-01-01T11:00:00.000Z', // After take control - }; - - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, { - [takeControlAction.reportActionID]: takeControlAction, - [submittedAndClosedAction.reportActionID]: submittedAndClosedAction, - }); - - // Set session as manager (normal approver) - await Onyx.set(ONYXKEYS.SESSION, { - email: managerEmail, - accountID: managerAccountID, - }); - - // Manager approves the report - approveMoneyRequest(expenseReport); - await waitForBatchedUpdates(); - - // Should be submitted to admin (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); - }); }); describe('approveMoneyRequest with normal approval chain', () => { @@ -8974,18 +8936,18 @@ describe('actions/IOU', () => { employeeList: { [employeeEmail]: { email: employeeEmail, - role: 'user', + role: CONST.POLICY.ROLE.USER, submitsTo: managerEmail, }, [managerEmail]: { email: managerEmail, - role: 'user', + role: CONST.POLICY.ROLE.USER, submitsTo: adminEmail, forwardsTo: adminEmail, }, [adminEmail]: { email: adminEmail, - role: 'admin', + role: CONST.POLICY.ROLE.ADMIN, submitsTo: '', forwardsTo: '', }, From 4b6dc18c65eb8b3b721a63ffcee386c96dd144ba Mon Sep 17 00:00:00 2001 From: fahimj Date: Wed, 15 Oct 2025 07:32:11 +0700 Subject: [PATCH 24/24] Remove unused personal details entry from ReportUtilsTest --- tests/unit/ReportUtilsTest.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 2092ace54df7..67b3aa2dbf19 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -269,10 +269,6 @@ const personalDetails: PersonalDetailsList = { accountID: 8, login: CONST.EMAIL.GUIDES_DOMAIN, }, - '9': { - accountID: 9, - login: 'manager@test.com', - }, }; const rules = {