diff --git a/src/components/MoneyRequestConfirmationFields/Provider.tsx b/src/components/MoneyRequestConfirmationFields/Provider.tsx index bbee4df05fbe..3e695821b282 100644 --- a/src/components/MoneyRequestConfirmationFields/Provider.tsx +++ b/src/components/MoneyRequestConfirmationFields/Provider.tsx @@ -1,3 +1,4 @@ +import type {ConfirmationFieldViewRefs} from '@components/MoneyRequestConfirmationList/hooks/useScrollToFlaggedField'; import type {MeasurableInput} from '@components/SelectionList/SelectionListWithSections/types'; import type {IOUAction, IOUType} from '@src/CONST'; @@ -70,6 +71,9 @@ type ProviderProps = { /** Scrolls the surface so an inline field's input is not hidden behind the keyboard when focused (new manual expense flow) */ scrollFocusedInputIntoView?: (input: MeasurableInput) => void; + /** The refs the amount, merchant and date fields attach their outer view to, so a blocked confirm can scroll to them */ + fieldViewRefs?: ConfirmationFieldViewRefs; + /** Submits the whole expense (used by inline inputs to keep Enter-to-confirm on hardware-keyboard setups) */ onSubmitForm?: () => void; @@ -101,6 +105,7 @@ function Provider({ isOdometerDistanceRequest = false, isGPSDistanceRequest = false, scrollFocusedInputIntoView, + fieldViewRefs, onSubmitForm, onTaxAmountEmptyChange, children, @@ -126,6 +131,7 @@ function Provider({ isOdometerDistanceRequest, isGPSDistanceRequest, scrollFocusedInputIntoView, + fieldViewRefs, onSubmitForm, onTaxAmountEmptyChange, }; diff --git a/src/components/MoneyRequestConfirmationFields/context.ts b/src/components/MoneyRequestConfirmationFields/context.ts index 2979665143fd..c1e9b6a8f5ea 100644 --- a/src/components/MoneyRequestConfirmationFields/context.ts +++ b/src/components/MoneyRequestConfirmationFields/context.ts @@ -1,3 +1,4 @@ +import type {ConfirmationFieldViewRefs} from '@components/MoneyRequestConfirmationList/hooks/useScrollToFlaggedField'; import type {MeasurableInput} from '@components/SelectionList/SelectionListWithSections/types'; import type {IOUAction, IOUType} from '@src/CONST'; @@ -40,6 +41,9 @@ type ConfirmationFieldsContextValue = { /** Scrolls the surface so an inline field's input is not hidden behind the keyboard when focused (new manual expense flow). */ scrollFocusedInputIntoView?: (input: MeasurableInput) => void; + /** The refs the amount, merchant and date fields attach their outer view to, so a blocked confirm can scroll to them */ + fieldViewRefs?: ConfirmationFieldViewRefs; + /** Submits the whole expense. Used by inline inputs to keep Enter-to-confirm on hardware-keyboard setups (new manual expense flow). */ onSubmitForm?: () => void; diff --git a/src/components/MoneyRequestConfirmationList/confirmAction.ts b/src/components/MoneyRequestConfirmationList/confirmAction.ts index ceae1d385c2f..0206969f74af 100644 --- a/src/components/MoneyRequestConfirmationList/confirmAction.ts +++ b/src/components/MoneyRequestConfirmationList/confirmAction.ts @@ -15,6 +15,8 @@ import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage'; import type {OnyxEntry} from 'react-native-onyx'; +import type {ValidationResult} from './hooks/useConfirmationValidation'; + type BuildConfirmActionParams = { /** IOU type being confirmed (submit / split / track / pay / invoice) */ iouType: IOUType; @@ -34,11 +36,17 @@ type BuildConfirmActionParams = { /** Whether the current user is a delegate without permission to pay */ isDelegateAccessRestricted: boolean; - /** Pure validator that returns the first failing translation key, or null on success */ - validate: (paymentType?: PaymentMethodType) => {errorKey: TranslationPaths; shouldSetDidConfirmSplit?: boolean} | {errorKey: null} | null; + /** Returns the first check that blocks confirming, `{errorKey: null}` when none does, or null when validation can't run yet */ + validate: (paymentType?: PaymentMethodType) => ValidationResult | null; + + /** Sets the form error for a check that blocks confirming */ + setConfirmError: (error: TranslationPaths) => void; + + /** Drops the form error the last confirm set, since validation checks it again on the next confirm */ + clearConfirmError: () => void; - /** Setter for the form-level error key */ - setFormError: (error: TranslationPaths | '') => void; + /** Shows the errors of the amount, date and merchant fields, which render on the fields rather than in the form error */ + revealFieldErrors: () => void; /** Setter for the "user already confirmed split" flag */ setDidConfirmSplit: (value: boolean) => void; @@ -56,10 +64,10 @@ type BuildConfirmActionParams = { /** * Owns the click-confirm action for the Money Request confirmation flow. * - * Validation always runs first and its result drives form-error state. Past it there are three - * branches: (1) invoice-without-company-info routes to the company info step; (2) non-PAY types - * invoke `onConfirm`; (3) PAY types run delegate-access gating and invoke `onSendMoney` with the - * chosen payment method. + * Validation always runs first. A field error is revealed on its field, dropping a form error an earlier confirm + * set, and any other failure becomes the form error. Past validation there are three branches: (1) invoice-without-company-info routes to the + * company info step; (2) non-PAY types invoke `onConfirm`; (3) PAY types run delegate-access gating and + * invoke `onSendMoney` with the chosen payment method. */ function buildConfirmAction({ iouType, @@ -69,7 +77,9 @@ function buildConfirmAction({ formError, isDelegateAccessRestricted, validate, - setFormError, + setConfirmError, + clearConfirmError, + revealFieldErrors, setDidConfirmSplit, showDelegateNoAccessModal, onConfirm, @@ -81,11 +91,17 @@ function buildConfirmAction({ return; } + if ('isFieldError' in result) { + clearConfirmError(); + revealFieldErrors(); + return; + } + if (result.errorKey) { if (result.shouldSetDidConfirmSplit) { setDidConfirmSplit(true); } - setFormError(result.errorKey); + setConfirmError(result.errorKey); return; } diff --git a/src/components/MoneyRequestConfirmationList/getConfirmationFieldErrors.ts b/src/components/MoneyRequestConfirmationList/getConfirmationFieldErrors.ts new file mode 100644 index 000000000000..128bc012d557 --- /dev/null +++ b/src/components/MoneyRequestConfirmationList/getConfirmationFieldErrors.ts @@ -0,0 +1,172 @@ +/** + * The errors the amount, date and merchant fields of the confirmation show under their inputs. They are computed + * from the transaction on every render, so an error goes away as soon as its field is fixed and nothing has to + * remember to clear it. The same map blocks confirmation, flags each field and picks the field a blocked confirm + * scrolls to, so those three can't disagree about what is missing. + */ +import {isP2PRecipient, isParticipantP2P} from '@libs/IOUUtils'; +import {getConfirmationAmount, isConfirmationAmountMissing, isConfirmationDateMissing, isConfirmationMerchantMissing, isValidMoneyRequestAmount} from '@libs/MoneyRequestUtils'; +import {getMerchant, isScanRequest} from '@libs/TransactionUtils'; +import {isInvalidMerchantValue, isMerchantBlank, isValidInputLength} from '@libs/ValidationUtils'; + +import type {IOUType} from '@src/CONST'; +import CONST from '@src/CONST'; +import type {TranslationPaths} from '@src/languages/types'; +import type * as OnyxTypes from '@src/types/onyx'; + +import type {OnyxEntry} from 'react-native-onyx'; + +type ConfirmationFieldErrors = { + amount?: TranslationPaths; + date?: TranslationPaths; + merchant?: TranslationPaths; +}; + +type ConfirmationField = keyof ConfirmationFieldErrors; + +type GetConfirmationFieldErrorsParams = { + /** Total amount of the expense, in the smallest currency unit */ + iouAmount: number; + + iouType: IOUType; + + iouMerchant: string | undefined; + + /** Whether this surface offers manual entry of the amount / merchant / date. False for test receipts and moved tracked expenses. */ + canEnterScanFieldsManually: boolean; + + /** Whether the expense goes to a workspace chat, the only destination where a manually entered Scan needs a merchant */ + isPolicyExpenseChat: boolean; + + /** Whether the date field is rendered. A hidden date can't be filled in, so it is never reported missing. */ + shouldShowDate: boolean; + + /** Whether the confirmation is read-only. A read-only date is populated server-side, so it can't be missing. */ + isReadOnly: boolean; + + isEditingSplitBill: boolean | undefined; + + isMerchantRequired: boolean | undefined; + + isDistanceRequest: boolean; + + isTimeRequest: boolean; + + currentUserAccountID: number; +}; + +type GetSiblingFieldErrorsParams = Omit & { + /** The recipient on screen. Every receipt goes to it, even when another receipt's own draft wasn't updated. */ + participants: OnyxTypes.Transaction['participants']; +}; + +type SiblingFieldErrors = { + transactionID: string; + fieldErrors: ConfirmationFieldErrors; +}; + +/** Returned whenever no field error is shown, so the error state passed down the footer keeps its identity while nothing is flagged. */ +const NO_FIELD_ERRORS: Readonly = Object.freeze({}); + +function getAmountFieldError(transaction: OnyxEntry, params: GetConfirmationFieldErrorsParams): TranslationPaths | undefined { + const {iouAmount, iouType, canEnterScanFieldsManually, isPolicyExpenseChat, isEditingSplitBill, isDistanceRequest, isTimeRequest, currentUserAccountID} = params; + const firstParticipant = transaction?.participants?.at(0); + const isP2P = isParticipantP2P(firstParticipant); + + // This runs before the $0 check so an amount the user never entered reads as missing, whoever the expense goes to. + if (isConfirmationAmountMissing(transaction, canEnterScanFieldsManually, isPolicyExpenseChat)) { + return 'common.error.fieldRequired'; + } + + // A manual expense to a person can't be $0, but scan, time and distance amounts are exempt since they're read off the receipt or computed. + if (!isScanRequest(transaction) && !isTimeRequest && !isDistanceRequest && iouAmount === 0 && isP2P) { + return 'common.error.invalidAmount'; + } + + // A typed Scan amount is checked like a manual one, because a Scan can pick its 1:1 recipient first and so never hits the participant picker's negative amount guard. + const canAmountBeTyped = transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.MANUAL || canEnterScanFieldsManually; + if ( + canAmountBeTyped && + transaction?.isAmountSet && + !isTimeRequest && + !isDistanceRequest && + !isEditingSplitBill && + !isValidMoneyRequestAmount(iouAmount, iouType, !isP2PRecipient(firstParticipant, currentUserAccountID), isP2P) + ) { + return 'common.error.invalidAmount'; + } + + return undefined; +} + +function getMerchantFieldError(transaction: OnyxEntry, params: GetConfirmationFieldErrorsParams): TranslationPaths | undefined { + const {iouMerchant, canEnterScanFieldsManually, isPolicyExpenseChat, isMerchantRequired} = params; + const merchant = iouMerchant ?? ''; + + // The confirmation saves the merchant as typed, so its length is checked before trimming. + if (!isValidInputLength(merchant, CONST.MERCHANT_NAME_MAX_BYTES).isValid) { + return 'iou.error.invalidMerchant'; + } + + // The amount, merchant and date of a Scan are all-or-nothing, so once one is entered a blank merchant is missing. + if (isConfirmationMerchantMissing(transaction, canEnterScanFieldsManually, isPolicyExpenseChat)) { + return 'common.error.fieldRequired'; + } + + const isBlank = isMerchantBlank(transaction?.isMerchantSet, merchant); + const isTypedValueInvalid = !isBlank && isInvalidMerchantValue(merchant.trim()); + + if (!isMerchantRequired) { + // An optional merchant is only checked once the user typed one in. + return transaction?.isMerchantSet && isTypedValueInvalid ? 'iou.error.invalidMerchant' : undefined; + } + + // A required merchant left blank reads the same as the other required fields, and only a typed one is "invalid". + if (isBlank) { + return 'common.error.fieldRequired'; + } + return isTypedValueInvalid ? 'iou.error.invalidMerchant' : undefined; +} + +/** Returns the error each inline field of the confirmation would show right now, keyed by field. Empty when nothing blocks. */ +function getConfirmationFieldErrors(transaction: OnyxEntry, params: GetConfirmationFieldErrorsParams): ConfirmationFieldErrors { + const {shouldShowDate, isReadOnly, canEnterScanFieldsManually, isPolicyExpenseChat} = params; + const amount = getAmountFieldError(transaction, params); + const date = isConfirmationDateMissing(transaction, shouldShowDate, isReadOnly, canEnterScanFieldsManually, isPolicyExpenseChat) ? 'common.error.fieldRequired' : undefined; + const merchant = getMerchantFieldError(transaction, params); + + if (!amount && !date && !merchant) { + return NO_FIELD_ERRORS; + } + + return { + ...(amount && {amount}), + ...(date && {date}), + ...(merchant && {merchant}), + }; +} + +/** Checks emptiness by content, so it gives the same answer whether or not the map is the shared `NO_FIELD_ERRORS` */ +function hasFieldErrors(fieldErrors: ConfirmationFieldErrors): boolean { + return Object.keys(fieldErrors).length > 0; +} + +/** Returns the first other receipt of a multi-scan with a field error, since Create submits every receipt at once. */ +function getSiblingFieldErrors(siblingTransactions: OnyxTypes.Transaction[] | undefined, {participants, ...params}: GetSiblingFieldErrorsParams): SiblingFieldErrors | undefined { + for (const sibling of siblingTransactions ?? []) { + const fieldErrors = getConfirmationFieldErrors({...sibling, participants}, {...params, iouAmount: getConfirmationAmount(sibling), iouMerchant: getMerchant(sibling)}); + if (hasFieldErrors(fieldErrors)) { + return {transactionID: sibling.transactionID, fieldErrors}; + } + } + return undefined; +} + +/** Whether a field error blocks confirming, on the receipt on screen or on another receipt of a multi-scan. */ +function hasBlockingFieldErrors(fieldErrors: ConfirmationFieldErrors, siblingFieldErrors: SiblingFieldErrors | undefined): boolean { + return hasFieldErrors(fieldErrors) || !!siblingFieldErrors; +} + +export default getConfirmationFieldErrors; +export {NO_FIELD_ERRORS, getSiblingFieldErrors, hasBlockingFieldErrors, hasFieldErrors}; +export type {ConfirmationField, ConfirmationFieldErrors, GetConfirmationFieldErrorsParams, SiblingFieldErrors}; diff --git a/src/components/MoneyRequestConfirmationList/hooks/useConfirmationFieldErrors.ts b/src/components/MoneyRequestConfirmationList/hooks/useConfirmationFieldErrors.ts new file mode 100644 index 000000000000..8e3e568693a8 --- /dev/null +++ b/src/components/MoneyRequestConfirmationList/hooks/useConfirmationFieldErrors.ts @@ -0,0 +1,75 @@ +/** + * Owns when the amount, date and merchant errors of the money request confirmation become visible, and which of those + * fields a blocked confirm brings into view. + */ +import type {ConfirmationField, ConfirmationFieldErrors, SiblingFieldErrors} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import {hasBlockingFieldErrors, hasFieldErrors, NO_FIELD_ERRORS} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; + +import {useState} from 'react'; + +/** The order the fields appear in on every confirmation, so the first one flagged is the first one the user reaches */ +const FIELDS_IN_SCREEN_ORDER: ConfirmationField[] = ['amount', 'merchant', 'date']; + +type UseConfirmationFieldErrorsParams = { + /** The receipt on screen */ + transactionID: string | undefined; + + /** The errors the amount, date and merchant fields would show now, from `useConfirmationValidation` */ + fieldErrors: ConfirmationFieldErrors; + + /** The first other receipt of a multi-scan with a field error, from `useConfirmationValidation` */ + siblingFieldErrors: SiblingFieldErrors | undefined; + + /** Brings another receipt of a multi-scan on screen */ + onSwitchToTransaction: ((transactionID: string) => void) | undefined; + + /** Scrolls the given field of the given receipt into view */ + onRevealField: (field: ConfirmationField, transactionID: string | undefined) => void; +}; + +type UseConfirmationFieldErrorsResult = { + /** The field errors to render. Empty until a confirm is blocked by one of them. */ + visibleFieldErrors: ConfirmationFieldErrors; + + /** Shows the field errors after a confirm they blocked, bringing the receipt and the field they belong to into view */ + revealFieldErrors: () => void; +}; + +/** The errors themselves are derived from the transaction, so only the fact that a confirm was blocked is kept as state. */ +function useConfirmationFieldErrors({ + transactionID, + fieldErrors, + siblingFieldErrors, + onSwitchToTransaction, + onRevealField, +}: UseConfirmationFieldErrorsParams): UseConfirmationFieldErrorsResult { + const [shouldShowFieldErrors, setShouldShowFieldErrors] = useState(false); + + // Hide the errors again once nothing is left to fix, so a field the user empties afterwards is only flagged on the next confirm. + if (shouldShowFieldErrors && !hasBlockingFieldErrors(fieldErrors, siblingFieldErrors)) { + setShouldShowFieldErrors(false); + } + + const revealFieldErrors = () => { + setShouldShowFieldErrors(true); + + // The receipt on screen is fixed first, so another receipt is only brought on screen once this one has nothing left to fix. + const hasFieldErrorsOnScreen = hasFieldErrors(fieldErrors); + if (!hasFieldErrorsOnScreen && siblingFieldErrors) { + onSwitchToTransaction?.(siblingFieldErrors.transactionID); + } + + const blockingReceipt = hasFieldErrorsOnScreen ? {transactionID, fieldErrors} : siblingFieldErrors; + const firstFlaggedField = FIELDS_IN_SCREEN_ORDER.find((field) => !!blockingReceipt?.fieldErrors[field]); + if (firstFlaggedField) { + onRevealField(firstFlaggedField, blockingReceipt?.transactionID); + } + }; + + return { + visibleFieldErrors: shouldShowFieldErrors ? fieldErrors : NO_FIELD_ERRORS, + revealFieldErrors, + }; +} + +export default useConfirmationFieldErrors; diff --git a/src/components/MoneyRequestConfirmationList/hooks/useConfirmationListData.ts b/src/components/MoneyRequestConfirmationList/hooks/useConfirmationListData.ts index 773e0edad1d7..f3f770518624 100644 --- a/src/components/MoneyRequestConfirmationList/hooks/useConfirmationListData.ts +++ b/src/components/MoneyRequestConfirmationList/hooks/useConfirmationListData.ts @@ -9,13 +9,12 @@ import usePrevious from '@hooks/usePrevious'; import {getCategoryDescriptionHint, isCategoryDescriptionRequired} from '@libs/CategoryUtils'; import {isMovingTransactionFromTrackExpense as isMovingTransactionFromTrackExpenseUtil} from '@libs/IOUUtils'; -import {shouldShowConfirmationDate} from '@libs/MoneyRequestUtils'; +import {getConfirmationAmount, isConfirmationMerchantRequired, shouldShowConfirmationDate} from '@libs/MoneyRequestUtils'; import {hasEnabledOptions} from '@libs/OptionsListUtils'; import {arePolicyRulesEnabled, isTaxTrackingEnabled} from '@libs/PolicyUtils'; -import {getCategory, getCurrency, getMerchant, getRateID, hasValidModifiedAmount, isScanRequest as isScanRequestUtil} from '@libs/TransactionUtils'; +import {getCategory, getCurrency, getMerchant, getRateID, isScanRequest as isScanRequestUtil} from '@libs/TransactionUtils'; import CONST from '@src/CONST'; -import type {TranslationPaths} from '@src/languages/types'; import {useIsFocused} from '@react-navigation/native'; import {useEffect, useRef, useState} from 'react'; @@ -23,19 +22,14 @@ import {useEffect, useRef, useState} from 'react'; import type {ConfirmationDistanceState, ConfirmationListData, UseConfirmationListDataParams} from './types'; import useConfirmationAmount from './useConfirmationAmount'; +import useConfirmationFieldErrors from './useConfirmationFieldErrors'; import useConfirmationPolicyData from './useConfirmationPolicyData'; import useConfirmationValidation from './useConfirmationValidation'; import useFormErrorManagement from './useFormErrorManagement'; import useParticipantSection from './useParticipantSection'; +import useScrollToFlaggedField from './useScrollToFlaggedField'; import useTransactionReportForConfirmation from './useTransactionReportForConfirmation'; -/** - * The errors the amount / merchant / date fields render inline rather than in the footer. Raising one of these is - * only visible if those fields are on screen, so every reveal site reads this same set: the shared confirmation - * switches to the transaction whose fields need fixing, and the scan variant expands "Show more". - */ -const INLINE_FIELD_ERROR_KEYS = new Set(['common.error.fieldRequired', 'common.error.invalidAmount', 'iou.error.invalidMerchant']); - /** * Everything the confirmation surface needs regardless of which expense type is being confirmed: the participant * rows, the form errors, the validation gate, and the confirm button, all derived from an already resolved policy. @@ -64,7 +58,7 @@ function useConfirmationListDataWithPolicy({ isConfirming, shouldShowSmartScanFields = true, canEnterScanFieldsManually = false, - partiallyManuallyFilledScanID, + siblingTransactions, hasSmartScanFailed, shouldHideToSection = false, onConfirm, @@ -103,8 +97,9 @@ function useConfirmationListDataWithPolicy({ const scrollFocusedInputIntoView = (input: MeasurableInput) => { listRef.current?.scrollInputIntoView(input); }; + const {fieldViewRefs, scrollToField} = useScrollToFlaggedField(listRef, transaction?.transactionID); - const iouAmount = hasValidModifiedAmount(transaction) ? Number(transaction?.modifiedAmount) : (transaction?.amount ?? 0); + const iouAmount = getConfirmationAmount(transaction); const iouCurrencyCode = getCurrency(transaction); const iouMerchant = getMerchant(transaction); const iouCategory = getCategory(transaction); @@ -129,6 +124,7 @@ function useConfirmationListDataWithPolicy({ // In the share flow, the caller knows it is a scan before the draft is initialized. const isScanRequest = isScanRequestProp || isScanRequestUtil(transaction); + const isMerchantRequired = isConfirmationMerchantRequired({isPolicyExpenseChat, isScanRequest, isEditingSplitBill, shouldShowMerchant}); const {amountToBeUsed, formattedAmount, formattedAmountPerAttendee} = useConfirmationAmount({ transaction, @@ -160,14 +156,12 @@ function useConfirmationListDataWithPolicy({ const isTypeSplit = iouType === CONST.IOU.TYPE.SPLIT; const shouldShowReadOnlySplits = isPolicyExpenseChat || isReadOnly || isScanRequest; - // Both the validation gate and the clear gate below key off this, so it is computed once here rather than - // being re-derived per hook, where the two could be updated independently. + // The date field is rendered under the same condition, so a hidden date is never reported missing. const shouldShowDate = shouldShowConfirmationDate(shouldShowSmartScanFields, isDistanceRequest); - const {formError, setFormError, clearFormErrors, shouldDisplayFieldError, isMerchantEmpty, isMerchantFieldValid, isMerchantRequired, errorMessage} = useFormErrorManagement({ + const {formError, setFormError, clearFormErrors, setConfirmError, clearConfirmError, shouldDisplayFieldError, errorMessage} = useFormErrorManagement({ transaction, transactionReport, - iouMerchant, iouCategory, iouAttendees, policy, @@ -175,19 +169,12 @@ function useConfirmationListDataWithPolicy({ policyCategories, currentUserPersonalDetails, isEditingSplitBill, - isPolicyExpenseChat, - isScanRequest, - canEnterScanFieldsManually, - partiallyManuallyFilledScanID, - shouldShowMerchant, hasSmartScanFailed, didConfirmSplit, routeError, isTypeSplit, shouldShowReadOnlySplits, isDistanceRequest, - isReadOnly, - shouldShowDate, }); const isCategoryRequired = !!policy?.requiresCategory && !isTypeInvoice; @@ -228,7 +215,7 @@ function useConfirmationListDataWithPolicy({ onOpenParticipantPicker, }); - const {validate} = useConfirmationValidation({ + const {validate, fieldErrors, siblingFieldErrors} = useConfirmationValidation({ transaction, transactionReport, transactionID, @@ -246,9 +233,6 @@ function useConfirmationListDataWithPolicy({ currentUserPersonalDetails, isEditingSplitBill, isMerchantRequired, - isMerchantFieldValid, - isMerchantEmpty, - shouldDisplayFieldError, shouldShowTax, isDistanceRequest, isDistanceRequestWithPendingRoute, @@ -258,20 +242,19 @@ function useConfirmationListDataWithPolicy({ routeError, canEnterScanFieldsManually, isPolicyExpenseChat, - partiallyManuallyFilledScanID, + siblingTransactions, isReadOnly, shouldShowDate, isTaxAmountEmpty, }); - // The partially filled receipt may not be the one on screen, so bring it into view to show its inline errors. - const validateAndRevealFields: typeof validate = (paymentType) => { - const result = validate(paymentType); - if (result?.errorKey && INLINE_FIELD_ERROR_KEYS.has(result.errorKey) && partiallyManuallyFilledScanID && partiallyManuallyFilledScanID !== transactionID) { - onSwitchToTransaction?.(partiallyManuallyFilledScanID); - } - return result; - }; + const {visibleFieldErrors, revealFieldErrors} = useConfirmationFieldErrors({ + transactionID, + fieldErrors, + siblingFieldErrors, + onSwitchToTransaction, + onRevealField: scrollToField, + }); const confirm = buildConfirmAction({ iouType, @@ -280,8 +263,10 @@ function useConfirmationListDataWithPolicy({ routeError, formError, isDelegateAccessRestricted, - validate: validateAndRevealFields, - setFormError, + validate, + setConfirmError, + clearConfirmError, + revealFieldErrors, setDidConfirmSplit, showDelegateNoAccessModal, onConfirm, @@ -298,7 +283,7 @@ function useConfirmationListDataWithPolicy({ shouldShowTax, hasParticipantSection: sections.length > 0, }; - const errorState = {shouldDisplayFieldError, formError, clearFormErrors, setFormError}; + const errorState = {shouldDisplayFieldError, formError, fieldErrors: visibleFieldErrors, clearFormErrors, setFormError}; return { /** Handed straight to `ConfirmationListLayout`. The variant adds its `listFooterContent`, its `fieldFlags`, and its controllers as children. */ @@ -346,6 +331,7 @@ function useConfirmationListDataWithPolicy({ isScanRequest, isEditingSplitBill, scrollFocusedInputIntoView, + fieldViewRefs, onSubmitForm: confirm, onTaxAmountEmptyChange: setIsTaxAmountEmpty, }, @@ -429,4 +415,4 @@ function useConfirmationListData(props: MoneyRequestConfirmationListProps & Pick } export default useConfirmationListData; -export {INLINE_FIELD_ERROR_KEYS, useConfirmationListDataWithPolicy}; +export {useConfirmationListDataWithPolicy}; diff --git a/src/components/MoneyRequestConfirmationList/hooks/useConfirmationValidation.ts b/src/components/MoneyRequestConfirmationList/hooks/useConfirmationValidation.ts index 4d46fba083fd..80c853607cbe 100644 --- a/src/components/MoneyRequestConfirmationList/hooks/useConfirmationValidation.ts +++ b/src/components/MoneyRequestConfirmationList/hooks/useConfirmationValidation.ts @@ -1,26 +1,18 @@ +import type {ConfirmationFieldErrors, SiblingFieldErrors} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import getConfirmationFieldErrors, {getSiblingFieldErrors, hasFieldErrors} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; + import {useCurrencyListActions} from '@hooks/useCurrencyList'; import {isValidPerDiemExpenseAmount} from '@libs/actions/IOU/PerDiem'; import {getIsMissingAttendeesViolation} from '@libs/AttendeeUtils'; import {isCategoryMissing} from '@libs/CategoryUtils'; import {convertToFrontendAmountAsString} from '@libs/CurrencyUtils'; -import {isP2PRecipient} from '@libs/IOUUtils'; -import {isConfirmationAmountMissing, isConfirmationDateMissing, isTaxAmountInvalid, isValidMoneyRequestAmount, validateAmount} from '@libs/MoneyRequestUtils'; +import {isTaxAmountInvalid, validateAmount} from '@libs/MoneyRequestUtils'; import type {getTagLists as getTagListsFn} from '@libs/PolicyUtils'; import {canSubmitPerDiemExpenseFromWorkspace, isAttendeeTrackingEnabled} from '@libs/PolicyUtils'; import {hasEnabledTags, hasMatchingTag} from '@libs/TagsOptionsListUtils'; import {isValidTimeExpenseAmount} from '@libs/TimeTrackingUtils'; -import { - areRequiredFieldsEmpty, - getCalculatedTaxAmount, - getTag, - getTaxAmount, - hasTaxRateWithMatchingValue, - isMerchantMissing, - isPartiallyEnteredScanExpense, - isScanRequest as isScanRequestUtil, -} from '@libs/TransactionUtils'; -import {isValidInputLength} from '@libs/ValidationUtils'; +import {areRequiredFieldsEmpty, getCalculatedTaxAmount, getTag, getTaxAmount, hasTaxRateWithMatchingValue} from '@libs/TransactionUtils'; import type {IOUType} from '@src/CONST'; import CONST from '@src/CONST'; @@ -32,7 +24,24 @@ import type {CurrentUserPersonalDetails} from '@src/types/onyx/PersonalDetails'; import type {OnyxEntry} from 'react-native-onyx'; -type ValidationResult = {errorKey: TranslationPaths; shouldSetDidConfirmSplit?: boolean} | {errorKey: null}; +type ValidationResult = + | { + errorKey: TranslationPaths; + shouldSetDidConfirmSplit?: boolean; + } + | {isFieldError: true} + | {errorKey: null}; + +type UseConfirmationValidationResult = { + /** Returns the first check that blocks confirmation, `{errorKey: null}` when none does, or null when validation can't run yet */ + validate: (paymentType?: PaymentMethodType) => ValidationResult | null; + + /** The errors the amount, date and merchant fields would show now. `validate` blocks on the same map. */ + fieldErrors: ConfirmationFieldErrors; + + /** The first other receipt of a multi-scan with a field error. It is only looked for once the receipt on screen has none, since that one is fixed first. */ + siblingFieldErrors: SiblingFieldErrors | undefined; +}; type UseConfirmationValidationParams = { /** Transaction being validated */ @@ -85,15 +94,6 @@ type UseConfirmationValidationParams = { /** Whether the merchant field is required for this flow */ isMerchantRequired: boolean | undefined; - /** Whether the merchant value passes full validation (length, required, disallowed values) */ - isMerchantFieldValid: boolean; - - /** Whether the merchant field is empty / partial (from {@link useFormErrorManagement}) */ - isMerchantEmpty: boolean; - - /** When editing a split bill, whether per-field errors should be shown (SmartScan failure paths) */ - shouldDisplayFieldError: boolean; - /** Whether the tax section is enabled for this policy */ shouldShowTax: boolean; @@ -115,14 +115,14 @@ type UseConfirmationValidationParams = { /** Truthy when the route to the confirmation page has a known error */ routeError: string | null | undefined; - /** Whether this surface offers manual entry of the amount / merchant / date. False for splits, test receipts and moved tracked expenses. */ + /** Whether this surface offers manual entry of the amount / merchant / date. False for test receipts and moved tracked expenses. */ canEnterScanFieldsManually: boolean; /** Whether the expense goes to a workspace chat, the only destination where a manually entered Scan needs a merchant */ isPolicyExpenseChat: boolean; - /** ID of a partially filled Scan among the transactions being confirmed. Can name a receipt other than this one. */ - partiallyManuallyFilledScanID?: string; + /** The other receipts of a multi-scan, which Create submits together with this one */ + siblingTransactions?: OnyxTypes.Transaction[]; /** Whether the confirmation fields are read-only (date is not inline-editable) */ isReadOnly: boolean; @@ -136,8 +136,11 @@ type UseConfirmationValidationParams = { /** * Runs the set of pure validation checks for the Money Request confirmation - * flow and returns either a translation key describing the first failure, or - * a success signal. + * flow and returns a translation key describing the first failure, a field + * error signal when the amount, date or merchant field blocks, or a success + * signal. The amount, date and merchant checks come from + * `getConfirmationFieldErrors`, whose map is returned too so those fields render + * exactly the errors that block confirmation. * * Side effects (setting form error, firing onConfirm / onSendMoney, * navigating to the company info route, showing the delegate-no-access modal) @@ -166,9 +169,6 @@ function useConfirmationValidation({ currentUserPersonalDetails, isEditingSplitBill, isMerchantRequired, - isMerchantFieldValid, - isMerchantEmpty, - shouldDisplayFieldError, shouldShowTax, isDistanceRequest, isDistanceRequestWithPendingRoute, @@ -178,18 +178,28 @@ function useConfirmationValidation({ routeError, canEnterScanFieldsManually, isPolicyExpenseChat, - partiallyManuallyFilledScanID, + siblingTransactions, isReadOnly, shouldShowDate, isTaxAmountEmpty, -}: UseConfirmationValidationParams): {validate: (paymentType?: PaymentMethodType) => ValidationResult | null} { +}: UseConfirmationValidationParams): UseConfirmationValidationResult { const {getCurrencyDecimals} = useCurrencyListActions(); const selectedParticipantsCount = selectedParticipants.length; - // The Scan confirmation reveals the amount / merchant / date fields behind "Show more". Each is optional there: - // a field left blank is still read off the receipt, but one the user does fill in is subject to the same - // validation as a manually entered one. - const shouldValidateEnteredAmount = transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.MANUAL || canEnterScanFieldsManually; - const validate = (paymentType?: PaymentMethodType): ValidationResult | null => { + const fieldErrorParams = { + iouType, + canEnterScanFieldsManually, + isPolicyExpenseChat, + shouldShowDate, + isReadOnly, + isEditingSplitBill, + isMerchantRequired, + isDistanceRequest, + isTimeRequest, + currentUserAccountID: currentUserPersonalDetails.accountID, + }; + const fieldErrors = getConfirmationFieldErrors(transaction, {...fieldErrorParams, iouAmount, iouMerchant}); + const siblingFieldErrors = !hasFieldErrors(fieldErrors) ? getSiblingFieldErrors(siblingTransactions, {...fieldErrorParams, participants: transaction?.participants}) : undefined; + const validateReceiptOnScreen = (paymentType?: PaymentMethodType): ValidationResult | null => { if (!!routeError || !transactionID) { return null; } @@ -198,59 +208,8 @@ function useConfirmationValidation({ return {errorKey: 'iou.error.noParticipantSelected'}; } - const firstParticipant = transaction?.participants?.at(0); - const isP2P = !!(firstParticipant?.accountID && !firstParticipant?.isPolicyExpenseChat && !firstParticipant?.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) { - return {errorKey: 'common.error.invalidAmount'}; - } - // `isConfirmationAmountMissing` only applies to manually entered amounts. Per diem, distance and time set the - // amount programmatically, and a scan reads it off the receipt whenever the user leaves the field blank. - if (isConfirmationAmountMissing(transaction, canEnterScanFieldsManually, isPolicyExpenseChat)) { - return {errorKey: 'common.error.fieldRequired'}; - } - // The three fields are all-or-nothing, so a partially filled set is blocked and each blank field flags inline. - if (isPartiallyEnteredScanExpense(transaction, canEnterScanFieldsManually, isPolicyExpenseChat)) { - return {errorKey: 'common.error.fieldRequired'}; - } - // On a multi-scan the same rule has to hold for the receipts that are not on screen, since Create submits - // all of them at once. The caller brings the offending one into view so its blank fields raise this inline. - if (partiallyManuallyFilledScanID) { - return {errorKey: 'common.error.fieldRequired'}; - } - if ( - shouldValidateEnteredAmount && - transaction?.isAmountSet && - !isTimeRequest && - !isDistanceRequest && - !isEditingSplitBill && - // P2P chats don't support negative amounts. The Scan flow lets the user pick a 1:1 recipient before entering - // the amount, so the participant picker's negative-amount guard never applies and the block has to live here. - !isValidMoneyRequestAmount(iouAmount, iouType, !isP2PRecipient(firstParticipant, currentUserPersonalDetails.accountID), isP2P) - ) { - return {errorKey: 'common.error.invalidAmount'}; - } - // The date is an inline, clearable required field for every type that shows it (manual, distance, time, - // invoice, ...). Block confirmation when the user cleared it. - if (isConfirmationDateMissing(transaction, shouldShowDate, isReadOnly, canEnterScanFieldsManually, isPolicyExpenseChat)) { - return {errorKey: 'common.error.fieldRequired'}; - } - const merchantValue = iouMerchant ?? ''; - const {isValid: isMerchantLengthValid} = isValidInputLength(merchantValue, CONST.MERCHANT_NAME_MAX_BYTES); - - if (!isMerchantLengthValid) { - return {errorKey: 'iou.error.invalidMerchant'}; - } - if (isMerchantRequired) { - if (!isEditingSplitBill && !isMerchantFieldValid) { - return {errorKey: 'iou.error.invalidMerchant'}; - } - if (isEditingSplitBill && (isMerchantEmpty || (shouldDisplayFieldError && isMerchantMissing(transaction)))) { - return {errorKey: 'iou.error.invalidMerchant'}; - } - } else if (transaction?.isMerchantSet && !isMerchantFieldValid) { - return {errorKey: 'iou.error.invalidMerchant'}; + if (hasFieldErrors(fieldErrors)) { + return {isFieldError: true}; } if (iouCategory.length > CONST.API_TRANSACTION_CATEGORY_MAX_LENGTH) { @@ -358,8 +317,17 @@ function useConfirmationValidation({ return {errorKey: null}; }; - return {validate}; + // Create submits every receipt, so another receipt's field errors block it too, but only once the receipt on screen passes all its checks. + const validate = (paymentType?: PaymentMethodType): ValidationResult | null => { + const result = validateReceiptOnScreen(paymentType); + if (siblingFieldErrors && result && 'errorKey' in result && result.errorKey === null) { + return {isFieldError: true}; + } + return result; + }; + + return {validate, fieldErrors, siblingFieldErrors}; } export default useConfirmationValidation; -export type {UseConfirmationValidationParams}; +export type {UseConfirmationValidationParams, ValidationResult}; diff --git a/src/components/MoneyRequestConfirmationList/hooks/useDistanceConfirmationListData.ts b/src/components/MoneyRequestConfirmationList/hooks/useDistanceConfirmationListData.ts index 2d6c6361f5e0..5be7e9ea23e5 100644 --- a/src/components/MoneyRequestConfirmationList/hooks/useDistanceConfirmationListData.ts +++ b/src/components/MoneyRequestConfirmationList/hooks/useDistanceConfirmationListData.ts @@ -3,7 +3,8 @@ import type {MoneyRequestConfirmationListProps} from '@components/MoneyRequestCo import useBlockDistanceRequest from '@hooks/useBlockDistanceRequest'; import {isMovingTransactionFromTrackExpense as isMovingTransactionFromTrackExpenseUtil} from '@libs/IOUUtils'; -import {getCurrency, hasValidModifiedAmount, isManualDistanceRequest as isManualDistanceRequestUtil} from '@libs/TransactionUtils'; +import {getConfirmationAmount} from '@libs/MoneyRequestUtils'; +import {getCurrency, isManualDistanceRequest as isManualDistanceRequestUtil} from '@libs/TransactionUtils'; import {useConfirmationListDataWithPolicy} from './useConfirmationListData'; import useConfirmationPolicyData from './useConfirmationPolicyData'; @@ -20,7 +21,7 @@ function useDistanceConfirmationListData(props: MoneyRequestConfirmationListProp const policyData = useConfirmationPolicyData({transaction, policyID, action, iouType, isPerDiemRequest}); const {policy, policyForMovingExpenses} = policyData; - const iouAmount = hasValidModifiedAmount(transaction) ? Number(transaction?.modifiedAmount) : (transaction?.amount ?? 0); + const iouAmount = getConfirmationAmount(transaction); const iouCurrencyCode = getCurrency(transaction); const distanceState = useDistanceRequestState({ diff --git a/src/components/MoneyRequestConfirmationList/hooks/useFormErrorManagement.ts b/src/components/MoneyRequestConfirmationList/hooks/useFormErrorManagement.ts index 0c27fd8991af..6747bb60f853 100644 --- a/src/components/MoneyRequestConfirmationList/hooks/useFormErrorManagement.ts +++ b/src/components/MoneyRequestConfirmationList/hooks/useFormErrorManagement.ts @@ -1,10 +1,8 @@ import useDebouncedState from '@hooks/useDebouncedState'; import useLocalize from '@hooks/useLocalize'; -import {isConfirmationAmountMissing, isConfirmationDateMissing, isConfirmationMerchantMissing} from '@libs/MoneyRequestUtils'; import {isAttendeeTrackingEnabled} from '@libs/PolicyUtils'; -import {areRequiredFieldsEmpty, getTag, hasMissingSmartscanFields, isMerchantMissing} from '@libs/TransactionUtils'; -import {isInvalidMerchantValue, isUntypedPlaceholderMerchant, isValidInputLength} from '@libs/ValidationUtils'; +import {areRequiredFieldsEmpty, getTag, hasMissingSmartscanFields} from '@libs/TransactionUtils'; import {getIsViolationFixed} from '@libs/Violations/ViolationsUtils'; import CONST from '@src/CONST'; @@ -16,7 +14,7 @@ import type {CurrentUserPersonalDetails} from '@src/types/onyx/PersonalDetails'; import type {OnyxEntry} from 'react-native-onyx'; import {useIsFocused} from '@react-navigation/native'; -import {useEffect, useRef} from 'react'; +import {useEffect, useRef, useState} from 'react'; type UseFormErrorManagementParams = { /** Transaction being confirmed */ @@ -25,9 +23,6 @@ type UseFormErrorManagementParams = { /** Report the IOU is being created on */ transactionReport: OnyxEntry; - /** Current merchant value entered for the IOU */ - iouMerchant: string | undefined; - /** Currently selected category */ iouCategory: string; @@ -48,21 +43,6 @@ type UseFormErrorManagementParams = { /** Whether we are editing an existing split bill */ isEditingSplitBill: boolean | undefined; - /** Whether this is a policy-expense chat (drives merchant requirement) */ - isPolicyExpenseChat: boolean; - - /** Whether the IOU was started from a SmartScan flow */ - isScanRequest: boolean; - - /** Whether this surface offers manual entry of the amount / merchant / date. False for splits, test receipts and moved tracked expenses. */ - canEnterScanFieldsManually: boolean; - - /** ID of a partially filled Scan among the transactions being confirmed. Can name a receipt other than this one. */ - partiallyManuallyFilledScanID?: string; - - /** Whether the merchant field should be visible in the UI */ - shouldShowMerchant: boolean; - /** Whether SmartScan failed to read the receipt */ hasSmartScanFailed: boolean | undefined; @@ -80,12 +60,6 @@ type UseFormErrorManagementParams = { /** Whether the transaction is a distance request (its amount is read-only, so amount errors are not shown inline) */ isDistanceRequest: boolean; - - /** Whether the date field is rendered (a hidden date can't be missing, so it never keeps the required error alive) */ - shouldShowDate: boolean; - - /** Whether the confirmation is read-only (a read-only date is populated server-side, so it can't be missing) */ - isReadOnly: boolean; }; type UseFormErrorManagementResult = { @@ -101,17 +75,14 @@ type UseFormErrorManagementResult = { /** Clears the current form error if it matches one of the provided keys */ clearFormErrors: (errors: string[]) => void; - /** Whether per-field errors should be shown (only true when editing a split bill) */ - shouldDisplayFieldError: boolean; - - /** Whether the merchant field is currently empty / partial */ - isMerchantEmpty: boolean; + /** Sets the form error for a check that blocked confirming, and remembers that the confirm set it */ + setConfirmError: (error: TranslationPaths) => void; - /** Whether the merchant field is required for this flow */ - isMerchantRequired: boolean; + /** Drops the form error only while it is the one the last confirm set, so an error raised outside validation stays */ + clearConfirmError: () => void; - /** Whether the current merchant value passes validation */ - isMerchantFieldValid: boolean; + /** Whether per-field errors should be shown (only true when editing a split bill) */ + shouldDisplayFieldError: boolean; /** Whether a previously surfaced violation has been resolved */ isViolationFixed: boolean; @@ -120,21 +91,26 @@ type UseFormErrorManagementResult = { errorMessage: string | undefined; }; +/** Whether the attendees or tax amount field shows this error itself, so the footer leaves it out */ +function isErrorShownOnItsOwnField(error: TranslationPaths | ''): boolean { + return error === 'violations.missingAttendees' || error === 'iou.error.invalidTaxAmount'; +} + /** * Owns the form-error state for the Money Request confirmation flow. * * Holds a debounced form-error string, exposes setters and clearing helpers used by the - * controllers, and derives merchant validity, the violation-fixed flag, and the user- - * visible error message. `shouldDisplayFieldError` is gated on edit-split-bill mode so - * field-level errors only render in that flow. `errorMessage` prefers `routeError`, - * then suppresses errors that are already surfaced inline (missingAttendees, the tax amount - * error, and the manual-flow amount/date/merchant required/invalid errors, except the distance-amount - * error which has no inline surface) so they don't show twice. + * controllers, and derives the violation-fixed flag and the user-visible error message. + * `shouldDisplayFieldError` is gated on edit-split-bill mode so field-level errors only render in + * that flow. `errorMessage` prefers `routeError`, then leaves out the errors their own field already + * shows (missingAttendees, the tax amount error, and the invalid amount an edited split flags as the + * user types) so they don't show twice. + * The amount, date and merchant errors of a blocked confirm aren't held here at all: they are derived + * by `getConfirmationFieldErrors` and shown through `useConfirmationFieldErrors`. */ function useFormErrorManagement({ transaction, transactionReport, - iouMerchant, iouCategory, iouAttendees, policy, @@ -142,19 +118,12 @@ function useFormErrorManagement({ policyCategories, currentUserPersonalDetails, isEditingSplitBill, - isPolicyExpenseChat, - isScanRequest, - canEnterScanFieldsManually, - partiallyManuallyFilledScanID, - shouldShowMerchant, hasSmartScanFailed, didConfirmSplit, routeError, isTypeSplit, shouldShowReadOnlySplits, isDistanceRequest, - shouldShowDate, - isReadOnly, }: UseFormErrorManagementParams): UseFormErrorManagementResult { const isFocused = useIsFocused(); const {translate} = useLocalize(); @@ -169,27 +138,25 @@ function useFormErrorManagement({ setFormError(''); }; - const shouldDisplayFieldError: boolean = - !!isEditingSplitBill && - ((!!hasSmartScanFailed && hasMissingSmartscanFields(transaction, transactionReport)) || (didConfirmSplit && areRequiredFieldsEmpty(transaction, transactionReport))); + // Validation raises a confirm's own error again on the next confirm, so a confirm blocked by a field error can drop it, but an error set outside validation would not come back and has to stay. + const [confirmErrorKey, setConfirmErrorKey] = useState(''); - const isMerchantEmpty = !iouMerchant || isMerchantMissing(transaction); - const isMerchantRequired = isPolicyExpenseChat && (!isScanRequest || !!isEditingSplitBill) && shouldShowMerchant; - const isMerchantFieldValid = (() => { - const merchantValue = iouMerchant ?? ''; - const trimmedMerchant = merchantValue.trim(); - const {isValid} = isValidInputLength(merchantValue, CONST.MERCHANT_NAME_MAX_BYTES); - - if (!isValid) { - return false; - } + const setConfirmError = (error: TranslationPaths) => { + setConfirmErrorKey(error); + setFormError(error); + }; - if (!trimmedMerchant || isUntypedPlaceholderMerchant(transaction?.isMerchantSet, trimmedMerchant)) { - return !isMerchantRequired; + const clearConfirmError = () => { + if (!formError || formError !== confirmErrorKey) { + return; } + setConfirmErrorKey(''); + setFormError(''); + }; - return !isInvalidMerchantValue(trimmedMerchant); - })(); + const shouldDisplayFieldError: boolean = + !!isEditingSplitBill && + ((!!hasSmartScanFailed && hasMissingSmartscanFields(transaction, transactionReport)) || (didConfirmSplit && areRequiredFieldsEmpty(transaction, transactionReport))); const isViolationFixed = getIsViolationFixed(formError, { category: iouCategory, @@ -214,27 +181,6 @@ function useFormErrorManagement({ formErrorRef.current = formError; }, [formError]); - useEffect(() => { - if (formErrorRef.current !== 'iou.error.invalidMerchant' || !isMerchantFieldValid) { - return; - } - setFormError(''); - }, [isMerchantFieldValid, setFormError]); - - // These reuse the very predicates `useConfirmationValidation` raises `common.error.fieldRequired` from, so the - // clear side can never drift from the validation side and strand a required error that can no longer be cleared (#96568). - const isAmountRequiredMissing = isConfirmationAmountMissing(transaction, canEnterScanFieldsManually, isPolicyExpenseChat); - const isDateRequiredMissing = isConfirmationDateMissing(transaction, shouldShowDate, isReadOnly, canEnterScanFieldsManually, isPolicyExpenseChat); - const isMerchantRequiredMissing = isConfirmationMerchantMissing(transaction, canEnterScanFieldsManually, isPolicyExpenseChat); - useEffect(() => { - // The predicates above only see the transaction on screen, so the ID keeps the error alive while another - // receipt of a multi-scan is still partially filled. - if (formErrorRef.current !== 'common.error.fieldRequired' || isAmountRequiredMissing || isDateRequiredMissing || isMerchantRequiredMissing || !!partiallyManuallyFilledScanID) { - return; - } - setFormError(''); - }, [isAmountRequiredMissing, isDateRequiredMissing, isMerchantRequiredMissing, partiallyManuallyFilledScanID, setFormError]); - useEffect(() => { const currentFormError = formErrorRef.current; if (shouldDisplayFieldError && didConfirmSplit) { @@ -260,34 +206,22 @@ function useFormErrorManagement({ } }, [isFocused, shouldDisplayFieldError, hasSmartScanFailed, didConfirmSplit, isViolationFixed, setFormError]); - // The amount/date/merchant fields surface these required/invalid errors inline, so repeating them at the bottom of - // the form would show "This field is required" twice. - // `common.error.invalidAmount` is the one exception: it is only surfaced inline while the editable amount input is - // rendered. Distance requests disable that input, and the read-only menu row it falls back to doesn't show the error, - // so the distance-amount error stays in the footer. Otherwise an invalid distance expense would fail silently. - const isSuppressedInline = (error: TranslationPaths | ''): boolean => - error === 'common.error.fieldRequired' || error === 'iou.error.invalidMerchant' || (!isDistanceRequest && error === 'common.error.invalidAmount'); + // An edited split sets this error as the user types and its amount input shows it, so the footer skips it; a distance amount has no input, so its error stays. + const isShownOnAmountField = (error: TranslationPaths | ''): boolean => !isDistanceRequest && error === 'common.error.invalidAmount'; const computeErrorMessage = (): string | undefined => { if (routeError) { return routeError; } - // This runs ahead of the split branch below because splits render those same inline fields, so they duplicate the same way. - if (isSuppressedInline(formError)) { + // This runs ahead of the split branch below because splits render the same amount input, so the error duplicates the same way. + if (isShownOnAmountField(formError)) { return undefined; } if (isTypeSplit && !shouldShowReadOnlySplits) { - // Splits render the debounced value, so the suppression has to be re-checked against it. `formError` clears - // the instant the user fixes the field while `debouncedFormError` lags by USE_DEBOUNCED_STATE_DELAY, and - // without this the suppressed error would pop into the footer for that window (#96565). - return debouncedFormError && !isSuppressedInline(debouncedFormError) ? translate(debouncedFormError) : undefined; - } - // Don't show error at the bottom of the form for missing attendees — the field surfaces it inline. - if (formError === 'violations.missingAttendees') { - return undefined; + // The debounced value lags the live one after a fix, so it is checked too, or the amount error would flash into the footer for that moment (#96565). + return debouncedFormError && !isShownOnAmountField(debouncedFormError) ? translate(debouncedFormError) : undefined; } - // The tax amount error is a parameterized message surfaced inline on the tax amount field, so skip it here. - if (formError === 'iou.error.invalidTaxAmount') { + if (isErrorShownOnItsOwnField(formError)) { return undefined; } return formError ? translate(formError) : undefined; @@ -299,13 +233,13 @@ function useFormErrorManagement({ debouncedFormError, setFormError, clearFormErrors, + setConfirmError, + clearConfirmError, shouldDisplayFieldError, - isMerchantEmpty, - isMerchantRequired, - isMerchantFieldValid, isViolationFixed, errorMessage, }; } export default useFormErrorManagement; +export {isErrorShownOnItsOwnField}; diff --git a/src/components/MoneyRequestConfirmationList/hooks/useScrollToFlaggedField.ts b/src/components/MoneyRequestConfirmationList/hooks/useScrollToFlaggedField.ts new file mode 100644 index 000000000000..b8048388e88d --- /dev/null +++ b/src/components/MoneyRequestConfirmationList/hooks/useScrollToFlaggedField.ts @@ -0,0 +1,45 @@ +/** + * Scrolls the money request confirmation to the field a blocked confirm flagged first, so its error is seen even when + * the user had scrolled past that field. Each field attaches its outer view to the ref it is given here. + */ +import type {ConfirmationField} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import type {SelectionListWithSectionsHandle} from '@components/SelectionList/SelectionListWithSections/types'; + +import type {RefObject} from 'react'; +import type {View} from 'react-native'; + +import {useEffect, useRef, useState} from 'react'; + +type ConfirmationFieldViewRefs = Record>; + +type ViewToReveal = { + viewRef: RefObject; + transactionID: string | undefined; +}; + +function useScrollToFlaggedField(listRef: RefObject, transactionIDOnScreen: string | undefined) { + const amountViewRef = useRef(null); + const merchantViewRef = useRef(null); + const dateViewRef = useRef(null); + const fieldViewRefs: ConfirmationFieldViewRefs = {amount: amountViewRef, merchant: merchantViewRef, date: dateViewRef}; + + // A new object for every blocked confirm, so confirming again scrolls again even when the same field is flagged. + const [viewToReveal, setViewToReveal] = useState(null); + const revealedViewRef = useRef(null); + + // The scroll waits until the flagged receipt is on screen with its error rendered, since revealing can open "Show more" or switch receipts first. + useEffect(() => { + if (!viewToReveal || viewToReveal === revealedViewRef.current || viewToReveal.transactionID !== transactionIDOnScreen) { + return; + } + revealedViewRef.current = viewToReveal; + listRef.current?.scrollToReveal(viewToReveal.viewRef.current); + }, [viewToReveal, transactionIDOnScreen, listRef]); + + const scrollToField = (field: ConfirmationField, transactionID: string | undefined) => setViewToReveal({viewRef: fieldViewRefs[field], transactionID}); + + return {fieldViewRefs, scrollToField}; +} + +export default useScrollToFlaggedField; +export type {ConfirmationFieldViewRefs}; diff --git a/src/components/MoneyRequestConfirmationList/sections/AmountField.tsx b/src/components/MoneyRequestConfirmationList/sections/AmountField.tsx index 2f825c5ca7a8..3b1c145129cc 100644 --- a/src/components/MoneyRequestConfirmationList/sections/AmountField.tsx +++ b/src/components/MoneyRequestConfirmationList/sections/AmountField.tsx @@ -20,7 +20,6 @@ import { import {convertToBackendAmount, convertToFrontendAmountAsString, getLocalizedCurrencySymbol} from '@libs/CurrencyUtils'; import {canUseTouchScreen} from '@libs/DeviceCapabilities'; import {calculateAmount, isMovingTransactionFromTrackExpense, isP2PRecipient, isParticipantP2P} from '@libs/IOUUtils'; -import {isConfirmationAmountMissing} from '@libs/MoneyRequestUtils'; import Navigation from '@libs/Navigation/Navigation'; import navigationRef from '@libs/Navigation/navigationRef'; import {shouldEnableNegative} from '@libs/ReportUtils'; @@ -59,6 +58,10 @@ type AmountFieldProps = { shouldShowTimeRequestFields: boolean; shouldDisplayFieldError: boolean; formError: string; + + /** The amount entry of the errors a blocked confirm revealed, from `getConfirmationFieldErrors`. Distance and time leave it out, since their amount is computed. */ + fieldError?: TranslationPaths; + policy: OnyxEntry; clearFormErrors: (errors: string[]) => void; setFormError: (error: TranslationPaths | '') => void; @@ -73,13 +76,14 @@ function AmountField({ shouldShowTimeRequestFields, shouldDisplayFieldError, formError, + fieldError, policy, clearFormErrors, setFormError, }: AmountFieldProps) { const isScreenFocused = useIsFocused(); const navigation = useNavigation(); - const {isEditingSplitBill, canEnterScanFieldsManually, isPolicyExpenseChat, isReadOnly, didConfirm, transactionID, action, iouType, reportID, reportActionID} = useConfirmationFields(); + const {isEditingSplitBill, canEnterScanFieldsManually, isReadOnly, didConfirm, transactionID, action, iouType, reportID, reportActionID, fieldViewRefs} = useConfirmationFields(); // Filled by the forms that offer a receipt from beside the amount field rather than from a full-width empty // state, with their compact add-receipt button. const {amountTrailingAction, shouldUseDropdownRows} = useExpenseFormLayout(); @@ -114,20 +118,13 @@ function AmountField({ // so its row is locked the same way until the scan finishes. const canOpenAmountPage = !isReadOnly && !isDistanceRequest && !shouldShowTimeRequestFields && !isScanningReceipt; const isP2P = isParticipantP2P(getMoneyRequestParticipantsFromReport(report, currentUserPersonalDetails.accountID).at(0)); - // `common.error.fieldRequired` is shared with the date field, so only surface it on the amount input when the - // amount itself is the missing value. `isConfirmationAmountMissing` is the same predicate validation raises the - // error from, so a scan expense (where the amount is read off the receipt whenever the user leaves the field - // blank) can't show a phantom required error under a field that is deliberately empty. - const shouldShowAmountRequiredError = formError === 'common.error.fieldRequired' && isConfirmationAmountMissing(transactionSlice, canEnterScanFieldsManually, isPolicyExpenseChat); - const shouldShowAmountInvalidError = formError === 'common.error.invalidAmount'; - const shouldShowAmountMissingError = shouldDisplayFieldError && amountIsMissing; + // Editing a split bill flags an invalid amount as the user types, through the form error rather than the field errors. + const amountErrorKey = formError === 'common.error.invalidAmount' ? 'common.error.invalidAmount' : fieldError; let amountFieldErrorText = ''; - if (shouldShowAmountInvalidError) { - amountFieldErrorText = translate('common.error.invalidAmount'); - } else if (shouldShowAmountRequiredError) { - amountFieldErrorText = translate('common.error.fieldRequired'); - } else if (shouldShowAmountMissingError) { + if (amountErrorKey) { + amountFieldErrorText = translate(amountErrorKey); + } else if (shouldDisplayFieldError && amountIsMissing) { amountFieldErrorText = translate('common.error.enterAmount'); } @@ -364,7 +361,10 @@ function AmountField({ launcherRef={currencyButtonRef} /> {!isAmountFieldDisabled ? ( - + ; @@ -49,9 +53,9 @@ type DateFieldProps = { reportActionID: string | undefined; }; -function DateField({shouldDisplayFieldError, didConfirm, isReadOnly, formError, transactionID, action, iouType, reportID, reportActionID}: DateFieldProps) { +function DateField({shouldDisplayFieldError, didConfirm, isReadOnly, fieldError, transactionID, action, iouType, reportID, reportActionID}: DateFieldProps) { const {getCurrencyDecimals, getCurrencySymbol} = useCurrencyListActions(); - const {isEditingSplitBill, canEnterScanFieldsManually, isPolicyExpenseChat} = useConfirmationFields(); + const {isEditingSplitBill, canEnterScanFieldsManually, fieldViewRefs} = useConfirmationFields(); const {shouldUseDropdownRows} = useExpenseFormLayout(); const styles = useThemeStyles(); const {translate} = useLocalize(); @@ -82,10 +86,7 @@ function DateField({shouldDisplayFieldError, didConfirm, isReadOnly, formError, const shouldShowAutomaticHint = shouldShowEmptyDate && !isDatePickerOpen && !hasAnyManuallyEnteredScanField(dateState); const dateErrorText = shouldDisplayFieldError && createdMissing ? translate('common.error.enterDate') : ''; - - // A draft always carries a date, so on a partially filled Scan the all-or-nothing rule stands in for `createdMissing`. - const isDateRequiredMissing = isPartiallyEnteredScanExpense(dateState, canEnterScanFieldsManually, isPolicyExpenseChat) ? !dateState?.isCreatedSet : createdMissing; - const inlineDateErrorText = formError === 'common.error.fieldRequired' && isDateRequiredMissing ? translate('common.error.fieldRequired') : ''; + const inlineDateErrorText = fieldError ? translate(fieldError) : ''; const handleDateChange = (newDate: string) => { if (!transactionID) { @@ -133,7 +134,10 @@ function DateField({shouldDisplayFieldError, didConfirm, isReadOnly, formError, if (!isReadOnly) { return ( // The bordered rows carry their own vertical margin, so an extra bottom margin here would stack with it. - + + : undefined} + sentryLabel={CONST.SENTRY_LABEL.REQUEST_CONFIRMATION_LIST.MERCHANT_FIELD} /> ); diff --git a/src/components/MoneyRequestConfirmationList/types.ts b/src/components/MoneyRequestConfirmationList/types.ts index 3ad91016263c..b8cc9043c0c3 100644 --- a/src/components/MoneyRequestConfirmationList/types.ts +++ b/src/components/MoneyRequestConfirmationList/types.ts @@ -74,8 +74,8 @@ type MoneyRequestConfirmationListProps = { /** Whether manual entry of amount / merchant / date is offered. */ canEnterScanFieldsManually?: boolean; - /** ID of a partially filled Scan among the transactions being confirmed. */ - partiallyManuallyFilledScanID?: string; + /** The other transactions being confirmed with this one, which Create submits together with it. */ + siblingTransactions?: OnyxTypes.Transaction[]; /** Brings another confirmed transaction on screen. */ onSwitchToTransaction?: (transactionID: string) => void; diff --git a/src/components/MoneyRequestConfirmationList/variants/ScanConfirmationList.tsx b/src/components/MoneyRequestConfirmationList/variants/ScanConfirmationList.tsx index a076c6946d32..92d951a8cf7d 100644 --- a/src/components/MoneyRequestConfirmationList/variants/ScanConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList/variants/ScanConfirmationList.tsx @@ -1,6 +1,8 @@ import ConfirmationListLayout from '@components/MoneyRequestConfirmationList/ConfirmationListLayout'; import FieldAutoSelector from '@components/MoneyRequestConfirmationList/FieldAutoSelector'; -import useConfirmationListData, {INLINE_FIELD_ERROR_KEYS} from '@components/MoneyRequestConfirmationList/hooks/useConfirmationListData'; +import {hasFieldErrors} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import useConfirmationListData from '@components/MoneyRequestConfirmationList/hooks/useConfirmationListData'; +import {isErrorShownOnItsOwnField} from '@components/MoneyRequestConfirmationList/hooks/useFormErrorManagement'; import SplitBillController from '@components/MoneyRequestConfirmationList/SplitBillController'; import TaxController from '@components/MoneyRequestConfirmationList/TaxController'; import type {MoneyRequestConfirmationListProps} from '@components/MoneyRequestConfirmationList/types'; @@ -10,6 +12,8 @@ import useIsInLandscapeMode from '@hooks/useIsInLandscapeMode'; import {hasAnyManuallyEnteredScanField} from '@libs/TransactionUtils'; +import type {TranslationPaths} from '@src/languages/types'; + import React, {useState} from 'react'; /** @@ -17,7 +21,7 @@ import React, {useState} from 'react'; * screen and the optional fields collapse behind a show-more button, so it owns that state. */ function ScanConfirmationList(props: MoneyRequestConfirmationListProps) { - const {transaction, canEnterScanFieldsManually = false, partiallyManuallyFilledScanID} = props; + const {transaction, canEnterScanFieldsManually = false} = props; const isInLandscapeMode = useIsInLandscapeMode(); @@ -40,14 +44,16 @@ function ScanConfirmationList(props: MoneyRequestConfirmationListProps) { }); }; - // Reveal the collapsed fields when the user already filled one of them in, so those values aren't hidden behind - // "Show more", or when one of them raises an inline error, or opening the section and pressing Create looks like - // it did nothing. Done during render so the expense never paints collapsed first and it survives a remount. - // In a multi-scan the error stays set while another receipt is partially filled, so only expand the receipt it belongs to. + // The fields expand during render when they hold a value or an error, so the expense never paints collapsed, even after a remount, and Create never looks like it did nothing. const hasManuallyEnteredFields = canEnterScanFieldsManually && hasAnyManuallyEnteredScanField(transaction); - const doesFormErrorBelongToThisReceipt = !partiallyManuallyFilledScanID || partiallyManuallyFilledScanID === transactionID; - const hasInlineFieldError = INLINE_FIELD_ERROR_KEYS.has(data.footerProps.errorState.formError) && doesFormErrorBelongToThisReceipt; - if (transactionID && !showMoreFields && (hasManuallyEnteredFields || hasInlineFieldError)) { + const {formError, fieldErrors} = data.footerProps.errorState; + // The attendees and tax amount fields sit behind "Show more". Every receipt shares one form error, so it opens only the receipt on screen when it is raised. + const [lastFormError, setLastFormError] = useState(''); + const isNewErrorOnCollapsedField = formError !== lastFormError && isErrorShownOnItsOwnField(formError); + if (formError !== lastFormError) { + setLastFormError(formError); + } + if (transactionID && !showMoreFields && (hasManuallyEnteredFields || hasFieldErrors(fieldErrors) || isNewErrorOnCollapsedField)) { setRevealedTransactionIDs([...revealedTransactionIDs, transactionID]); } diff --git a/src/components/MoneyRequestConfirmationListFooter/fieldGroupTypes.ts b/src/components/MoneyRequestConfirmationListFooter/fieldGroupTypes.ts index f88b812d17c4..fdb9c7f7e297 100644 --- a/src/components/MoneyRequestConfirmationListFooter/fieldGroupTypes.ts +++ b/src/components/MoneyRequestConfirmationListFooter/fieldGroupTypes.ts @@ -1,3 +1,5 @@ +import type {ConfirmationFieldErrors} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; + import type {MileageRate} from '@libs/DistanceRequestUtils'; import type {TranslationPaths} from '@src/languages/types'; @@ -54,6 +56,10 @@ type VisibilityFlags = { type ErrorState = { shouldDisplayFieldError: boolean; formError: TranslationPaths | ''; + + /** The amount, date and merchant errors to show under those inputs. Empty until a confirm is blocked by one of them. */ + fieldErrors: ConfirmationFieldErrors; + clearFormErrors: (errors: string[]) => void; setFormError: (error: TranslationPaths | '') => void; }; diff --git a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/ClassificationFields.tsx b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/ClassificationFields.tsx index 99913742f874..7311e9570ad4 100644 --- a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/ClassificationFields.tsx +++ b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/ClassificationFields.tsx @@ -147,7 +147,7 @@ function ClassificationFields({ shouldDisplayFieldError={errorState.shouldDisplayFieldError} didConfirm={didConfirm} isReadOnly={isReadOnly} - formError={errorState.formError} + fieldError={errorState.fieldErrors.date} transactionID={transactionID} action={action} iouType={iouType} diff --git a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/DistanceDetailsFields.tsx b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/DistanceDetailsFields.tsx index dba2566b1df6..a883153c405e 100644 --- a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/DistanceDetailsFields.tsx +++ b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/DistanceDetailsFields.tsx @@ -46,7 +46,10 @@ function DistanceDetailsFields({policy, amountDisplay, distanceData, isDescripti isDistanceRequest shouldShowTimeRequestFields={false} policy={policy} - {...errorState} + shouldDisplayFieldError={errorState.shouldDisplayFieldError} + formError={errorState.formError} + clearFormErrors={errorState.clearFormErrors} + setFormError={errorState.setFormError} /> )} diff --git a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/ManualDetailsFields.tsx b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/ManualDetailsFields.tsx index 6a0a048fbec2..19b6c84be5c6 100644 --- a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/ManualDetailsFields.tsx +++ b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/ManualDetailsFields.tsx @@ -40,7 +40,11 @@ function ManualDetailsFields({policy, amountDisplay, requiredFlags, errorState}: isDistanceRequest={false} shouldShowTimeRequestFields={false} policy={policy} - {...errorState} + shouldDisplayFieldError={errorState.shouldDisplayFieldError} + formError={errorState.formError} + fieldError={errorState.fieldErrors.amount} + clearFormErrors={errorState.clearFormErrors} + setFormError={errorState.setFormError} /> )} @@ -48,7 +52,7 @@ function ManualDetailsFields({policy, amountDisplay, requiredFlags, errorState}: )} diff --git a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/TimeDetailsFields.tsx b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/TimeDetailsFields.tsx index ea76bcd46d1c..eec98ec1d3d4 100644 --- a/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/TimeDetailsFields.tsx +++ b/src/components/MoneyRequestConfirmationListFooter/fieldGroups/detailsFields/TimeDetailsFields.tsx @@ -40,7 +40,10 @@ function TimeDetailsFields({policy, amountDisplay, isDescriptionRequired, descri isDistanceRequest={false} shouldShowTimeRequestFields policy={policy} - {...errorState} + shouldDisplayFieldError={errorState.shouldDisplayFieldError} + formError={errorState.formError} + clearFormErrors={errorState.clearFormErrors} + setFormError={errorState.setFormError} /> )} diff --git a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx index 3ef8b5d33bdd..cb9758ed0cff 100644 --- a/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx +++ b/src/components/SelectionList/SelectionListWithSections/BaseSelectionListWithSections.tsx @@ -122,7 +122,7 @@ function BaseSelectionListWithSectionsImpl({ const targetScrollIndex = initialScrollIndex ?? (initialFocusedIndex < 0 ? undefined : initialFocusedIndex); const listRef = useRef> | null>(null); const {scrollToIndex, debouncedScrollToIndex} = useSelectionListScroll(listRef, flattenedData); - const {containerRef, trackScrollOffset, scrollInputIntoView} = useScrollToFocusedInput(listRef, isKeyboardShown); + const {containerRef, trackScrollOffset, scrollInputIntoView, scrollToReveal} = useScrollToFocusedInput(listRef, isKeyboardShown); const {focusedIndex, setFocusedIndex, isKeyboardNavigating, setHasKeyBeenPressed} = useSelectionListKeyboardFocus({ initialFocusedIndex, @@ -217,8 +217,9 @@ function BaseSelectionListWithSectionsImpl({ updateExternalTextInputFocus, getFocusedOption: getFocusedItem, scrollInputIntoView, + scrollToReveal, }), - [focusTextInput, scrollToIndex, clearInputAfterSelect, updateAndScrollToFocusedIndex, updateExternalTextInputFocus, getFocusedItem, scrollInputIntoView], + [focusTextInput, scrollToIndex, clearInputAfterSelect, updateAndScrollToFocusedIndex, updateExternalTextInputFocus, getFocusedItem, scrollInputIntoView, scrollToReveal], ); const syncedSearchValue = searchValueForFocusSync ?? textInputOptions?.value; diff --git a/src/components/SelectionList/SelectionListWithSections/types.ts b/src/components/SelectionList/SelectionListWithSections/types.ts index e810695ccaf7..26f3769b3aac 100644 --- a/src/components/SelectionList/SelectionListWithSections/types.ts +++ b/src/components/SelectionList/SelectionListWithSections/types.ts @@ -66,6 +66,9 @@ type SelectionListWithSectionsHandle = { /** Scrolls the list so an input rendered inside `listFooterContent` is not hidden behind the keyboard. */ scrollInputIntoView: (input: MeasurableInput) => void; + + /** Scrolls the list just enough to show a view rendered inside `listFooterContent`, when that view is out of sight. */ + scrollToReveal: (view: MeasurableInput) => void; }; type SectionHeader = { diff --git a/src/components/SelectionList/hooks/useScrollToFocusedInput/getScrollDeltaToReveal.ts b/src/components/SelectionList/hooks/useScrollToFocusedInput/getScrollDeltaToReveal.ts new file mode 100644 index 000000000000..b1d89f6e5f68 --- /dev/null +++ b/src/components/SelectionList/hooks/useScrollToFocusedInput/getScrollDeltaToReveal.ts @@ -0,0 +1,32 @@ +/** Extra space (px) left between a view the list scrolls to and the edge of the list's visible area */ +const EXTRA_SCROLL_PADDING = 16; + +type VerticalSpan = { + /** Where the span starts, measured from the top of the window */ + top: number; + + /** How tall the span is */ + height: number; +}; + +/** + * Returns how far the list has to scroll for the view to be in sight: negative to scroll up, 0 when it already is. + * A view taller than the visible area is brought in by its bottom edge, where a form field shows its error. + */ +function getScrollDeltaToReveal(view: VerticalSpan, visibleArea: VerticalSpan): number { + const visibleTop = visibleArea.top + EXTRA_SCROLL_PADDING; + const visibleBottom = visibleArea.top + visibleArea.height - EXTRA_SCROLL_PADDING; + const viewBottom = view.top + view.height; + + if (view.height > visibleBottom - visibleTop) { + const isBottomInSight = viewBottom >= visibleTop && viewBottom <= visibleBottom; + return isBottomInSight ? 0 : viewBottom - visibleBottom; + } + if (view.top < visibleTop) { + return view.top - visibleTop; + } + return Math.max(0, viewBottom - visibleBottom); +} + +export default getScrollDeltaToReveal; +export {EXTRA_SCROLL_PADDING}; diff --git a/src/components/SelectionList/hooks/useScrollToFocusedInput/index.native.ts b/src/components/SelectionList/hooks/useScrollToFocusedInput/index.native.ts index abaf86522b37..7fa2afe4634c 100644 --- a/src/components/SelectionList/hooks/useScrollToFocusedInput/index.native.ts +++ b/src/components/SelectionList/hooks/useScrollToFocusedInput/index.native.ts @@ -10,8 +10,7 @@ import {KeyboardEvents} from 'react-native-keyboard-controller'; import type {UseScrollToFocusedInput} from './types'; -/** Extra space (px) left between the focused input and the top of the visible list area after scrolling. */ -const EXTRA_SCROLL_PADDING = 16; +import getScrollDeltaToReveal, {EXTRA_SCROLL_PADDING} from './getScrollDeltaToReveal'; type MeasureInWindowCallback = (x: number, y: number, width: number, height: number) => void; @@ -110,7 +109,25 @@ const useScrollToFocusedInput: UseScrollToFocusedInput = (listRef, isKeyboardSho }, CONST.MAX_TRANSITION_DURATION_MS); }; - return {containerRef, trackScrollOffset, scrollInputIntoView}; + const scrollToReveal = (view: MeasurableInput) => { + const list = listRef.current; + // The container also holds the footer below the list, so only the scroll view's own frame is the visible area. + const scroller = list?.getNativeScrollRef(); + if (!list || !isMeasurable(view) || !isMeasurable(scroller)) { + return; + } + scroller.measureInWindow((scrollerX, scrollerY, scrollerWidth, scrollerHeight) => { + view.measureInWindow((viewX, viewY, viewWidth, viewHeight) => { + const delta = getScrollDeltaToReveal({top: viewY, height: viewHeight}, {top: scrollerY, height: scrollerHeight}); + if (delta === 0) { + return; + } + list.scrollToOffset({offset: scrollOffsetRef.current + delta, animated: true}); + }); + }); + }; + + return {containerRef, trackScrollOffset, scrollInputIntoView, scrollToReveal}; }; export default useScrollToFocusedInput; diff --git a/src/components/SelectionList/hooks/useScrollToFocusedInput/index.ts b/src/components/SelectionList/hooks/useScrollToFocusedInput/index.ts index 6ae9ecfdef1e..665439155054 100644 --- a/src/components/SelectionList/hooks/useScrollToFocusedInput/index.ts +++ b/src/components/SelectionList/hooks/useScrollToFocusedInput/index.ts @@ -1,3 +1,5 @@ +import type {MeasurableInput} from '@components/SelectionList/SelectionListWithSections/types'; + import type {ComponentRef} from 'react'; import type {View} from 'react-native'; @@ -5,15 +7,34 @@ import {useRef} from 'react'; import type {UseScrollToFocusedInput} from './types'; +import getScrollDeltaToReveal from './getScrollDeltaToReveal'; + const noop = () => {}; /** - * No-op on web: browsers automatically scroll a focused input into view, so there's nothing to do here. + * Browsers already scroll a focused input into view, so on web only revealing a view that isn't focused needs work. * `containerRef` is still returned so the list can attach it harmlessly. */ -const useScrollToFocusedInput: UseScrollToFocusedInput = () => { +const useScrollToFocusedInput: UseScrollToFocusedInput = (listRef) => { const containerRef = useRef | null>(null); - return {containerRef, trackScrollOffset: noop, scrollInputIntoView: noop}; + + const scrollToReveal = (view: MeasurableInput) => { + const list = listRef.current; + // `scrollIntoView` can't keep a gap from the list's edges, so the list scrolls by the same amount native does. + const scroller = list?.getNativeScrollRef(); + if (!list || !(view instanceof HTMLElement) || !(scroller instanceof HTMLElement)) { + return; + } + const viewRect = view.getBoundingClientRect(); + const scrollerRect = scroller.getBoundingClientRect(); + const delta = getScrollDeltaToReveal({top: viewRect.top, height: viewRect.height}, {top: scrollerRect.top, height: scrollerRect.height}); + if (delta === 0) { + return; + } + list.scrollToOffset({offset: scroller.scrollTop + delta, animated: true}); + }; + + return {containerRef, trackScrollOffset: noop, scrollInputIntoView: noop, scrollToReveal}; }; export default useScrollToFocusedInput; diff --git a/src/components/SelectionList/hooks/useScrollToFocusedInput/types.ts b/src/components/SelectionList/hooks/useScrollToFocusedInput/types.ts index c54797dc58ee..f3fdeedb4980 100644 --- a/src/components/SelectionList/hooks/useScrollToFocusedInput/types.ts +++ b/src/components/SelectionList/hooks/useScrollToFocusedInput/types.ts @@ -13,9 +13,12 @@ type UseScrollToFocusedInputResult = { /** Scrolls the list so the given input is visible above the keyboard. Safe to call from an input's `onFocus`. */ scrollInputIntoView: (input: MeasurableInput) => void; + + /** Scrolls the list just enough to show a view rendered inside its footer, when that view is out of sight. A view taller than the list is brought in by its bottom edge. */ + scrollToReveal: (view: MeasurableInput) => void; }; -type UseScrollToFocusedInput = (listRef: RefObject, 'scrollToOffset'> | null>, isKeyboardShown: boolean) => UseScrollToFocusedInputResult; +type UseScrollToFocusedInput = (listRef: RefObject, 'scrollToOffset' | 'getNativeScrollRef'> | null>, isKeyboardShown: boolean) => UseScrollToFocusedInputResult; // eslint-disable-next-line import/prefer-default-export export type {UseScrollToFocusedInput}; diff --git a/src/libs/MoneyRequestUtils.ts b/src/libs/MoneyRequestUtils.ts index 865478e05009..df0f41bdbcf5 100644 --- a/src/libs/MoneyRequestUtils.ts +++ b/src/libs/MoneyRequestUtils.ts @@ -10,7 +10,14 @@ import type {ManuallyEnteredScanFields} from './TransactionUtils'; import {convertToBackendAmount, convertToFrontendAmountAsInteger} from './CurrencyUtils'; import replaceAllDigits from './replaceAllDigits'; import {isExpenseReport, isExpenseRequest, isPolicyExpenseChat} from './ReportUtils'; -import {doesMoneyRequestDraftHaveUserInput, haveWaypointAddressesChanged, isCreatedMissing, isExpenseUnreported, isPartiallyEnteredScanExpense} from './TransactionUtils'; +import { + doesMoneyRequestDraftHaveUserInput, + hasValidModifiedAmount, + haveWaypointAddressesChanged, + isCreatedMissing, + isExpenseUnreported, + isPartiallyEnteredScanExpense, +} from './TransactionUtils'; import {getMerchantError} from './ValidationUtils'; /** @@ -246,14 +253,34 @@ function shouldShowConfirmationDate(shouldShowSmartScanFields: boolean, isDistan return shouldShowSmartScanFields || isDistanceRequest; } +/** The amount the money request confirmation works with: an edited amount when there is one, otherwise the original. */ +function getConfirmationAmount(transaction: OnyxEntry): number { + return hasValidModifiedAmount(transaction) ? Number(transaction?.modifiedAmount) : (transaction?.amount ?? 0); +} + +type ConfirmationMerchantRequirementParams = { + isPolicyExpenseChat: boolean; + isScanRequest: boolean; + isEditingSplitBill: boolean | undefined; + shouldShowMerchant: boolean; +}; + +/** + * Whether the merchant is required on the money request confirmation surface. Only a workspace chat requires one, and + * a scan leaves it to SmartScan unless its split is being edited. + */ +function isConfirmationMerchantRequired(params: ConfirmationMerchantRequirementParams): boolean { + return params.isPolicyExpenseChat && (!params.isScanRequest || !!params.isEditingSplitBill) && params.shouldShowMerchant; +} + /** * Whether the required amount is still missing on the money request confirmation surface. * `isAmountSet` is only ever set by the manual flow (scan, per diem, distance and time populate the amount * programmatically and never set it), so the manual gate is part of the predicate rather than of each call site. * A Scan the user started filling in counts too: its three revealed fields are all-or-nothing, so once any of them * carries a value the blank ones are missing rather than SmartScan's to read. - * This is the single source of truth shared by the validation that raises `common.error.fieldRequired`, the effect - * that clears it once the field is filled, and the amount field that renders it inline, so the three never drift. + * `getConfirmationFieldErrors` reads it to build the amount error, and the field and the blocked confirm both read + * that one map, so an amount reported missing is always the one shown as missing. */ function isConfirmationAmountMissing(transaction: OnyxEntry, canEnterScanFieldsManually = false, shouldRequireMerchant = true): boolean { if (isPartiallyEnteredScanExpense(transaction, canEnterScanFieldsManually, shouldRequireMerchant)) { @@ -272,9 +299,9 @@ function isConfirmationMerchantMissing(transaction: OnyxEntry, @@ -291,9 +318,11 @@ function isConfirmationDateMissing( export { addLeadingZero, + getConfirmationAmount, isConfirmationAmountMissing, isConfirmationDateMissing, isConfirmationMerchantMissing, + isConfirmationMerchantRequired, shouldShowConfirmationDate, replaceAllDigits, stripCommaFromAmount, diff --git a/src/libs/ValidationUtils.ts b/src/libs/ValidationUtils.ts index bd35e8830789..1e5b7b0211a3 100644 --- a/src/libs/ValidationUtils.ts +++ b/src/libs/ValidationUtils.ts @@ -898,6 +898,15 @@ function isUntypedPlaceholderMerchant(isMerchantSet: boolean | undefined, mercha return !isMerchantSet && isInvalidMerchantValue(merchant); } +/** + * Checks if the merchant field is empty from the user's point of view: nothing but whitespace, or a placeholder the + * flow seeded and the user never typed. + */ +function isMerchantBlank(isMerchantSet: boolean | undefined, merchant?: string): boolean { + const trimmedMerchant = merchant?.trim() ?? ''; + return !trimmedMerchant || isUntypedPlaceholderMerchant(isMerchantSet, trimmedMerchant); +} + /** * Validates a 4-digit PIN for UK/EU Expensify Card. * PIN must be exactly 4 digits and not in the list of invalid/weak PINs. @@ -972,6 +981,7 @@ export { getMerchantError, getMerchantErrorMessage, isUntypedPlaceholderMerchant, + isMerchantBlank, isValidPIN, containsHtmlTag, }; diff --git a/src/libs/actions/IOU/Split.ts b/src/libs/actions/IOU/Split.ts index 2bf7661a0655..875f4d93c80d 100644 --- a/src/libs/actions/IOU/Split.ts +++ b/src/libs/actions/IOU/Split.ts @@ -967,6 +967,8 @@ function completeSplitBill({ key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, value: { ...updatedTransaction, + // Only the confirmation reads this draft flag. A saved scan would take it as an unconfirmed amount, and a failed request can't remove it. + isMerchantSet: null, receipt: { state: CONST.IOU.RECEIPT_STATE.OPEN, }, @@ -1377,6 +1379,11 @@ function setDraftSplitTransaction( }) : null; + // getUpdatedTransaction only applies the fields an expense stores, so the draft's own flag for a typed merchant is copied over here. + if (updatedTransaction && Object.hasOwn(transactionChanges, 'isMerchantSet')) { + updatedTransaction.isMerchantSet = transactionChanges.isMerchantSet; + } + Onyx.merge(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`, updatedTransaction); } diff --git a/src/pages/iou/request/step/IOURequestStepConfirmation/index.tsx b/src/pages/iou/request/step/IOURequestStepConfirmation/index.tsx index d7fca9bb98a0..e07d8c0e096a 100644 --- a/src/pages/iou/request/step/IOURequestStepConfirmation/index.tsx +++ b/src/pages/iou/request/step/IOURequestStepConfirmation/index.tsx @@ -52,7 +52,6 @@ import { isDistanceRequest as isDistanceRequestTransactionUtils, isManualDistanceRequest as isManualDistanceRequestTransactionUtils, isOdometerDistanceRequest as isOdometerDistanceRequestTransactionUtils, - isPartiallyEnteredScanExpense, isScanning, isScanRequest, } from '@libs/TransactionUtils'; @@ -369,8 +368,8 @@ function IOURequestStepConfirmationContent({ } } - // The confirmation only validates the transaction it shows, so find the partially filled one across all receipts. - const partiallyManuallyFilledScanID = transactions.find((item) => isPartiallyEnteredScanExpense(item, canEnterScanFieldsManually, isPolicyExpenseChat))?.transactionID; + // While the next receipt loads the pager still shows the previous one, so filter by the one shown. + const siblingTransactions = transactions.filter((item) => item.transactionID !== transaction?.transactionID); const isFromGlobalCreate = transaction?.isFromGlobalCreate === true || transaction?.isFromFloatingActionButton === true; @@ -653,7 +652,7 @@ function IOURequestStepConfirmationContent({ isPerDiemRequest, shouldShowSmartScanFields, canEnterScanFieldsManually, - partiallyManuallyFilledScanID, + siblingTransactions, onSwitchToTransaction: setCurrentTransactionID, action, isConfirmed, diff --git a/tests/actions/IOUTest/SplitTest.ts b/tests/actions/IOUTest/SplitTest.ts index 427bcdd4598b..c0f14ce6e0da 100644 --- a/tests/actions/IOUTest/SplitTest.ts +++ b/tests/actions/IOUTest/SplitTest.ts @@ -1317,7 +1317,7 @@ describe('split expense', () => { getCurrencyDecimals: getCurrencyDecimalsLocal, chatReportID: reportID, reportAction: iouAction, - updatedTransaction: splitTransaction ? {...splitTransaction, amount: 100, modifiedAmount: 100, modifiedCurrency: CONST.CURRENCY.USD} : undefined, + updatedTransaction: splitTransaction ? {...splitTransaction, amount: 100, modifiedAmount: 100, modifiedCurrency: CONST.CURRENCY.USD, isMerchantSet: true} : undefined, sessionAccountID: RORY_ACCOUNT_ID, isASAPSubmitBetaEnabled: false, quickAction: undefined, @@ -1342,9 +1342,10 @@ describe('split expense', () => { await mockFetch?.resume?.(); await waitForBatchedUpdates(); - // Then the split goes back to being a scan request so the user can retry from the original state + // Then the split goes back to being a scan request so the user can retry from the original state, without the draft's typed-merchant flag, which a scan would read as an unconfirmed amount splitTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${splitTransactionID}`); expect(splitTransaction?.iouRequestType).toBe(CONST.IOU.REQUEST_TYPE.SCAN); + expect(splitTransaction?.isMerchantSet).toBeUndefined(); expect(splitTransaction?.errors).toBeTruthy(); }); @@ -10701,6 +10702,27 @@ describe('setDraftSplitTransaction', () => { // Then the action no longer falls back by itself, so the null merge clears the draft expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`)).toBeFalsy(); }); + + it('remembers whether the merchant was typed, which the expense edit itself does not carry', async () => { + // Given a split draft whose merchant the user never typed + const transactionID = 'typed-merchant'; + const draftTransaction = createRandomTransaction(1); + await Onyx.merge(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`, draftTransaction); + await waitForBatchedUpdates(); + + // When the user types a merchant, and later clears it again + setDraftSplitTransaction(transactionID, draftTransaction, {merchant: '(none)', isMerchantSet: true}, getCurrencyDecimalsLocal, getCurrencySymbolLocal); + await waitForBatchedUpdates(); + const typedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`); + setDraftSplitTransaction(transactionID, typedDraft, {merchant: '', isMerchantSet: false}, getCurrencyDecimalsLocal, getCurrencySymbolLocal); + await waitForBatchedUpdates(); + const clearedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`); + + // Then the draft records each state, so a placeholder the user typed can be told apart from one the split was seeded with + expect(typedDraft?.modifiedMerchant).toBe('(none)'); + expect(typedDraft?.isMerchantSet).toBe(true); + expect(clearedDraft?.isMerchantSet).toBe(false); + }); }); describe('initSplitExpenseItemData', () => { diff --git a/tests/ui/MoneyRequestReportFooter.tsx b/tests/ui/MoneyRequestReportFooter.tsx index 1c7c6cb4ec05..b087f9b3ab59 100644 --- a/tests/ui/MoneyRequestReportFooter.tsx +++ b/tests/ui/MoneyRequestReportFooter.tsx @@ -11,7 +11,6 @@ import ScreenWrapper from '@components/ScreenWrapper'; import initOnyxDerivedValues from '@userActions/OnyxDerived'; import CONST from '@src/CONST'; -import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Transaction} from '@src/types/onyx'; @@ -122,7 +121,7 @@ const renderMoneyRequestConfirmationListFooter = async (transaction: Transaction shouldShowTax: false, hasParticipantSection: false, }, - errorState: {shouldDisplayFieldError: false, formError: 'common.error.fieldRequired' as TranslationPaths, clearFormErrors: jest.fn(), setFormError: jest.fn()}, + errorState: {shouldDisplayFieldError: false, formError: '' as const, fieldErrors: {}, clearFormErrors: jest.fn(), setFormError: jest.fn()}, receiptOptions: { receiptFilename: '', receiptPath: '', diff --git a/tests/unit/MoneyRequestUtilsTest.ts b/tests/unit/MoneyRequestUtilsTest.ts index b04f479ee702..7cd54ca007c5 100644 --- a/tests/unit/MoneyRequestUtilsTest.ts +++ b/tests/unit/MoneyRequestUtilsTest.ts @@ -6,6 +6,7 @@ import { handleNegativeAmountFlipping, isConfirmationAmountMissing, isConfirmationDateMissing, + isConfirmationMerchantRequired, isValidMerchant, isValidMoneyRequestAmount, validateAmount, @@ -267,6 +268,23 @@ describe('ReportActionsUtils', () => { }); }); + describe('isConfirmationMerchantRequired', () => { + const workspaceManualExpense = {isPolicyExpenseChat: true, isScanRequest: false, isEditingSplitBill: false, shouldShowMerchant: true}; + + it.each([ + ['a manual expense to a workspace, where only the user can enter it', {}, true], + ['the split of a workspace scan being edited, which is only editable once SmartScan left fields blank', {isScanRequest: true, isEditingSplitBill: true}, true], + ['a scan, whose merchant SmartScan reads off the receipt', {isScanRequest: true}, false], + ['an expense to a person, which does not need a merchant', {isPolicyExpenseChat: false}, false], + ['a confirmation that hides the merchant field, which cannot be filled in', {shouldShowMerchant: false}, false], + ])('decides whether the merchant is required on %s', (_case, overrides, isRequired) => { + // Given a manual workspace expense that differs in this way, if at all + // When its merchant requirement is checked + // Then the merchant is required only where a workspace needs one and nothing but the user can fill it in + expect(isConfirmationMerchantRequired({...workspaceManualExpense, ...overrides})).toBe(isRequired); + }); + }); + describe('isValidMerchant', () => { const iouReport = { reportID: '1', diff --git a/tests/unit/components/MoneyRequestConfirmationList/AmountField.test.tsx b/tests/unit/components/MoneyRequestConfirmationList/AmountField.test.tsx index a77d623f270c..fb2ef16ee928 100644 --- a/tests/unit/components/MoneyRequestConfirmationList/AmountField.test.tsx +++ b/tests/unit/components/MoneyRequestConfirmationList/AmountField.test.tsx @@ -133,6 +133,7 @@ const amountFieldProps: React.ComponentProps = { shouldShowTimeRequestFields: false, shouldDisplayFieldError: false, formError: 'iou.error.invalidAmount', + fieldError: undefined, policy: undefined, clearFormErrors: jest.fn(), setFormError: jest.fn(), diff --git a/tests/unit/components/MoneyRequestConfirmationList/DateField.test.tsx b/tests/unit/components/MoneyRequestConfirmationList/DateField.test.tsx index d54b7cc0e956..c8067431182d 100644 --- a/tests/unit/components/MoneyRequestConfirmationList/DateField.test.tsx +++ b/tests/unit/components/MoneyRequestConfirmationList/DateField.test.tsx @@ -53,7 +53,7 @@ const dateFieldProps: React.ComponentProps = { shouldDisplayFieldError: false, didConfirm: false, isReadOnly: false, - formError: '', + fieldError: undefined, transactionID: '1', action: CONST.IOU.ACTION.CREATE, iouType: CONST.IOU.TYPE.CREATE, diff --git a/tests/unit/components/MoneyRequestConfirmationList/MerchantField.test.tsx b/tests/unit/components/MoneyRequestConfirmationList/MerchantField.test.tsx index cfdbf0dcf647..ac12cc151752 100644 --- a/tests/unit/components/MoneyRequestConfirmationList/MerchantField.test.tsx +++ b/tests/unit/components/MoneyRequestConfirmationList/MerchantField.test.tsx @@ -1,4 +1,4 @@ -import {render, renderHook, screen} from '@testing-library/react-native'; +import {fireEvent, render, renderHook, screen} from '@testing-library/react-native'; import MenuItem from '@components/MenuItem'; import ConfirmationFieldsProvider from '@components/MoneyRequestConfirmationFields/Provider'; @@ -7,6 +7,8 @@ import MerchantField from '@components/MoneyRequestConfirmationList/sections/Mer import useThemeStyles from '@hooks/useThemeStyles'; +import {setDraftSplitTransaction} from '@userActions/IOU/Split'; + import CONST from '@src/CONST'; import React from 'react'; @@ -32,7 +34,7 @@ function renderMerchantField(props: Partial @@ -83,15 +85,42 @@ describe('MerchantField read-only dropdown row', () => { }); it.each([ - {formError: '', shouldDisplayFieldError: true, expectedError: 'common.error.fieldRequired'}, - {formError: 'iou.error.invalidMerchant', shouldDisplayFieldError: false, expectedError: 'iou.error.invalidMerchant'}, - ])('shows $expectedError on the locked merchant row', ({formError, shouldDisplayFieldError, expectedError}) => { + {fieldError: undefined, shouldDisplayFieldError: true, expectedError: 'common.error.fieldRequired'}, + {fieldError: 'iou.error.invalidMerchant' as const, shouldDisplayFieldError: false, expectedError: 'iou.error.invalidMerchant'}, + ])('shows $expectedError on the locked merchant row', ({fieldError, shouldDisplayFieldError, expectedError}) => { // Given a merchant validation failure that must remain visible even when the field is locked // When the bordered confirmation form displays that failure - renderMerchantField({formError, shouldDisplayFieldError}); + renderMerchantField({fieldError, shouldDisplayFieldError}); // Then the actual validation message is preserved and the row still has no edit control expect(screen.getByText(expectedError)).toBeOnTheScreen(); expect(screen.queryByRole('button')).toBeNull(); }); }); + +describe('MerchantField on an edited split', () => { + it('records that the merchant was typed, so a placeholder the user typed can be told apart from one the split was seeded with', () => { + // Given an editable split showing its merchant input + render( + + + , + ); + + // When the user types a merchant + fireEvent.changeText(screen.getByLabelText('common.merchant'), 'Hilton'); + + // Then the split draft gets the merchant together with the flag that it was typed + expect(setDraftSplitTransaction).toHaveBeenCalledWith('1', undefined, {merchant: 'Hilton', isMerchantSet: true}, expect.any(Function), expect.any(Function)); + }); +}); diff --git a/tests/unit/components/MoneyRequestConfirmationList/ScanConfirmationList.test.tsx b/tests/unit/components/MoneyRequestConfirmationList/ScanConfirmationList.test.tsx new file mode 100644 index 000000000000..d22fc9fbe634 --- /dev/null +++ b/tests/unit/components/MoneyRequestConfirmationList/ScanConfirmationList.test.tsx @@ -0,0 +1,113 @@ +import {render} from '@testing-library/react-native'; + +import ConfirmationListLayout from '@components/MoneyRequestConfirmationList/ConfirmationListLayout'; +import type {ConfirmationFieldErrors} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import type {MoneyRequestConfirmationListProps} from '@components/MoneyRequestConfirmationList/types'; +import ScanConfirmationList from '@components/MoneyRequestConfirmationList/variants/ScanConfirmationList'; + +import CONST from '@src/CONST'; +import type {TranslationPaths} from '@src/languages/types'; +import type * as OnyxTypes from '@src/types/onyx'; + +import React from 'react'; + +let mockFieldErrors: ConfirmationFieldErrors = {}; +let mockFormError: TranslationPaths | '' = ''; +let mockTransactionID = 'txn1'; + +jest.mock('@components/MoneyRequestConfirmationList/hooks/useConfirmationListData', () => () => ({ + layoutProps: {transactionID: mockTransactionID}, + footerProps: {errorState: {fieldErrors: mockFieldErrors, formError: mockFormError}}, + taxControllerProps: {}, + splitBillControllerProps: {}, + fieldAutoSelectProps: {}, +})); +jest.mock('@components/MoneyRequestConfirmationList/ConfirmationListLayout', () => jest.fn(() => null)); +jest.mock('@components/MoneyRequestConfirmationList/TaxController', () => () => null); +jest.mock('@components/MoneyRequestConfirmationList/SplitBillController', () => () => null); +jest.mock('@components/MoneyRequestConfirmationList/FieldAutoSelector', () => () => null); +jest.mock('@components/MoneyRequestConfirmationListFooter/variants/ScanFooter', () => () => null); +jest.mock('@hooks/useIsInLandscapeMode', () => () => false); + +const mockedLayout = jest.mocked(ConfirmationListLayout); + +// A fresh scan the user hasn't typed anything into, so nothing on its own opens "Show more" +const untouchedScan = { + transactionID: 'txn1', + reportID: 'report1', + amount: 0, + currency: 'USD', + merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, + created: '2026-11-05', + comment: {}, + iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, +} as OnyxTypes.Transaction; + +const listProps: MoneyRequestConfirmationListProps = { + transaction: untouchedScan, + iouType: CONST.IOU.TYPE.SUBMIT, + action: CONST.IOU.ACTION.CREATE, + selectedParticipants: [], + onOpenParticipantPicker: () => {}, + receiptOptions: {shouldDisplayReceipt: true}, + canEnterScanFieldsManually: true, +}; + +function isLastRenderCompact(): boolean | undefined { + return mockedLayout.mock.calls.at(-1)?.at(0)?.isCompactMode; +} + +describe('ScanConfirmationList', () => { + beforeEach(() => { + mockFieldErrors = {}; + mockFormError = ''; + mockTransactionID = 'txn1'; + mockedLayout.mockClear(); + }); + + it('keeps the fields collapsed behind "Show more" while nothing needs them', () => { + // Given an untouched scan with no field errors + // When it renders + render(); + + // Then the receipt fills the screen and the fields stay collapsed, since SmartScan will fill them in + expect(isLastRenderCompact()).toBe(true); + }); + + it('opens "Show more" when a blocked confirm reveals field errors, so pressing Create does not look like it did nothing', () => { + // Given a scan whose fields are collapsed + const {rerender} = render(); + expect(isLastRenderCompact()).toBe(true); + + // When a confirm is blocked by its missing amount and the field errors are revealed + mockFieldErrors = {amount: 'common.error.fieldRequired'}; + rerender(); + + // Then the fields expand, so the amount shows its error under the input + expect(isLastRenderCompact()).toBe(false); + }); + + it('opens "Show more" for an attendee error only on the receipt it was raised on', () => { + // Given a multi-scan whose receipt on screen has its fields collapsed + const {rerender} = render(); + + // When a confirm is blocked by missing attendees, an error only the attendees field behind "Show more" shows + mockFormError = 'violations.missingAttendees'; + rerender(); + + // Then the fields expand, so the error is on screen + expect(isLastRenderCompact()).toBe(false); + + // When the user pages to another receipt while that error is still set + mockTransactionID = 'txn2'; + rerender( + , + ); + + // Then that receipt stays collapsed, since every receipt shares the form error and this one doesn't have it + expect(isLastRenderCompact()).toBe(true); + }); +}); diff --git a/tests/unit/components/MoneyRequestConfirmationList/getConfirmationFieldErrors.test.ts b/tests/unit/components/MoneyRequestConfirmationList/getConfirmationFieldErrors.test.ts new file mode 100644 index 000000000000..b8a29ec40950 --- /dev/null +++ b/tests/unit/components/MoneyRequestConfirmationList/getConfirmationFieldErrors.test.ts @@ -0,0 +1,263 @@ +import type {GetConfirmationFieldErrorsParams} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import getConfirmationFieldErrors, {getSiblingFieldErrors, NO_FIELD_ERRORS} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; + +import CONST from '@src/CONST'; +import type * as OnyxTypes from '@src/types/onyx'; +import type {Participant} from '@src/types/onyx/IOU'; + +const WORKSPACE_PARTICIPANT = {accountID: 0, isPolicyExpenseChat: true, policyID: 'policy1'} as Participant; +const P2P_PARTICIPANT = {accountID: 2, isPolicyExpenseChat: false} as Participant; + +const baseParams: GetConfirmationFieldErrorsParams = { + iouAmount: 1000, + iouType: CONST.IOU.TYPE.SUBMIT, + iouMerchant: 'Coffee Shop', + canEnterScanFieldsManually: false, + isPolicyExpenseChat: true, + shouldShowDate: true, + isReadOnly: false, + isEditingSplitBill: false, + isMerchantRequired: true, + isDistanceRequest: false, + isTimeRequest: false, + currentUserAccountID: 1, +}; + +// A merchant that still holds the "Expense" placeholder the draft was seeded with +const blankRequiredMerchantParams: Partial = { + iouMerchant: CONST.TRANSACTION.DEFAULT_MERCHANT, +}; + +function createManualTransaction(overrides: Partial = {}): OnyxTypes.Transaction { + return { + transactionID: 'txn1', + reportID: 'report1', + amount: 1000, + currency: 'USD', + merchant: 'Coffee Shop', + created: '2026-11-05', + comment: {}, + iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL, + isAmountSet: true, + isMerchantSet: true, + participants: [WORKSPACE_PARTICIPANT], + ...overrides, + }; +} + +function createScanDraft(overrides: Partial = {}): OnyxTypes.Transaction { + return createManualTransaction({ + iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, + amount: 0, + merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, + isAmountSet: false, + isMerchantSet: false, + isCreatedSet: false, + ...overrides, + }); +} + +const scanParams: Partial = { + canEnterScanFieldsManually: true, + isMerchantRequired: false, + iouAmount: 0, + iouMerchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, +}; + +describe('getConfirmationFieldErrors', () => { + it('returns the shared empty map when every field is filled in', () => { + // Given a manual workspace expense with an amount, a merchant and a date + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(createManualTransaction(), baseParams); + + // Then nothing is reported, and the shared empty map comes back so the footer's error state keeps its identity + expect(fieldErrors).toBe(NO_FIELD_ERRORS); + }); + + it('flags both the amount and a blank required merchant on a manual workspace expense', () => { + // Given a manual workspace expense with neither an amount nor a merchant + const transaction = createManualTransaction({amount: 0, isAmountSet: false, merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, isMerchantSet: false}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, {...baseParams, ...blankRequiredMerchantParams, iouAmount: 0}); + + // Then both fields read as required, so each of them is flagged under its own input + expect(fieldErrors).toEqual({amount: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}); + }); + + it('reports a required merchant the user typed but that is not allowed as invalid, not missing', () => { + // Given a workspace expense whose typed merchant fails validation + const transaction = createManualTransaction({merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, isMerchantSet: true}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, {...baseParams, iouMerchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT}); + + // Then the merchant asks for a valid value instead of saying the field is required + expect(fieldErrors).toEqual({merchant: 'iou.error.invalidMerchant'}); + }); + + it.each([ + ['the "(none)" placeholder the split was seeded with', CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, undefined, 'common.error.fieldRequired'], + ['a merchant the user cleared', '', false, 'common.error.fieldRequired'], + ['the "(none)" placeholder the user typed', CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, true, 'iou.error.invalidMerchant'], + ] as const)('reports %s on an edited split the same way as on a new expense', (_case, merchant, isMerchantSet, expectedError) => { + // Given a workspace split being edited with this merchant + const transaction = createManualTransaction({merchant, isMerchantSet}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, { + ...baseParams, + iouType: CONST.IOU.TYPE.SPLIT, + iouMerchant: merchant, + isEditingSplitBill: true, + }); + + // Then a merchant the field shows as blank reads as required, and only one the user typed reads as invalid + expect(fieldErrors).toEqual({merchant: expectedError}); + }); + + it('flags a placeholder value the user typed into an optional merchant', () => { + // Given an expense to a user, whose merchant is optional, where the user typed the "Expense" placeholder themselves + const transaction = createManualTransaction({merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, isMerchantSet: true, participants: [P2P_PARTICIPANT]}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, { + ...baseParams, + iouMerchant: CONST.TRANSACTION.DEFAULT_MERCHANT, + isPolicyExpenseChat: false, + isMerchantRequired: false, + }); + + // Then the merchant is invalid, since a typed placeholder is an entry the user made rather than a blank field + expect(fieldErrors).toEqual({merchant: 'iou.error.invalidMerchant'}); + }); + + it('keeps the date flagged until it is filled in, even once the amount is', () => { + // Given a manual expense whose amount was entered while the date is still empty + const transaction = createManualTransaction({created: ''}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, baseParams); + + // Then the date is still reported + expect(fieldErrors).toEqual({date: 'common.error.fieldRequired'}); + }); + + it.each([ + {name: 'hidden', overrides: {shouldShowDate: false}}, + {name: 'read-only', overrides: {isReadOnly: true}}, + ])('never reports a $name date as missing, since it cannot be filled in', ({overrides}) => { + // Given an expense with no date on a confirmation that doesn't let the user enter one + const transaction = createManualTransaction({created: ''}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, {...baseParams, ...overrides}); + + // Then the date doesn't block confirmation + expect(fieldErrors).toBe(NO_FIELD_ERRORS); + }); + + it('leaves an untouched scan to SmartScan', () => { + // Given a scan whose amount, merchant and date the user left blank + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(createScanDraft(), {...baseParams, ...scanParams}); + + // Then nothing is reported, since SmartScan reads all three off the receipt + expect(fieldErrors).toBe(NO_FIELD_ERRORS); + }); + + it.each([ + ['the merchant and the date', {merchant: 'Hilton', isMerchantSet: true, created: '2026-11-05', isCreatedSet: true}, {iouMerchant: 'Hilton'}, {amount: 'common.error.fieldRequired'}], + ['only the amount', {amount: 2500, isAmountSet: true}, {iouAmount: 2500}, {date: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}], + ] as const)('flags the blank fields on a workspace scan where the user entered %s', (_entered, transactionOverrides, paramsOverrides, expectedFieldErrors) => { + // Given a scan headed for a workspace where the user entered some of the amount, merchant and date by hand + const transaction = createScanDraft(transactionOverrides); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, {...baseParams, ...scanParams, ...paramsOverrides}); + + // Then the fields left blank are required, because the three are all-or-nothing once one is entered + expect(fieldErrors).toEqual(expectedFieldErrors); + }); + + it.each([ + ['an amount never entered as required', false, 'common.error.fieldRequired'], + ['a typed $0 amount as invalid', true, 'common.error.invalidAmount'], + ] as const)('reports %s on an expense to a person', (_case, isAmountSet, expectedError) => { + // Given a manual expense to a user whose amount is $0, either left blank or typed + const transaction = createManualTransaction({amount: 0, isAmountSet, participants: [P2P_PARTICIPANT]}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, {...baseParams, iouAmount: 0, isPolicyExpenseChat: false, isMerchantRequired: false}); + + // Then a blank amount reads as missing, the same as on a workspace expense, while a typed $0 is invalid because a 1:1 expense can't be $0 + expect(fieldErrors).toEqual({amount: expectedError}); + }); + + it('does not check the amount of a distance expense, which is computed rather than typed', () => { + // Given a distance expense whose computed amount is zero while its route loads + const transaction = createManualTransaction({amount: 0, iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MAP, participants: [P2P_PARTICIPANT]}); + + // When its field errors are computed + const fieldErrors = getConfirmationFieldErrors(transaction, {...baseParams, iouAmount: 0, isDistanceRequest: true, isPolicyExpenseChat: false, isMerchantRequired: false}); + + // Then the amount field isn't flagged, since its errors belong to the footer for this type + expect(fieldErrors).toBe(NO_FIELD_ERRORS); + }); +}); + +describe('getSiblingFieldErrors', () => { + const siblingParams = {...baseParams, canEnterScanFieldsManually: true, isMerchantRequired: false, participants: [WORKSPACE_PARTICIPANT]}; + const completeFields = {amount: 2500, isAmountSet: true, merchant: 'Hilton', isMerchantSet: true, isCreatedSet: true}; + + it('finds nothing without other receipts, or when they were left for SmartScan', () => { + // Given a single expense, and a multi-scan whose other receipt the user never touched + // When the other receipts are checked + // Then nothing blocks, since SmartScan fills in a receipt nobody typed into + expect(getSiblingFieldErrors(undefined, siblingParams)).toBeUndefined(); + expect(getSiblingFieldErrors([createScanDraft()], siblingParams)).toBeUndefined(); + }); + + it('reports a partially filled receipt with the fields it is missing', () => { + // Given a multi-scan to a workspace whose other receipt only has its amount entered + const sibling = createScanDraft({transactionID: 'txn2', amount: 2500, isAmountSet: true}); + + // When the other receipts are checked + // Then that receipt is reported with its blank fields, so it can be brought on screen + expect(getSiblingFieldErrors([sibling], siblingParams)).toEqual({transactionID: 'txn2', fieldErrors: {date: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}}); + }); + + it('reports a complete receipt that holds an invalid merchant, using its own values', () => { + // Given a multi-scan whose other receipt is fully filled in with a merchant that isn't allowed + const sibling = createScanDraft({...completeFields, transactionID: 'txn2', merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT}); + + // When the other receipts are checked with the valid merchant of the receipt on screen in the params + // Then the other receipt's own merchant is what gets checked + expect(getSiblingFieldErrors([sibling], siblingParams)).toEqual({transactionID: 'txn2', fieldErrors: {merchant: 'iou.error.invalidMerchant'}}); + }); + + it('checks an amount against the recipient on screen, even when the draft of the other receipt has none', () => { + // Given a multi-scan whose other receipt has a negative amount and no recipient of its own in its draft + const sibling = createScanDraft({...completeFields, transactionID: 'txn2', amount: -1234, participants: undefined}); + const params = {...siblingParams, iouType: CONST.IOU.TYPE.CREATE, isPolicyExpenseChat: false}; + + // When the recipient on screen is a person, and then a workspace + const toPerson = getSiblingFieldErrors([sibling], {...params, participants: [P2P_PARTICIPANT]}); + const toWorkspace = getSiblingFieldErrors([sibling], {...params, participants: [WORKSPACE_PARTICIPANT]}); + + // Then the negative amount only blocks for a person, because every receipt goes to the same recipient + expect(toPerson).toEqual({transactionID: 'txn2', fieldErrors: {amount: 'common.error.invalidAmount'}}); + expect(toWorkspace).toBeUndefined(); + }); + + it('returns the first receipt that has a field error', () => { + // Given a multi-scan with a valid receipt followed by two receipts holding invalid merchants + const valid = createScanDraft({...completeFields, transactionID: 'txn2'}); + const firstInvalid = createScanDraft({...completeFields, transactionID: 'txn3', merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT}); + const secondInvalid = createScanDraft({...completeFields, transactionID: 'txn4', merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT}); + + // When the other receipts are checked + // Then the first one with an error is the one to bring on screen + expect(getSiblingFieldErrors([valid, firstInvalid, secondInvalid], siblingParams)?.transactionID).toBe('txn3'); + }); +}); diff --git a/tests/unit/components/SelectionList/getScrollDeltaToReveal.test.ts b/tests/unit/components/SelectionList/getScrollDeltaToReveal.test.ts new file mode 100644 index 000000000000..084eabd28d08 --- /dev/null +++ b/tests/unit/components/SelectionList/getScrollDeltaToReveal.test.ts @@ -0,0 +1,25 @@ +import getScrollDeltaToReveal, {EXTRA_SCROLL_PADDING} from '@components/SelectionList/hooks/useScrollToFocusedInput/getScrollDeltaToReveal'; + +// The visible part of the list runs from 100 to 600 in the window, so a view is in sight between 116 and 584 +const VISIBLE_AREA = {top: 100, height: 500}; +const VISIBLE_TOP = VISIBLE_AREA.top + EXTRA_SCROLL_PADDING; +const VISIBLE_BOTTOM = VISIBLE_AREA.top + VISIBLE_AREA.height - EXTRA_SCROLL_PADDING; + +describe('getScrollDeltaToReveal', () => { + it.each([ + ['a view that fits and sits above the list scrolls up until its top is the padding below the list top', {top: 20, height: 60}, 20 - VISIBLE_TOP], + ['a view that fits and sits below the list scrolls down until its bottom is the padding above the list bottom', {top: 560, height: 60}, 620 - VISIBLE_BOTTOM], + ['a view that is already in sight leaves the list where it is', {top: 300, height: 60}, 0], + ['a tall view running past both edges scrolls until its bottom is in sight', {top: 50, height: 700}, 750 - VISIBLE_BOTTOM], + ['a tall view whose top is in sight but whose bottom is clipped scrolls down to its bottom', {top: 200, height: 600}, 800 - VISIBLE_BOTTOM], + ['a tall view that is entirely above the list scrolls up to its bottom', {top: -700, height: 600}, -100 - VISIBLE_BOTTOM], + ['a tall view whose bottom is already in sight leaves the list where it is', {top: 20, height: 500}, 0], + ])('%s', (_case, view, expectedDelta) => { + // Given a view at this position relative to a list whose visible area runs from 116 to 584 once the padding is taken off + // When the scroll needed to reveal it is computed + const delta = getScrollDeltaToReveal(view, VISIBLE_AREA); + + // Then the list moves only as far as needed, and a view taller than the list is judged by its bottom, where a field shows its error + expect(delta).toBe(expectedDelta); + }); +}); diff --git a/tests/unit/components/SelectionList/useScrollToFocusedInput.test.ts b/tests/unit/components/SelectionList/useScrollToFocusedInput.test.ts new file mode 100644 index 000000000000..f26556f096d1 --- /dev/null +++ b/tests/unit/components/SelectionList/useScrollToFocusedInput.test.ts @@ -0,0 +1,101 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import useScrollToFocusedInput from '@components/SelectionList/hooks/useScrollToFocusedInput/index.native'; +import type * as WebScrollToFocusedInputModule from '@components/SelectionList/hooks/useScrollToFocusedInput/index.ts'; + +import type {FlashListRef} from '@shopify/flash-list'; +import type {ComponentRef} from 'react'; +import type {NativeScrollEvent, NativeSyntheticEvent, View} from 'react-native'; + +import createMock from '../../../utils/createMock'; + +type MeasureInWindowCallback = (x: number, y: number, width: number, height: number) => void; + +const LIST_TOP = 100; +const LIST_HEIGHT = 500; +const SCROLL_OFFSET = 300; +const PADDING = 16; + +describe('useScrollToFocusedInput (native)', () => { + describe('scrollToReveal', () => { + /** A view that reports this position in the window */ + function createViewAt(y: number, height: number) { + return {measureInWindow: (callback: MeasureInWindowCallback) => callback(0, y, 375, height)}; + } + + function renderScrolledList() { + // A scrolled list inside a container that also holds a footer below it, so the container is taller than the visible part of the list + const scrollToOffset = jest.fn(); + const scroller = {...createViewAt(LIST_TOP, LIST_HEIGHT), scrollTo: jest.fn()}; + const {result} = renderHook(() => + useScrollToFocusedInput({current: {scrollToOffset, getNativeScrollRef: () => createMock['getNativeScrollRef']>>(scroller)}}, false), + ); + result.current.containerRef.current = createMock>(createViewAt(LIST_TOP, LIST_HEIGHT + 200)); + act(() => result.current.trackScrollOffset(createMock>({nativeEvent: {contentOffset: {y: SCROLL_OFFSET}}}))); + return {scrollToOffset, scrollToReveal: result.current.scrollToReveal}; + } + + it('brings up a view that sits below the list until its bottom is in view', () => { + // Given a field whose bottom is below the visible part of the list, but above the bottom of the container + const {scrollToOffset, scrollToReveal} = renderScrolledList(); + + // When it is revealed + scrollToReveal(createViewAt(560, 60)); + + // Then the list scrolls from where it is until the field's bottom sits the padding above the bottom of the list, so the footer doesn't cover it + expect(scrollToOffset).toHaveBeenCalledWith({offset: SCROLL_OFFSET + 560 + 60 - (LIST_TOP + LIST_HEIGHT - PADDING), animated: true}); + }); + + it('leaves the list where it is when the view is already in sight', () => { + // Given a field that is already in the visible part of the list + const {scrollToOffset, scrollToReveal} = renderScrolledList(); + + // When it is revealed + scrollToReveal(createViewAt(300, 60)); + + // Then the list doesn't move, so a visible error never makes the form jump + expect(scrollToOffset).not.toHaveBeenCalled(); + }); + }); +}); + +describe('useScrollToFocusedInput (web)', () => { + const useWebScrollToFocusedInput = jest.requireActual('@components/SelectionList/hooks/useScrollToFocusedInput/index.ts').default; + + /** An element that reports this position in the window */ + function createElementAt(top: number, height: number) { + const element = document.createElement('div'); + jest.spyOn(element, 'getBoundingClientRect').mockReturnValue(createMock({top, height})); + return element; + } + + function renderScrolledList() { + const scrollToOffset = jest.fn(); + const scroller = createElementAt(LIST_TOP, LIST_HEIGHT); + scroller.scrollTop = SCROLL_OFFSET; + const {result} = renderHook(() => useWebScrollToFocusedInput({current: {scrollToOffset, getNativeScrollRef: jest.fn().mockReturnValue(scroller)}}, false)); + return {scrollToOffset, scrollToReveal: result.current.scrollToReveal}; + } + + it('scrolls the list by the same rule as native, from its current scroll position', () => { + // Given a scrolled list and a field the user scrolled past, now above the visible part of the list + const {scrollToOffset, scrollToReveal} = renderScrolledList(); + + // When it is revealed + scrollToReveal(createElementAt(20, 60)); + + // Then the list scrolls up until the field sits the padding below the top of the list, the same as on native + expect(scrollToOffset).toHaveBeenCalledWith({offset: SCROLL_OFFSET + 20 - (LIST_TOP + PADDING), animated: true}); + }); + + it('leaves the list where it is when the view is already in sight', () => { + // Given a field that is already in the visible part of the list + const {scrollToOffset, scrollToReveal} = renderScrolledList(); + + // When it is revealed + scrollToReveal(createElementAt(300, 60)); + + // Then the list doesn't move + expect(scrollToOffset).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/hooks/confirmAction.test.ts b/tests/unit/hooks/confirmAction.test.ts index 19156aabe73c..5ed0bcf39d35 100644 --- a/tests/unit/hooks/confirmAction.test.ts +++ b/tests/unit/hooks/confirmAction.test.ts @@ -24,7 +24,9 @@ function makeBase(overrides: Partial = {}): Params { formError: '', isDelegateAccessRestricted: false, validate: jest.fn(() => ({errorKey: null})), - setFormError: jest.fn(), + setConfirmError: jest.fn(), + clearConfirmError: jest.fn(), + revealFieldErrors: jest.fn(), setDidConfirmSplit: jest.fn(), showDelegateNoAccessModal: jest.fn(), onConfirm: jest.fn(), @@ -47,26 +49,55 @@ describe('buildConfirmAction', () => { }); it('returns without calling onConfirm when validate returns null', () => { + // Given a confirmation whose validation stops it without an error to show const params = makeBase({validate: jest.fn(() => null)}); + + // When the user confirms buildConfirmAction(params)({paymentType: undefined}); + + // Then nothing is submitted and no error is set, since validation already handled the press expect(params.onConfirm).not.toHaveBeenCalled(); - expect(params.setFormError).not.toHaveBeenCalled(); + expect(params.setConfirmError).not.toHaveBeenCalled(); }); it('sets form error when validation returns an errorKey', () => { - const params = makeBase({validate: jest.fn(() => ({errorKey: 'iou.error.invalidMerchant'}))}); + // Given a confirmation with no participant, an error that has no field of its own + const params = makeBase({validate: jest.fn(() => ({errorKey: 'iou.error.noParticipantSelected'}))}); + + // When the user confirms + buildConfirmAction(params)({paymentType: undefined}); + + // Then the error becomes the form error shown above the button, and no field errors are revealed + expect(params.setConfirmError).toHaveBeenCalledWith('iou.error.noParticipantSelected'); + expect(params.revealFieldErrors).not.toHaveBeenCalled(); + expect(params.onConfirm).not.toHaveBeenCalled(); + }); + + it('reveals a field error on its field and drops the error an earlier confirm set', () => { + // Given a confirmation whose amount field is missing a value, while an error from an earlier confirm is still set + const params = makeBase({formError: 'violations.categoryOutOfPolicy', validate: jest.fn(() => ({isFieldError: true as const}))}); + + // When the user confirms buildConfirmAction(params)({paymentType: undefined}); - expect(params.setFormError).toHaveBeenCalledWith('iou.error.invalidMerchant'); + + // Then the field errors are revealed and the earlier confirm's error is dropped, since validation checks it again on the next confirm + expect(params.revealFieldErrors).toHaveBeenCalledTimes(1); + expect(params.clearConfirmError).toHaveBeenCalledTimes(1); expect(params.onConfirm).not.toHaveBeenCalled(); }); it('sets didConfirmSplit when validation result has shouldSetDidConfirmSplit', () => { + // Given an edited split whose SmartScan failed, so validation asks to remember that the split was confirmed const params = makeBase({ validate: jest.fn(() => ({errorKey: 'iou.error.genericSmartscanFailureMessage', shouldSetDidConfirmSplit: true})), }); + + // When the user confirms buildConfirmAction(params)({paymentType: undefined}); + + // Then the split is marked as confirmed and the SmartScan failure becomes the form error expect(params.setDidConfirmSplit).toHaveBeenCalledWith(true); - expect(params.setFormError).toHaveBeenCalledWith('iou.error.genericSmartscanFailureMessage'); + expect(params.setConfirmError).toHaveBeenCalledWith('iou.error.genericSmartscanFailureMessage'); }); it('does not call onConfirm when formError is already set', () => { @@ -87,13 +118,13 @@ describe('buildConfirmAction', () => { it('blocks the company-info step for an invalid invoice instead of routing past validation (#96579)', () => { // Given an invoice whose sender workspace has no company info yet, and a cleared date mockHasInvoicingDetails.mockReturnValueOnce(false); - const params = makeBase({iouType: CONST.IOU.TYPE.INVOICE, validate: jest.fn(() => ({errorKey: 'common.error.fieldRequired'}))}); + const params = makeBase({iouType: CONST.IOU.TYPE.INVOICE, validate: jest.fn(() => ({isFieldError: true as const}))}); // When the user confirms buildConfirmAction(params)({paymentType: undefined}); // Then the error surfaces and the company info step - which sends the invoice without revalidating - is not reached - expect(params.setFormError).toHaveBeenCalledWith('common.error.fieldRequired'); + expect(params.revealFieldErrors).toHaveBeenCalledTimes(1); expect(mockNavigate).not.toHaveBeenCalled(); expect(params.onConfirm).not.toHaveBeenCalled(); }); diff --git a/tests/unit/hooks/confirmationRequiredFieldsFlow.test.ts b/tests/unit/hooks/confirmationRequiredFieldsFlow.test.ts new file mode 100644 index 000000000000..9719c212a782 --- /dev/null +++ b/tests/unit/hooks/confirmationRequiredFieldsFlow.test.ts @@ -0,0 +1,209 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import buildConfirmAction from '@components/MoneyRequestConfirmationList/confirmAction'; +import useConfirmationFieldErrors from '@components/MoneyRequestConfirmationList/hooks/useConfirmationFieldErrors'; +import useConfirmationValidation from '@components/MoneyRequestConfirmationList/hooks/useConfirmationValidation'; +import useFormErrorManagement from '@components/MoneyRequestConfirmationList/hooks/useFormErrorManagement'; + +import {isConfirmationMerchantRequired} from '@libs/MoneyRequestUtils'; +import {getMerchant} from '@libs/TransactionUtils'; + +import CONST from '@src/CONST'; +import type * as OnyxTypes from '@src/types/onyx'; +import type {Participant} from '@src/types/onyx/IOU'; +import type {CurrentUserPersonalDetails} from '@src/types/onyx/PersonalDetails'; + +import type * as ReactNavigationModule from '@react-navigation/native'; + +jest.mock('@hooks/useLocalize', () => () => ({translate: (key: string) => key})); +jest.mock('@hooks/useCurrencyList', () => ({useCurrencyListActions: () => ({getCurrencyDecimals: () => 2})})); +jest.mock('@libs/Navigation/Navigation', () => ({navigate: jest.fn(), getActiveRoute: () => '/'})); +jest.mock('@userActions/Policy/Policy', () => ({hasInvoicingDetails: jest.fn(() => true)})); +let mockIsFocused = true; +jest.mock('@react-navigation/native', () => { + const actual = jest.requireActual('@react-navigation/native'); + return {...actual, useIsFocused: () => mockIsFocused}; +}); + +const WORKSPACE_PARTICIPANT = {accountID: 0, isPolicyExpenseChat: true, policyID: 'policy1'} as Participant; +const CURRENT_USER = {accountID: 1} as CurrentUserPersonalDetails; +const mockOnConfirm = jest.fn(); +const mockOnRevealField = jest.fn(); + +// A manual expense to a workspace, as the draft starts: no amount entered and the "Expense" placeholder as its merchant +const blankDraft: OnyxTypes.Transaction = { + transactionID: 'txn1', + reportID: 'report1', + amount: 0, + currency: 'USD', + merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, + created: '2026-11-05', + comment: {}, + iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL, + isAmountSet: false, + isMerchantSet: false, + participants: [WORKSPACE_PARTICIPANT], +}; + +/** Wires the form error, validation, field error and confirm pieces together the way `useConfirmationListData` does, on a workspace that tracks tax. */ +function useManualWorkspaceConfirmation( + transaction: OnyxTypes.Transaction, + {isTaxAmountEmpty = false, siblingTransactions}: {isTaxAmountEmpty?: boolean; siblingTransactions?: OnyxTypes.Transaction[]} = {}, +) { + const iouMerchant = getMerchant(transaction); + const isMerchantRequired = isConfirmationMerchantRequired({isPolicyExpenseChat: true, isScanRequest: false, isEditingSplitBill: false, shouldShowMerchant: true}); + const {formError, setConfirmError, clearConfirmError, errorMessage} = useFormErrorManagement({ + transaction, + transactionReport: undefined, + iouCategory: '', + iouAttendees: [], + policy: undefined, + policyTags: undefined, + policyCategories: undefined, + currentUserPersonalDetails: CURRENT_USER, + isEditingSplitBill: false, + hasSmartScanFailed: false, + didConfirmSplit: false, + routeError: undefined, + isTypeSplit: false, + shouldShowReadOnlySplits: true, + isDistanceRequest: false, + }); + const {validate, fieldErrors, siblingFieldErrors} = useConfirmationValidation({ + transaction, + transactionReport: undefined, + transactionID: transaction.transactionID, + iouType: CONST.IOU.TYPE.SUBMIT, + iouAmount: transaction.amount, + iouMerchant, + iouCategory: '', + iouCurrencyCode: 'USD', + iouAttendees: [], + policy: undefined, + policyTags: undefined, + policyTagLists: [], + policyCategories: undefined, + selectedParticipants: [WORKSPACE_PARTICIPANT], + currentUserPersonalDetails: CURRENT_USER, + isEditingSplitBill: false, + isMerchantRequired, + shouldShowTax: true, + isDistanceRequest: false, + isDistanceRequestWithPendingRoute: false, + isPerDiemRequest: false, + isMovingTransactionFromTrackExpense: false, + isTimeRequest: false, + routeError: undefined, + canEnterScanFieldsManually: false, + isPolicyExpenseChat: true, + isReadOnly: false, + shouldShowDate: true, + isTaxAmountEmpty, + siblingTransactions, + }); + const {visibleFieldErrors, revealFieldErrors} = useConfirmationFieldErrors({ + transactionID: transaction.transactionID, + fieldErrors, + siblingFieldErrors, + onSwitchToTransaction: undefined, + onRevealField: mockOnRevealField, + }); + const confirm = buildConfirmAction({ + iouType: CONST.IOU.TYPE.SUBMIT, + policy: undefined, + transactionID: transaction.transactionID, + routeError: undefined, + formError, + isDelegateAccessRestricted: false, + validate, + setConfirmError, + clearConfirmError, + revealFieldErrors, + setDidConfirmSplit: jest.fn(), + showDelegateNoAccessModal: jest.fn(), + onConfirm: mockOnConfirm, + }); + + return {confirm, visibleFieldErrors, footerErrorMessage: errorMessage}; +} + +describe('required fields on a manual workspace expense', () => { + beforeEach(() => { + mockOnConfirm.mockClear(); + mockOnRevealField.mockClear(); + mockIsFocused = true; + }); + + it('keeps the errors shown when the user leaves the page and comes back without filling anything in', () => { + // Given a blocked confirm on a manual workspace expense with neither an amount nor a merchant + const {result, rerender} = renderHook((transaction: OnyxTypes.Transaction) => useManualWorkspaceConfirmation(transaction), {initialProps: blankDraft}); + act(() => result.current.confirm()); + + // When the user opens another page, such as the category picker, and comes back, which clears the form error + mockIsFocused = false; + rerender(blankDraft); + mockIsFocused = true; + rerender(blankDraft); + + // Then both fields are still flagged, since they are still blank + expect(result.current.visibleFieldErrors).toEqual({amount: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}); + }); + + it('does not let the tax error of a removed expense block the one left on screen', () => { + // Given a confirmation of several expenses where Create was blocked by the empty tax amount of the one on screen + const expenseWithEmptyTax = {...blankDraft, amount: 2500, isAmountSet: true, merchant: 'Hilton', isMerchantSet: true}; + const {result, rerender} = renderHook( + ({transaction, isTaxAmountEmpty}: {transaction: OnyxTypes.Transaction; isTaxAmountEmpty: boolean}) => useManualWorkspaceConfirmation(transaction, {isTaxAmountEmpty}), + { + initialProps: {transaction: expenseWithEmptyTax, isTaxAmountEmpty: true}, + }, + ); + act(() => result.current.confirm()); + expect(result.current.footerErrorMessage).toBe('iou.error.invalidAmount'); + + // When the user removes that expense, and Create on the one left is blocked by its blank merchant + const remainingExpense = {...expenseWithEmptyTax, transactionID: 'txn2', merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, isMerchantSet: false}; + rerender({transaction: remainingExpense, isTaxAmountEmpty: false}); + act(() => result.current.confirm()); + + // Then only the blank merchant explains the block, and the removed expense's tax error is gone from the footer + expect(result.current.visibleFieldErrors).toEqual({merchant: 'common.error.fieldRequired'}); + expect(result.current.footerErrorMessage).toBeUndefined(); + + // When the user fills in the merchant and presses Create again + rerender({transaction: {...remainingExpense, merchant: 'Office Depot', isMerchantSet: true}, isTaxAmountEmpty: false}); + act(() => result.current.confirm()); + + // Then the expense is confirmed, since the tax error belonged to the expense that was removed + expect(mockOnConfirm).toHaveBeenCalledTimes(1); + }); + + it('does not submit from a valid expense while another expense still holds an invalid value', () => { + // Given a confirmation of two expenses where Create was blocked by a typed merchant that isn't allowed + const invalidExpense: OnyxTypes.Transaction = {...blankDraft, amount: 2500, isAmountSet: true, merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, isMerchantSet: true}; + const validExpense: OnyxTypes.Transaction = {...invalidExpense, transactionID: 'txn2', merchant: 'Hilton'}; + const {result, rerender} = renderHook( + ({transaction, siblingTransactions}: {transaction: OnyxTypes.Transaction; siblingTransactions: OnyxTypes.Transaction[]}) => + useManualWorkspaceConfirmation(transaction, {siblingTransactions}), + {initialProps: {transaction: invalidExpense, siblingTransactions: [validExpense]}}, + ); + act(() => result.current.confirm()); + expect(result.current.visibleFieldErrors).toEqual({merchant: 'iou.error.invalidMerchant'}); + + // When the user pages to the valid expense and presses Create there + rerender({transaction: validExpense, siblingTransactions: [invalidExpense]}); + mockOnRevealField.mockClear(); + act(() => result.current.confirm()); + + // Then nothing is submitted, and the invalid merchant of the other expense is the field brought into view, since Create submits both expenses + expect(mockOnConfirm).not.toHaveBeenCalled(); + expect(mockOnRevealField).toHaveBeenCalledWith('merchant', invalidExpense.transactionID); + + // When the other expense's merchant is fixed and the user presses Create + rerender({transaction: validExpense, siblingTransactions: [{...invalidExpense, merchant: 'Office Depot'}]}); + act(() => result.current.confirm()); + + // Then both expenses are confirmed + expect(mockOnConfirm).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/unit/hooks/useConfirmationFieldErrors.test.ts b/tests/unit/hooks/useConfirmationFieldErrors.test.ts new file mode 100644 index 000000000000..3d67edc38068 --- /dev/null +++ b/tests/unit/hooks/useConfirmationFieldErrors.test.ts @@ -0,0 +1,104 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import type {ConfirmationField, ConfirmationFieldErrors, SiblingFieldErrors} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import {NO_FIELD_ERRORS} from '@components/MoneyRequestConfirmationList/getConfirmationFieldErrors'; +import useConfirmationFieldErrors from '@components/MoneyRequestConfirmationList/hooks/useConfirmationFieldErrors'; + +type Props = { + fieldErrors: ConfirmationFieldErrors; + siblingFieldErrors?: SiblingFieldErrors; + onSwitchToTransaction?: (transactionID: string) => void; + onRevealField: (field: ConfirmationField, transactionID: string | undefined) => void; +}; + +const ON_SCREEN_TRANSACTION_ID = 'txn1'; +const MISSING_AMOUNT: ConfirmationFieldErrors = {amount: 'common.error.fieldRequired'}; +const PARTIALLY_FILLED_SIBLING: SiblingFieldErrors = {transactionID: 'txn2', fieldErrors: MISSING_AMOUNT}; + +function renderFieldErrors(initialProps: Props) { + return renderHook( + ({fieldErrors, siblingFieldErrors, onSwitchToTransaction, onRevealField}: Props) => + useConfirmationFieldErrors({transactionID: ON_SCREEN_TRANSACTION_ID, fieldErrors, siblingFieldErrors, onSwitchToTransaction, onRevealField}), + {initialProps}, + ); +} + +describe('useConfirmationFieldErrors', () => { + it('keeps the field errors hidden until a confirm is blocked by them', () => { + // Given a confirmation whose amount is missing, before the user pressed the confirm button + // When it renders + const {result} = renderFieldErrors({fieldErrors: MISSING_AMOUNT, onRevealField: jest.fn()}); + + // Then the field doesn't flag anything yet + expect(result.current.visibleFieldErrors).toBe(NO_FIELD_ERRORS); + }); + + it.each([ + ['the amount when the merchant is flagged too', {amount: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}, undefined, 'amount', ON_SCREEN_TRANSACTION_ID], + ['the merchant when the date is flagged too', {date: 'common.error.fieldRequired', merchant: 'iou.error.invalidMerchant'}, undefined, 'merchant', ON_SCREEN_TRANSACTION_ID], + ['the date when it is the only one flagged', {date: 'common.error.fieldRequired'}, undefined, 'date', ON_SCREEN_TRANSACTION_ID], + [ + 'the flagged field of another receipt when the one on screen has none', + NO_FIELD_ERRORS, + {transactionID: 'txn2', fieldErrors: {merchant: 'iou.error.invalidMerchant'}}, + 'merchant', + 'txn2', + ], + ] as const)('scrolls to %s once a confirm is blocked', (_case, fieldErrors, siblingFieldErrors, expectedField, expectedTransactionID) => { + // Given a confirmation whose receipt on screen, or another receipt of the multi-scan, holds these field errors + const onRevealField = jest.fn(); + const {result} = renderFieldErrors({fieldErrors, siblingFieldErrors, onRevealField}); + + // When the confirm is blocked by them + act(() => result.current.revealFieldErrors()); + + // Then the first flagged field in screen order is scrolled into view, so the user reaches the error they meet first even if they had scrolled past it + expect(onRevealField).toHaveBeenCalledWith(expectedField, expectedTransactionID); + }); + + it('keeps the errors shown while any field is still missing, and hides them once all are filled in', () => { + // Given a blocked confirm with both the amount and the merchant missing + const {result, rerender} = renderFieldErrors({fieldErrors: {amount: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}, onRevealField: jest.fn()}); + act(() => result.current.revealFieldErrors()); + + // When only the amount is filled in + rerender({fieldErrors: {merchant: 'common.error.fieldRequired'}, onRevealField: jest.fn()}); + + // Then the merchant is still flagged + expect(result.current.visibleFieldErrors).toEqual({merchant: 'common.error.fieldRequired'}); + + // When the merchant is filled in as well, and the user then empties a field again + rerender({fieldErrors: NO_FIELD_ERRORS, onRevealField: jest.fn()}); + rerender({fieldErrors: MISSING_AMOUNT, onRevealField: jest.fn()}); + + // Then it is only flagged on the next blocked confirm, as before the first one + expect(result.current.visibleFieldErrors).toBe(NO_FIELD_ERRORS); + }); + + it('keeps an invalid value on the receipt on screen in view before moving to a partially filled sibling', () => { + // Given a multi-scan whose receipt on screen has a typed merchant that isn't allowed, while a sibling receipt is partially filled + const onSwitchToTransaction = jest.fn(); + const onRevealField = jest.fn(); + const {result, rerender} = renderFieldErrors({ + fieldErrors: {merchant: 'iou.error.invalidMerchant'}, + siblingFieldErrors: PARTIALLY_FILLED_SIBLING, + onSwitchToTransaction, + onRevealField, + }); + + // When the confirm is blocked + act(() => result.current.revealFieldErrors()); + + // Then the receipt on screen stays and its merchant is scrolled into view, so its error is seen + expect(onSwitchToTransaction).not.toHaveBeenCalled(); + expect(onRevealField).toHaveBeenLastCalledWith('merchant', ON_SCREEN_TRANSACTION_ID); + + // When the user fixes the merchant and confirms again + rerender({fieldErrors: NO_FIELD_ERRORS, siblingFieldErrors: PARTIALLY_FILLED_SIBLING, onSwitchToTransaction, onRevealField}); + act(() => result.current.revealFieldErrors()); + + // Then the partially filled sibling is brought on screen and scrolled to its missing amount, since the receipt on screen has nothing left to fix + expect(onSwitchToTransaction).toHaveBeenCalledWith('txn2'); + expect(onRevealField).toHaveBeenLastCalledWith('amount', 'txn2'); + }); +}); diff --git a/tests/unit/hooks/useConfirmationValidation.test.ts b/tests/unit/hooks/useConfirmationValidation.test.ts index 4967c8d3892d..daa9aa112a7a 100644 --- a/tests/unit/hooks/useConfirmationValidation.test.ts +++ b/tests/unit/hooks/useConfirmationValidation.test.ts @@ -91,9 +91,6 @@ const baseParams = { currentUserPersonalDetails: {accountID: 1} as CurrentUserPersonalDetails, isEditingSplitBill: false, isMerchantRequired: false, - isMerchantFieldValid: true, - isMerchantEmpty: false, - shouldDisplayFieldError: false, shouldShowTax: false, isDistanceRequest: false, isDistanceRequestWithPendingRoute: false, @@ -140,37 +137,74 @@ describe('useConfirmationValidation', () => { }); it('returns invalidMerchant when merchant exceeds max bytes', () => { + // Given a merchant one byte longer than the backend accepts const longMerchant = 'A'.repeat(CONST.MERCHANT_NAME_MAX_BYTES + 1); const {result} = renderHook(() => useConfirmationValidation({...baseParams, iouMerchant: longMerchant})); - expect(result.current.validate()).toEqual({errorKey: 'iou.error.invalidMerchant'}); + + // When the expense's field errors are computed + // Then the merchant field blocks confirmation as invalid, since the request would be rejected + expect(result.current.fieldErrors).toEqual({merchant: 'iou.error.invalidMerchant'}); }); - it('returns invalidMerchant when merchant is required but empty', () => { - const {result} = renderHook(() => useConfirmationValidation({...baseParams, isMerchantRequired: true, isMerchantEmpty: true, isMerchantFieldValid: false})); - expect(result.current.validate()).toEqual({errorKey: 'iou.error.invalidMerchant'}); + it('returns the field errors it blocks on, so the fields show exactly what stops confirmation', () => { + // Given a manual workspace expense with neither an amount nor a merchant + const {result} = renderHook(() => + useConfirmationValidation( + createValidationParamsForParticipant( + POLICY_EXPENSE_CHAT_PARTICIPANT, + {iouAmount: 0, iouMerchant: CONST.TRANSACTION.DEFAULT_MERCHANT, isMerchantRequired: true, isPolicyExpenseChat: true}, + {amount: 0, isAmountSet: false, merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, isMerchantSet: false}, + ), + ), + ); + + // When its validation is read + // Then both fields are reported, and the confirm is blocked as a field error so they show them + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}); + expect(result.current.validate()).toEqual({isFieldError: true}); }); it('returns invalidMerchant when merchant is not required but was explicitly set to an invalid value', () => { + // Given an expense whose merchant is optional, but which the user typed with a value that fails validation const {result} = renderHook(() => useConfirmationValidation({ ...baseParams, + iouMerchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, isMerchantRequired: false, - isMerchantFieldValid: false, - transaction: createMock({transactionID: 'txn1', comment: {}, amount: 100, isMerchantSet: true}), + transaction: createMock({ + transactionID: 'txn1', + comment: {}, + amount: 100, + merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, + isMerchantSet: true, + }), }), ); - expect(result.current.validate()).toEqual({errorKey: 'iou.error.invalidMerchant'}); + + // When its field errors are computed + // Then the merchant field blocks confirmation, since an optional merchant is still checked once the user typed one + expect(result.current.fieldErrors).toEqual({merchant: 'iou.error.invalidMerchant'}); }); it('returns null when merchant is not required and was not explicitly set', () => { + // Given an expense whose merchant is optional and still holds the "(none)" placeholder the user never typed const {result} = renderHook(() => useConfirmationValidation({ ...baseParams, + iouMerchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, isMerchantRequired: false, - isMerchantFieldValid: false, - transaction: createMock({transactionID: 'txn1', comment: {}, amount: 100, isMerchantSet: false}), + transaction: createMock({ + transactionID: 'txn1', + comment: {}, + amount: 100, + merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, + isMerchantSet: false, + }), }), ); + + // When the user confirms + // Then nothing blocks it, since an untyped placeholder in an optional field is just a blank merchant expect(result.current.validate()).toEqual({errorKey: null}); }); @@ -312,6 +346,7 @@ describe('useConfirmationValidation', () => { }); it('returns fieldRequired for manual expense when amount is not set with a policy expense chat participant', () => { + // Given a manual workspace expense whose amount the user never entered const {result} = renderHook(() => useConfirmationValidation( createValidationParamsForParticipant( @@ -323,7 +358,10 @@ describe('useConfirmationValidation', () => { ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the amount field blocks confirmation as required, since a manual expense can't be created without an amount + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired'}); }); it('does not return fieldRequired for scan expense when amount is not set', () => { @@ -372,6 +410,7 @@ describe('useConfirmationValidation', () => { describe('policy expense chat participant (workspace submit/create)', () => { it.each(IOU_TYPES_WITH_STANDARD_VALIDATION)('returns fieldRequired for unset manual amount when iouType is %s', (iouType) => { + // Given a manual workspace expense of this type whose amount the user never entered const {result} = renderHook(() => useConfirmationValidation( createValidationParamsForParticipant( @@ -384,10 +423,14 @@ describe('useConfirmationValidation', () => { ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then every type blocks on the same required amount, so the rule doesn't depend on what is being created + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired'}); }); it('returns fieldRequired for PAY with unset manual amount before payment method is checked', () => { + // Given a payment whose amount the user never entered const {result} = renderHook(() => useConfirmationValidation( createValidationParamsForParticipant( @@ -400,8 +443,12 @@ describe('useConfirmationValidation', () => { ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); - expect(result.current.validate(CONST.IOU.PAYMENT_TYPE.ELSEWHERE)).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When the user confirms, with or without a payment method + // Then the missing amount blocks first, so picking a payment method can't send money without an amount + expect(result.current.validate()).toEqual({isFieldError: true}); + expect(result.current.validate(CONST.IOU.PAYMENT_TYPE.ELSEWHERE)).toEqual({isFieldError: true}); + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired'}); }); it('returns errorKey: null when manual amount is explicitly set to zero for submit', () => { @@ -414,19 +461,24 @@ describe('useConfirmationValidation', () => { }); it('returns invalidAmount when manual amount is explicitly set to zero for invoice', () => { + // Given an invoice to a workspace where the user typed a $0 amount const {result} = renderHook(() => useConfirmationValidation( createValidationParamsForParticipant(POLICY_EXPENSE_CHAT_PARTICIPANT, {...zeroAmountParams, iouType: CONST.IOU.TYPE.INVOICE}, {amount: 0, isAmountSet: true}), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the amount field blocks confirmation as invalid, since an invoice can't be for $0 even where a $0 expense is allowed + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); }); describe('P2P participant', () => { it.each([CONST.IOU.TYPE.SUBMIT, CONST.IOU.TYPE.CREATE, CONST.IOU.TYPE.PAY, CONST.IOU.TYPE.SPLIT])( - 'returns invalidAmount (not fieldRequired) for unset manual amount when iouType is %s', + 'returns fieldRequired for unset manual amount when iouType is %s', (iouType) => { + // Given a manual expense of this type to a person, with no amount entered const {result} = renderHook(() => useConfirmationValidation( createValidationParamsForParticipant( @@ -439,7 +491,10 @@ describe('useConfirmationValidation', () => { ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the amount reads as missing, the same as for a workspace + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired'}); }, ); @@ -460,22 +515,34 @@ describe('useConfirmationValidation', () => { }); it('returns invalidAmount when manual amount is explicitly set to zero', () => { + // Given a manual expense to a person where the user typed a $0 amount const {result} = renderHook(() => useConfirmationValidation(createValidationParamsForParticipant(P2P_PARTICIPANT, zeroAmountParams, {amount: 0, isAmountSet: true}))); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the amount field blocks confirmation, since a 1:1 expense can't be $0 + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); it('returns invalidAmount when manual amount is explicitly set to zero for invoice', () => { + // Given an invoice to a person where the user typed a $0 amount const {result} = renderHook(() => useConfirmationValidation(createValidationParamsForParticipant(P2P_PARTICIPANT, {...zeroAmountParams, iouType: CONST.IOU.TYPE.INVOICE}, {amount: 0, isAmountSet: true})), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the amount field blocks confirmation as invalid, the same as any other 1:1 expense + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); }); describe('self-DM participant', () => { it('returns fieldRequired for unset manual amount', () => { + // Given a manual expense to the self DM whose amount the user never entered const {result} = renderHook(() => useConfirmationValidation(createValidationParamsForParticipant(SELF_DM_PARTICIPANT, zeroAmountParams, {isAmountSet: false}))); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the amount reads as required, like any manual expense whose amount was never entered + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired'}); }); it('returns errorKey: null when manual amount is explicitly set to zero', () => { @@ -487,8 +554,12 @@ describe('useConfirmationValidation', () => { describe('amount validation — P2P zero amount guard', () => { it('returns invalidAmount for P2P manual submit with zero amount', () => { + // Given a manual expense submitted to a person for $0 const {result} = renderHook(() => useConfirmationValidation(createValidationParamsForParticipant(P2P_PARTICIPANT, {iouAmount: 0}, {amount: 0, isAmountSet: true}))); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the $0 guard blocks confirmation on the amount field, since money can't be requested from a person for nothing + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); it('returns errorKey: null for policy expense chat participant with zero amount', () => { @@ -697,6 +768,7 @@ describe('useConfirmationValidation', () => { describe('amount validation — split bill editing', () => { it('returns invalidAmount when editing a split bill with zero amount and required fields otherwise filled', () => { + // Given a split between people being edited down to a $0 amount const splitParticipants = [P2P_PARTICIPANT, {accountID: 3, isPolicyExpenseChat: false} as Participant]; const {result} = renderHook(() => useConfirmationValidation({ @@ -712,11 +784,15 @@ describe('useConfirmationValidation', () => { transactionReport: createMock({type: CONST.REPORT.TYPE.IOU}), }), ); - // P2P zero-amount guard runs before the split-bill-specific invalidAmount check. - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When the user saves the split + // Then the 1:1 $0 guard on the amount field blocks confirmation, since it runs before the split-specific amount check + expect(result.current.validate()).toEqual({isFieldError: true}); + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); - it('returns invalidAmount for split IOU type with unset manual amount and P2P participants', () => { + it('returns fieldRequired for split IOU type with unset manual amount and P2P participants', () => { + // Given a split between people whose amount the user never entered const splitParticipants = [P2P_PARTICIPANT, {accountID: 3, isPolicyExpenseChat: false} as Participant]; const {result} = renderHook(() => useConfirmationValidation( @@ -731,10 +807,14 @@ describe('useConfirmationValidation', () => { ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the amount reads as missing, since an amount the user never entered is checked before the 1:1 $0 guard + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired'}); }); it('returns fieldRequired for split IOU type with unset manual amount and a policy expense chat participant', () => { + // Given a workspace split whose amount the user never entered const {result} = renderHook(() => useConfirmationValidation( createValidationParamsForParticipant( @@ -747,10 +827,14 @@ describe('useConfirmationValidation', () => { ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the amount reads as required, like any manual expense whose amount was never entered + expect(result.current.fieldErrors).toEqual({amount: 'common.error.fieldRequired'}); }); it('returns invalidAmount for split IOU type with zero amount and isAmountSet true', () => { + // Given a split where the user typed a $0 amount const splitParticipants = [POLICY_EXPENSE_CHAT_PARTICIPANT, {accountID: 3, isPolicyExpenseChat: false} as Participant]; const {result} = renderHook(() => useConfirmationValidation( @@ -765,7 +849,10 @@ describe('useConfirmationValidation', () => { ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the amount field blocks confirmation, since a split of $0 has nothing to divide + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); it('returns iou.error.invalidAmount when editing split bill with zero amount and isAmountSet true', () => { @@ -791,11 +878,16 @@ describe('useConfirmationValidation', () => { describe('date validation — inline required date', () => { it('returns fieldRequired for manual expense when the date is removed', () => { + // Given a manual expense whose date the user cleared const {result} = renderHook(() => useConfirmationValidation(createValidationParamsForParticipant(POLICY_EXPENSE_CHAT_PARTICIPANT, baseParams, {created: '', isAmountSet: true}))); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the date field blocks confirmation as required, since an expense can't be created without a date + expect(result.current.fieldErrors).toEqual({date: 'common.error.fieldRequired'}); }); it('returns fieldRequired for distance expense when the date is removed', () => { + // Given a distance expense whose date the user cleared const {result} = renderHook(() => useConfirmationValidation({ ...baseParams, @@ -810,10 +902,14 @@ describe('useConfirmationValidation', () => { selectedParticipants: [POLICY_EXPENSE_CHAT_PARTICIPANT], }), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the date field blocks confirmation, since the date also decides which distance rate applies + expect(result.current.fieldErrors).toEqual({date: 'common.error.fieldRequired'}); }); it('returns fieldRequired for time expense when the date is removed', () => { + // Given a time expense whose date the user cleared const {result} = renderHook(() => useConfirmationValidation({ ...baseParams, @@ -828,16 +924,23 @@ describe('useConfirmationValidation', () => { selectedParticipants: [POLICY_EXPENSE_CHAT_PARTICIPANT], }), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the date field blocks confirmation, the same as for a manual expense + expect(result.current.fieldErrors).toEqual({date: 'common.error.fieldRequired'}); }); it('returns fieldRequired for invoice when the date is removed', () => { + // Given an invoice whose date the user cleared const {result} = renderHook(() => useConfirmationValidation( createValidationParamsForParticipant(POLICY_EXPENSE_CHAT_PARTICIPANT, {...baseParams, iouType: CONST.IOU.TYPE.INVOICE}, {created: '', isAmountSet: true}), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the date field blocks confirmation, so the company info step that follows is never reached without a date + expect(result.current.fieldErrors).toEqual({date: 'common.error.fieldRequired'}); }); it('does not return fieldRequired when the date is present', () => { @@ -912,7 +1015,6 @@ describe('useConfirmationValidation', () => { isPolicyExpenseChat: true, iouAmount: 0, iouMerchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, - isMerchantEmpty: true, ...overrides, transaction: createTransactionBase({ iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, @@ -930,21 +1032,29 @@ describe('useConfirmationValidation', () => { }); it.each([ - ['merchant', {isMerchantSet: true, merchant: 'Starbucks'}, {iouMerchant: 'Starbucks', isMerchantEmpty: false}], - ['amount', {isAmountSet: true, amount: 1000}, {iouAmount: 1000}], - ['date', {isCreatedSet: true, created: '2025-01-15'}, {}], - ])('requires the other two once the %s is entered, since the three are all-or-nothing', (_field, transactionOverrides, overrides) => { + ['merchant', {isMerchantSet: true, merchant: 'Starbucks'}, {iouMerchant: 'Starbucks'}, {amount: 'common.error.fieldRequired', date: 'common.error.fieldRequired'}], + ['amount', {isAmountSet: true, amount: 1000}, {iouAmount: 1000}, {date: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}], + ['date', {isCreatedSet: true, created: '2025-01-15'}, {}, {amount: 'common.error.fieldRequired', merchant: 'common.error.fieldRequired'}], + ])('requires the other two once the %s is entered, since the three are all-or-nothing', (_field, transactionOverrides, overrides, expectedFieldErrors) => { + // Given a workspace scan where the user entered only this one of the amount, merchant and date const {result} = renderHook(() => useConfirmationValidation(createScanValidationParams(transactionOverrides, overrides))); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the two blank fields are required, since entering any of them means SmartScan no longer fills them in + expect(result.current.fieldErrors).toEqual(expectedFieldErrors); }); it.each([ ['merchant', {isAmountSet: true, amount: 1000, isCreatedSet: true, created: '2025-01-15'}, {iouAmount: 1000}], - ['amount', {isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}, {iouMerchant: 'Starbucks', isMerchantEmpty: false}], - ['date', {isAmountSet: true, amount: 1000, isMerchantSet: true, merchant: 'Starbucks'}, {iouAmount: 1000, iouMerchant: 'Starbucks', isMerchantEmpty: false}], - ])('still blocks confirmation while only the %s is left blank', (_field, transactionOverrides, overrides) => { + ['amount', {isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}, {iouMerchant: 'Starbucks'}], + ['date', {isAmountSet: true, amount: 1000, isMerchantSet: true, merchant: 'Starbucks'}, {iouAmount: 1000, iouMerchant: 'Starbucks'}], + ])('still blocks confirmation while only the %s is left blank', (blankField, transactionOverrides, overrides) => { + // Given a workspace scan where the user entered two of the three fields and left this one blank const {result} = renderHook(() => useConfirmationValidation(createScanValidationParams(transactionOverrides, overrides))); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When its field errors are computed + // Then the last blank field is still required, so a half-entered scan can't be submitted + expect(result.current.fieldErrors).toEqual({[blankField]: 'common.error.fieldRequired'}); }); it('does not require the merchant when the expense goes to a chat that does not require one', () => { @@ -962,11 +1072,11 @@ describe('useConfirmationValidation', () => { // Given a P2P scan with only the amount entered const params = createScanValidationParams({isAmountSet: true, amount: 1000}, {iouAmount: 1000, isPolicyExpenseChat: false}); - // When the user confirms it + // When the confirmation renders const {result} = renderHook(() => useConfirmationValidation(params)); // Then the blank date still blocks confirmation - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + expect(result.current.fieldErrors).toEqual({date: 'common.error.fieldRequired'}); }); it('passes again once the user clears the fields back to an untouched scan', () => { @@ -979,7 +1089,7 @@ describe('useConfirmationValidation', () => { useConfirmationValidation( createScanValidationParams( {isAmountSet: true, amount: 1000, isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}, - {iouAmount: 1000, iouMerchant: 'Starbucks', isMerchantEmpty: false}, + {iouAmount: 1000, iouMerchant: 'Starbucks'}, ), ), ); @@ -987,28 +1097,32 @@ describe('useConfirmationValidation', () => { }); it('validates the entered amount the same way a manually entered one is validated', () => { + // Given a scan to a person where the user filled in all three fields with a $0 amount const {result} = renderHook(() => useConfirmationValidation( createScanValidationParams( {isAmountSet: true, amount: 0, isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}, - {iouAmount: 0, iouMerchant: 'Starbucks', isMerchantEmpty: false}, + {iouAmount: 0, iouMerchant: 'Starbucks'}, ), ), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + + // When its field errors are computed + // Then the typed amount is rejected like a manual one, since it replaces what SmartScan would have read + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); it('blocks a negative entered amount when the expense goes to a 1:1 user', () => { // Given a scan addressed to a P2P recipient, where the To field is picked before the amount is entered const filledFields = {isAmountSet: true, amount: -1234, isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}; - // When the user fills in a negative amount and confirms + // When the user fills in a negative amount const {result} = renderHook(() => - useConfirmationValidation(createScanValidationParams(filledFields, {iouType: CONST.IOU.TYPE.CREATE, iouAmount: -1234, iouMerchant: 'Starbucks', isMerchantEmpty: false})), + useConfirmationValidation(createScanValidationParams(filledFields, {iouType: CONST.IOU.TYPE.CREATE, iouAmount: -1234, iouMerchant: 'Starbucks'})), ); // Then it is rejected, since P2P chats don't support negative amounts - expect(result.current.validate()).toEqual({errorKey: 'common.error.invalidAmount'}); + expect(result.current.fieldErrors).toEqual({amount: 'common.error.invalidAmount'}); }); it.each([ @@ -1026,7 +1140,6 @@ describe('useConfirmationValidation', () => { iouType: CONST.IOU.TYPE.CREATE, iouAmount: -1234, iouMerchant: 'Starbucks', - isMerchantEmpty: false, selectedParticipants: [participant], }), ), @@ -1036,29 +1149,57 @@ describe('useConfirmationValidation', () => { expect(result.current.validate()).toEqual({errorKey: null}); }); - it('blocks confirmation when another receipt in a multi-scan is the partially filled one', () => { - // The transaction on screen is a complete manual scan, but a sibling receipt is not. + const COMPLETE_SCAN_FIELDS = {isAmountSet: true, amount: 1000, isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}; + + function createSiblingReceipt(overrides: Partial): OnyxTypes.Transaction { + return createTransactionBase({ + transactionID: 'other-txn', + iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, + receipt: {source: 'https://example.com/receipt.jpg', state: CONST.IOU.RECEIPT_STATE.SCAN_READY}, + merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, + ...overrides, + }); + } + + it('blocks confirmation when another receipt in a multi-scan holds a field error', () => { + // Given a multi-scan to a person whose receipt on screen is valid while a sibling receipt holds a negative amount + const siblingTransactions = [createSiblingReceipt({...COMPLETE_SCAN_FIELDS, amount: -1234})]; const {result} = renderHook(() => - useConfirmationValidation( - createScanValidationParams( - {isAmountSet: true, amount: 1000, isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}, - {iouAmount: 1000, iouMerchant: 'Starbucks', isMerchantEmpty: false, partiallyManuallyFilledScanID: 'other-txn'}, - ), - ), + useConfirmationValidation(createScanValidationParams(COMPLETE_SCAN_FIELDS, {iouType: CONST.IOU.TYPE.CREATE, iouAmount: 1000, iouMerchant: 'Starbucks', siblingTransactions})), ); - expect(result.current.validate()).toEqual({errorKey: 'common.error.fieldRequired'}); + + // When the user confirms from the valid receipt + // Then it is blocked, since Create would otherwise submit the invalid receipt along with this one + expect(result.current.validate()).toEqual({isFieldError: true}); + expect(result.current.siblingFieldErrors).toEqual({transactionID: 'other-txn', fieldErrors: {amount: 'common.error.invalidAmount'}}); }); - it('lets a multi-scan through once no receipt is left partially filled', () => { - const {result} = renderHook(() => - useConfirmationValidation( - createScanValidationParams( - {isAmountSet: true, amount: 1000, isMerchantSet: true, merchant: 'Starbucks', isCreatedSet: true, created: '2025-01-15'}, - {iouAmount: 1000, iouMerchant: 'Starbucks', isMerchantEmpty: false, partiallyManuallyFilledScanID: undefined}, - ), - ), - ); - expect(result.current.validate()).toEqual({errorKey: null}); + it('runs every check on the receipt on screen before another receipt with a field error blocks confirmation', () => { + // Given a multi-scan on a Control workspace whose receipt on screen is in a category that requires attendees but has none, while another receipt holds a typed merchant that isn't allowed + const controlPolicy: OnyxTypes.Policy = {...createRandomPolicy(1), type: CONST.POLICY.TYPE.CORPORATE, isAttendeeTrackingEnabled: true}; + const policyCategories = {Meals: createMock({name: 'Meals', enabled: true, areAttendeesRequired: true})}; + const siblingTransactions = [createSiblingReceipt({...COMPLETE_SCAN_FIELDS, merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT})]; + const params = createScanValidationParams(COMPLETE_SCAN_FIELDS, { + iouAmount: 1000, + iouMerchant: 'Starbucks', + iouCategory: 'Meals', + policy: controlPolicy, + policyCategories, + currentUserPersonalDetails: createMock({accountID: 1, email: 'owner@example.com'}), + siblingTransactions, + }); + const {result, rerender} = renderHook((props: UseConfirmationValidationParams) => useConfirmationValidation(props), {initialProps: params}); + + // When the user confirms from the receipt on screen + // Then its missing attendees block first, so this receipt is fixed before Create moves on to the other one + expect(result.current.validate()).toEqual({errorKey: 'violations.missingAttendees'}); + + // When an attendee is added and the user confirms again + rerender({...params, iouAttendees: [{email: 'guest@example.com', displayName: 'Guest', avatarUrl: ''}]}); + + // Then the other receipt's merchant blocks confirmation, since Create submits both receipts + expect(result.current.validate()).toEqual({isFieldError: true}); + expect(result.current.siblingFieldErrors).toEqual({transactionID: 'other-txn', fieldErrors: {merchant: 'iou.error.invalidMerchant'}}); }); it('requires nothing on surfaces that do not expose the scan fields (splits, test receipts)', () => { diff --git a/tests/unit/hooks/useFormErrorManagement.test.tsx b/tests/unit/hooks/useFormErrorManagement.test.tsx index d21b7d7ff64f..8b96dc01cb0d 100644 --- a/tests/unit/hooks/useFormErrorManagement.test.tsx +++ b/tests/unit/hooks/useFormErrorManagement.test.tsx @@ -29,7 +29,6 @@ type Params = Parameters[0]; const baseParams: Params = { transaction: createMock({transactionID: 'txn1', amount: 100, merchant: 'Coffee', comment: {}}), transactionReport: undefined, - iouMerchant: 'Coffee', iouCategory: '', iouAttendees: [], policy: undefined, @@ -37,32 +36,12 @@ const baseParams: Params = { policyCategories: undefined, currentUserPersonalDetails: {accountID: 1} as CurrentUserPersonalDetails, isEditingSplitBill: false, - isPolicyExpenseChat: false, - isScanRequest: false, - canEnterScanFieldsManually: false, - shouldShowMerchant: true, hasSmartScanFailed: false, didConfirmSplit: false, routeError: undefined, isTypeSplit: false, shouldShowReadOnlySplits: false, isDistanceRequest: false, - shouldShowDate: false, - isReadOnly: false, -}; - -// A manual draft the user never typed a merchant into: `initMoneyRequest` seeds it with the "Expense" placeholder. -const placeholderMerchantTransaction = createMock({ - transactionID: 'txn1', - amount: 100, - merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, - isMerchantSet: false, - comment: {}, -}); - -const placeholderMerchantParams: Partial = { - transaction: placeholderMerchantTransaction, - iouMerchant: CONST.TRANSACTION.DEFAULT_MERCHANT, }; function Wrapper({children}: {children: React.ReactNode}) { @@ -96,16 +75,6 @@ describe('useFormErrorManagement', () => { expect(result.current.shouldDisplayFieldError).toBe(true); }); - it('isMerchantRequired is true only for policyExpenseChat with non-scan request and shouldShowMerchant', () => { - const {result: required} = renderHook(() => useFormErrorManagement({...baseParams, isPolicyExpenseChat: true, isScanRequest: false, shouldShowMerchant: true}), {wrapper: Wrapper}); - const {result: notRequired} = renderHook( - () => useFormErrorManagement({...baseParams, isPolicyExpenseChat: true, isScanRequest: true, shouldShowMerchant: true, isEditingSplitBill: false}), - {wrapper: Wrapper}, - ); - expect(required.current.isMerchantRequired).toBe(true); - expect(notRequired.current.isMerchantRequired).toBe(false); - }); - it('clearFormErrors only clears the error when it matches the provided list', async () => { const {result} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); act(() => result.current.setFormError('iou.error.invalidMerchant')); @@ -120,6 +89,44 @@ describe('useFormErrorManagement', () => { expect(result.current.formError).toBe(''); }); + it('drops the error a confirm set once a later confirm asks for it to be cleared', () => { + // Given a confirm that was blocked by an empty tax amount + const {result} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); + act(() => result.current.setConfirmError('iou.error.invalidAmount')); + expect(result.current.formError).toBe('iou.error.invalidAmount'); + + // When a later confirm is blocked by a field error instead + act(() => result.current.clearConfirmError()); + + // Then the error is dropped, since validation checks it again on the next confirm + expect(result.current.formError).toBe(''); + }); + + it('keeps an error set outside validation when a confirm asks to clear its own', () => { + // Given a form error the distance rate check raised + const {result} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); + act(() => result.current.setFormError('iou.error.invalidRate')); + + // When a confirm blocked by a field error asks to clear its own error + act(() => result.current.clearConfirmError()); + + // Then the distance rate error stays, since validation doesn't check it again and it still has to block confirming + expect(result.current.formError).toBe('iou.error.invalidRate'); + }); + + it('keeps an error that replaced the one a confirm set', () => { + // Given a confirm blocked by an empty tax amount, after which the distance rate check raised its own error + const {result} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); + act(() => result.current.setConfirmError('iou.error.invalidAmount')); + act(() => result.current.setFormError('iou.error.invalidRate')); + + // When a confirm blocked by a field error asks to clear its own error + act(() => result.current.clearConfirmError()); + + // Then the distance rate error stays, since it is no longer the one the confirm set + expect(result.current.formError).toBe('iou.error.invalidRate'); + }); + it('errorMessage returns routeError when present', () => { const {result} = renderHook(() => useFormErrorManagement({...baseParams, routeError: 'route.error.unreachable'}), {wrapper: Wrapper}); expect(result.current.errorMessage).toBe('route.error.unreachable'); @@ -131,14 +138,15 @@ describe('useFormErrorManagement', () => { expect(result.current.errorMessage).toBeUndefined(); }); - it('errorMessage suppresses required/invalid amount errors (surfaced inline)', () => { - const {result: required} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); - act(() => required.current.setFormError('common.error.fieldRequired')); - expect(required.current.errorMessage).toBeUndefined(); + it('errorMessage leaves out the invalid amount error the amount input already shows', () => { + // Given an edited split, whose amount input flags an invalid amount through the form error as the user types + const {result} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); + + // When that error is set + act(() => result.current.setFormError('common.error.invalidAmount')); - const {result: invalid} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); - act(() => invalid.current.setFormError('common.error.invalidAmount')); - expect(invalid.current.errorMessage).toBeUndefined(); + // Then the footer doesn't repeat it under the button + expect(result.current.errorMessage).toBeUndefined(); }); it('errorMessage still shows the invalid amount error for a distance request (no inline surface)', () => { @@ -200,184 +208,4 @@ describe('useFormErrorManagement', () => { jest.useRealTimers(); } }); - - it('errorMessage suppresses the invalid merchant error (surfaced inline)', () => { - const {result} = renderHook(() => useFormErrorManagement(baseParams), {wrapper: Wrapper}); - act(() => result.current.setFormError('iou.error.invalidMerchant')); - expect(result.current.errorMessage).toBeUndefined(); - }); - - it('treats the placeholder merchant of an untouched draft as empty, so it is only invalid while a merchant is required', () => { - const {result: required} = renderHook(() => useFormErrorManagement({...baseParams, ...placeholderMerchantParams, isPolicyExpenseChat: true}), {wrapper: Wrapper}); - const {result: notRequired} = renderHook(() => useFormErrorManagement({...baseParams, ...placeholderMerchantParams, isPolicyExpenseChat: false}), {wrapper: Wrapper}); - - expect(required.current.isMerchantFieldValid).toBe(false); - expect(notRequired.current.isMerchantFieldValid).toBe(true); - }); - - it('keeps a placeholder merchant the user typed themselves invalid', () => { - const {result} = renderHook( - () => - useFormErrorManagement({ - ...baseParams, - ...placeholderMerchantParams, - transaction: {...placeholderMerchantTransaction, isMerchantSet: true}, - isPolicyExpenseChat: false, - }), - {wrapper: Wrapper}, - ); - - expect(result.current.isMerchantFieldValid).toBe(false); - }); - - it('leaves the merchant optional on a scan the user has started filling in, since a blank field is still scanned', () => { - const scanParams: Partial = { - isScanRequest: true, - isPolicyExpenseChat: false, - iouMerchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, - }; - const scanDraft = { - transactionID: 'txn1', - amount: 0, - merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, - comment: {}, - iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, - }; - - const {result: untouched} = renderHook(() => useFormErrorManagement({...baseParams, ...scanParams, transaction: createMock(scanDraft)}), { - wrapper: Wrapper, - }); - const {result: amountEntered} = renderHook( - () => useFormErrorManagement({...baseParams, ...scanParams, transaction: createMock({...scanDraft, amount: 1000, isAmountSet: true})}), - {wrapper: Wrapper}, - ); - - expect(untouched.current.isMerchantRequired).toBe(false); - expect(amountEntered.current.isMerchantRequired).toBe(false); - }); - - it('clears the invalid merchant error once the recipient changes from a workspace chat to a user (#96593)', () => { - // Given an untouched manual draft (still carrying the placeholder merchant) headed for a workspace chat - const {result, rerender} = renderHook( - ({isPolicyExpenseChat}: {isPolicyExpenseChat: boolean}) => useFormErrorManagement({...baseParams, ...placeholderMerchantParams, isPolicyExpenseChat}), - {wrapper: Wrapper, initialProps: {isPolicyExpenseChat: true}}, - ); - - // When confirming surfaces the "please enter a valid merchant" error - act(() => result.current.setFormError('iou.error.invalidMerchant')); - expect(result.current.formError).toBe('iou.error.invalidMerchant'); - - // Then switching the recipient to a user drops the merchant requirement and clears the error - rerender({isPolicyExpenseChat: false}); - expect(result.current.formError).toBe(''); - }); - - const manualRequiredParams = ({ - isAmountSet, - created, - shouldShowDate = true, - isReadOnly = false, - }: { - isAmountSet: boolean; - created: string; - shouldShowDate?: boolean; - isReadOnly?: boolean; - }): Params => ({ - ...baseParams, - shouldShowDate, - isReadOnly, - transaction: createMock({ - transactionID: 'txn1', - iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL, - isAmountSet, - created, - comment: {}, - }), - }); - - it('clears the required error once the amount is set when the date field is hidden, so a hidden date cannot strand it', () => { - // Given a confirmation that doesn't render the date field (a scan flow before its fields show), with no date yet - const {result, rerender} = renderHook((props: Params) => useFormErrorManagement(props), { - wrapper: Wrapper, - initialProps: manualRequiredParams({isAmountSet: false, created: '', shouldShowDate: false}), - }); - act(() => result.current.setFormError('common.error.fieldRequired')); - - // When the amount is filled, the empty date must not keep the error alive since it has no inline surface - rerender(manualRequiredParams({isAmountSet: true, created: '', shouldShowDate: false})); - expect(result.current.formError).toBe(''); - }); - - it('clears the required error once the amount is set on a read-only confirmation, where the date is populated server-side', () => { - const {result, rerender} = renderHook((props: Params) => useFormErrorManagement(props), { - wrapper: Wrapper, - initialProps: manualRequiredParams({isAmountSet: false, created: '', isReadOnly: true}), - }); - act(() => result.current.setFormError('common.error.fieldRequired')); - - rerender(manualRequiredParams({isAmountSet: true, created: '', isReadOnly: true})); - expect(result.current.formError).toBe(''); - }); - - it('keeps the required error alive while another receipt of a multi-scan is still partially filled', () => { - // Given a confirmation showing a complete receipt while a sibling one is partially filled. Confirming switches to - // the sibling, and the error has to survive the render where the complete receipt is still the one on screen. - const completeReceiptParams = (partiallyManuallyFilledScanID?: string): Params => ({ - ...baseParams, - canEnterScanFieldsManually: true, - partiallyManuallyFilledScanID, - transaction: createMock({ - transactionID: 'txn1', - iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, - isAmountSet: true, - isMerchantSet: true, - isCreatedSet: true, - comment: {}, - }), - }); - - const partiallyFilledReceiptParams = (partiallyManuallyFilledScanID?: string): Params => ({ - ...completeReceiptParams(partiallyManuallyFilledScanID), - transaction: createMock({ - transactionID: 'txn1', - iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, - isAmountSet: false, - isMerchantSet: true, - isCreatedSet: true, - comment: {}, - }), - }); - - const {result, rerender} = renderHook((props: Params) => useFormErrorManagement(props), { - wrapper: Wrapper, - initialProps: partiallyFilledReceiptParams('txn1'), - }); - act(() => result.current.setFormError('common.error.fieldRequired')); - - // Moving to a receipt that has nothing missing would clear the error, but a sibling is still partially filled - rerender(completeReceiptParams('txn2')); - expect(result.current.formError).toBe('common.error.fieldRequired'); - - // Once no receipt is partially filled any more the error clears, so confirmation is no longer blocked - rerender(completeReceiptParams(undefined)); - expect(result.current.formError).toBe(''); - }); - - it('keeps the shared required error until both the amount and the date are filled (#96568)', () => { - // Given a manual expense confirmed with both the amount and the date empty - const {result, rerender} = renderHook((props: Params) => useFormErrorManagement(props), { - wrapper: Wrapper, - initialProps: manualRequiredParams({isAmountSet: false, created: ''}), - }); - act(() => result.current.setFormError('common.error.fieldRequired')); - expect(result.current.formError).toBe('common.error.fieldRequired'); - - // When only the amount is filled, the shared error must survive so the date keeps showing it inline - rerender(manualRequiredParams({isAmountSet: true, created: ''})); - expect(result.current.formError).toBe('common.error.fieldRequired'); - - // Then filling the date too clears it, so confirmation is no longer blocked - rerender(manualRequiredParams({isAmountSet: true, created: '2026-07-29'})); - expect(result.current.formError).toBe(''); - }); }); diff --git a/tests/unit/hooks/useScrollToFlaggedField.test.ts b/tests/unit/hooks/useScrollToFlaggedField.test.ts new file mode 100644 index 000000000000..d42ba1a41c9e --- /dev/null +++ b/tests/unit/hooks/useScrollToFlaggedField.test.ts @@ -0,0 +1,57 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import useScrollToFlaggedField from '@components/MoneyRequestConfirmationList/hooks/useScrollToFlaggedField'; +import type {SelectionListWithSectionsHandle} from '@components/SelectionList/SelectionListWithSections/types'; + +import type {View} from 'react-native'; + +import createMock from '../../utils/createMock'; + +function renderWithMerchantView(transactionIDOnScreen: string) { + const scrollToReveal = jest.fn(); + const listRef = {current: createMock({scrollToReveal})}; + const {result, rerender} = renderHook(({transactionID}: {transactionID: string}) => useScrollToFlaggedField(listRef, transactionID), { + initialProps: {transactionID: transactionIDOnScreen}, + }); + const merchantView = createMock({}); + result.current.fieldViewRefs.merchant.current = merchantView; + return {result, rerender, scrollToReveal, merchantView}; +} + +describe('useScrollToFlaggedField', () => { + it('scrolls the list to the view of the flagged field, and again every time a confirm is blocked by it', () => { + // Given a confirmation whose merchant field has attached its outer view + const {result, scrollToReveal, merchantView} = renderWithMerchantView('txn1'); + + // When a blocked confirm flags the merchant, and a second confirm flags it again + act(() => result.current.scrollToField('merchant', 'txn1')); + act(() => result.current.scrollToField('merchant', 'txn1')); + + // Then the list scrolls to the merchant's view both times, so a repeated confirm still brings its error into view + expect(scrollToReveal).toHaveBeenCalledTimes(2); + expect(scrollToReveal).toHaveBeenLastCalledWith(merchantView); + }); + + it('waits for the flagged receipt to be on screen before scrolling, and only scrolls once for it', () => { + // Given a multi-scan whose blocked confirm flagged the merchant of another receipt, while the pager still shows the previous receipt + const {result, rerender, scrollToReveal, merchantView} = renderWithMerchantView('txn1'); + act(() => result.current.scrollToField('merchant', 'txn2')); + + // Then nothing scrolls yet, since the merchant view still belongs to the previous receipt + expect(scrollToReveal).not.toHaveBeenCalled(); + + // When the flagged receipt comes on screen + rerender({transactionID: 'txn2'}); + + // Then the list scrolls to its merchant + expect(scrollToReveal).toHaveBeenCalledTimes(1); + expect(scrollToReveal).toHaveBeenLastCalledWith(merchantView); + + // When the user pages away and back to that receipt without confirming again + rerender({transactionID: 'txn1'}); + rerender({transactionID: 'txn2'}); + + // Then the list stays where the user left it + expect(scrollToReveal).toHaveBeenCalledTimes(1); + }); +});