diff --git a/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx b/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx index e613ee1a4fce..0813063b1088 100644 --- a/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx +++ b/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx @@ -304,6 +304,10 @@ function ExpenseReportListItemInner({ delegateEmail, delegateAccountID, isTrackIntentUser, + // Pass the row-scoped, live violations (keyed by this report's snapshot transactions) instead of the + // whole TRANSACTION_VIOLATIONS collection, so the Approve action reads live data without re-rendering + // every row on unrelated violation changes. + allViolations: liveViolationsForSnapshotTransactions, conciergeChat, }); }, [ @@ -346,6 +350,7 @@ function ExpenseReportListItemInner({ delegateEmail, delegateAccountID, isTrackIntentUser, + liveViolationsForSnapshotTransactions, conciergeChat, ]); diff --git a/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx b/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx index 8912b377b525..631597e0cb0e 100644 --- a/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx +++ b/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx @@ -40,6 +40,7 @@ import type {ColorValue} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import {isTrackIntentUserSelector} from '@selectors/Onboarding'; +import {transactionViolationsByIDsSelector} from '@selectors/TransactionViolations'; import React, {useMemo} from 'react'; import {View} from 'react-native'; // Use the original useOnyx hook to get the real-time personal details list data from Onyx and not from the snapshot @@ -282,6 +283,9 @@ function ReportListItemHeaderInner({ ); const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); + const reportTransactionIDs = (reportItem.transactions ?? []).map((transaction) => transaction.transactionID); + const [allViolations] = originalUseOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {selector: transactionViolationsByIDsSelector(reportTransactionIDs)}); + const {currentUserAccountID, currentUserLogin, introSelected, betas, isSelfTourViewed, activePolicy, chatReportPolicy, amountOwed, delegateEmail, delegateAccountID, conciergeChat} = useReportPaymentContext({ chatReportPolicyID: chatReport?.policyID, @@ -333,6 +337,7 @@ function ReportListItemHeaderInner({ delegateEmail, delegateAccountID, isTrackIntentUser, + allViolations, conciergeChat, }); }; diff --git a/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx b/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx index f7c793f0b5cb..95a3e565e107 100644 --- a/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx +++ b/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx @@ -38,10 +38,10 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import {personalDetailsLoginSelector} from '@src/selectors/PersonalDetails'; import {isActionLoadingSelector} from '@src/selectors/ReportMetaData'; -import type {Policy, Report, ReportAction, ReportActions} from '@src/types/onyx'; +import type {Policy, Report, ReportAction, ReportActions, TransactionViolations} from '@src/types/onyx'; import type {TransactionViolation} from '@src/types/onyx/TransactionViolation'; -import type {OnyxEntry} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {isTrackIntentUserSelector} from '@selectors/Onboarding'; // NOTE: The narrow-layout rendering of this component has a static twin in @@ -132,7 +132,9 @@ function TransactionListItemInner({ const [transactionThreadReport] = originalUseOnyx(`${ONYXKEYS.COLLECTION.REPORT}${transactionItem?.reportAction?.childReportID}`); const [submitterLogin] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: personalDetailsLoginSelector(transactionItem?.report?.ownerAccountID)}); const [transaction] = originalUseOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${getNonEmptyStringOnyxID(transactionItem.transactionID)}`); - const [transactionViolationsForRow] = originalUseOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${getNonEmptyStringOnyxID(transactionItem.transactionID)}`); + const transactionViolationsKey = `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${getNonEmptyStringOnyxID(transactionItem.transactionID)}` as const; + const [transactionViolationsForRow] = originalUseOnyx(transactionViolationsKey); + const allViolations: OnyxCollection = {[transactionViolationsKey]: transactionViolationsForRow}; const parentReportActionID = transactionItem?.reportAction?.reportActionID; const [parentReportAction] = originalUseOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(transactionItem.reportID)}`, { selector: (reportActions: OnyxEntry): OnyxEntry => reportActions?.[`${parentReportActionID}`], @@ -240,6 +242,7 @@ function TransactionListItemInner({ delegateEmail, delegateAccountID, isTrackIntentUser, + allViolations, conciergeChat, }); }; diff --git a/src/libs/actions/IOU/UpdateMoneyRequest.ts b/src/libs/actions/IOU/UpdateMoneyRequest.ts index 271a0ca8410c..792e0b588733 100644 --- a/src/libs/actions/IOU/UpdateMoneyRequest.ts +++ b/src/libs/actions/IOU/UpdateMoneyRequest.ts @@ -621,6 +621,7 @@ type UpdateMoneyRequestVendorParams = { parentReport?: OnyxEntry; policy?: OnyxEntry; delegateAccountID: number | undefined; + transactionViolations: OnyxEntry; }; /** @@ -631,7 +632,17 @@ type UpdateMoneyRequestVendorParams = { * * Passing `vendorID=''` clears the vendor from the transaction. */ -function updateMoneyRequestVendor({transactionID, vendorID, vendorName, transaction, transactionThreadReport, parentReport, policy, delegateAccountID}: UpdateMoneyRequestVendorParams) { +function updateMoneyRequestVendor({ + transactionID, + vendorID, + vendorName, + transaction, + transactionThreadReport, + parentReport, + policy, + delegateAccountID, + transactionViolations, +}: UpdateMoneyRequestVendorParams) { // Fall back to the cached Onyx transaction when the caller doesn't pass one so failureData can // restore the actual previous vendor on API failure instead of clearing it. const resolvedTransaction = transaction ?? getAllTransactions()?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; @@ -759,9 +770,7 @@ function updateMoneyRequestVendor({transactionID, vendorID, vendorName, transact // resolves it (no vendor → no inactive-vendor). Without this, the stale violation persists // in Onyx until some unrelated recalculation fires, keeping the expense incorrectly flagged. const violationsKey = `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}` as const; - // TODO: https://github.com/Expensify/App/issues/66512 - // eslint-disable-next-line @typescript-eslint/no-deprecated - const currentViolations = getAllTransactionViolations()[violationsKey] ?? []; + const currentViolations = transactionViolations ?? []; if (currentViolations.some((violation) => violation.name === CONST.VIOLATIONS.INACTIVE_VENDOR)) { optimisticData.push({ onyxMethod: Onyx.METHOD.SET, diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index fdcf6bbf90a9..b03663b092ed 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -88,6 +88,7 @@ import type { ReportActions, SaveSearch, Transaction, + TransactionViolations, } from '@src/types/onyx'; import type {PaymentInformation} from '@src/types/onyx/LastPaymentMethod'; import type {ConnectionName} from '@src/types/onyx/Policy'; @@ -106,7 +107,6 @@ import Onyx from 'react-native-onyx'; import type {AdditionalPayOnyxData} from './IOU/PayMoneyRequest'; import type {RejectMoneyRequestData} from './IOU/RejectMoneyRequest'; -import {getAllTransactionViolations} from './IOU'; import {payMoneyRequest} from './IOU/PayMoneyRequest'; import {prepareRejectMoneyRequestData, rejectMoneyRequest} from './IOU/RejectMoneyRequest'; import {approveMoneyRequest} from './IOU/ReportWorkflow'; @@ -240,6 +240,7 @@ type HandleActionButtonPressParams = { delegateEmail?: string; delegateAccountID: number | undefined; isTrackIntentUser: boolean | undefined; + allViolations: OnyxCollection; conciergeChat: OnyxEntry; getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals']; }; @@ -279,6 +280,7 @@ function handleActionButtonPress({ delegateEmail, delegateAccountID, isTrackIntentUser, + allViolations, conciergeChat, getCurrencyDecimals, }: HandleActionButtonPressParams) { @@ -366,6 +368,7 @@ function handleActionButtonPress({ delegateAccountID, isTrackIntentUser, ownerLogin: submitterLogin, + allViolations, getCurrencyDecimals, }); return; @@ -665,6 +668,7 @@ type GetApproveActionCallbackParams = { delegateAccountID: number | undefined; isTrackIntentUser: boolean | undefined; ownerLogin: string | undefined; + allViolations: OnyxCollection; getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals']; }; @@ -685,6 +689,7 @@ function getApproveActionCallback({ delegateAccountID, isTrackIntentUser, ownerLogin, + allViolations, getCurrencyDecimals, }: GetApproveActionCallbackParams) { if (!item.reportID) { @@ -692,8 +697,7 @@ function getApproveActionCallback({ } const reportPolicy = policy ?? snapshotPolicy; - // eslint-disable-next-line @typescript-eslint/no-deprecated -- using deprecated getAllTransactionViolations until #66512 migrates this call - const hasViolations = hasViolationsReportUtils(item.reportID, getAllTransactionViolations(), currentUserAccountID, currentUserLogin ?? ''); + const hasViolations = hasViolationsReportUtils(item.reportID, allViolations, currentUserAccountID, currentUserLogin ?? ''); const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, betas); approveMoneyRequest({ diff --git a/src/pages/iou/request/step/IOURequestStepVendor.tsx b/src/pages/iou/request/step/IOURequestStepVendor.tsx index 6221e1447fbb..2ee5e1a33401 100644 --- a/src/pages/iou/request/step/IOURequestStepVendor.tsx +++ b/src/pages/iou/request/step/IOURequestStepVendor.tsx @@ -62,6 +62,7 @@ function IOURequestStepVendor({ isPerDiemRequest: isPerDiemRequest(transaction), }); const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.parentReportID)}`); + const [transactionViolations] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${getNonEmptyStringOnyxID(transactionID)}`); const delegateAccountID = useDelegateAccountID(); const isFeatureAvailable = hasVendorFeature(policy, isBetaEnabled(CONST.BETAS.VENDOR_MATCHING)); @@ -120,6 +121,7 @@ function IOURequestStepVendor({ parentReport, policy, delegateAccountID, + transactionViolations, }); } navigateBack(); diff --git a/tests/actions/IOUTest/UpdateMoneyRequestVendorTest.ts b/tests/actions/IOUTest/UpdateMoneyRequestVendorTest.ts index b51edb834eb7..522a24776282 100644 --- a/tests/actions/IOUTest/UpdateMoneyRequestVendorTest.ts +++ b/tests/actions/IOUTest/UpdateMoneyRequestVendorTest.ts @@ -54,43 +54,84 @@ describe('updateMoneyRequestVendor', () => { const getOnyxDataArg = () => getRequiredWriteCall(writeSpy.mock.calls, 0)[2]; - it('clears an existing inactive-vendor violation optimistically when a vendor is picked', async () => { - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, [otherViolation, inactiveVendorViolation]); - await waitForBatchedUpdates(); - - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-active', vendorName: 'Active Vendor', transaction: baseTransaction, delegateAccountID: undefined}); + it('clears an existing inactive-vendor violation optimistically when a vendor is picked', () => { + // The violations are passed in as a parameter (sourced from useOnyx in the component) rather than + // read from the global Onyx collection, so nothing is set on Onyx here. + updateMoneyRequestVendor({ + transactionID: TRANSACTION_ID, + vendorID: 'v-active', + vendorName: 'Active Vendor', + transaction: baseTransaction, + delegateAccountID: undefined, + transactionViolations: [otherViolation, inactiveVendorViolation], + }); const violationsUpdate = getRequiredOnyxUpdate(getOnyxDataArg(), 'optimisticData', `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, Onyx.METHOD.SET, true); expect(violationsUpdate.value).toEqual([otherViolation]); }); - it('clears an existing inactive-vendor violation optimistically when the vendor is cleared', async () => { - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, [inactiveVendorViolation]); - await waitForBatchedUpdates(); - - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: '', vendorName: '', transaction: baseTransaction, delegateAccountID: undefined}); + it('clears an existing inactive-vendor violation optimistically when the vendor is cleared', () => { + updateMoneyRequestVendor({ + transactionID: TRANSACTION_ID, + vendorID: '', + vendorName: '', + transaction: baseTransaction, + delegateAccountID: undefined, + transactionViolations: [inactiveVendorViolation], + }); const violationsUpdate = getRequiredOnyxUpdate(getOnyxDataArg(), 'optimisticData', `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, Onyx.METHOD.SET, true); expect(violationsUpdate.value).toEqual([]); }); - it('restores the original violation list in failureData so a server rejection rolls back cleanly', async () => { + it('restores the original violation list in failureData so a server rejection rolls back cleanly', () => { const original = [otherViolation, inactiveVendorViolation]; - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, original); - await waitForBatchedUpdates(); - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-active', vendorName: 'Active Vendor', transaction: baseTransaction, delegateAccountID: undefined}); + updateMoneyRequestVendor({ + transactionID: TRANSACTION_ID, + vendorID: 'v-active', + vendorName: 'Active Vendor', + transaction: baseTransaction, + delegateAccountID: undefined, + transactionViolations: original, + }); const failureViolations = getRequiredOnyxUpdate(getOnyxDataArg(), 'failureData', `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, Onyx.METHOD.SET, true); expect(failureViolations.value).toEqual(original); }); - it('does not write a violations update when there was no inactive-vendor violation to clear', async () => { - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, [otherViolation]); + it('does not write a violations update when there was no inactive-vendor violation to clear', () => { + updateMoneyRequestVendor({ + transactionID: TRANSACTION_ID, + vendorID: 'v-active', + vendorName: 'Active Vendor', + transaction: baseTransaction, + delegateAccountID: undefined, + transactionViolations: [otherViolation], + }); + + const violationsUpdate = getRequiredOnyxUpdates(getOnyxDataArg(), 'optimisticData').some( + (entry) => isObject(entry) && entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, + ); + expect(violationsUpdate).toBe(false); + }); + + it('ignores the global Onyx violations collection and uses the passed transactionViolations parameter', async () => { + // Given: Onyx holds an inactive-vendor violation, but the parameter passes none. + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, [inactiveVendorViolation]); await waitForBatchedUpdates(); - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-active', vendorName: 'Active Vendor', transaction: baseTransaction, delegateAccountID: undefined}); + // When: a vendor is picked while passing an empty violations parameter + updateMoneyRequestVendor({ + transactionID: TRANSACTION_ID, + vendorID: 'v-active', + vendorName: 'Active Vendor', + transaction: baseTransaction, + delegateAccountID: undefined, + transactionViolations: [], + }); + // Then: no violations update is written, proving the parameter (not the global collection) drives the logic. const violationsUpdate = getRequiredOnyxUpdates(getOnyxDataArg(), 'optimisticData').some( (entry) => isObject(entry) && entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, ); @@ -105,7 +146,7 @@ describe('updateMoneyRequestVendor', () => { }); await waitForBatchedUpdates(); - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', delegateAccountID: undefined}); + updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', delegateAccountID: undefined, transactionViolations: undefined}); const transactionFailure = getRequiredOnyxUpdate(getOnyxDataArg(), 'failureData', `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, Onyx.METHOD.MERGE, true); expect(transactionFailure.value).toEqual({ @@ -118,14 +159,21 @@ describe('updateMoneyRequestVendor', () => { // No transaction arg + nothing in Onyx — the prior vendor is unknown, so we must not // write `vendor: null` and silently clear whatever the server actually has. The // pendingFields-clear entry still runs (so the offline indicator clears on rejection). - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', delegateAccountID: undefined}); + updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', delegateAccountID: undefined, transactionViolations: undefined}); const transactionFailure = getRequiredOnyxUpdate(getOnyxDataArg(), 'failureData', `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, Onyx.METHOD.MERGE, true); expect(transactionFailure.value).toEqual({pendingFields: {vendor: null}}); }); it('writes pendingFields.vendor = UPDATE in optimisticData so the offline indicator surfaces', () => { - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', transaction: baseTransaction, delegateAccountID: undefined}); + updateMoneyRequestVendor({ + transactionID: TRANSACTION_ID, + vendorID: 'v-new', + vendorName: 'New Vendor', + transaction: baseTransaction, + delegateAccountID: undefined, + transactionViolations: undefined, + }); const transactionOptimistic = getRequiredOnyxUpdate(getOnyxDataArg(), 'optimisticData', `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, Onyx.METHOD.MERGE, true); // The selected vendor's display name is persisted alongside the externalID so the title still @@ -137,7 +185,14 @@ describe('updateMoneyRequestVendor', () => { }); it('clears pendingFields.vendor in successData when the server confirms the write', () => { - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', transaction: baseTransaction, delegateAccountID: undefined}); + updateMoneyRequestVendor({ + transactionID: TRANSACTION_ID, + vendorID: 'v-new', + vendorName: 'New Vendor', + transaction: baseTransaction, + delegateAccountID: undefined, + transactionViolations: undefined, + }); const transactionSuccess = getRequiredOnyxUpdate(getOnyxDataArg(), 'successData', `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, Onyx.METHOD.MERGE, true); expect(transactionSuccess.value).toEqual({pendingFields: {vendor: null}}); @@ -146,7 +201,7 @@ describe('updateMoneyRequestVendor', () => { it('clears pendingFields.vendor in failureData when the server rejects the write', () => { // Even without a prior snapshot to roll the vendor itself back, the pending indicator must // clear on failure — otherwise the row stays stuck in "pending" forever after a server reject. - updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', delegateAccountID: undefined}); + updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', vendorName: 'New Vendor', delegateAccountID: undefined, transactionViolations: undefined}); const transactionFailure = getRequiredOnyxUpdate(getOnyxDataArg(), 'failureData', `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, Onyx.METHOD.MERGE, true); expect(transactionFailure.value).toMatchObject({pendingFields: {vendor: null}}); @@ -183,6 +238,7 @@ describe('updateMoneyRequestVendor', () => { transaction: baseTransaction, transactionThreadReport, delegateAccountID: undefined, + transactionViolations: undefined, }); const optimisticAction = findOptimisticModifiedExpense(); @@ -209,6 +265,7 @@ describe('updateMoneyRequestVendor', () => { transaction: transactionWithVendor, transactionThreadReport, delegateAccountID: undefined, + transactionViolations: undefined, }); const optimisticAction = findOptimisticModifiedExpense(); @@ -232,6 +289,7 @@ describe('updateMoneyRequestVendor', () => { transaction: transactionWithVendor, transactionThreadReport, delegateAccountID: undefined, + transactionViolations: undefined, }); const optimisticAction = findOptimisticModifiedExpense(); @@ -248,6 +306,7 @@ describe('updateMoneyRequestVendor', () => { vendorName: 'New Vendor', transaction: baseTransaction, delegateAccountID: undefined, + transactionViolations: undefined, }); const reportActionsUpdate = getRequiredOnyxUpdates(getOnyxDataArg(), 'optimisticData').some( diff --git a/tests/unit/Search/handleActionButtonPressTest.ts b/tests/unit/Search/handleActionButtonPressTest.ts index c168bfa7eb16..c987ad249f99 100644 --- a/tests/unit/Search/handleActionButtonPressTest.ts +++ b/tests/unit/Search/handleActionButtonPressTest.ts @@ -1,15 +1,18 @@ import type {TransactionReportGroupListItemType} from '@components/Search/SearchList/ListItem/types'; +import * as ReportWorkflow from '@libs/actions/IOU/ReportWorkflow'; import {handleActionButtonPress, handleBulkPayItemSelected} from '@libs/actions/Search'; import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute'; import Navigation from '@libs/Navigation/Navigation'; +// eslint-disable-next-line no-restricted-imports -- namespace import needed to spy on hasViolations in the approve-action test +import * as ReportUtils from '@libs/ReportUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES'; -import type {LastPaymentMethod, Policy, Report, SearchResults} from '@src/types/onyx'; +import type {LastPaymentMethod, Policy, Report, SearchResults, TransactionViolations} from '@src/types/onyx'; -import type {OnyxEntry} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; @@ -334,8 +337,8 @@ describe('handleActionButtonPress', () => { Onyx.merge(ONYXKEYS.NVP_LAST_PAYMENT_METHOD, mockLastPaymentMethod); }); - const snapshotReport = mockSnapshotForItem?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${mockReportItemWithHold.reportID}`] ?? {}; - const snapshotPolicy = mockSnapshotForItem?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${mockReportItemWithHold.policyID}`] ?? {}; + const snapshotReport = (mockSnapshotForItem?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${mockReportItemWithHold.reportID}`] ?? {}) as Report; + const snapshotPolicy = (mockSnapshotForItem?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${mockReportItemWithHold.policyID}`] ?? {}) as Policy; test('Should not navigate to item when report has one transaction on hold and action is approve', () => { const goToItem = jest.fn(() => {}); @@ -345,8 +348,8 @@ describe('handleActionButtonPress', () => { hash: searchHash, item: mockReportItemWithHold, goToItem, - snapshotReport: snapshotReport as Report, - snapshotPolicy: snapshotPolicy as Policy, + snapshotReport, + snapshotPolicy, submitterLogin: undefined, lastPaymentMethod: mockLastPaymentMethod, personalPolicyID: undefined, @@ -354,11 +357,12 @@ describe('handleActionButtonPress', () => { amountOwed: undefined, userBillingGracePeriodEnds: undefined, onHoldMenuOpen: jest.fn(), - policy: snapshotPolicy as Policy, + policy: snapshotPolicy, chatReportActions: undefined, currentUserAccountID: 1206, delegateAccountID: undefined, isTrackIntentUser: false, + allViolations: undefined, }); expect(goToItem).not.toHaveBeenCalled(); }); @@ -371,8 +375,8 @@ describe('handleActionButtonPress', () => { hash: searchHash, item: mockReportItemWithHold, goToItem: jest.fn(), - snapshotReport: snapshotReport as Report, - snapshotPolicy: snapshotPolicy as Policy, + snapshotReport, + snapshotPolicy, submitterLogin: undefined, lastPaymentMethod: mockLastPaymentMethod, personalPolicyID: undefined, @@ -380,11 +384,12 @@ describe('handleActionButtonPress', () => { ownerBillingGracePeriodEnd: undefined, amountOwed: undefined, onHoldMenuOpen, - policy: snapshotPolicy as Policy, + policy: snapshotPolicy, chatReportActions: undefined, currentUserAccountID: 1206, delegateAccountID: undefined, isTrackIntentUser: false, + allViolations: undefined, }); expect(onHoldMenuOpen).toHaveBeenCalledWith(mockReportItemWithHold, CONST.IOU.REPORT_ACTION_TYPE.APPROVE); @@ -398,22 +403,65 @@ describe('handleActionButtonPress', () => { hash: searchHash, item: updatedMockReportItem, goToItem, - snapshotReport: snapshotReport as Report, - snapshotPolicy: snapshotPolicy as Policy, + snapshotReport, + snapshotPolicy, submitterLogin: undefined, lastPaymentMethod: mockLastPaymentMethod, personalPolicyID: undefined, ownerBillingGracePeriodEnd: undefined, amountOwed: undefined, userBillingGracePeriodEnds: undefined, - policy: snapshotPolicy as Policy, + policy: snapshotPolicy, chatReportActions: undefined, currentUserAccountID: 1206, delegateAccountID: undefined, isTrackIntentUser: false, + allViolations: undefined, }); expect(goToItem).toHaveBeenCalledTimes(0); }); + + test('Should compute hasViolations from the passed allViolations param (not the global Onyx collection) and forward it to approveMoneyRequest', () => { + // Given: a report item with no held expenses so the approve action reaches getApproveActionCallback, + // and a violations collection passed explicitly through the params. + const allViolations: OnyxCollection = { + [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}1049531721038862176`]: [{name: CONST.VIOLATIONS.MISSING_CATEGORY, type: CONST.VIOLATION_TYPES.VIOLATION}], + }; + + const hasViolationsMock = jest.spyOn(ReportUtils, 'hasViolations').mockReturnValue(true); + const approveMoneyRequestMock = jest.spyOn(ReportWorkflow, 'approveMoneyRequest').mockImplementation(jest.fn()); + + // When: the approve action button is pressed + handleActionButtonPress({ + hash: searchHash, + item: updatedMockReportItem, + goToItem: jest.fn(), + snapshotReport, + snapshotPolicy, + submitterLogin: undefined, + lastPaymentMethod: mockLastPaymentMethod, + personalPolicyID: undefined, + ownerBillingGracePeriodEnd: undefined, + amountOwed: undefined, + userBillingGracePeriodEnds: undefined, + policy: snapshotPolicy, + chatReportActions: undefined, + currentUserAccountID: 1206, + delegateAccountID: undefined, + isTrackIntentUser: false, + allViolations, + conciergeChat: undefined, + getCurrencyDecimals: getCurrencyDecimalsLocal, + }); + + // Then: hasViolations is evaluated against the passed collection, proving the deprecated global getter is no longer used, + // and the resulting value is forwarded to approveMoneyRequest. + expect(hasViolationsMock).toHaveBeenCalledWith(updatedMockReportItem.reportID, allViolations, 1206, ''); + expect(approveMoneyRequestMock).toHaveBeenCalledWith(expect.objectContaining({hasViolations: true})); + + hasViolationsMock.mockRestore(); + approveMoneyRequestMock.mockRestore(); + }); }); describe('handleBulkPayItemSelected', () => {