diff --git a/src/components/MoneyReportHeaderPrimaryAction/MarkAsResolvedPrimaryAction.tsx b/src/components/MoneyReportHeaderPrimaryAction/MarkAsResolvedPrimaryAction.tsx index 31094d5b90a1..b980f1c23176 100644 --- a/src/components/MoneyReportHeaderPrimaryAction/MarkAsResolvedPrimaryAction.tsx +++ b/src/components/MoneyReportHeaderPrimaryAction/MarkAsResolvedPrimaryAction.tsx @@ -2,9 +2,14 @@ import Button from '@components/Button'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; +import useOnyx from '@hooks/useOnyx'; + +import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import {markRejectViolationAsResolved} from '@userActions/IOU/RejectMoneyRequest'; +import ONYXKEYS from '@src/ONYXKEYS'; + import React from 'react'; import type {SimpleActionProps} from './types'; @@ -14,6 +19,7 @@ import useTransactionThreadData from './useTransactionThreadData'; function MarkAsResolvedPrimaryAction({reportID, chatReportID}: SimpleActionProps) { const {translate} = useLocalize(); const {transaction, transactionThreadReport} = useTransactionThreadData(reportID, chatReportID); + const [transactionViolations] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${getNonEmptyStringOnyxID(transaction?.transactionID)}`); const {isOffline} = useNetwork(); @@ -24,7 +30,7 @@ function MarkAsResolvedPrimaryAction({reportID, chatReportID}: SimpleActionProps if (!transaction?.transactionID) { return; } - markRejectViolationAsResolved(transaction.transactionID, isOffline, transactionThreadReport?.reportID); + markRejectViolationAsResolved(transaction.transactionID, isOffline, transactionViolations, transactionThreadReport?.reportID); }} text={translate('iou.reject.markAsResolved')} /> diff --git a/src/components/MoneyRequestHeaderPrimaryAction.tsx b/src/components/MoneyRequestHeaderPrimaryAction.tsx index 9d14567b94ff..edc00c3fb1ef 100644 --- a/src/components/MoneyRequestHeaderPrimaryAction.tsx +++ b/src/components/MoneyRequestHeaderPrimaryAction.tsx @@ -109,7 +109,7 @@ function MoneyRequestHeaderPrimaryAction({reportID}: MoneyRequestHeaderPrimaryAc if (!transaction?.transactionID) { return; } - markRejectViolationAsResolved(transaction.transactionID, isOffline, reportID); + markRejectViolationAsResolved(transaction.transactionID, isOffline, rawTransactionViolations, reportID); }} /> ); diff --git a/src/libs/actions/IOU/RejectMoneyRequest.ts b/src/libs/actions/IOU/RejectMoneyRequest.ts index e97cf5e97baf..d901c15c97be 100644 --- a/src/libs/actions/IOU/RejectMoneyRequest.ts +++ b/src/libs/actions/IOU/RejectMoneyRequest.ts @@ -933,16 +933,12 @@ function rejectMoneyRequest( return urlToNavigateBack; } -function markRejectViolationAsResolved(transactionID: string, isOffline: boolean, reportID?: string) { +function markRejectViolationAsResolved(transactionID: string, isOffline: boolean, transactionViolations: OnyxEntry, reportID?: string) { if (!reportID) { return; } - // TODO: https://github.com/Expensify/App/issues/66512 - // eslint-disable-next-line @typescript-eslint/no-deprecated - const allTransactionViolations = getAllTransactionViolations(); - - const currentViolations = allTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`]; + const currentViolations = transactionViolations; const updatedViolations = currentViolations?.filter((violation) => violation.name !== CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE); const optimisticMarkedAsResolvedReportAction = buildOptimisticMarkedAsResolvedReportAction(); diff --git a/tests/actions/IOUTest/RejectMoneyRequestTest.ts b/tests/actions/IOUTest/RejectMoneyRequestTest.ts index dd1bc9497b4c..2ff12d775190 100644 --- a/tests/actions/IOUTest/RejectMoneyRequestTest.ts +++ b/tests/actions/IOUTest/RejectMoneyRequestTest.ts @@ -425,19 +425,21 @@ describe('actions/IOU/RejectMoneyRequest', () => { let transaction: OnyxEntry; let iouReport: OnyxEntry; + const rejectViolations = [ + { + name: CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE, + type: CONST.VIOLATION_TYPES.WARNING, + data: {comment: 'Test reject reason'}, + }, + ]; + beforeEach(async () => { transaction = createRandomTransaction(1); iouReport = createRandomReport(1, undefined); await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction?.transactionID}`, transaction); await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${iouReport?.reportID}`, iouReport); - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction?.transactionID}`, [ - { - name: CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE, - type: CONST.VIOLATION_TYPES.WARNING, - data: {comment: 'Test reject reason'}, - }, - ]); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction?.transactionID}`, rejectViolations); await waitForBatchedUpdates(); }); @@ -451,7 +453,7 @@ describe('actions/IOU/RejectMoneyRequest', () => { if (!transaction?.transactionID || !iouReport?.reportID) { throw new Error('Required transaction or report data is missing'); } - markRejectViolationAsResolved(transaction.transactionID, false, iouReport.reportID); + markRejectViolationAsResolved(transaction.transactionID, false, rejectViolations, iouReport.reportID); await waitForBatchedUpdates(); // Then: Verify violation is removed @@ -470,7 +472,7 @@ describe('actions/IOU/RejectMoneyRequest', () => { if (!transaction?.transactionID || !iouReport?.reportID) { throw new Error('Required transaction or report data is missing'); } - markRejectViolationAsResolved(transaction.transactionID, true, iouReport.reportID); + markRejectViolationAsResolved(transaction.transactionID, true, rejectViolations, iouReport.reportID); await waitForBatchedUpdates(); // Then: Verify violation is removed @@ -493,7 +495,7 @@ describe('actions/IOU/RejectMoneyRequest', () => { } // When: Mark violation as resolved - markRejectViolationAsResolved(transaction.transactionID, false, iouReport.reportID); + markRejectViolationAsResolved(transaction.transactionID, false, rejectViolations, iouReport.reportID); await waitForBatchedUpdates(); // Then: API.write should be called with the correct command and transactionID @@ -515,7 +517,7 @@ describe('actions/IOU/RejectMoneyRequest', () => { } // When: Mark violation as resolved while online - markRejectViolationAsResolved(transaction.transactionID, false, iouReport.reportID); + markRejectViolationAsResolved(transaction.transactionID, false, rejectViolations, iouReport.reportID); await waitForBatchedUpdates(); // Then: notifyNewAction should be called @@ -533,7 +535,7 @@ describe('actions/IOU/RejectMoneyRequest', () => { } // When: Mark violation as resolved without reportID - markRejectViolationAsResolved(transaction.transactionID, false, undefined); + markRejectViolationAsResolved(transaction.transactionID, false, rejectViolations, undefined); await waitForBatchedUpdates(); // Then: API.write should not be called @@ -544,5 +546,45 @@ describe('actions/IOU/RejectMoneyRequest', () => { writeSpy.mockRestore(); }); + + it('uses the passed transactionViolations parameter instead of the global Onyx collection', async () => { + // eslint-disable-next-line rulesdir/no-multiple-api-calls + const writeSpy = jest.spyOn(API, 'write').mockImplementation(jest.fn()); + + if (!transaction?.transactionID || !iouReport?.reportID) { + throw new Error('Required transaction or report data is missing'); + } + + // Given: Onyx holds an empty violation set, different from what is passed in, + // to prove the function relies on the parameter and not the global collection. + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`, []); + await waitForBatchedUpdates(); + + const passedViolations = [ + {name: CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE, type: CONST.VIOLATION_TYPES.WARNING, data: {comment: 'Test reject reason'}}, + {name: CONST.VIOLATIONS.MISSING_CATEGORY, type: CONST.VIOLATION_TYPES.VIOLATION}, + ]; + + // When: Mark violation as resolved with the passed violations + markRejectViolationAsResolved(transaction.transactionID, false, passedViolations, iouReport.reportID); + await waitForBatchedUpdates(); + + // Then: the optimistic update removes only AUTO_REPORTED_REJECTED_EXPENSE and keeps the other + // violation from the parameter — proving the parameter (not the empty Onyx value) was used. + expect(writeSpy).toHaveBeenCalledWith( + WRITE_COMMANDS.MARK_TRANSACTION_VIOLATION_AS_RESOLVED, + expect.anything(), + expect.objectContaining({ + optimisticData: expect.arrayContaining([ + expect.objectContaining({ + key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`, + value: [{name: CONST.VIOLATIONS.MISSING_CATEGORY, type: CONST.VIOLATION_TYPES.VIOLATION}], + }), + ]), + }), + ); + + writeSpy.mockRestore(); + }); }); });