Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,10 @@ function ExpenseReportListItemInner<TItem extends ListItem>({
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,
});
}, [
currentSearchHash,
Expand Down Expand Up @@ -339,6 +343,7 @@ function ExpenseReportListItemInner<TItem extends ListItem>({
delegateEmail,
delegateAccountID,
isTrackIntentUser,
liveViolationsForSnapshotTransactions,
]);

const handleSelectionButtonPress = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,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
Expand Down Expand Up @@ -281,6 +282,9 @@ function ReportListItemHeaderInner<TItem extends ListItem>({
);
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, nextStep, chatReportPolicy, amountOwed, delegateEmail, delegateAccountID} =
useReportPaymentContext({
reportID: reportItem.reportID,
Expand Down Expand Up @@ -332,6 +336,7 @@ function ReportListItemHeaderInner<TItem extends ListItem>({
delegateEmail,
delegateAccountID,
isTrackIntentUser,
allViolations,
});
};
return !isLargeScreenWidth ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ function TransactionListItemInner<TItem extends ListItem>({
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 allViolations = {[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionItem.transactionID}`]: transactionViolationsForRow};
const parentReportActionID = transactionItem?.reportAction?.reportActionID;
const [parentReportAction] = originalUseOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(transactionItem.reportID)}`, {
selector: (reportActions: OnyxEntry<ReportActions>): OnyxEntry<ReportAction> => reportActions?.[`${parentReportActionID}`],
Expand Down Expand Up @@ -233,6 +234,7 @@ function TransactionListItemInner<TItem extends ListItem>({
delegateEmail,
delegateAccountID,
isTrackIntentUser,
allViolations,
});
};

Expand Down
16 changes: 12 additions & 4 deletions src/libs/actions/IOU/UpdateMoneyRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ type UpdateMoneyRequestVendorParams = {
parentReport?: OnyxEntry<OnyxTypes.Report>;
policy?: OnyxEntry<OnyxTypes.Policy>;
delegateAccountID: number | undefined;
transactionViolations: OnyxEntry<OnyxTypes.TransactionViolations>;
};

/**
Expand All @@ -608,7 +609,16 @@ type UpdateMoneyRequestVendorParams = {
*
* Passing `vendorID=''` clears the vendor from the transaction.
*/
function updateMoneyRequestVendor({transactionID, vendorID, transaction, transactionThreadReport, parentReport, policy, delegateAccountID}: UpdateMoneyRequestVendorParams) {
function updateMoneyRequestVendor({
transactionID,
vendorID,
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}`];
Expand Down Expand Up @@ -736,9 +746,7 @@ function updateMoneyRequestVendor({transactionID, vendorID, transaction, transac
// 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,
Expand Down
10 changes: 7 additions & 3 deletions src/libs/actions/Search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ import type {
ReportNextStepDeprecated,
SaveSearch,
Transaction,
TransactionViolations,
} from '@src/types/onyx';
import type {PaymentInformation} from '@src/types/onyx/LastPaymentMethod';
import type {ConnectionName} from '@src/types/onyx/Policy';
Expand All @@ -94,7 +95,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';
Expand Down Expand Up @@ -229,6 +229,7 @@ type HandleActionButtonPressParams = {
delegateEmail?: string;
delegateAccountID: number | undefined;
isTrackIntentUser: boolean | undefined;
allViolations: OnyxCollection<TransactionViolations>;
};

function handleActionButtonPress({
Expand Down Expand Up @@ -267,6 +268,7 @@ function handleActionButtonPress({
delegateEmail,
delegateAccountID,
isTrackIntentUser,
allViolations,
}: HandleActionButtonPressParams) {
// The transactionIDList is needed to handle actions taken on `status:""` where transactions on single expense reports can be approved/paid.
// We need the transactionID to display the loading indicator for that list item's action.
Expand Down Expand Up @@ -351,6 +353,7 @@ function handleActionButtonPress({
delegateEmail,
isTrackIntentUser,
ownerLogin: submitterLogin,
allViolations,
});
return;
case CONST.SEARCH.ACTION_TYPES.SUBMIT: {
Expand Down Expand Up @@ -623,6 +626,7 @@ type GetApproveActionCallbackParams = {
delegateEmail?: string;
isTrackIntentUser: boolean | undefined;
ownerLogin: string | undefined;
allViolations: OnyxCollection<TransactionViolations>;
};

function getApproveActionCallback({
Expand All @@ -642,14 +646,14 @@ function getApproveActionCallback({
delegateEmail,
isTrackIntentUser,
ownerLogin,
allViolations,
}: GetApproveActionCallbackParams) {
if (!item.reportID) {
return;
}

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({
Expand Down
2 changes: 2 additions & 0 deletions src/pages/iou/request/step/IOURequestStepVendor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,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));
Expand Down Expand Up @@ -114,6 +115,7 @@ function IOURequestStepVendor({
parentReport,
policy,
delegateAccountID,
transactionViolations,
});
}
navigateBack();
Expand Down
65 changes: 43 additions & 22 deletions tests/actions/IOUTest/UpdateMoneyRequestVendorTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,48 +59,65 @@ describe('updateMoneyRequestVendor', () => {
return firstCall?.at(2) as OnyxDataArg | undefined;
};

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', 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',
transaction: baseTransaction,
delegateAccountID: undefined,
transactionViolations: [otherViolation, inactiveVendorViolation],
});

const onyxData = getOnyxDataArg();
const violationsUpdate = onyxData?.optimisticData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`);
expect(violationsUpdate).toBeDefined();
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: '', transaction: baseTransaction, delegateAccountID: undefined});
it('clears an existing inactive-vendor violation optimistically when the vendor is cleared', () => {
updateMoneyRequestVendor({
transactionID: TRANSACTION_ID,
vendorID: '',
transaction: baseTransaction,
delegateAccountID: undefined,
transactionViolations: [inactiveVendorViolation],
});

const onyxData = getOnyxDataArg();
const violationsUpdate = onyxData?.optimisticData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`);
expect(violationsUpdate).toBeDefined();
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', transaction: baseTransaction, delegateAccountID: undefined});
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-active', transaction: baseTransaction, delegateAccountID: undefined, transactionViolations: original});

const onyxData = getOnyxDataArg();
const failureViolations = onyxData?.failureData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`);
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', transaction: baseTransaction, delegateAccountID: undefined, transactionViolations: [otherViolation]});

const onyxData = getOnyxDataArg();
const violationsUpdate = onyxData?.optimisticData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`);
expect(violationsUpdate).toBeUndefined();
});

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', transaction: baseTransaction, delegateAccountID: undefined});
// When: a vendor is picked while passing an empty violations parameter
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-active', transaction: baseTransaction, delegateAccountID: undefined, transactionViolations: []});

// Then: no violations update is written, proving the parameter (not the global collection) drives the logic.
const onyxData = getOnyxDataArg();
const violationsUpdate = onyxData?.optimisticData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`);
expect(violationsUpdate).toBeUndefined();
Expand All @@ -114,7 +131,7 @@ describe('updateMoneyRequestVendor', () => {
});
await waitForBatchedUpdates();

updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', delegateAccountID: undefined});
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', delegateAccountID: undefined, transactionViolations: undefined});

const onyxData = getOnyxDataArg();
const transactionFailure = onyxData?.failureData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
Expand All @@ -128,15 +145,15 @@ 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', delegateAccountID: undefined});
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', delegateAccountID: undefined, transactionViolations: undefined});

const onyxData = getOnyxDataArg();
const transactionFailure = onyxData?.failureData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
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', transaction: baseTransaction, delegateAccountID: undefined});
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', transaction: baseTransaction, delegateAccountID: undefined, transactionViolations: undefined});

const onyxData = getOnyxDataArg();
const transactionOptimistic = onyxData?.optimisticData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
Expand All @@ -147,7 +164,7 @@ describe('updateMoneyRequestVendor', () => {
});

it('clears pendingFields.vendor in successData when the server confirms the write', () => {
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', transaction: baseTransaction, delegateAccountID: undefined});
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', transaction: baseTransaction, delegateAccountID: undefined, transactionViolations: undefined});

const onyxData = getOnyxDataArg();
const transactionSuccess = onyxData?.successData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
Expand All @@ -157,7 +174,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', delegateAccountID: undefined});
updateMoneyRequestVendor({transactionID: TRANSACTION_ID, vendorID: 'v-new', delegateAccountID: undefined, transactionViolations: undefined});

const onyxData = getOnyxDataArg();
const transactionFailure = onyxData?.failureData.find((entry) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
Expand Down Expand Up @@ -193,6 +210,7 @@ describe('updateMoneyRequestVendor', () => {
transaction: baseTransaction,
transactionThreadReport,
delegateAccountID: undefined,
transactionViolations: undefined,
});

const optimisticAction = findOptimisticModifiedExpense();
Expand All @@ -218,6 +236,7 @@ describe('updateMoneyRequestVendor', () => {
transaction: transactionWithVendor,
transactionThreadReport,
delegateAccountID: undefined,
transactionViolations: undefined,
});

const optimisticAction = findOptimisticModifiedExpense();
Expand All @@ -240,6 +259,7 @@ describe('updateMoneyRequestVendor', () => {
transaction: transactionWithVendor,
transactionThreadReport,
delegateAccountID: undefined,
transactionViolations: undefined,
});

const optimisticAction = findOptimisticModifiedExpense();
Expand All @@ -255,6 +275,7 @@ describe('updateMoneyRequestVendor', () => {
vendorID: 'v-new',
transaction: baseTransaction,
delegateAccountID: undefined,
transactionViolations: undefined,
});

const onyxData = getOnyxDataArg();
Expand Down
Loading
Loading