Skip to content

Commit c99f4df

Browse files
luacmartinsluacmartins
authored
Merge pull request Expensify#84700 from nkdengineer/fix/80255
fix: Prevent self approval still display in expense detail after set approver for workspace
2 parents 0bdacd6 + cbe28fa commit c99f4df

5 files changed

Lines changed: 390 additions & 14 deletions

File tree

‎src/components/MoneyReportHeaderPrimaryAction/SubmitPrimaryAction.tsx‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import useTransactionsAndViolationsForReport from '@hooks/useTransactionsAndViol
1616
import {search} from '@libs/actions/Search';
1717
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
1818
import {getFilteredReportActionsForReportView} from '@libs/ReportActionsUtils';
19-
import {getNextApproverAccountID, hasViolations as hasViolationsReportUtils, isReportOwner, shouldBlockSubmitDueToStrictPolicyRules} from '@libs/ReportUtils';
19+
import {hasViolations as hasViolationsReportUtils, shouldBlockSubmitDueToPreventSelfApproval, shouldBlockSubmitDueToStrictPolicyRules} from '@libs/ReportUtils';
2020
import {hasAnyPendingRTERViolation as hasAnyPendingRTERViolationTransactionUtils} from '@libs/TransactionUtils';
2121
import {submitReport} from '@userActions/IOU/ReportWorkflow';
2222
import {markPendingRTERTransactionsAsCash} from '@userActions/Transaction';
@@ -57,10 +57,7 @@ function SubmitPrimaryAction({reportID}: SubmitPrimaryActionProps) {
5757
};
5858
const confirmPendingRTERAndProceed = useConfirmPendingRTERAndProceed(hasAnyPendingRTERViolation, handleMarkPendingRTERTransactionsAsCash);
5959

60-
const nextApproverAccountID = getNextApproverAccountID(moneyRequestReport);
61-
const isSubmitterSameAsNextApprover =
62-
isReportOwner(moneyRequestReport) && (nextApproverAccountID === moneyRequestReport?.ownerAccountID || moneyRequestReport?.managerID === moneyRequestReport?.ownerAccountID);
63-
const isBlockSubmitDueToPreventSelfApproval = isSubmitterSameAsNextApprover && policy?.preventSelfApproval;
60+
const isBlockSubmitDueToPreventSelfApproval = shouldBlockSubmitDueToPreventSelfApproval(moneyRequestReport, policy);
6461
const isBlockSubmitDueToStrictPolicyRules = shouldBlockSubmitDueToStrictPolicyRules(
6562
moneyRequestReport?.reportID,
6663
violations,

‎src/libs/NextStepUtils.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
isInvoiceReport,
2222
isOpenExpenseReport,
2323
isPayer,
24-
isReportOwner,
24+
shouldBlockSubmitDueToPreventSelfApproval,
2525
} from './ReportUtils';
2626
import {hasSubmissionBlockingViolations} from './TransactionUtils';
2727

