From 3ec0542bf73eb70e661b022fda578e51fc9ad278 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 19 Apr 2024 15:14:43 +0800 Subject: [PATCH 1/4] replace policy.submitsTo with PolicyUtils.getSubmitToAccountID --- src/libs/NextStepUtils.ts | 8 +++++--- src/libs/PolicyUtils.ts | 32 +++++++++++++++++++++++++++++++- src/libs/ReportUtils.ts | 9 +++++---- src/libs/actions/IOU.ts | 2 +- tests/unit/NextStepUtilsTest.ts | 32 ++++++++++++++++++++++++++------ 5 files changed, 68 insertions(+), 15 deletions(-) diff --git a/src/libs/NextStepUtils.ts b/src/libs/NextStepUtils.ts index 3b8fce748f458..66f63ddfd27f4 100644 --- a/src/libs/NextStepUtils.ts +++ b/src/libs/NextStepUtils.ts @@ -12,6 +12,7 @@ import type {EmptyObject} from '@src/types/utils/EmptyObject'; import DateUtils from './DateUtils'; import EmailUtils from './EmailUtils'; import * as PersonalDetailsUtils from './PersonalDetailsUtils'; +import * as PolicyUtils from './PolicyUtils'; import * as ReportUtils from './ReportUtils'; let currentUserAccountID = -1; @@ -81,12 +82,13 @@ function buildNextStep( const {policyID = '', ownerAccountID = -1, managerID = -1} = report; const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`] ?? ({} as Policy); - const {submitsTo, harvesting, preventSelfApproval, autoReportingFrequency, autoReportingOffset} = policy; + const {harvesting, preventSelfApproval, autoReportingFrequency, autoReportingOffset} = policy; + const submitToAccountID = PolicyUtils.getSubmitToAccountID(policy, ownerAccountID); const isOwner = currentUserAccountID === ownerAccountID; const isManager = currentUserAccountID === managerID; - const isSelfApproval = currentUserAccountID === submitsTo; + const isSelfApproval = currentUserAccountID === submitToAccountID; const ownerLogin = PersonalDetailsUtils.getLoginsByAccountIDs([ownerAccountID])[0] ?? ''; - const managerDisplayName = isSelfApproval ? 'you' : ReportUtils.getDisplayNameForParticipant(submitsTo) ?? ''; + const managerDisplayName = isSelfApproval ? 'you' : ReportUtils.getDisplayNameForParticipant(submitToAccountID) ?? ''; const type: ReportNextStep['type'] = 'neutral'; let optimisticNextStep: ReportNextStep | null; diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index b4cf4b164a19d..40c7f0b6a2e1b 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -11,7 +11,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import getPolicyIDFromState from './Navigation/getPolicyIDFromState'; import Navigation, {navigationRef} from './Navigation/Navigation'; import type {RootStackParamList, State} from './Navigation/types'; -import {getPersonalDetailByEmail} from './PersonalDetailsUtils'; +import {getAccountIDsByLogins, getLoginsByAccountIDs, getPersonalDetailByEmail} from './PersonalDetailsUtils'; type MemberEmailsToAccountIDs = Record; @@ -311,6 +311,35 @@ function isPolicyFeatureEnabled(policy: OnyxEntry | EmptyObject, feature return Boolean(policy?.[featureName]); } +function getApprovalWorkflow(policy: OnyxEntry | EmptyObject): ValueOf { + if (policy?.type === CONST.POLICY.TYPE.PERSONAL) { + return CONST.POLICY.APPROVAL_MODE.OPTIONAL; + } + + return policy?.approvalMode ?? CONST.POLICY.APPROVAL_MODE.ADVANCED; +} + +function getDefaultApprover(policy: OnyxEntry | EmptyObject): string { + return policy?.approver ?? policy?.owner ?? ''; +} + +function getSubmitToAccountID(policy: OnyxEntry | EmptyObject, employeeAccountID: number): number { + const employeeLogin = getLoginsByAccountIDs([employeeAccountID])[0]; + const defaultApprover = getDefaultApprover(policy); + // For policy using the optional or basic workflow, the manager is the policy default approver. + if (([CONST.POLICY.APPROVAL_MODE.OPTIONAL, CONST.POLICY.APPROVAL_MODE.BASIC] as Array>).includes(getApprovalWorkflow(policy))) { + return getAccountIDsByLogins([defaultApprover])[0]; + } + + const employee = policy?.employeeList?.[employeeLogin]; + + if (!employee) { + return -1; + } + + return getAccountIDsByLogins([employee.submitsTo ?? defaultApprover])[0]; +} + /** * Get the currently selected policy ID stored in the navigation state. */ @@ -357,6 +386,7 @@ export { getTaxByID, hasPolicyCategoriesError, getPolicyIDFromNavigationState, + getSubmitToAccountID, }; export type {MemberEmailsToAccountIDs}; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index a15e1937dbe2b..6c4dcfc71a571 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3430,9 +3430,10 @@ function buildOptimisticExpenseReport(chatReportID: string, policyID: string, pa lastVisibleActionCreated: DateUtils.getDBTime(), }; + const submitToAccountID = PolicyUtils.getSubmitToAccountID(policy, payeeAccountID); // The account defined in the policy submitsTo field is the approver/ manager for this report - if (policy?.submitsTo) { - expenseReport.managerID = policy.submitsTo; + if (submitToAccountID) { + expenseReport.managerID = submitToAccountID; } const titleReportField = getTitleReportField(getReportFieldsByPolicyID(policyID) ?? {}); @@ -5870,9 +5871,9 @@ function isAllowedToApproveExpenseReport(report: OnyxEntry, approverAcco function isAllowedToSubmitDraftExpenseReport(report: OnyxEntry): boolean { const policy = getPolicy(report?.policyID); - const {submitsTo} = policy; + const submitToAccountID = PolicyUtils.getSubmitToAccountID(policy, report?.ownerAccountID ?? -1); - return isAllowedToApproveExpenseReport(report, submitsTo); + return isAllowedToApproveExpenseReport(report, submitToAccountID); } /** diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 7fed15335e2af..e74d2978d7717 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -5612,7 +5612,7 @@ function submitReport(expenseReport: OnyxTypes.Report) { const parameters: SubmitReportParams = { reportID: expenseReport.reportID, - managerAccountID: policy.submitsTo ?? expenseReport.managerID, + managerAccountID: PolicyUtils.getSubmitToAccountID(policy, expenseReport.ownerAccountID ?? -1) ?? expenseReport.managerID, reportActionID: optimisticSubmittedReportAction.reportActionID, }; diff --git a/tests/unit/NextStepUtilsTest.ts b/tests/unit/NextStepUtilsTest.ts index 072a06748da90..7af6d35302bf1 100644 --- a/tests/unit/NextStepUtilsTest.ts +++ b/tests/unit/NextStepUtilsTest.ts @@ -22,7 +22,6 @@ describe('libs/NextStepUtils', () => { // Important props id: policyID, owner: currentUserEmail, - submitsTo: currentUserAccountID, harvesting: { enabled: false, }, @@ -51,6 +50,11 @@ describe('libs/NextStepUtils', () => { login: strangeEmail, avatar: '', }, + [currentUserAccountID]: { + accountID: currentUserAccountID, + login: currentUserEmail, + avatar: '', + }, }, ...policyCollectionDataSet, }).then(waitForBatchedUpdates); @@ -341,10 +345,14 @@ describe('libs/NextStepUtils', () => { ]; return Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { - submitsTo: currentUserAccountID, preventSelfApproval: true, + employeeList: { + [currentUserEmail]: { + submitsTo: currentUserEmail, + }, + }, }).then(() => { - const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.OPEN); + const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.OPEN, undefined, true); expect(result).toMatchObject(optimisticNextStep); }); @@ -403,7 +411,11 @@ describe('libs/NextStepUtils', () => { ]; return Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { - submitsTo: strangeAccountID, + employeeList: { + [currentUserEmail]: { + submitsTo: strangeEmail, + }, + }, }).then(() => { const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.SUBMITTED); @@ -438,9 +450,17 @@ describe('libs/NextStepUtils', () => { }, ]; - const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.SUBMITTED); + return Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { + employeeList: { + [strangeEmail]: { + submitsTo: currentUserEmail, + }, + }, + }).then(() => { + const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.SUBMITTED); - expect(result).toMatchObject(optimisticNextStep); + expect(result).toMatchObject(optimisticNextStep); + }); }); test('submit and close approval mode', () => { From 28e8945cc5138ec1e91d2b5b81b28ed43272a373 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 19 Apr 2024 15:38:33 +0800 Subject: [PATCH 2/4] remove code that is added when debugging --- tests/unit/NextStepUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/NextStepUtilsTest.ts b/tests/unit/NextStepUtilsTest.ts index 7af6d35302bf1..65ce7352b6e62 100644 --- a/tests/unit/NextStepUtilsTest.ts +++ b/tests/unit/NextStepUtilsTest.ts @@ -352,7 +352,7 @@ describe('libs/NextStepUtils', () => { }, }, }).then(() => { - const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.OPEN, undefined, true); + const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.OPEN); expect(result).toMatchObject(optimisticNextStep); }); From bae212fd87ccb52eb6a1a12f82f9b5b417c2dffc Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 19 Apr 2024 22:52:53 +0800 Subject: [PATCH 3/4] tidying up and update comment --- src/libs/PolicyUtils.ts | 2 +- src/libs/ReportUtils.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index 40c7f0b6a2e1b..225e422a0ffcd 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -326,13 +326,13 @@ function getDefaultApprover(policy: OnyxEntry | EmptyObject): string { function getSubmitToAccountID(policy: OnyxEntry | EmptyObject, employeeAccountID: number): number { const employeeLogin = getLoginsByAccountIDs([employeeAccountID])[0]; const defaultApprover = getDefaultApprover(policy); + // For policy using the optional or basic workflow, the manager is the policy default approver. if (([CONST.POLICY.APPROVAL_MODE.OPTIONAL, CONST.POLICY.APPROVAL_MODE.BASIC] as Array>).includes(getApprovalWorkflow(policy))) { return getAccountIDsByLogins([defaultApprover])[0]; } const employee = policy?.employeeList?.[employeeLogin]; - if (!employee) { return -1; } diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 6c4dcfc71a571..64aceba13ac42 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3430,8 +3430,8 @@ function buildOptimisticExpenseReport(chatReportID: string, policyID: string, pa lastVisibleActionCreated: DateUtils.getDBTime(), }; + // Get the approver/manager for this report to properly display the optimistic data const submitToAccountID = PolicyUtils.getSubmitToAccountID(policy, payeeAccountID); - // The account defined in the policy submitsTo field is the approver/ manager for this report if (submitToAccountID) { expenseReport.managerID = submitToAccountID; } From 10ff1935692695a473f78ffaffb336db1e2975d9 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Mon, 22 Apr 2024 23:13:25 +0800 Subject: [PATCH 4/4] add comment --- src/libs/PolicyUtils.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index de2731bba752e..731dc5700c8ee 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -333,6 +333,9 @@ function getDefaultApprover(policy: OnyxEntry | EmptyObject): string { return policy?.approver ?? policy?.owner ?? ''; } +/** + * Returns the accountID to whom the given employeeAccountID submits reports to in the given Policy. + */ function getSubmitToAccountID(policy: OnyxEntry | EmptyObject, employeeAccountID: number): number { const employeeLogin = getLoginsByAccountIDs([employeeAccountID])[0]; const defaultApprover = getDefaultApprover(policy);