Skip to content

Commit

Permalink
Merge pull request Expensify#39563 from Expensify/vit-39337
Browse files Browse the repository at this point in the history
Fix submit and close next steps for optimistically created policy
  • Loading branch information
MariaHCD authored Apr 8, 2024
2 parents ce06355 + 2d9027e commit ef41626
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 34 deletions.
18 changes: 16 additions & 2 deletions src/libs/NextStepUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function buildNextStep(

const {policyID = '', ownerAccountID = -1, managerID = -1} = report;
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`] ?? ({} as Policy);
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;
Expand Down Expand Up @@ -172,7 +172,7 @@ function buildNextStep(
}

// Prevented self submitting
if ((isPreventSelfApprovalEnabled ?? preventSelfApproval) && isSelfApproval) {
if (preventSelfApproval && isSelfApproval) {
optimisticNextStep.message = [
{
text: "Oops! Looks like you're submitting to ",
Expand Down Expand Up @@ -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
Expand Down
39 changes: 19 additions & 20 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4743,8 +4743,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
? [
Expand All @@ -4769,11 +4769,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,
},
]
: [
{
Expand All @@ -4787,6 +4782,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,
Expand Down Expand Up @@ -4822,24 +4823,22 @@ 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(
{
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'),
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`,
value: currentNextStep,
},
);
});
}

if (parentReport?.reportID) {
Expand Down
15 changes: 15 additions & 0 deletions src/libs/actions/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1951,6 +1951,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,
},
},
},
{
Expand Down Expand Up @@ -2009,6 +2014,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,
Expand Down Expand Up @@ -2497,6 +2507,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,
Expand Down
3 changes: 0 additions & 3 deletions src/types/onyx/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,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;

Expand Down
110 changes: 109 additions & 1 deletion tests/actions/IOUTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3048,7 +3048,11 @@ describe('actions/IOU', () => {
let chatReport: OnyxEntry<OnyxTypes.Report>;
return waitForBatchedUpdates()
.then(() => {
PolicyActions.createWorkspace(CARLOS_EMAIL, true, "Carlos's Workspace");
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();
})
.then(
Expand Down Expand Up @@ -3144,6 +3148,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<OnyxTypes.Report>;
let chatReport: OnyxEntry<OnyxTypes.Report>;
return waitForBatchedUpdates()
.then(() => {
PolicyActions.createWorkspace(CARLOS_EMAIL, true, "Carlos's Workspace");
return waitForBatchedUpdates();
})
.then(
() =>
new Promise<void>((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<void>((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<void>((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<void>((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 = '💸💸💸💸';
Expand Down
26 changes: 18 additions & 8 deletions tests/unit/NextStepUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -553,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();
});
});
});
});

0 comments on commit ef41626

Please sign in to comment.