Skip to content

Fix/90661 #91326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ChavdaSachin wants to merge 5 commits into from
Closed

Fix/90661 #91326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
39facc2
Fix amount field validation for invoices
ChavdaSachin May 21, 2026
d6b128c
Add test for zero amount invoice
ChavdaSachin May 21, 2026
a1ad59d
Fix zero-amount self-DM FAB submission
ChavdaSachin May 21, 2026
73bda69
Add tests for zero-amount trackExpense
ChavdaSachin May 21, 2026
7759af0
correct the isP2P check
ChavdaSachin May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/components/MoneyRequestConfirmationList/hooks/useConfirmationValidation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type {OnyxEntry} from 'react-native-onyx';
import {useCurrencyListActions} from '@hooks/useCurrencyList';
import {isValidPerDiemExpenseAmount} from '@libs/actions/IOU/PerDiem';
import {getIsMissingAttendeesViolation} from '@libs/AttendeeUtils';
import {validateAmount} from '@libs/MoneyRequestUtils';
import {isValidMoneyRequestAmount, validateAmount} from '@libs/MoneyRequestUtils';
import type {getTagLists as getTagListsFn} from '@libs/PolicyUtils';
import {isAttendeeTrackingEnabled} from '@libs/PolicyUtils';
import {hasEnabledTags, hasMatchingTag} from '@libs/TagsOptionsListUtils';
Expand Down Expand Up @@ -157,10 +157,12 @@ function useConfirmationValidation({
}

const firstParticipant = transaction?.participants?.at(0);
const isP2P = !!(firstParticipant?.accountID && !firstParticipant?.isPolicyExpenseChat);
const isSelfDM = !!firstParticipant?.isSelfDM;
const isP2P = !!(firstParticipant?.accountID && !firstParticipant?.isPolicyExpenseChat && !isSelfDM);

// P2P manual submit: $0 is invalid unless scan/time/distance (same guard as legacy inline confirm).
if (!isScanRequestUtil(transaction) && !isTimeRequest && !isDistanceRequest && iouAmount === 0 && isP2P) {
// Zero or invalid amounts are blocked for invoice, pay, split, and P2P submit/request flows.
// Scan, time, distance, and per-diem requests have their own amount rules below.
if (!isScanRequestUtil(transaction) && !isTimeRequest && !isDistanceRequest && !isPerDiemRequest && !isValidMoneyRequestAmount(iouAmount, iouType, true, isP2P, isSelfDM)) {
return {errorKey: 'common.error.invalidAmount'};
}
if (isNewManualExpenseFlowEnabled && !transaction?.isAmountSet) {
Comment on lines +165 to 168
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check amount-set state before invalid-amount validation

When NEW_MANUAL_EXPENSE_FLOW is enabled and the amount input is still untouched (transaction.isAmountSet is false), this new guard now returns common.error.invalidAmount for INVOICE, PAY, and SPLIT because iouAmount defaults to 0. That bypasses the intended required-field path immediately below and shows the wrong validation state for an empty field. This regression is introduced by broadening the invalid-amount check at this point in the flow; the isAmountSet check should run first (or be folded into this condition) so empty amounts still surface common.error.fieldRequired in manual-expense confirmation.

Useful? React with 👍 / 👎.

Expand Down
10 changes: 6 additions & 4 deletions src/components/MoneyRequestConfirmationList/sections/AmountField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import {clearMoneyRequestAmount, getMoneyRequestParticipantsFromReport, setMoneyRequestAmount} from '@libs/actions/IOU/MoneyRequest';
import {convertToBackendAmount, convertToFrontendAmountAsString, getLocalizedCurrencySymbol} from '@libs/CurrencyUtils';
import {calculateAmount} from '@libs/IOUUtils';
import {isValidMoneyRequestAmount} from '@libs/MoneyRequestUtils';
import Navigation from '@libs/Navigation/Navigation';
import {shouldEnableNegative} from '@libs/ReportUtils';
import {isAmountMissing} from '@libs/TransactionUtils';
Expand Down Expand Up @@ -88,9 +89,10 @@ function AmountField({

const isAmountFieldDisabled = didConfirm || isReadOnly || shouldShowTimeRequestFields || isDistanceRequest;
const firstParticipant = transaction?.participants?.at(0);
const isP2P = isNewManualExpenseFlowEnabled
? isParticipantP2P(getMoneyRequestParticipantsFromReport(report, currentUserPersonalDetails.accountID).at(0))
: !!(firstParticipant?.accountID && !firstParticipant?.isPolicyExpenseChat);
const participantForAmountValidation =
isNewManualExpenseFlowEnabled && firstParticipant ? firstParticipant : getMoneyRequestParticipantsFromReport(report, currentUserPersonalDetails.accountID).at(0);
const isSelfDMParticipant = !!participantForAmountValidation?.isSelfDM;
const isP2P = isParticipantP2P(participantForAmountValidation);
const shouldShowAmountRequiredError = formError === 'common.error.fieldRequired';
const shouldShowAmountInvalidError = formError === 'common.error.invalidAmount';

Expand Down Expand Up @@ -236,7 +238,7 @@ function AmountField({
return;
}

const isInlineAmountInvalid = parsedAmount === 0 && isP2P;
const isInlineAmountInvalid = !isDistanceRequest && !shouldShowTimeRequestFields && !isValidMoneyRequestAmount(parsedAmount, iouType, allowNegative, isP2P, isSelfDMParticipant);

if (isInlineAmountInvalid && shouldDisplayFieldError) {
setFormError('common.error.invalidAmount');
Expand Down
5 changes: 3 additions & 2 deletions src/libs/MoneyRequestUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ const nonZeroMoneyRequestTypes = new Set<ValueOf<typeof CONST.IOU.TYPE>>([CONST.
* @param allowNegative - Whether negative amounts are allowed
* @param isIOUReport - Whether this is an IOU report (zero amounts not allowed)
* @param isP2P - Whether this is a peer-to-peer transaction
* @param isSelfDM - Whether the expense is being sent to the user's self DM
*/
function isValidMoneyRequestAmount(amount: number | undefined, iouType: ValueOf<typeof CONST.IOU.TYPE>, allowNegative = true, isP2P = false): boolean {
function isValidMoneyRequestAmount(amount: number | undefined, iouType: ValueOf<typeof CONST.IOU.TYPE>, allowNegative = true, isP2P = false, isSelfDM = false): boolean {
if (amount === undefined || amount === null || Number.isNaN(amount)) {
return false;
}
Expand All @@ -148,7 +149,7 @@ function isValidMoneyRequestAmount(amount: number | undefined, iouType: ValueOf<

const absoluteAmount = Math.abs(amount);

if ((iouType === CONST.IOU.TYPE.REQUEST || iouType === CONST.IOU.TYPE.SUBMIT) && isP2P) {
if ((iouType === CONST.IOU.TYPE.REQUEST || iouType === CONST.IOU.TYPE.SUBMIT) && isP2P && !isSelfDM) {
return absoluteAmount >= 1;
}

Expand Down
20 changes: 19 additions & 1 deletion src/pages/iou/request/step/IOURequestStepConfirmation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import {
} from '@libs/TransactionUtils';
import {getIOURequestPolicyID, getMoneyRequestParticipantsFromReport, setMoneyRequestParticipants, setMoneyRequestParticipantsFromReport} from '@userActions/IOU/MoneyRequest';
import {setMoneyRequestReceipt} from '@userActions/IOU/Receipt';
import {setTransactionReport} from '@userActions/Transaction';
import {removeDraftTransaction, replaceDefaultDraftTransaction} from '@userActions/TransactionEdit';
import CONST from '@src/CONST';
import type {IOUType} from '@src/CONST';
Expand Down Expand Up @@ -114,6 +115,7 @@ function IOURequestStepConfirmation({
report: reportReal,
reportDraft,
route,
navigation,
transaction: initialTransaction,
isLoadingTransaction,
shouldHideHeader = false,
Expand Down Expand Up @@ -343,12 +345,28 @@ function IOURequestStepConfirmation({
if (!activeTransactionID) {
return;
}

const firstParticipant = participantsList.at(0);
if (isNewManualExpenseFlowEnabled && firstParticipant?.isSelfDM && selfDMReport?.reportID && iouType !== CONST.IOU.TYPE.SPLIT) {
for (const draftTransaction of transactions) {
const selfDMParticipants = getMoneyRequestParticipantsFromReport(selfDMReport, currentUserPersonalDetails.accountID).map((participant) => ({
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ PERF-13 (docs)

getMoneyRequestParticipantsFromReport(selfDMReport, currentUserPersonalDetails.accountID).map(...) is called inside the for (const draftTransaction of transactions) loop but does not depend on draftTransaction. The result is identical on every iteration, creating redundant O(n) computation.

Hoist the selfDMParticipants computation above the loop:

const selfDMParticipants = getMoneyRequestParticipantsFromReport(selfDMReport, currentUserPersonalDetails.accountID).map((participant) => ({
    ...participant,
    iouType: CONST.IOU.TYPE.TRACK,
}));
for (const draftTransaction of transactions) {
    setMoneyRequestParticipants(draftTransaction.transactionID, selfDMParticipants);
    setTransactionReport(draftTransaction.transactionID, {reportID: CONST.REPORT.UNREPORTED_REPORT_ID}, true);
}

Reviewed at: 7759af0 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

Copy link
Copy Markdown
Contributor

@ShridharGoel ShridharGoel May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChavdaSachin What do you think about this?

...participant,
iouType: CONST.IOU.TYPE.TRACK,
}));
setMoneyRequestParticipants(draftTransaction.transactionID, selfDMParticipants);
setTransactionReport(draftTransaction.transactionID, {reportID: CONST.REPORT.UNREPORTED_REPORT_ID}, true);
}
Navigation.setParams({iouType: CONST.IOU.TYPE.TRACK, reportID: selfDMReport.reportID}, route.key, navigation.getState()?.key);
closeParticipantPicker();
return;
}

setMoneyRequestParticipants(activeTransactionID, participantsList);
if (participantsList.length > 0) {
closeParticipantPicker();
}
},
[activeTransactionID, closeParticipantPicker],
[activeTransactionID, closeParticipantPicker, currentUserPersonalDetails.accountID, isNewManualExpenseFlowEnabled, iouType, navigation, route.key, selfDMReport, transactions],
);

useEffect(() => {
Expand Down
5 changes: 5 additions & 0 deletions tests/unit/MoneyRequestUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ describe('ReportActionsUtils', () => {
expect(isValidMoneyRequestAmount(0, CONST.IOU.TYPE.SUBMIT, allowNegative, isP2P)).toBe(false);
});

it('should allow zero amount for self DM', () => {
expect(isValidMoneyRequestAmount(0, CONST.IOU.TYPE.SUBMIT, allowNegative, isP2P, true)).toBe(true);
expect(isValidMoneyRequestAmount(0, CONST.IOU.TYPE.REQUEST, allowNegative, isP2P, true)).toBe(true);
});

it('should return true for amounts >= 1 cent', () => {
expect(isValidMoneyRequestAmount(1, CONST.IOU.TYPE.REQUEST, allowNegative, isP2P)).toBe(true);
expect(isValidMoneyRequestAmount(100, CONST.IOU.TYPE.REQUEST, allowNegative, isP2P)).toBe(true);
Expand Down
34 changes: 34 additions & 0 deletions tests/unit/hooks/useConfirmationValidation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,40 @@ describe('useConfirmationValidation', () => {
expect(result.current.validate()).toEqual({errorKey: 'iou.error.invalidAmount'});
});

it('returns invalidAmount for invoice with zero amount', () => {
const {result} = renderHook(() =>
useConfirmationValidation({
...baseParams,
iouType: CONST.IOU.TYPE.INVOICE,
iouAmount: 0,
transaction: {
transactionID: 'txn1',
amount: 0,
comment: {},
participants: [{accountID: 2, isPolicyExpenseChat: true}],
} as unknown as OnyxTypes.Transaction,
}),
);
expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'});
});

it('allows zero amount for self DM submit expense', () => {
const {result} = renderHook(() =>
useConfirmationValidation({
...baseParams,
iouAmount: 0,
transaction: {
transactionID: 'txn1',
amount: 0,
isAmountSet: true,
comment: {},
participants: [{accountID: 0, reportID: 'self-dm', isSelfDM: true, selected: true}],
} as unknown as OnyxTypes.Transaction,
}),
);
expect(result.current.validate()).toEqual({errorKey: null});
});

it('returns errorKey: null on successful non-PAY validation', () => {
const {result} = renderHook(() => useConfirmationValidation(baseParams));
expect(result.current.validate()).toEqual({errorKey: null});
Expand Down
Loading