Skip to content

Commit 187d32f

Browse files
authored
Merge pull request #89250 from truph01/fix/78578
fix: cleanup RejectedExpense violation when moving expense
2 parents cb7d3f8 + 63d79aa commit 187d32f

9 files changed

Lines changed: 1051 additions & 182 deletions

File tree

src/libs/ReportUtils.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,15 +2156,15 @@ function pushTransactionViolationsOnyxData(
21562156
for (const {transactions, violations} of nonInvoiceReportTransactionsAndViolations) {
21572157
for (const transaction of Object.values(transactions)) {
21582158
const existingViolations = violations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`];
2159-
const optimisticViolations = ViolationsUtils.getViolationsOnyxData(
2160-
transaction,
2161-
existingViolations ?? [],
2162-
optimisticPolicy,
2163-
optimisticTagLists,
2164-
optimisticCategories,
2159+
const optimisticViolations = ViolationsUtils.getViolationsOnyxData({
2160+
updatedTransaction: transaction,
2161+
transactionViolations: existingViolations ?? [],
2162+
policy: optimisticPolicy,
2163+
policyTagList: optimisticTagLists,
2164+
policyCategories: optimisticCategories,
21652165
hasDependentTags,
2166-
false,
2167-
);
2166+
isInvoiceTransaction: false,
2167+
});
21682168

21692169
if (!isEmptyObject(optimisticViolations)) {
21702170
onyxData.optimisticData?.push(optimisticViolations);

src/libs/Violations/ViolationsUtils.ts

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -350,18 +350,31 @@ const ViolationsUtils = {
350350
* Checks a transaction for policy violations and returns an object with Onyx method, key and updated transaction
351351
* violations.
352352
*/
353-
getViolationsOnyxData(
354-
updatedTransaction: Transaction,
355-
transactionViolations: TransactionViolation[],
356-
policy: Policy,
357-
policyTagList: PolicyTagLists,
358-
policyCategories: PolicyCategories,
359-
hasDependentTags: boolean,
360-
isInvoiceTransaction: boolean,
361-
isSelfDM?: boolean,
362-
iouReport?: OnyxEntry<Report>,
363-
isFromExpenseReport?: boolean,
364-
): OnyxUpdate<typeof ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS> {
353+
getViolationsOnyxData({
354+
updatedTransaction,
355+
transactionViolations,
356+
policy,
357+
policyTagList,
358+
policyCategories,
359+
hasDependentTags,
360+
isInvoiceTransaction,
361+
isSelfDM,
362+
iouReport,
363+
isFromExpenseReport,
364+
shouldRemoveRejectedExpenseViolation,
365+
}: {
366+
updatedTransaction: Transaction;
367+
transactionViolations: TransactionViolation[];
368+
policy: Policy;
369+
policyTagList: PolicyTagLists;
370+
policyCategories: PolicyCategories;
371+
hasDependentTags: boolean;
372+
isInvoiceTransaction: boolean;
373+
isSelfDM?: boolean;
374+
iouReport?: OnyxEntry<Report>;
375+
isFromExpenseReport?: boolean;
376+
shouldRemoveRejectedExpenseViolation?: boolean;
377+
}): OnyxUpdate<typeof ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS> {
365378
const isScanning = TransactionUtils.isScanning(updatedTransaction);
366379
const isScanRequest = TransactionUtils.isScanRequest(updatedTransaction);
367380
const isPartialTransaction = TransactionUtils.isPartialTransaction(updatedTransaction);
@@ -375,8 +388,9 @@ const ViolationsUtils = {
375388

376389
let newTransactionViolations = [...transactionViolations];
377390

378-
// Remove AUTO_REPORTED_REJECTED_EXPENSE violation when the submitter edits the expense
379-
if (iouReport && isFromExpenseReport && isCurrentUserSubmitter(iouReport)) {
391+
// Remove AUTO_REPORTED_REJECTED_EXPENSE violation when the submitter edits the expense, when the transaction is moved to a different report,
392+
// or when explicitly requested (e.g. from changeTransactionsReport)
393+
if (shouldRemoveRejectedExpenseViolation || (iouReport && isFromExpenseReport && isCurrentUserSubmitter(iouReport))) {
380394
const hasRejectedExpenseViolation = newTransactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE);
381395
if (hasRejectedExpenseViolation) {
382396
newTransactionViolations = newTransactionViolations.filter((violation) => violation.name !== CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE);

src/libs/actions/IOU/BulkEdit.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -352,18 +352,18 @@ function updateMultipleMoneyRequests({
352352
: optimisticViolations;
353353
const transactionPolicyTagList = policyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${transactionPolicy?.id}`] ?? {};
354354
const transactionPolicyCategories = policyCategories?.[`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${transactionPolicy?.id}`] ?? {};
355-
optimisticViolationsData = ViolationsUtils.getViolationsOnyxData(
355+
optimisticViolationsData = ViolationsUtils.getViolationsOnyxData({
356356
updatedTransaction,
357-
optimisticViolations,
358-
transactionPolicy,
359-
transactionPolicyTagList,
360-
transactionPolicyCategories,
361-
hasDependentTags(transactionPolicy, transactionPolicyTagList),
362-
isInvoiceReportReportUtils(iouReport),
363-
isSelfDM(iouReport),
357+
transactionViolations: optimisticViolations,
358+
policy: transactionPolicy,
359+
policyTagList: transactionPolicyTagList,
360+
policyCategories: transactionPolicyCategories,
361+
hasDependentTags: hasDependentTags(transactionPolicy, transactionPolicyTagList),
362+
isInvoiceTransaction: isInvoiceReportReportUtils(iouReport),
363+
isSelfDM: isSelfDM(iouReport),
364364
iouReport,
365365
isFromExpenseReport,
366-
);
366+
});
367367
optimisticData.push(optimisticViolationsData);
368368
failureData.push({
369369
onyxMethod: Onyx.METHOD.MERGE,

src/libs/actions/IOU/MoneyRequestBuilder.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,15 +1078,15 @@ function buildOnyxDataForMoneyRequest(moneyRequestParams: BuildOnyxDataForMoneyR
10781078
if (!policy || !isPaidGroupPolicy(policy) || transaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID) {
10791079
return onyxData;
10801080
}
1081-
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData(
1082-
transaction,
1083-
[],
1081+
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData({
1082+
updatedTransaction: transaction,
1083+
transactionViolations: [],
10841084
policy,
1085-
policyTagList ?? {},
1086-
policyCategories ?? {},
1087-
hasDependentTags(policy, policyTagList ?? {}),
1088-
false,
1089-
);
1085+
policyTagList: policyTagList ?? {},
1086+
policyCategories: policyCategories ?? {},
1087+
hasDependentTags: hasDependentTags(policy, policyTagList ?? {}),
1088+
isInvoiceTransaction: false,
1089+
});
10901090

10911091
if (violationsOnyxData) {
10921092
const reportTransactions = [...getReportTransactions(iou.report.reportID).filter((reportTransaction) => reportTransaction.transactionID !== transaction.transactionID), transaction];

src/libs/actions/IOU/Receipt.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,15 @@ function detachReceipt(
101101

102102
if (transactionPolicy && isPaidGroupPolicy(transactionPolicy) && newTransaction) {
103103
const currentTransactionViolations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`] ?? [];
104-
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData(
105-
newTransaction,
106-
currentTransactionViolations,
107-
transactionPolicy,
108-
transactionPolicyTagList ?? {},
109-
transactionPolicyCategories ?? {},
110-
hasDependentTags(transactionPolicy, transactionPolicyTagList ?? {}),
111-
isInvoiceReportReportUtils(expenseReport),
112-
);
104+
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData({
105+
updatedTransaction: newTransaction,
106+
transactionViolations: currentTransactionViolations,
107+
policy: transactionPolicy,
108+
policyTagList: transactionPolicyTagList ?? {},
109+
policyCategories: transactionPolicyCategories ?? {},
110+
hasDependentTags: hasDependentTags(transactionPolicy, transactionPolicyTagList ?? {}),
111+
isInvoiceTransaction: isInvoiceReportReportUtils(expenseReport),
112+
});
113113
optimisticData.push(violationsOnyxData);
114114
failureData.push({
115115
onyxMethod: Onyx.METHOD.MERGE,
@@ -237,15 +237,15 @@ function replaceReceipt({transactionID, file, source, state, transactionPolicy,
237237

238238
if (transactionPolicy && isPaidGroupPolicy(transactionPolicy) && newTransaction) {
239239
const currentTransactionViolations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`] ?? [];
240-
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData(
241-
newTransaction,
242-
currentTransactionViolations,
243-
transactionPolicy,
244-
transactionPolicyTagList ?? {},
245-
transactionPolicyCategories ?? {},
246-
hasDependentTags(transactionPolicy, transactionPolicyTagList ?? {}),
247-
isInvoiceReportReportUtils(expenseReport),
248-
);
240+
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData({
241+
updatedTransaction: newTransaction,
242+
transactionViolations: currentTransactionViolations,
243+
policy: transactionPolicy,
244+
policyTagList: transactionPolicyTagList ?? {},
245+
policyCategories: transactionPolicyCategories ?? {},
246+
hasDependentTags: hasDependentTags(transactionPolicy, transactionPolicyTagList ?? {}),
247+
isInvoiceTransaction: isInvoiceReportReportUtils(expenseReport),
248+
});
249249
optimisticData.push(violationsOnyxData);
250250
failureData.push({
251251
onyxMethod: Onyx.METHOD.MERGE,

src/libs/actions/IOU/UpdateMoneyRequest.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,18 +1647,18 @@ function getUpdateMoneyRequestParams(params: GetUpdateMoneyRequestParamsType): U
16471647
);
16481648
}
16491649

1650-
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData(
1650+
const violationsOnyxData = ViolationsUtils.getViolationsOnyxData({
16511651
updatedTransaction,
1652-
optimisticViolations,
1652+
transactionViolations: optimisticViolations,
16531653
policy,
1654-
policyTagList ?? {},
1655-
policyCategories ?? {},
1656-
hasDependentTags(policy, policyTagList ?? {}),
1657-
isInvoice,
1658-
isSelfDM(iouReport),
1654+
policyTagList: policyTagList ?? {},
1655+
policyCategories: policyCategories ?? {},
1656+
hasDependentTags: hasDependentTags(policy, policyTagList ?? {}),
1657+
isInvoiceTransaction: isInvoice,
1658+
isSelfDM: isSelfDM(iouReport),
16591659
iouReport,
16601660
isFromExpenseReport,
1661-
);
1661+
});
16621662
optimisticData.push(violationsOnyxData);
16631663
failureData.push({
16641664
onyxMethod: Onyx.METHOD.MERGE,

src/libs/actions/Transaction.ts

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,15 +1270,16 @@ function changeTransactionsReport({
12701270
let transactionReimbursable = transaction.reimbursable;
12711271
// 2. Calculate transaction violations if moving transaction to a workspace
12721272
if (isPaidGroupPolicy(policy) && policy?.id) {
1273-
const violationData = ViolationsUtils.getViolationsOnyxData(
1274-
transactionForViolations,
1275-
currentTransactionViolations[transaction.transactionID] ?? [],
1273+
const violationData = ViolationsUtils.getViolationsOnyxData({
1274+
updatedTransaction: transactionForViolations,
1275+
transactionViolations: currentTransactionViolations[transaction.transactionID] ?? [],
12761276
policy,
1277-
policyTagList ?? {},
1278-
policyCategories ?? {},
1279-
policyHasDependentTags,
1280-
false,
1281-
);
1277+
policyTagList: policyTagList ?? {},
1278+
policyCategories: policyCategories ?? {},
1279+
hasDependentTags: policyHasDependentTags,
1280+
isInvoiceTransaction: false,
1281+
shouldRemoveRejectedExpenseViolation: true,
1282+
});
12821283
optimisticData.push(violationData);
12831284
failureData.push({
12841285
onyxMethod: Onyx.METHOD.MERGE,
@@ -1668,15 +1669,15 @@ function changeTransactionsReport({
16681669
if (!isPaidGroupPolicy(policy) || !policy?.id) {
16691670
continue;
16701671
}
1671-
const violationData = ViolationsUtils.getViolationsOnyxData(
1672-
transaction,
1673-
currentTransactionViolations[transaction.transactionID] ?? [],
1672+
const violationData = ViolationsUtils.getViolationsOnyxData({
1673+
updatedTransaction: transaction,
1674+
transactionViolations: currentTransactionViolations[transaction.transactionID] ?? [],
16741675
policy,
1675-
policyTagList ?? {},
1676-
policyCategories ?? {},
1677-
policyHasDependentTags,
1678-
false,
1679-
);
1676+
policyTagList: policyTagList ?? {},
1677+
policyCategories: policyCategories ?? {},
1678+
hasDependentTags: policyHasDependentTags,
1679+
isInvoiceTransaction: false,
1680+
});
16801681
if (Array.isArray(violationData.value) && hasSubmissionBlockingViolationInList(violationData.value)) {
16811682
shouldFixViolations = true;
16821683
}

tests/unit/TransactionTest.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {ReportCollectionDataSet} from '@src/types/onyx/Report';
1818
import type {OnyxData} from '@src/types/onyx/Request';
1919
import type {UpdateMoneyRequestDataKeys} from '../../src/libs/actions/IOU/UpdateMoneyRequest';
2020
import * as TransactionUtils from '../../src/libs/TransactionUtils';
21-
import type {PersonalDetails, PolicyTagLists, RecentWaypoint, Report, ReportAction, ReportActions, Transaction} from '../../src/types/onyx';
21+
import type {PersonalDetails, Policy, PolicyTagLists, RecentWaypoint, Report, ReportAction, ReportActions, Transaction} from '../../src/types/onyx';
2222
import createRandomPolicy from '../utils/collections/policies';
2323
import createRandomPolicyCategories from '../utils/collections/policyCategory';
2424
import {createExpenseReport, createRandomReport} from '../utils/collections/reports';
@@ -1336,6 +1336,50 @@ describe('Transaction', () => {
13361336
expect(missingTagViolations.some((violation) => violation.data?.tagName === 'City')).toBe(true);
13371337
});
13381338

1339+
it('removes AUTO_REPORTED_REJECTED_EXPENSE from transaction violations when moving to a paid group policy', async () => {
1340+
const policyID = '1001';
1341+
const transaction = generateTransaction({reportID: FAKE_OLD_REPORT_ID});
1342+
const oldIOUAction = createIOUAction(transaction);
1343+
const newExpenseReport = {
1344+
...createExpenseReport(Number(FAKE_NEW_REPORT_ID)),
1345+
policyID,
1346+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
1347+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
1348+
ownerAccountID: CURRENT_USER_ID,
1349+
};
1350+
const policy: Policy = {
1351+
...createRandomPolicy(Number(policyID), CONST.POLICY.TYPE.TEAM),
1352+
requiresTag: false,
1353+
requiresCategory: false,
1354+
};
1355+
1356+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction);
1357+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${FAKE_OLD_REPORT_ID}`, {[oldIOUAction.reportActionID]: oldIOUAction});
1358+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`, [
1359+
{name: CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true},
1360+
]);
1361+
await waitForBatchedUpdates();
1362+
1363+
const allTransactions = {
1364+
[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]: transaction,
1365+
};
1366+
1367+
changeTransactionsReport({
1368+
transactionIDs: [transaction.transactionID],
1369+
isASAPSubmitBetaEnabled: false,
1370+
accountID: CURRENT_USER_ID,
1371+
email: 'test@example.com',
1372+
newReport: newExpenseReport,
1373+
policy,
1374+
allTransactions,
1375+
policyTagList: {},
1376+
});
1377+
await waitForBatchedUpdates();
1378+
1379+
const updatedViolations = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`);
1380+
expect(updatedViolations?.some((violation) => violation.name === CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE)).toBe(false);
1381+
});
1382+
13391383
it('should auto-select a valid distance rate when moving a distance expense with an invalid P2P rate to a workspace', async () => {
13401384
const policyID = '100';
13411385
const validRateID = 'valid_rate_1';

0 commit comments

Comments
 (0)