Skip to content

Commit 4cdf916

Browse files
jasperhuanggjasperhuangg
authored
Merge pull request Expensify#67941 from mkzie2/mkzie2-issue/67173
fix: approve button is present after submitting a scan expense with missing amount
2 parents 3ddd02c + e6a10fc commit 4cdf916

5 files changed

Lines changed: 96 additions & 18 deletions

File tree

‎src/components/MoneyReportHeader.tsx‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {getThreadReportIDsForTransactions, getTotalAmountForIOUReportPreviewButt
2727
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
2828
import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types';
2929
import type {ReportsSplitNavigatorParamList, SearchFullscreenNavigatorParamList, SearchReportParamList} from '@libs/Navigation/types';
30-
import {buildOptimisticNextStepForPreventSelfApprovalsEnabled} from '@libs/NextStepUtils';
30+
import {getReportNextStep} from '@libs/NextStepUtils';
3131
import {isSecondaryActionAPaymentOption, selectPaymentType} from '@libs/PaymentUtils';
3232
import type {KYCFlowEvent, TriggerKYCFlow} from '@libs/PaymentUtils';
3333
import {getConnectedIntegration, getValidConnectedIntegration} from '@libs/PolicyUtils';
@@ -40,7 +40,6 @@ import {
4040
getArchiveReason,
4141
getIntegrationExportIcon,
4242
getIntegrationNameFromExportMessage as getIntegrationNameFromExportMessageUtils,
43-
getNextApproverAccountID,
4443
getNonHeldAndFullAmount,
4544
getTransactionsWithReceipts,
4645
hasHeldExpenses as hasHeldExpensesReportUtils,
@@ -50,7 +49,6 @@ import {
5049
isExported as isExportedUtils,
5150
isInvoiceReport as isInvoiceReportUtil,
5251
isProcessingReport,
53-
isReportOwner,
5452
navigateOnDeleteExpense,
5553
navigateToDetailsPage,
5654
} from '@libs/ReportUtils';
@@ -363,12 +361,7 @@ function MoneyReportHeader({
363361
const shouldShowStatusBar =
364362
hasAllPendingRTERViolations || shouldShowBrokenConnectionViolation || hasOnlyHeldExpenses || hasScanningReceipt || isPayAtEndExpense || hasOnlyPendingTransactions || hasDuplicates;
365363

366-
// 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
367-
// We should always show this optimistic message for policies with preventSelfApproval
368-
// to avoid any flicker during transitions between online/offline states
369-
const nextApproverAccountID = getNextApproverAccountID(moneyRequestReport);
370-
const isSubmitterSameAsNextApprover = isReportOwner(moneyRequestReport) && nextApproverAccountID === moneyRequestReport?.ownerAccountID;
371-
const optimisticNextStep = isSubmitterSameAsNextApprover && policy?.preventSelfApproval ? buildOptimisticNextStepForPreventSelfApprovalsEnabled() : nextStep;
364+
const optimisticNextStep = getReportNextStep(nextStep, moneyRequestReport, transactions, policy);
372365

373366
const shouldShowNextStep = isFromPaidPolicy && !isInvoiceReport && !shouldShowStatusBar;
374367
const {nonHeldAmount, fullAmount, hasValidNonHeldAmount} = getNonHeldAndFullAmount(moneyRequestReport, shouldShowPayButton);

‎src/libs/NextStepUtils.ts‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
55
import type {ValueOf} from 'type-fest';
66
import CONST from '@src/CONST';
77
import ONYXKEYS from '@src/ONYXKEYS';
8-
import type {Beta, Policy, Report, ReportNextStep, TransactionViolations} from '@src/types/onyx';
8+
import type {Beta, Policy, Report, ReportNextStep, Transaction, TransactionViolations} from '@src/types/onyx';
99
import type {Message} from '@src/types/onyx/ReportNextStep';
1010
import type DeepValueOf from '@src/types/utils/DeepValueOf';
1111
import EmailUtils from './EmailUtils';
@@ -20,8 +20,12 @@ import {
2020
hasViolations as hasViolationsReportUtils,
2121
isExpenseReport,
2222
isInvoiceReport,
23+
isOpenExpenseReport,
2324
isPayer,
25+
isProcessingReport,
26+
isReportOwner,
2427
} from './ReportUtils';
28+
import {isPendingCardOrIncompleteTransaction, isPendingCardOrScanningTransaction} from './TransactionUtils';
2529

2630
let currentUserAccountID = -1;
2731
let currentUserEmail = '';
@@ -124,6 +128,53 @@ function buildOptimisticNextStepForPreventSelfApprovalsEnabled() {
124128
return optimisticNextStep;
125129
}
126130

131+
function buildOptimisticFixIssueNextStep() {
132+
const optimisticNextStep: ReportNextStep = {
133+
type: 'neutral',
134+
icon: CONST.NEXT_STEP.ICONS.HOURGLASS,
135+
message: [
136+
{
137+
text: 'Waiting for ',
138+
},
139+
{
140+
text: `you`,
141+
type: 'strong',
142+
},
143+
{
144+
text: ' to ',
145+
},
146+
{
147+
text: 'fix the issue(s)',
148+
},
149+
],
150+
};
151+
152+
return optimisticNextStep;
153+
}
154+
155+
function getReportNextStep(currentNextStep: ReportNextStep | undefined, moneyRequestReport: OnyxEntry<Report>, transactions: Array<OnyxEntry<Transaction>>, policy: OnyxEntry<Policy>) {
156+
const nextApproverAccountID = getNextApproverAccountID(moneyRequestReport);
157+
158+
if (isOpenExpenseReport(moneyRequestReport) && transactions.length > 0 && transactions.every((transaction) => isPendingCardOrIncompleteTransaction(transaction))) {
159+
return buildOptimisticFixIssueNextStep();
160+
}
161+
162+
if (isProcessingReport(moneyRequestReport) && transactions.length > 0 && transactions.every((transaction) => isPendingCardOrScanningTransaction(transaction))) {
163+
return buildOptimisticFixIssueNextStep();
164+
}
165+
166+
const isSubmitterSameAsNextApprover = isReportOwner(moneyRequestReport) && nextApproverAccountID === moneyRequestReport?.ownerAccountID;
167+
168+
// 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
169+
// We should always show this optimistic message for policies with preventSelfApproval
170+
// to avoid any flicker during transitions between online/offline states
171+
if (isSubmitterSameAsNextApprover && policy?.preventSelfApproval) {
172+
return buildOptimisticNextStepForPreventSelfApprovalsEnabled();
173+
}
174+
175+
return currentNextStep;
176+
}
177+
127178
/**
128179
* Generates an optimistic nextStep based on a current report status and other properties.
129180
*
@@ -498,4 +549,4 @@ function buildNextStep(
498549
return optimisticNextStep;
499550
}
500551

501-
export {parseMessage, buildNextStep, buildOptimisticNextStepForPreventSelfApprovalsEnabled};
552+
export {parseMessage, buildNextStep, buildOptimisticNextStepForPreventSelfApprovalsEnabled, getReportNextStep};

‎src/libs/ReportPrimaryActionUtils.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ import {
4343
hasPendingRTERViolation as hasPendingRTERViolationTransactionUtils,
4444
isDuplicate,
4545
isOnHold as isOnHoldTransactionUtils,
46-
isPending,
46+
isPendingCardOrIncompleteTransaction,
47+
isPendingCardOrScanningTransaction,
4748
isScanning,
4849
shouldShowBrokenConnectionViolationForMultipleTransactions,
4950
shouldShowBrokenConnectionViolation as shouldShowBrokenConnectionViolationTransactionUtils,
@@ -83,7 +84,7 @@ function isSubmitAction(report: Report, reportTransactions: Transaction[], polic
8384
const isManualSubmitEnabled = getCorrectedAutoReportingFrequency(policy) === CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MANUAL;
8485
const transactionAreComplete = reportTransactions.every((transaction) => transaction.amount !== 0 || transaction.modifiedAmount !== 0);
8586

86-
if (reportTransactions.length > 0 && reportTransactions.every((transaction) => isPending(transaction))) {
87+
if (reportTransactions.length > 0 && reportTransactions.every((transaction) => isPendingCardOrIncompleteTransaction(transaction))) {
8788
return false;
8889
}
8990

@@ -128,7 +129,7 @@ function isApproveAction(report: Report, reportTransactions: Transaction[], poli
128129
return false;
129130
}
130131

131-
if (reportTransactions.length > 0 && reportTransactions.every((transaction) => isPending(transaction))) {
132+
if (reportTransactions.length > 0 && reportTransactions.every((transaction) => isPendingCardOrScanningTransaction(transaction))) {
132133
return false;
133134
}
134135

‎src/libs/TransactionUtils/index.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,10 @@ function isPendingCardOrScanningTransaction(transaction: OnyxEntry<Transaction>)
233233
return (isExpensifyCardTransaction(transaction) && isPending(transaction)) || isPartialTransaction(transaction) || (isScanRequest(transaction) && isScanning(transaction));
234234
}
235235

236+
function isPendingCardOrIncompleteTransaction(transaction: OnyxEntry<Transaction>): boolean {
237+
return (isExpensifyCardTransaction(transaction) && isPending(transaction)) || (isAmountMissing(transaction) && isMerchantMissing(transaction));
238+
}
239+
236240
/**
237241
* Optimistically generate a transaction.
238242
*
@@ -1976,6 +1980,7 @@ export {
19761980
isDemoTransaction,
19771981
shouldShowViolation,
19781982
isUnreportedAndHasInvalidDistanceRateTransaction,
1983+
isPendingCardOrIncompleteTransaction,
19791984
getTransactionViolationsOfTransaction,
19801985
isExpenseSplit,
19811986
};

‎tests/unit/ReportPrimaryActionUtilsTest.ts‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ describe('getPrimaryAction', () => {
8080
);
8181
});
8282

83-
it('should not return SUBMIT option for admin with only pending transactions', async () => {
83+
it('should not return SUBMIT option for admin with only pending/incomplete transactions', async () => {
8484
const report = {
8585
reportID: REPORT_ID,
8686
type: CONST.REPORT.TYPE.EXPENSE,
@@ -99,9 +99,22 @@ describe('getPrimaryAction', () => {
9999
amount: 10,
100100
merchant: 'Merchant',
101101
date: '2025-01-01',
102+
bank: CONST.EXPENSIFY_CARD.BANK,
102103
} as unknown as Transaction;
103104

104-
expect(getReportPrimaryAction({report, chatReport, reportTransactions: [transaction], violations: {}, policy: policy as Policy, isChatReportArchived: false})).toBe('');
105+
const transaction1 = {
106+
reportID: `${REPORT_ID}`,
107+
amount: 0,
108+
modifiedAmount: 0,
109+
receipt: {
110+
source: 'test',
111+
state: CONST.IOU.RECEIPT_STATE.SCAN_FAILED,
112+
},
113+
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
114+
modifiedMerchant: undefined,
115+
} as unknown as Transaction;
116+
117+
expect(getReportPrimaryAction({report, chatReport, reportTransactions: [transaction, transaction1], violations: {}, policy: policy as Policy, isChatReportArchived: false})).toBe('');
105118
});
106119

107120
it('should return Approve for report being processed', async () => {
@@ -123,6 +136,8 @@ describe('getPrimaryAction', () => {
123136
comment: {
124137
hold: 'Hold',
125138
},
139+
amount: 10,
140+
merchant: 'merchant',
126141
} as unknown as Transaction;
127142

128143
expect(getReportPrimaryAction({report, chatReport, reportTransactions: [transaction], violations: {}, policy: policy as Policy, isChatReportArchived: false})).toBe(
@@ -157,7 +172,7 @@ describe('getPrimaryAction', () => {
157172
expect(getReportPrimaryAction({report, chatReport, reportTransactions: [transaction], violations: {}, policy: policy as Policy, isChatReportArchived: false})).toBe('');
158173
});
159174

160-
it('should return empty for report being processed but transactions are pending', async () => {
175+
it('should return empty for report being processed but transactions are pending/partial', async () => {
161176
const report = {
162177
reportID: REPORT_ID,
163178
type: CONST.REPORT.TYPE.EXPENSE,
@@ -177,9 +192,22 @@ describe('getPrimaryAction', () => {
177192
amount: 10,
178193
merchant: 'Merchant',
179194
date: '2025-01-01',
195+
bank: CONST.EXPENSIFY_CARD.BANK,
180196
} as unknown as Transaction;
181197

182-
expect(getReportPrimaryAction({report, chatReport, reportTransactions: [transaction], violations: {}, policy: policy as Policy, isChatReportArchived: false})).toBe('');
198+
const transaction1 = {
199+
reportID: `${REPORT_ID}`,
200+
amount: 0,
201+
modifiedAmount: 0,
202+
receipt: {
203+
source: 'test',
204+
state: CONST.IOU.RECEIPT_STATE.SCAN_FAILED,
205+
},
206+
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
207+
modifiedMerchant: undefined,
208+
} as unknown as Transaction;
209+
210+
expect(getReportPrimaryAction({report, chatReport, reportTransactions: [transaction, transaction1], violations: {}, policy: policy as Policy, isChatReportArchived: false})).toBe('');
183211
});
184212

185213
it('should return PAY for submitted invoice report if paid as personal', async () => {

0 commit comments

Comments
 (0)