@@ -363,8 +363,6 @@ function getReportNextStep(
363363
currentUserEmail: string,
364364
currentUserAccountID: number,
365365
) {
366-
const nextApproverAccountID = getNextApproverAccountID(moneyRequestReport);
367-
368366
if (
369367
isOpenExpenseReport(moneyRequestReport) &&
370368
transactions.length > 0 &&
@@ -376,13 +374,10 @@ function getReportNextStep(
376374
return buildOptimisticFixIssueNextStep(moneyRequestReport?.ownerAccountID ?? -1);
377375
}
378376

379-
const isSubmitterSameAsNextApprover =
380-
isReportOwner(moneyRequestReport) && (nextApproverAccountID === moneyRequestReport?.ownerAccountID || moneyRequestReport?.managerID === moneyRequestReport?.ownerAccountID);
381-
382377
// When prevent self-approval is enabled & the current user is submitter AND they're submitting to themselves, we need to show the optimistic next step
383378
// We should always show this optimistic message for policies with preventSelfApproval
384379
// to avoid any flicker during transitions between online/offline states
385-
if (isSubmitterSameAsNextApprover && policy?.preventSelfApproval) {
380+
if (shouldBlockSubmitDueToPreventSelfApproval(moneyRequestReport, policy)) {
386381
return buildOptimisticNextStepForPreventSelfApprovalsEnabled();
387382
}
388383

@@ -826,6 +821,7 @@ export {
826821
buildOptimisticNextStepForDynamicExternalWorkflowSubmitError,
827822
buildOptimisticNextStepForDynamicExternalWorkflowApproveError,
828823
buildOptimisticNextStepForDEWOffline,
824+
buildOptimisticNextStepForPreventSelfApprovalsEnabled,
829825
// eslint-disable-next-line @typescript-eslint/no-deprecated
830826
buildNextStepNew,
831827
};

‎src/libs/ReportUtils.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9358,6 +9358,17 @@ function shouldBlockSubmitDueToStrictPolicyRules(
93589358
return hasAnyViolations(reportID, transactionViolations, currentUserAccountIDParam, currentUserEmailParam, reportTransactions);
93599359
}
93609360

9361+
function shouldBlockSubmitDueToPreventSelfApproval(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>): boolean {
9362+
if (!policy?.preventSelfApproval) {
9363+
return false;
9364+
}
9365+
9366+
const nextApproverAccountID = getNextApproverAccountID(report);
9367+
const isSubmitterSameAsNextApprover = isReportOwner(report) && nextApproverAccountID === report?.ownerAccountID;
9368+
const isSubmitterSameAsApprover = isReportOwner(report) && (report?.managerID === report?.ownerAccountID || nextApproverAccountID === report?.ownerAccountID);
9369+
return (isSubmitterSameAsNextApprover && isOpenExpenseReport(report)) || (isSubmitterSameAsApprover && isProcessingReport(report));
9370+
}
9371+
93619372
type ReportErrorsAndReportActionThatRequiresAttention = {
93629373
errors: ErrorFields;
93639374
reportAction?: OnyxEntry<ReportAction>;
@@ -13780,6 +13791,7 @@ export {
1378013791
getReportForHeader,
1378113792
isReportOpenOrUnsubmitted,
1378213793
getIconsForExpenseReport,
13794+
shouldBlockSubmitDueToPreventSelfApproval,
1378313795
getTransactionSortValue,
1378413796
isSortableColumnName,
1378513797
getLinkedIOUTransaction,

‎tests/unit/NextStepUtilsTest.ts‎

Lines changed: 207 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
import Onyx from 'react-native-onyx';
2+
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
23
// eslint-disable-next-line @typescript-eslint/no-deprecated
3-
import {buildNextStepNew, buildOptimisticNextStepForDynamicExternalWorkflowSubmitError, buildOptimisticNextStepForStrictPolicyRuleViolations} from '@libs/NextStepUtils';
4+
import {
5+
buildNextStepNew,
6+
buildOptimisticNextStepForDynamicExternalWorkflowSubmitError,
7+
buildOptimisticNextStepForPreventSelfApprovalsEnabled,
8+
buildOptimisticNextStepForStrictPolicyRuleViolations,
9+
getReportNextStep,
10+
} from '@libs/NextStepUtils';
411
import {buildOptimisticEmptyReport, buildOptimisticExpenseReport} from '@libs/ReportUtils';
512
import CONST from '@src/CONST';
613
import ONYXKEYS from '@src/ONYXKEYS';
7-
import type {Policy, Report, ReportNextStepDeprecated} from '@src/types/onyx';
14+
import type {Policy, Report, ReportNextStepDeprecated, Transaction, TransactionViolations} from '@src/types/onyx';
815
import {toCollectionDataSet} from '@src/types/utils/CollectionDataSet';
916
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
1017

@@ -1105,4 +1112,202 @@ describe('libs/NextStepUtils', () => {
11051112
});
11061113
});
11071114
});
1115+
1116+
describe('getReportNextStep', () => {
1117+
const currentUserEmail = 'current-user@expensify.com';
1118+
const currentUserAccountID = 37;
1119+
const policyID = 'policy-1';
1120+
1121+
beforeAll(() => {
1122+
Onyx.multiSet({
1123+
[ONYXKEYS.SESSION]: {email: currentUserEmail, accountID: currentUserAccountID},
1124+
[ONYXKEYS.PERSONAL_DETAILS_LIST]: {
1125+
[currentUserAccountID]: {
1126+
accountID: currentUserAccountID,
1127+
login: currentUserEmail,
1128+
avatar: '',
1129+
},
1130+
},
1131+
}).then(waitForBatchedUpdates);
1132+
});
1133+
1134+
it('returns the current next step when no special conditions are met', () => {
1135+
const report: Report = {
1136+
...buildOptimisticExpenseReport({
1137+
chatReportID: 'chat-1',
1138+
policyID,
1139+
payeeAccountID: 1,
1140+
total: -500,
1141+
currency: CONST.CURRENCY.USD,
1142+
betas: [CONST.BETAS.ALL],
1143+
}),
1144+
ownerAccountID: currentUserAccountID,
1145+
managerID: currentUserAccountID,
1146+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
1147+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
1148+
} as Report;
1149+
1150+
const currentNextStep: ReportNextStepDeprecated = {
1151+
type: 'neutral',
1152+
icon: CONST.NEXT_STEP.ICONS.HOURGLASS,
1153+
message: [{text: 'Current next step'}],
1154+
};
1155+
1156+
const result = getReportNextStep(currentNextStep, report, [], undefined, {}, currentUserEmail, currentUserAccountID);
1157+
expect(result).toBe(currentNextStep);
1158+
});
1159+
1160+
it('returns an optimistic fix issue next step when all transactions have submission-blocking violations', () => {
1161+
const report: Report = {
1162+
...buildOptimisticExpenseReport({
1163+
chatReportID: 'chat-2',
1164+
policyID,
1165+
payeeAccountID: 1,
1166+
total: -500,
1167+
currency: CONST.CURRENCY.USD,
1168+
betas: [CONST.BETAS.ALL],
1169+
}),
1170+
ownerAccountID: currentUserAccountID,
1171+
managerID: currentUserAccountID,
1172+
type: CONST.REPORT.TYPE.EXPENSE,
1173+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
1174+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
1175+
} as Report;
1176+
1177+
const transaction: Transaction = {
1178+
transactionID: 'txn-1',
1179+
reportID: report.reportID,
1180+
amount: -500,
1181+
currency: CONST.CURRENCY.USD,
1182+
} as Transaction;
1183+
1184+
const transactionViolations: OnyxCollection<TransactionViolations> = {
1185+
[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`]: [
1186+
{
1187+
name: CONST.VIOLATIONS.SMARTSCAN_FAILED,
1188+
type: CONST.VIOLATION_TYPES.VIOLATION,
1189+
},
1190+
],
1191+
};
1192+
1193+
const result = getReportNextStep(undefined, report, [transaction] as Array<OnyxEntry<Transaction>>, undefined, transactionViolations, currentUserEmail, currentUserAccountID);
1194+
1195+
expect(result).toEqual({
1196+
icon: CONST.NEXT_STEP.ICONS.HOURGLASS,
1197+
messageKey: CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_FIX_ISSUES,
1198+
actorAccountID: report.ownerAccountID,
1199+
});
1200+
});
1201+
1202+
it('returns an optimistic prevent self-approval next step when preventSelfApproval is enabled and submitter would submit to themselves', async () => {
1203+
const policy: Policy = {
1204+
id: policyID,
1205+
name: 'Policy',
1206+
role: CONST.POLICY.ROLE.ADMIN,
1207+
type: CONST.POLICY.TYPE.TEAM,
1208+
owner: currentUserEmail,
1209+
outputCurrency: CONST.CURRENCY.USD,
1210+
isPolicyExpenseChatEnabled: true,
1211+
reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES,
1212+
approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL,
1213+
approver: currentUserEmail,
1214+
preventSelfApproval: true,
1215+
employeeList: {
1216+
[currentUserEmail]: {
1217+
email: currentUserEmail,
1218+
role: CONST.POLICY.ROLE.ADMIN,
1219+
submitsTo: currentUserEmail,
1220+
},
1221+
},
1222+
};
1223+
1224+
const report: Report = {
1225+
...buildOptimisticExpenseReport({
1226+
chatReportID: 'chat-3',
1227+
policyID,
1228+
payeeAccountID: 1,
1229+
total: -500,
1230+
currency: CONST.CURRENCY.USD,
1231+
betas: [CONST.BETAS.ALL],
1232+
}),
1233+
ownerAccountID: currentUserAccountID,
1234+
policyID,
1235+
type: CONST.REPORT.TYPE.EXPENSE,
1236+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
1237+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
1238+
} as Report;
1239+
1240+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policy);
1241+
await waitForBatchedUpdates();
1242+
1243+
const result = getReportNextStep(undefined, report, [], policy, {}, currentUserEmail, currentUserAccountID);
1244+
expect(result).toEqual(buildOptimisticNextStepForPreventSelfApprovalsEnabled());
1245+
});
1246+
1247+
it('prioritizes the fix issue next step over the prevent self-approval next step when both conditions are true', async () => {
1248+
const policy: Policy = {
1249+
id: policyID,
1250+
name: 'Policy',
1251+
role: CONST.POLICY.ROLE.ADMIN,
1252+
type: CONST.POLICY.TYPE.TEAM,
1253+
owner: currentUserEmail,
1254+
outputCurrency: CONST.CURRENCY.USD,
1255+
isPolicyExpenseChatEnabled: true,
1256+
reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES,
1257+
approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL,
1258+
approver: currentUserEmail,
1259+
preventSelfApproval: true,
1260+
employeeList: {
1261+
[currentUserEmail]: {
1262+
email: currentUserEmail,
1263+
role: CONST.POLICY.ROLE.ADMIN,
1264+
submitsTo: currentUserEmail,
1265+
},
1266+
},
1267+
};
1268+
1269+
const report: Report = {
1270+
...buildOptimisticExpenseReport({
1271+
chatReportID: 'chat-4',
1272+
policyID,
1273+
payeeAccountID: 1,
1274+
total: -500,
1275+
currency: CONST.CURRENCY.USD,
1276+
betas: [CONST.BETAS.ALL],
1277+
}),
1278+
ownerAccountID: currentUserAccountID,
1279+
policyID,
1280+
type: CONST.REPORT.TYPE.EXPENSE,
1281+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
1282+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
1283+
} as Report;
1284+
1285+
const transaction: Transaction = {
1286+
transactionID: 'txn-2',
1287+
reportID: report.reportID,
1288+
amount: -500,
1289+
currency: CONST.CURRENCY.USD,
1290+
} as Transaction;
1291+
1292+
const transactionViolations: OnyxCollection<TransactionViolations> = {
1293+
[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`]: [
1294+
{
1295+
name: CONST.VIOLATIONS.NO_ROUTE,
1296+
type: CONST.VIOLATION_TYPES.VIOLATION,
1297+
},
1298+
],
1299+
};
1300+
1301+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policy);
1302+
await waitForBatchedUpdates();
1303+
1304+
const result = getReportNextStep(undefined, report, [transaction] as Array<OnyxEntry<Transaction>>, policy, transactionViolations, currentUserEmail, currentUserAccountID);
1305+
1306+
expect(result).toEqual({
1307+
messageKey: CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_FIX_ISSUES,
1308+
icon: CONST.NEXT_STEP.ICONS.HOURGLASS,
1309+
actorAccountID: report.ownerAccountID,
1310+
});
1311+
});
1312+
});
11081313
});

0 commit comments

Comments
 (0)