From df343436db6d1a3b523647dbcdbbddd545dd4c65 Mon Sep 17 00:00:00 2001 From: emkhalid Date: Sat, 16 May 2026 18:21:23 +0430 Subject: [PATCH 1/5] Fix next-step approver selection after report submit --- src/libs/ReportUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 8f689fa18919..59563e2e6140 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -302,7 +302,7 @@ import addTrailingForwardSlash from './UrlUtils'; import type {AvatarSource} from './UserAvatarUtils'; import {getDefaultAvatarURL} from './UserAvatarUtils'; import {generateAccountID} from './UserUtils'; -import {isInvalidMerchantValue} from './ValidationUtils'; +import {isInvalidMerchantValue, isValidAccountRoute} from './ValidationUtils'; import ViolationsUtils from './Violations/ViolationsUtils'; type AvatarRange = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18; @@ -4728,7 +4728,7 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals return deprecatedCurrentUserAccountID; } - return report?.managerID ?? submitToAccountID; + return isValidAccountRoute(submitToAccountID) ? submitToAccountID : report?.managerID; } if (approvalChain.length === 0) { From 3a2118d66cfe5a1e5f6c0b622f994c1ae4e40a47 Mon Sep 17 00:00:00 2001 From: emkhalid Date: Sat, 16 May 2026 18:31:47 +0430 Subject: [PATCH 2/5] Fix next approver fallback for unapproved reports tests --- tests/unit/ReportUtilsTest.ts | 60 +++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 6478f908203a..51e84fd4202b 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -441,7 +441,9 @@ const rules = { }; const employeeAccountID = 2; +const policyAdminAccountID = 1; const categoryApprover1Email = 'categoryapprover1@test.com'; +const categoryApprover1AccountID = 3; const categoryApprover2Email = 'categoryapprover2@test.com'; const tagApprover1Email = 'tagapprover1@test.com'; const tagApprover2Email = 'tagapprover2@test.com'; @@ -7845,6 +7847,64 @@ describe('ReportUtils', () => { }); }); + describe('getNextApproverAccountID', () => { + beforeEach(async () => { + await Onyx.clear(); + await Onyx.merge(ONYXKEYS.SESSION, {email: 'employee@test.com', accountID: employeeAccountID}); + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails); + await waitForBatchedUpdates(); + }); + + afterEach(async () => { + await Onyx.clear(); + await Onyx.merge(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID}); + await waitForBatchedUpdates(); + }); + + it('should prefer a valid submit target over a stale report manager when unapproving', async () => { + const policyID = 'next-approver-policy'; + const policyTest: Policy = { + ...createRandomPolicy(0), + id: policyID, + approver: 'owner@test.com', + owner: 'owner@test.com', + type: CONST.POLICY.TYPE.CORPORATE, + employeeList, + rules, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + }; + const expenseReport: Report = { + ...createRandomReport(100, undefined), + ownerAccountID: employeeAccountID, + managerID: policyAdminAccountID, + policyID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + const transaction: Transaction = { + ...createRandomTransaction(0), + category: 'cat1', + reportID: expenseReport.reportID, + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyTest); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); + await waitForBatchedUpdates(); + + expect(getNextApproverAccountID(expenseReport, true)).toBe(categoryApprover1AccountID); + }); + + it('should fall back to report manager when submit target is invalid', async () => { + const expenseReport: Report = { + ...createRandomReport(101, undefined), + ownerAccountID: employeeAccountID, + managerID: policyAdminAccountID, + type: CONST.REPORT.TYPE.EXPENSE, + }; + + expect(getNextApproverAccountID(expenseReport, true)).toBe(policyAdminAccountID); + }); + }); + describe('shouldReportShowSubscript', () => { afterEach(async () => { await Onyx.clear(); From f9894b841e41c3d2fd06d926619b6a766f9967bc Mon Sep 17 00:00:00 2001 From: emkhalid Date: Sun, 31 May 2026 12:32:01 +0430 Subject: [PATCH 3/5] Fix optimistic next-step approver without affecting unapprove flow --- src/libs/ReportUtils.ts | 4 +- src/libs/actions/IOU/ReportWorkflow.ts | 4 + tests/actions/IOUTest/ReportWorkflowTest.ts | 96 ++++++++++++++++++++- tests/unit/ReportUtilsTest.ts | 5 +- 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 5f196dc3029c..bb9efc4e39fc 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -309,7 +309,7 @@ import addTrailingForwardSlash from './UrlUtils'; import type {AvatarSource} from './UserAvatarUtils'; import {getDefaultAvatarURL} from './UserAvatarUtils'; import {generateAccountID} from './UserUtils'; -import {isInvalidMerchantValue, isValidAccountRoute} from './ValidationUtils'; +import {isInvalidMerchantValue} from './ValidationUtils'; import ViolationsUtils from './Violations/ViolationsUtils'; type AvatarRange = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18; @@ -4792,7 +4792,7 @@ function getNextApproverAccountID(report: OnyxEntry, isUnapproved = fals return deprecatedCurrentUserAccountID; } - return isValidAccountRoute(submitToAccountID) ? submitToAccountID : report?.managerID; + return report?.managerID ?? submitToAccountID; } if (approvalChain.length === 0) { diff --git a/src/libs/actions/IOU/ReportWorkflow.ts b/src/libs/actions/IOU/ReportWorkflow.ts index bf5f7977a823..217a2df43c35 100644 --- a/src/libs/actions/IOU/ReportWorkflow.ts +++ b/src/libs/actions/IOU/ReportWorkflow.ts @@ -63,6 +63,7 @@ import { isScanning, isScanningTransaction, } from '@libs/TransactionUtils'; +import {isValidAccountRoute} from '@libs/ValidationUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; @@ -1301,6 +1302,7 @@ function submitReport({ const adminAccountID = policy?.role === CONST.POLICY.ROLE.ADMIN ? currentUserAccountIDParam : undefined; const parentReport = getReportOrDraftReport(expenseReport.parentReportID); const managerID = getSubmitReportManagerAccountID(policy, expenseReport); + const optimisticNextStepApproverID = !isSubmitAndClosePolicy && isValidAccountRoute(managerID ?? CONST.DEFAULT_NUMBER_ID) ? managerID : undefined; const isCurrentUserManager = currentUserAccountIDParam === managerID; const optimisticSubmittedReportAction = buildOptimisticSubmittedReportAction( expenseReport?.total ?? 0, @@ -1326,6 +1328,7 @@ function submitReport({ hasViolations, isASAPSubmitBetaEnabled, isUnapprove: true, + bypassNextApproverID: optimisticNextStepApproverID, }); const optimisticNextStep = isDEWPolicy ? null @@ -1338,6 +1341,7 @@ function submitReport({ hasViolations, isASAPSubmitBetaEnabled, isUnapprove: true, + bypassNextApproverID: optimisticNextStepApproverID, }); const optimisticData: Array< OnyxUpdate diff --git a/tests/actions/IOUTest/ReportWorkflowTest.ts b/tests/actions/IOUTest/ReportWorkflowTest.ts index 01de01f90443..4db25011f703 100644 --- a/tests/actions/IOUTest/ReportWorkflowTest.ts +++ b/tests/actions/IOUTest/ReportWorkflowTest.ts @@ -30,7 +30,7 @@ import * as API from '@src/libs/API'; import DateUtils from '@src/libs/DateUtils'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; -import type {Policy, Report, ReportNameValuePairs} from '@src/types/onyx'; +import type {Policy, Report, ReportNameValuePairs, ReportNextStepDeprecated} from '@src/types/onyx'; import type ReportAction from '@src/types/onyx/ReportAction'; import type {ReportActions} from '@src/types/onyx/ReportAction'; import type {OnyxData} from '@src/types/onyx/Request'; @@ -1389,6 +1389,100 @@ describe('actions/IOU/ReportWorkflow', () => { const optimisticReportUpdate = onyxData.optimisticData?.find((update) => update.key === `${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`); expect((optimisticReportUpdate?.value as Report | undefined)?.managerID).toBe(adminAccountID); }); + + it('uses the rule approver in the optimistic next step when the existing report manager is stale', async () => { + // eslint-disable-next-line rulesdir/no-multiple-api-calls -- Inspecting API.write calls to verify submit payload and optimistic data. + const apiWriteSpy = jest.spyOn(API, 'write').mockImplementation(() => Promise.resolve()); + const policyID = '1'; + const submitterAccountID = 100; + const defaultApproverAccountID = 101; + const ruleApproverAccountID = 102; + const submitterEmail = 'submitter@example.com'; + const defaultApproverEmail = 'default-approver@example.com'; + const ruleApproverEmail = 'rule-approver@example.com'; + + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [submitterAccountID]: {accountID: submitterAccountID, login: submitterEmail}, + [defaultApproverAccountID]: {accountID: defaultApproverAccountID, login: defaultApproverEmail}, + [ruleApproverAccountID]: {accountID: ruleApproverAccountID, login: ruleApproverEmail}, + }); + + const policy: Policy = { + ...createRandomPolicy(Number(policyID)), + id: policyID, + type: CONST.POLICY.TYPE.CORPORATE, + approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, + approver: defaultApproverEmail, + owner: defaultApproverEmail, + employeeList: { + [submitterEmail]: { + email: submitterEmail, + submitsTo: defaultApproverEmail, + }, + }, + rules: { + approvalRules: [ + { + id: 'travel-rule', + applyWhen: [ + { + field: CONST.POLICY.FIELDS.CATEGORY, + condition: CONST.POLICY.RULE_CONDITIONS.MATCHES, + value: 'Travel', + }, + ], + approver: ruleApproverEmail, + }, + ], + }, + }; + const expenseReport: Report = { + ...createRandomReport(Number(policyID), undefined), + reportID: '1', + policyID, + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: submitterAccountID, + managerID: defaultApproverAccountID, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + total: 1000, + currency: CONST.CURRENCY.USD, + }; + const transaction: Transaction = { + ...createRandomTransaction(1), + reportID: expenseReport.reportID, + category: 'Travel', + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); + await waitForBatchedUpdates(); + + submitReport({ + expenseReport, + policy, + currentUserAccountIDParam: submitterAccountID, + currentUserEmailParam: submitterEmail, + hasViolations: false, + isASAPSubmitBetaEnabled: false, + expenseReportCurrentNextStepDeprecated: undefined, + userBillingGracePeriodEnds: undefined, + amountOwed: 0, + ownerBillingGracePeriodEnd: undefined, + delegateEmail: undefined, + }); + + const [, parameters, onyxData] = apiWriteSpy.mock.calls.at(-1) as [unknown, {managerAccountID?: number}, OnyxData]; + expect(parameters.managerAccountID).toBe(ruleApproverAccountID); + + const optimisticReportUpdate = onyxData.optimisticData?.find((update) => update.key === `${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`); + expect((optimisticReportUpdate?.value as Report | undefined)?.managerID).toBe(ruleApproverAccountID); + expect((optimisticReportUpdate?.value as Report | undefined)?.nextStep?.actorAccountID).toBe(ruleApproverAccountID); + + const optimisticDeprecatedNextStepUpdate = onyxData.optimisticData?.find((update) => update.key === `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`); + const optimisticDeprecatedNextStep = optimisticDeprecatedNextStepUpdate?.value as ReportNextStepDeprecated | undefined; + expect(optimisticDeprecatedNextStep?.message?.find((message) => message.type === 'strong')?.text).toBe(ruleApproverEmail); + }); + it('keeps the workspace chat outstanding when an admin submits after approver changes', async () => { // eslint-disable-next-line rulesdir/no-multiple-api-calls -- Inspecting optimistic parent chat data after submit from workspace chat. const apiWriteSpy = jest.spyOn(API, 'write').mockImplementation(() => Promise.resolve()); diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 4770292da3ab..4c8cc35b6e39 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -452,7 +452,6 @@ const rules = { const employeeAccountID = 2; const policyAdminAccountID = 1; const categoryApprover1Email = 'categoryapprover1@test.com'; -const categoryApprover1AccountID = 3; const categoryApprover2Email = 'categoryapprover2@test.com'; const tagApprover1Email = 'tagapprover1@test.com'; const tagApprover2Email = 'tagapprover2@test.com'; @@ -8018,7 +8017,7 @@ describe('ReportUtils', () => { await waitForBatchedUpdates(); }); - it('should prefer a valid submit target over a stale report manager when unapproving', async () => { + it('should use the report manager when unapproving even if the submit target differs', async () => { const policyID = 'next-approver-policy'; const policyTest: Policy = { ...createRandomPolicy(0), @@ -8047,7 +8046,7 @@ describe('ReportUtils', () => { await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); await waitForBatchedUpdates(); - expect(getNextApproverAccountID(expenseReport, true)).toBe(categoryApprover1AccountID); + expect(getNextApproverAccountID(expenseReport, true)).toBe(policyAdminAccountID); }); it('should fall back to report manager when submit target is invalid', async () => { From 8bf25e74cc07d49cfe1ca9003b5cbb76f788f0d8 Mon Sep 17 00:00:00 2001 From: emkhalid Date: Tue, 2 Jun 2026 19:00:20 +0430 Subject: [PATCH 4/5] Remove unnecessary getNextApproverAccountID tests --- tests/unit/ReportUtilsTest.ts | 59 ----------------------------------- 1 file changed, 59 deletions(-) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 13e10b4f3b3d..8cf8f2cc0b29 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -450,7 +450,6 @@ const rules = { }; const employeeAccountID = 2; -const policyAdminAccountID = 1; const categoryApprover1Email = 'categoryapprover1@test.com'; const categoryApprover2Email = 'categoryapprover2@test.com'; const tagApprover1Email = 'tagapprover1@test.com'; @@ -8003,64 +8002,6 @@ describe('ReportUtils', () => { }); }); - describe('getNextApproverAccountID', () => { - beforeEach(async () => { - await Onyx.clear(); - await Onyx.merge(ONYXKEYS.SESSION, {email: 'employee@test.com', accountID: employeeAccountID}); - await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails); - await waitForBatchedUpdates(); - }); - - afterEach(async () => { - await Onyx.clear(); - await Onyx.merge(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID}); - await waitForBatchedUpdates(); - }); - - it('should use the report manager when unapproving even if the submit target differs', async () => { - const policyID = 'next-approver-policy'; - const policyTest: Policy = { - ...createRandomPolicy(0), - id: policyID, - approver: 'owner@test.com', - owner: 'owner@test.com', - type: CONST.POLICY.TYPE.CORPORATE, - employeeList, - rules, - approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED, - }; - const expenseReport: Report = { - ...createRandomReport(100, undefined), - ownerAccountID: employeeAccountID, - managerID: policyAdminAccountID, - policyID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - const transaction: Transaction = { - ...createRandomTransaction(0), - category: 'cat1', - reportID: expenseReport.reportID, - }; - - await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyTest); - await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); - await waitForBatchedUpdates(); - - expect(getNextApproverAccountID(expenseReport, true)).toBe(policyAdminAccountID); - }); - - it('should fall back to report manager when submit target is invalid', async () => { - const expenseReport: Report = { - ...createRandomReport(101, undefined), - ownerAccountID: employeeAccountID, - managerID: policyAdminAccountID, - type: CONST.REPORT.TYPE.EXPENSE, - }; - - expect(getNextApproverAccountID(expenseReport, true)).toBe(policyAdminAccountID); - }); - }); - describe('shouldReportShowSubscript', () => { afterEach(async () => { await Onyx.clear(); From 60d7af48338201a83f82c2f78bedfb9cdd21f6e2 Mon Sep 17 00:00:00 2001 From: emkhalid Date: Thu, 4 Jun 2026 18:10:48 +0430 Subject: [PATCH 5/5] Simplify optimistic next-step approver validation --- src/libs/actions/IOU/ReportWorkflow.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/IOU/ReportWorkflow.ts b/src/libs/actions/IOU/ReportWorkflow.ts index c61ec2bbcb7b..9d42d25c11f1 100644 --- a/src/libs/actions/IOU/ReportWorkflow.ts +++ b/src/libs/actions/IOU/ReportWorkflow.ts @@ -1312,7 +1312,7 @@ function submitReport({ const adminAccountID = policy?.role === CONST.POLICY.ROLE.ADMIN ? currentUserAccountIDParam : undefined; const parentReport = getReportOrDraftReport(expenseReport.parentReportID); const managerID = getSubmitReportManagerAccountID(policy, expenseReport); - const optimisticNextStepApproverID = !isSubmitAndClosePolicy && isValidAccountRoute(managerID ?? CONST.DEFAULT_NUMBER_ID) ? managerID : undefined; + const optimisticNextStepApproverID = !isSubmitAndClosePolicy && managerID !== undefined && isValidAccountRoute(managerID) ? managerID : undefined; const isCurrentUserManager = currentUserAccountIDParam === managerID; const optimisticSubmittedReportAction = buildOptimisticSubmittedReportAction( expenseReport?.total ?? 0,