Skip to content

Commit

Permalink
Merge pull request Expensify#55345 from nkdengineer/fix/54996
Browse files Browse the repository at this point in the history
Prevent approving in expense report only has pending card/scan failure transactions
  • Loading branch information
luacmartins authored Feb 12, 2025
2 parents 63afa93 + ac46491 commit b10a56a
Show file tree
Hide file tree
Showing 4 changed files with 211 additions and 13 deletions.
21 changes: 18 additions & 3 deletions src/libs/SearchUIUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,18 @@ import {
isMoneyRequestReport,
isSettled,
} from './ReportUtils';
import {getAmount as getTransactionAmount, getCreated as getTransactionCreatedDate, getMerchant as getTransactionMerchant, isExpensifyCardTransaction, isPending} from './TransactionUtils';
import {
getMerchant,
getAmount as getTransactionAmount,
getCreated as getTransactionCreatedDate,
getMerchant as getTransactionMerchant,
isAmountMissing,
isExpensifyCardTransaction,
isPartialMerchant,
isPending,
isReceiptBeingScanned,
isScanRequest,
} from './TransactionUtils';

const columnNamesToSortingProperty = {
[CONST.SEARCH.TABLE_COLUMNS.TO]: 'formattedTo' as const,
Expand Down Expand Up @@ -347,10 +358,14 @@ function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTr
if (canIOUBePaid(report, chatReport, policy, allReportTransactions, false, chatReportRNVP, invoiceReceiverPolicy) && !hasOnlyHeldExpenses(report.reportID, allReportTransactions)) {
return CONST.SEARCH.ACTION_TYPES.PAY;
}
const hasOnlyPendingTransactions = allReportTransactions.length > 0 && allReportTransactions.every((t) => isExpensifyCardTransaction(t) && isPending(t));
const hasOnlyPendingCardOrScanningTransactions =
allReportTransactions.length > 0 &&
allReportTransactions.every(
(t) => (isExpensifyCardTransaction(t) && isPending(t)) || (isPartialMerchant(getMerchant(t)) && isAmountMissing(t)) || (isScanRequest(t) && isReceiptBeingScanned(t)),
);

const isAllowedToApproveExpenseReport = isAllowedToApproveExpenseReportUtils(report, undefined, policy);
if (canApproveIOU(report, policy) && isAllowedToApproveExpenseReport && !hasOnlyPendingTransactions) {
if (canApproveIOU(report, policy) && isAllowedToApproveExpenseReport && !hasOnlyPendingCardOrScanningTransactions) {
return CONST.SEARCH.ACTION_TYPES.APPROVE;
}

Expand Down
21 changes: 11 additions & 10 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8039,20 +8039,21 @@ function canApproveIOU(
const iouSettled = isSettled(iouReport?.reportID);
const reportNameValuePairs = chatReportRNVP ?? getReportNameValuePairs(iouReport?.reportID);
const isArchivedExpenseReport = isArchivedReport(reportNameValuePairs);
let isTransactionBeingScanned = false;
const reportTransactions = getReportTransactions(iouReport?.reportID);
for (const transaction of reportTransactions) {
const hasReceipt = hasReceiptTransactionUtils(transaction);
const isReceiptBeingScanned = isReceiptBeingScannedTransactionUtils(transaction);

// If transaction has receipt (scan) and its receipt is being scanned, we shouldn't be able to Approve
if (hasReceipt && isReceiptBeingScanned) {
isTransactionBeingScanned = true;
}
const hasOnlyPendingCardOrScanningTransactions =
reportTransactions.length > 0 &&
reportTransactions.every(
(transaction) =>
(isExpensifyCardTransaction(transaction) && isPending(transaction)) ||
(isPartialMerchant(getMerchant(transaction)) && isAmountMissing(transaction)) ||
(isScanRequestTransactionUtils(transaction) && isReceiptBeingScannedTransactionUtils(transaction)),
);
if (hasOnlyPendingCardOrScanningTransactions) {
return false;
}
const isPayAtEndExpenseReport = isPayAtEndExpenseReportReportUtils(iouReport?.reportID, reportTransactions);

return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedExpenseReport && !isTransactionBeingScanned && !isPayAtEndExpenseReport;
return reportTransactions.length > 0 && isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedExpenseReport && !isPayAtEndExpenseReport;
}

function canIOUBePaid(
Expand Down
177 changes: 177 additions & 0 deletions tests/actions/IOUTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import isEqual from 'lodash/isEqual';
import type {OnyxCollection, OnyxEntry, OnyxInputValue} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import {
canApproveIOU,
cancelPayment,
deleteMoneyRequest,
payMoneyRequest,
Expand Down Expand Up @@ -4472,4 +4473,180 @@ describe('actions/IOU', () => {
});
});
});

describe('canApproveIOU', () => {
it('should return false if we have only pending card transactions', async () => {
const policyID = '2';
const reportID = '1';
const fakePolicy: Policy = {
...createRandomPolicy(Number(policyID)),
type: CONST.POLICY.TYPE.TEAM,
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
};
const fakeReport: Report = {
...createRandomReport(Number(reportID)),
type: CONST.REPORT.TYPE.EXPENSE,
policyID,
};
const fakeTransaction1: Transaction = {
...createRandomTransaction(0),
reportID,
bank: CONST.EXPENSIFY_CARD.BANK,
status: CONST.TRANSACTION.STATUS.PENDING,
};
const fakeTransaction2: Transaction = {
...createRandomTransaction(1),
reportID,
bank: CONST.EXPENSIFY_CARD.BANK,
status: CONST.TRANSACTION.STATUS.PENDING,
};

await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`, fakeReport);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction1.transactionID}`, fakeTransaction1);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction2.transactionID}`, fakeTransaction2);

await waitForBatchedUpdates();

expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy();
});
it('should return false if we have only scan failure transactions', async () => {
const policyID = '2';
const reportID = '1';
const fakePolicy: Policy = {
...createRandomPolicy(Number(policyID)),
type: CONST.POLICY.TYPE.TEAM,
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
};
const fakeReport: Report = {
...createRandomReport(Number(reportID)),
type: CONST.REPORT.TYPE.EXPENSE,
policyID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
managerID: RORY_ACCOUNT_ID,
};
const fakeTransaction1: Transaction = {
...createRandomTransaction(0),
reportID,
amount: 0,
modifiedAmount: 0,
receipt: {
state: CONST.IOU.RECEIPT_STATE.SCANFAILED,
},
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
modifiedMerchant: undefined,
};
const fakeTransaction2: Transaction = {
...createRandomTransaction(1),
reportID,
amount: 0,
modifiedAmount: 0,
receipt: {
state: CONST.IOU.RECEIPT_STATE.SCANFAILED,
},
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
modifiedMerchant: undefined,
};

await Onyx.set(ONYXKEYS.COLLECTION.REPORT, {
[`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`]: fakeReport,
});
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`, fakeReport);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction1.transactionID}`, fakeTransaction1);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction2.transactionID}`, fakeTransaction2);

