From f07344be3a83bd25e0713dfdbd15efdd9952649e Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 18:16:23 +0100 Subject: [PATCH 01/12] Add some default policy values --- src/libs/actions/Policy.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libs/actions/Policy.ts b/src/libs/actions/Policy.ts index 3c34e823ac9a..8dc2a3cd0b3d 100644 --- a/src/libs/actions/Policy.ts +++ b/src/libs/actions/Policy.ts @@ -1938,6 +1938,11 @@ function createDraftInitialWorkspace(policyOwnerEmail = '', policyName = '', pol pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, customUnits, makeMeAdmin, + autoReporting: true, + approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL, + harvesting: { + enabled: true, + } }, }, { From 0860477bea7f31d39e314f5f0ee183d206546c99 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 18:33:35 +0100 Subject: [PATCH 02/12] Make sure we create the workspace with correct harvesting and approval mode defaults --- src/libs/actions/Policy.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libs/actions/Policy.ts b/src/libs/actions/Policy.ts index 8dc2a3cd0b3d..dabc1b0890e4 100644 --- a/src/libs/actions/Policy.ts +++ b/src/libs/actions/Policy.ts @@ -2001,6 +2001,11 @@ function createWorkspace(policyOwnerEmail = '', makeMeAdmin = false, policyName isPolicyExpenseChatEnabled: true, outputCurrency, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + autoReporting: true, + approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL, + harvesting: { + enabled: true, + }, customUnits, areCategoriesEnabled: true, areTagsEnabled: false, @@ -2489,6 +2494,11 @@ function createWorkspaceFromIOUPayment(iouReport: Report | EmptyObject): string // Setting the currency to USD as we can only add the VBBA for this policy currency right now outputCurrency: CONST.CURRENCY.USD, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + autoReporting: true, + approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL, + harvesting: { + enabled: true, + }, customUnits, areCategoriesEnabled: true, areTagsEnabled: false, From 3131c5d66431fdf5bc4a00cdaa7c24a3dbca8e91 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 18:35:51 +0100 Subject: [PATCH 03/12] Remove deprecated isPreventSelfApprovalEnabled policy property --- src/libs/NextStepUtils.ts | 4 ++-- src/types/onyx/Policy.ts | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libs/NextStepUtils.ts b/src/libs/NextStepUtils.ts index 5a19e68afe72..7a0b638ddce1 100644 --- a/src/libs/NextStepUtils.ts +++ b/src/libs/NextStepUtils.ts @@ -73,7 +73,7 @@ function buildNextStep( const {policyID = '', ownerAccountID = -1, managerID = -1} = report; const policy = ReportUtils.getPolicy(policyID); - const {submitsTo, harvesting, isPreventSelfApprovalEnabled, preventSelfApproval, autoReportingFrequency, autoReportingOffset} = policy; + const {submitsTo, harvesting, preventSelfApproval, autoReportingFrequency, autoReportingOffset} = policy; const isOwner = currentUserAccountID === ownerAccountID; const isManager = currentUserAccountID === managerID; const isSelfApproval = currentUserAccountID === submitsTo; @@ -164,7 +164,7 @@ function buildNextStep( } // Prevented self submitting - if ((isPreventSelfApprovalEnabled ?? preventSelfApproval) && isSelfApproval) { + if (preventSelfApproval && isSelfApproval) { optimisticNextStep.message = [ { text: "Oops! Looks like you're submitting to ", diff --git a/src/types/onyx/Policy.ts b/src/types/onyx/Policy.ts index 247eb64f48e9..d33190d9cb71 100644 --- a/src/types/onyx/Policy.ts +++ b/src/types/onyx/Policy.ts @@ -316,9 +316,6 @@ type Policy = OnyxCommon.OnyxValueWithOfflineFeedback< enabled: boolean; }; - /** @deprecated Whether the scheduled submit is enabled */ - isPreventSelfApprovalEnabled?: boolean; - /** Whether the self approval or submitting is enabled */ preventSelfApproval?: boolean; From ebf0eaa25c0ce9bc235db9968caa182e3eb38680 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 22:37:58 +0100 Subject: [PATCH 04/12] Create correct next steps when the report is submitted in the submit and close approval flow --- src/libs/NextStepUtils.ts | 14 ++++++++++++++ src/libs/actions/IOU.ts | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libs/NextStepUtils.ts b/src/libs/NextStepUtils.ts index 50a424bcee8d..0e76596ba8fa 100644 --- a/src/libs/NextStepUtils.ts +++ b/src/libs/NextStepUtils.ts @@ -255,6 +255,20 @@ function buildNextStep( break; } + // Generates an optimistic nextStep once a report has been closed for example in the case of Submit and Close approval flow + case CONST.REPORT.STATUS_NUM.CLOSED: + optimisticNextStep = { + type, + title: 'Finished!', + message: [ + { + text: 'No further action required!', + }, + ], + }; + + break; + // Generates an optimistic nextStep once a report has been approved case CONST.REPORT.STATUS_NUM.APPROVED: // Self review diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 5c92cd87a2bc..51a91e03622e 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -4782,8 +4782,8 @@ function submitReport(expenseReport: OnyxTypes.Report) { const parentReport = ReportUtils.getReport(expenseReport.parentReportID); const policy = getPolicy(expenseReport.policyID); const isCurrentUserManager = currentUserPersonalDetails.accountID === expenseReport.managerID; - const optimisticNextStep = NextStepUtils.buildNextStep(expenseReport, CONST.REPORT.STATUS_NUM.SUBMITTED); const isSubmitAndClosePolicy = PolicyUtils.isSubmitAndClose(policy); + const optimisticNextStep = NextStepUtils.buildNextStep(expenseReport, isSubmitAndClosePolicy ? CONST.REPORT.STATUS_NUM.CLOSED : CONST.REPORT.STATUS_NUM.SUBMITTED); const optimisticData: OnyxUpdate[] = !isSubmitAndClosePolicy ? [ From 26e41dc9fa5f46ac1593bfa1100ca6516f9d5c4b Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 22:42:11 +0100 Subject: [PATCH 05/12] Add unit test for the submit and close next steps --- src/libs/actions/IOU.ts | 5 ----- tests/unit/NextStepUtilsTest.ts | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 51a91e03622e..30c9bca9ca7d 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -4808,11 +4808,6 @@ function submitReport(expenseReport: OnyxTypes.Report) { statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, }, }, - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`, - value: optimisticNextStep, - }, ] : [ { diff --git a/tests/unit/NextStepUtilsTest.ts b/tests/unit/NextStepUtilsTest.ts index 200a8f52349e..a5bdda802315 100644 --- a/tests/unit/NextStepUtilsTest.ts +++ b/tests/unit/NextStepUtilsTest.ts @@ -442,6 +442,24 @@ describe('libs/NextStepUtils', () => { expect(result).toMatchObject(optimisticNextStep); }); + + test('submit and close approval mode', () => { + report.ownerAccountID = strangeAccountID; + optimisticNextStep.title = 'Finished:'; + optimisticNextStep.message = [ + { + text: 'No further action required!', + }, + ]; + + return Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { + approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL, + }).then(() => { + const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.CLOSED); + + expect(result).toMatchObject(optimisticNextStep); + }); + }); }); describe('it generates an optimistic nextStep once a report has been approved', () => { From bcb3110cc69b238fea606159e265464f27fc2237 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 22:43:32 +0100 Subject: [PATCH 06/12] Make sure the next steps are included in the optimistic data when submitting --- src/libs/actions/IOU.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 30c9bca9ca7d..333c869f4be4 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -4821,6 +4821,12 @@ function submitReport(expenseReport: OnyxTypes.Report) { }, ]; + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`, + value: optimisticNextStep, + }); + if (parentReport?.reportID) { optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, From 36c91d2365bd179e9829602c4da95df8ebf89413 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 22:44:39 +0100 Subject: [PATCH 07/12] Include the next steps in failure data when submitting no matter what approval mode the policy has --- src/libs/actions/IOU.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 333c869f4be4..344ec53568cc 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -4862,6 +4862,11 @@ function submitReport(expenseReport: OnyxTypes.Report) { stateNum: CONST.REPORT.STATE_NUM.OPEN, }, }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`, + value: currentNextStep, + }, ]; if (!isSubmitAndClosePolicy) { failureData.push( @@ -4874,11 +4879,6 @@ function submitReport(expenseReport: OnyxTypes.Report) { }, }, }, - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`, - value: currentNextStep, - }, ); } From 67c18cea42d8404072739bcfef770499e0cd13af Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 23:11:15 +0100 Subject: [PATCH 08/12] Update the unit tests --- tests/unit/NextStepUtilsTest.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/unit/NextStepUtilsTest.ts b/tests/unit/NextStepUtilsTest.ts index a5bdda802315..072a06748da9 100644 --- a/tests/unit/NextStepUtilsTest.ts +++ b/tests/unit/NextStepUtilsTest.ts @@ -445,7 +445,7 @@ describe('libs/NextStepUtils', () => { test('submit and close approval mode', () => { report.ownerAccountID = strangeAccountID; - optimisticNextStep.title = 'Finished:'; + optimisticNextStep.title = 'Finished!'; optimisticNextStep.message = [ { text: 'No further action required!', @@ -571,13 +571,5 @@ describe('libs/NextStepUtils', () => { expect(result).toMatchObject(optimisticNextStep); }); }); - - describe('it generates a nullable optimistic nextStep', () => { - test('closed status', () => { - const result = NextStepUtils.buildNextStep(report, CONST.REPORT.STATUS_NUM.CLOSED); - - expect(result).toBeNull(); - }); - }); }); }); From fc36c7c0769aaa189a24fe25a8a6790d6dc42338 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 23:12:26 +0100 Subject: [PATCH 09/12] Fix style --- src/libs/actions/IOU.ts | 16 +++++++--------- src/libs/actions/Policy.ts | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 344ec53568cc..d8c388045aa6 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -4869,17 +4869,15 @@ function submitReport(expenseReport: OnyxTypes.Report) { }, ]; if (!isSubmitAndClosePolicy) { - failureData.push( - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, - value: { - [optimisticSubmittedReportAction.reportActionID]: { - errors: ErrorUtils.getMicroSecondOnyxError('iou.error.other'), - }, + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, + value: { + [optimisticSubmittedReportAction.reportActionID]: { + errors: ErrorUtils.getMicroSecondOnyxError('iou.error.other'), }, }, - ); + }); } if (parentReport?.reportID) { diff --git a/src/libs/actions/Policy.ts b/src/libs/actions/Policy.ts index e11cef47303b..27ed99d00868 100644 --- a/src/libs/actions/Policy.ts +++ b/src/libs/actions/Policy.ts @@ -1954,7 +1954,7 @@ function createDraftInitialWorkspace(policyOwnerEmail = '', policyName = '', pol approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL, harvesting: { enabled: true, - } + }, }, }, { From 356bbd0f563f46b62510cd1c846ede02da3de526 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 23:13:02 +0100 Subject: [PATCH 10/12] Revert podfile lock changes --- ios/Podfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 9e6a52543c92..24ef0704be25 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -1921,8 +1921,8 @@ SPEC CHECKSUMS: SocketRocket: f32cd54efbe0f095c4d7594881e52619cfe80b17 Turf: 13d1a92d969ca0311bbc26e8356cca178ce95da2 VisionCamera: b6b6f46949eae83b71429c971162af337ef34fa3 - Yoga: 13c8ef87792450193e117976337b8527b49e8c03 + Yoga: e64aa65de36c0832d04e8c7bd614396c77a80047 PODFILE CHECKSUM: a431c146e1501391834a2f299a74093bac53b530 -COCOAPODS: 1.14.3 +COCOAPODS: 1.13.0 From eb24a0a7dddf05ea5185c45cf0f730cd22fe72b4 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 23:34:02 +0100 Subject: [PATCH 11/12] Update IOU tests --- tests/actions/IOUTest.ts | 108 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 1 deletion(-) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 2ce72a58aead..cfa37d4e6299 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -3051,7 +3051,9 @@ describe('actions/IOU', () => { let chatReport: OnyxEntry; return waitForBatchedUpdates() .then(() => { - PolicyActions.createWorkspace(CARLOS_EMAIL, true, "Carlos's Workspace"); + const policyID = PolicyActions.generatePolicyID(); + PolicyActions.createWorkspace(CARLOS_EMAIL, true, "Carlos's Workspace", policyID); + PolicyActions.setWorkspaceApprovalMode(policyID, CARLOS_EMAIL, CONST.POLICY.APPROVAL_MODE.BASIC); return waitForBatchedUpdates(); }) .then( @@ -3147,6 +3149,110 @@ describe('actions/IOU', () => { }), ); }); + it('correctly submits a report with Submit and Close approval mode', () => { + const amount = 10000; + const comment = '💸💸💸💸'; + const merchant = 'NASDAQ'; + let expenseReport: OnyxEntry; + let chatReport: OnyxEntry; + return waitForBatchedUpdates() + .then(() => { + PolicyActions.createWorkspace(CARLOS_EMAIL, true, "Carlos's Workspace"); + return waitForBatchedUpdates(); + }) + .then( + () => + new Promise((resolve) => { + const connectionID = Onyx.connect({ + key: ONYXKEYS.COLLECTION.REPORT, + waitForCollectionCallback: true, + callback: (allReports) => { + Onyx.disconnect(connectionID); + chatReport = Object.values(allReports ?? {}).find((report) => report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT) ?? null; + + resolve(); + }, + }); + }), + ) + .then(() => { + if (chatReport) { + IOU.requestMoney( + chatReport, + amount, + CONST.CURRENCY.USD, + '', + merchant, + RORY_EMAIL, + RORY_ACCOUNT_ID, + {login: CARLOS_EMAIL, accountID: CARLOS_ACCOUNT_ID, isPolicyExpenseChat: true, reportID: chatReport.reportID}, + comment, + {}, + ); + } + return waitForBatchedUpdates(); + }) + .then( + () => + new Promise((resolve) => { + const connectionID = Onyx.connect({ + key: ONYXKEYS.COLLECTION.REPORT, + waitForCollectionCallback: true, + callback: (allReports) => { + Onyx.disconnect(connectionID); + expenseReport = Object.values(allReports ?? {}).find((report) => report?.type === CONST.REPORT.TYPE.EXPENSE) ?? null; + Onyx.merge(`report_${expenseReport?.reportID}`, { + statusNum: 0, + stateNum: 0, + }); + resolve(); + }, + }); + }), + ) + .then( + () => + new Promise((resolve) => { + const connectionID = Onyx.connect({ + key: ONYXKEYS.COLLECTION.REPORT, + waitForCollectionCallback: true, + callback: (allReports) => { + Onyx.disconnect(connectionID); + expenseReport = Object.values(allReports ?? {}).find((report) => report?.type === CONST.REPORT.TYPE.EXPENSE) ?? null; + + // Verify report is a draft + expect(expenseReport?.stateNum).toBe(0); + expect(expenseReport?.statusNum).toBe(0); + resolve(); + }, + }); + }), + ) + .then(() => { + if (expenseReport) { + IOU.submitReport(expenseReport); + } + return waitForBatchedUpdates(); + }) + .then( + () => + new Promise((resolve) => { + const connectionID = Onyx.connect({ + key: ONYXKEYS.COLLECTION.REPORT, + waitForCollectionCallback: true, + callback: (allReports) => { + Onyx.disconnect(connectionID); + expenseReport = Object.values(allReports ?? {}).find((report) => report?.type === CONST.REPORT.TYPE.EXPENSE) ?? null; + + // Report is closed since the default policy settings is Submit and Close + expect(expenseReport?.stateNum).toBe(2); + expect(expenseReport?.statusNum).toBe(2); + resolve(); + }, + }); + }), + ); + }); it('correctly implements error handling', () => { const amount = 10000; const comment = '💸💸💸💸'; From 737788b6ffb4e4a2c2e00bff6c705638c015992e Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 3 Apr 2024 23:37:19 +0100 Subject: [PATCH 12/12] Add comment to explain test --- tests/actions/IOUTest.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index cfa37d4e6299..803ef9ba6be2 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -3053,6 +3053,8 @@ describe('actions/IOU', () => { .then(() => { const policyID = PolicyActions.generatePolicyID(); PolicyActions.createWorkspace(CARLOS_EMAIL, true, "Carlos's Workspace", policyID); + + // Change the approval mode for the policy since default is Submit and Close PolicyActions.setWorkspaceApprovalMode(policyID, CARLOS_EMAIL, CONST.POLICY.APPROVAL_MODE.BASIC); return waitForBatchedUpdates(); })