await waitForBatchedUpdates();

expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy();
});
it('should return false if all transactions are pending card or scan failure transaction', async () => {
const policyID = '2';
const reportID = '1';
const fakePolicy: Policy = {
...createRandomPolicy(Number(policyID)),
type: CONST.POLICY.TYPE.TEAM,
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
};
const fakeReport: Report = {
...createRandomReport(Number(reportID)),
type: CONST.REPORT.TYPE.EXPENSE,
policyID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
managerID: RORY_ACCOUNT_ID,
};
const fakeTransaction1: Transaction = {
...createRandomTransaction(0),
reportID,
bank: CONST.EXPENSIFY_CARD.BANK,
status: CONST.TRANSACTION.STATUS.PENDING,
};
const fakeTransaction2: Transaction = {
...createRandomTransaction(1),
reportID,
amount: 0,
modifiedAmount: 0,
receipt: {
state: CONST.IOU.RECEIPT_STATE.SCANFAILED,
},
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
modifiedMerchant: undefined,
};

await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`, fakeReport);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction1.transactionID}`, fakeTransaction1);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction2.transactionID}`, fakeTransaction2);

await waitForBatchedUpdates();

expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy();
});
it('should return true if at least one transactions is not pending card or scan failure transaction', async () => {
const policyID = '2';
const reportID = '1';
const fakePolicy: Policy = {
...createRandomPolicy(Number(policyID)),
type: CONST.POLICY.TYPE.TEAM,
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
};
const fakeReport: Report = {
...createRandomReport(Number(reportID)),
type: CONST.REPORT.TYPE.EXPENSE,
policyID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
managerID: RORY_ACCOUNT_ID,
};
const fakeTransaction1: Transaction = {
...createRandomTransaction(0),
reportID,
bank: CONST.EXPENSIFY_CARD.BANK,
status: CONST.TRANSACTION.STATUS.PENDING,
};
const fakeTransaction2: Transaction = {
...createRandomTransaction(1),
reportID,
amount: 0,
receipt: {
state: CONST.IOU.RECEIPT_STATE.SCANFAILED,
},
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
modifiedMerchant: undefined,
};
const fakeTransaction3: Transaction = {
...createRandomTransaction(2),
reportID,
amount: 100,
};

await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${fakeReport.reportID}`, fakeReport);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction1.transactionID}`, fakeTransaction1);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction2.transactionID}`, fakeTransaction2);
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${fakeTransaction3.transactionID}`, fakeTransaction3);

await waitForBatchedUpdates();

expect(canApproveIOU(fakeReport, fakePolicy)).toBeTruthy();
});
});
});
5 changes: 5 additions & 0 deletions tests/unit/DebugUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,11 @@ describe('DebugUtils', () => {
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
type: CONST.POLICY.TYPE.CORPORATE,
},
[`${ONYXKEYS.COLLECTION.TRANSACTION}1` as const]: {
amount: -100,
currency: CONST.CURRENCY.USD,
reportID: '2',
},
[ONYXKEYS.SESSION]: {
accountID: 12345,
},
Expand Down

0 comments on commit b10a56a

Please sign in to comment.