From b9f0d7fd992a40b7616bc81bbad15a1029fc088f Mon Sep 17 00:00:00 2001 From: Linh Date: Thu, 27 Aug 2026 18:14:53 +0700 Subject: [PATCH 1/5] Remove getReportTransactions callers' reliance on the deprecated default (Part 2) canApprove (ReportPreviewActionUtils.ts) is unexported and its one caller already always supplies transactions, so drop the deprecated-default fallback and make the param required. hasDuplicateTransactions (TransactionUtils.ts) gains a required reportTransactions param, sourced from data its one caller (a hook) already had via useTransactionsAndViolationsForReport. --- src/hooks/useMoneyReportHeaderStatusBar.ts | 2 +- src/libs/ReportPreviewActionUtils.ts | 10 ++++------ src/libs/TransactionUtils/index.ts | 4 +--- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/hooks/useMoneyReportHeaderStatusBar.ts b/src/hooks/useMoneyReportHeaderStatusBar.ts index 7c8842c93ad2..2bb170bb8895 100644 --- a/src/hooks/useMoneyReportHeaderStatusBar.ts +++ b/src/hooks/useMoneyReportHeaderStatusBar.ts @@ -91,7 +91,7 @@ function useMoneyReportHeaderStatusBar(reportID: string | undefined, chatReportI const hasOnlyHeldExpenses = hasOnlyHeldExpensesReportUtils(transactions); const isPayAtEndExpense = isPayAtEndExpenseTransactionUtils(transaction); const isReportSettled = isSettledReportUtils(moneyRequestReport); - const hasDuplicates = !isReportSettled && hasDuplicateTransactions(email ?? '', accountID, moneyRequestReport, ownerLogin, policy, allTransactionViolations); + const hasDuplicates = !isReportSettled && hasDuplicateTransactions(email ?? '', accountID, moneyRequestReport, ownerLogin, policy, allTransactionViolations, transactions); const shouldShowMarkAsResolved = isMarkAsResolvedAction(moneyRequestReport, transactionViolations); const shouldShowStatusBar = diff --git a/src/libs/ReportPreviewActionUtils.ts b/src/libs/ReportPreviewActionUtils.ts index 321e5a77299a..e7ef0107e36a 100644 --- a/src/libs/ReportPreviewActionUtils.ts +++ b/src/libs/ReportPreviewActionUtils.ts @@ -20,7 +20,6 @@ import {isAddExpenseAction} from './ReportPrimaryActionUtils'; import { getMoneyRequestSpendBreakdown, getParentReport, - getReportTransactions, hasExportError as hasExportErrorUtil, hasOnlyNonReimbursableTransactions, isClosedReport, @@ -77,7 +76,7 @@ function canSubmit( return isExpense && isSubmitter && isOpen && !isAnyReceiptBeingScanned && !!transactions && transactions.length > 0; } -function canApprove(report: Report, currentUserAccountID: number, reportMetadata: OnyxEntry, policy?: Policy, transactions?: Transaction[]) { +function canApprove(report: Report, currentUserAccountID: number, reportMetadata: OnyxEntry, policy: Policy | undefined, transactions: Transaction[]) { if (isSubmitterApproveBlockedOnSubmitWorkspace(policy, report.ownerAccountID, currentUserAccountID)) { return false; } @@ -87,14 +86,13 @@ function canApprove(report: Report, currentUserAccountID: number, reportMetadata const isApprovalEnabled = policy?.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL; const managerID = report.managerID ?? CONST.DEFAULT_NUMBER_ID; const isCurrentUserManager = managerID === currentUserAccountID; - const reportTransactions = transactions ?? getReportTransactions(report?.reportID); - const isAnyReceiptBeingScanned = transactions?.some((transaction) => isScanning(transaction)); + const isAnyReceiptBeingScanned = transactions.some((transaction) => isScanning(transaction)); if (isAnyReceiptBeingScanned) { return false; } - if (reportTransactions.length > 0 && reportTransactions.every((transaction) => isPending(transaction))) { + if (transactions.length > 0 && transactions.every((transaction) => isPending(transaction))) { return false; } @@ -110,7 +108,7 @@ function canApprove(report: Report, currentUserAccountID: number, reportMetadata return false; } - return isExpense && isProcessing && !!isApprovalEnabled && reportTransactions.length > 0 && isCurrentUserManager; + return isExpense && isProcessing && !!isApprovalEnabled && transactions.length > 0 && isCurrentUserManager; } function canPay( diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 984891ffa3dd..fe041e98190b 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2391,10 +2391,8 @@ function hasDuplicateTransactions( ownerLogin: string | undefined, policy: OnyxEntry, allTransactionViolations: OnyxCollection, + reportTransactions: Transaction[], ): boolean { - const transactionsByIouReportID = getReportTransactions(iouReport?.reportID); - const reportTransactions = transactionsByIouReportID; - return ( reportTransactions.length > 0 && reportTransactions.some((transaction) => From 2dc118cd129b8d3aed3fbe882a26e49af8f729cd Mon Sep 17 00:00:00 2001 From: Linh Date: Fri, 4 Sep 2026 16:09:25 +0700 Subject: [PATCH 2/5] Add unit test coverage for useMoneyReportHeaderStatusBar's hasDuplicateTransactions call Codecov flagged 0% patch coverage on PR #99656 for the line threading transactions into hasDuplicateTransactions. This hook had no test exercising its real implementation before now. --- .../useMoneyReportHeaderStatusBarTest.ts | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts diff --git a/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts new file mode 100644 index 000000000000..2f5889839774 --- /dev/null +++ b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts @@ -0,0 +1,120 @@ +import {renderHook} from '@testing-library/react-native'; + +import useMoneyReportHeaderStatusBar from '@hooks/useMoneyReportHeaderStatusBar'; + +import * as ReportActionsUtils from '@libs/ReportActionsUtils'; +import * as ReportPrimaryActionUtils from '@libs/ReportPrimaryActionUtils'; +import * as ReportUtils from '@libs/ReportUtils'; +import * as TransactionUtils from '@libs/TransactionUtils'; + +import CONST from '@src/CONST'; +import type {Report, Transaction} from '@src/types/onyx'; + +import createMock from '../../utils/createMock'; + +const REPORT_ID = 'report1'; +const CHAT_REPORT_ID = 'chatReport1'; + +// Prefixed with `mock` so they can be referenced inside the hoisted jest.mock factory below. +const mockTransaction1 = createMock({transactionID: 'transaction1', reportID: REPORT_ID}); +const mockTransaction2 = createMock({transactionID: 'transaction2', reportID: REPORT_ID}); +const mockMoneyRequestReport = createMock({reportID: REPORT_ID, type: 'iou'}); + +jest.mock('@hooks/useNetwork', () => ({ + __esModule: true, + default: () => ({isOffline: false}), +})); + +jest.mock('@hooks/useCurrentUserPersonalDetails', () => ({ + __esModule: true, + default: () => ({accountID: 1, email: 'test@example.com'}), +})); + +jest.mock('@hooks/usePaginatedReportActions', () => ({ + __esModule: true, + default: () => ({reportActions: []}), +})); + +jest.mock('@hooks/useReportTransactionsCollection', () => ({ + __esModule: true, + default: () => ({}), +})); + +jest.mock('@hooks/useTransactionViolations', () => ({ + __esModule: true, + default: () => [], +})); + +jest.mock('@hooks/useReportIsArchived', () => ({ + __esModule: true, + default: () => false, +})); + +jest.mock('@hooks/useTransactionsAndViolationsForReport', () => ({ + __esModule: true, + default: () => ({transactions: {transaction1: mockTransaction1, transaction2: mockTransaction2}, violations: {}}), +})); + +jest.mock('@hooks/useOnyx', () => ({ + __esModule: true, + default: (key: string) => { + if (key === `report_${REPORT_ID}`) { + return [mockMoneyRequestReport]; + } + return [undefined]; + }, +})); + +describe('useMoneyReportHeaderStatusBar - duplicate transactions', () => { + beforeEach(() => { + // Every other status-bar condition is stubbed to false so the test can isolate the hasDuplicates branch, + // which is the line this hook's PR changed (threading `transactions` into hasDuplicateTransactions + // instead of it recomputing them via the deprecated getReportTransactions default). + jest.spyOn(ReportActionsUtils, 'getFilteredReportActionsForReportView').mockReturnValue([]); + jest.spyOn(ReportActionsUtils, 'getOneTransactionThreadReportID').mockReturnValue(undefined); + jest.spyOn(ReportActionsUtils, 'getOriginalMessage').mockReturnValue(undefined); + jest.spyOn(ReportActionsUtils, 'isMoneyRequestAction').mockReturnValue(false); + jest.spyOn(ReportPrimaryActionUtils, 'isMarkAsResolvedAction').mockReturnValue(false); + jest.spyOn(ReportUtils, 'hasOnlyHeldExpenses').mockReturnValue(false); + jest.spyOn(ReportUtils, 'isSettled').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'allHavePendingRTERViolation').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'hasDuplicateTransactions').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'isBrokenConnectionViolation').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'hasReceipt').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'isPayAtEndExpense').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'isPending').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'isScanning').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'shouldSuppressBrokenConnectionStatus').mockReturnValue(false); + jest.spyOn(TransactionUtils, 'shouldShowBrokenConnectionViolationForMultipleTransactions').mockReturnValue(false); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('passes the report transactions through to hasDuplicateTransactions', () => { + renderHook(() => useMoneyReportHeaderStatusBar(REPORT_ID, CHAT_REPORT_ID)); + + const passedTransactions = jest.mocked(TransactionUtils.hasDuplicateTransactions).mock.calls[0]?.[6]; + expect(passedTransactions).toHaveLength(2); + expect(passedTransactions?.map((transaction) => transaction.transactionID)).toEqual(expect.arrayContaining(['transaction1', 'transaction2'])); + }); + + it('shows the duplicates status bar when hasDuplicateTransactions reports duplicates on an unsettled report', () => { + jest.spyOn(TransactionUtils, 'hasDuplicateTransactions').mockReturnValue(true); + + const {result} = renderHook(() => useMoneyReportHeaderStatusBar(REPORT_ID, CHAT_REPORT_ID)); + + expect(result.current.shouldShowStatusBar).toBe(true); + expect(result.current.statusBarType).toBe(CONST.REPORT.STATUS_BAR_TYPE.DUPLICATES); + }); + + it('does not show the duplicates status bar when the report is already settled, even if duplicates are found', () => { + jest.spyOn(TransactionUtils, 'hasDuplicateTransactions').mockReturnValue(true); + jest.spyOn(ReportUtils, 'isSettled').mockReturnValue(true); + + const {result} = renderHook(() => useMoneyReportHeaderStatusBar(REPORT_ID, CHAT_REPORT_ID)); + + expect(result.current.statusBarType).not.toBe(CONST.REPORT.STATUS_BAR_TYPE.DUPLICATES); + }); +}); From 707fc157ca8ba7e0b0a72ad2935f6a5504406993 Mon Sep 17 00:00:00 2001 From: Linh Date: Fri, 4 Sep 2026 16:24:33 +0700 Subject: [PATCH 3/5] Fix ESLint failures in useMoneyReportHeaderStatusBar test Use require() instead of a static namespace import for ReportUtils spies, since a wildcard import can't be statically proven not to touch the restricted isPaidGroupPolicy/isPaidGroupPolicyExpenseReport exports. Also switch to .at() for array element access per the project's array-access rule. --- .../unit/hooks/useMoneyReportHeaderStatusBarTest.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts index 2f5889839774..291821c2504d 100644 --- a/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts +++ b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts @@ -4,7 +4,6 @@ import useMoneyReportHeaderStatusBar from '@hooks/useMoneyReportHeaderStatusBar' import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportPrimaryActionUtils from '@libs/ReportPrimaryActionUtils'; -import * as ReportUtils from '@libs/ReportUtils'; import * as TransactionUtils from '@libs/TransactionUtils'; import CONST from '@src/CONST'; @@ -75,8 +74,11 @@ describe('useMoneyReportHeaderStatusBar - duplicate transactions', () => { jest.spyOn(ReportActionsUtils, 'getOriginalMessage').mockReturnValue(undefined); jest.spyOn(ReportActionsUtils, 'isMoneyRequestAction').mockReturnValue(false); jest.spyOn(ReportPrimaryActionUtils, 'isMarkAsResolvedAction').mockReturnValue(false); - jest.spyOn(ReportUtils, 'hasOnlyHeldExpenses').mockReturnValue(false); - jest.spyOn(ReportUtils, 'isSettled').mockReturnValue(false); + // isPaidGroupPolicy/isPaidGroupPolicyExpenseReport are billing-only and restricted from static import; + // this hook never touches them, but a namespace import can't be statically proven not to, so these + // ReportUtils spies go through require() instead (matches other tests spying on this module). + jest.spyOn(require('@libs/ReportUtils'), 'hasOnlyHeldExpenses').mockReturnValue(false); + jest.spyOn(require('@libs/ReportUtils'), 'isSettled').mockReturnValue(false); jest.spyOn(TransactionUtils, 'allHavePendingRTERViolation').mockReturnValue(false); jest.spyOn(TransactionUtils, 'hasDuplicateTransactions').mockReturnValue(false); jest.spyOn(TransactionUtils, 'isBrokenConnectionViolation').mockReturnValue(false); @@ -95,7 +97,7 @@ describe('useMoneyReportHeaderStatusBar - duplicate transactions', () => { it('passes the report transactions through to hasDuplicateTransactions', () => { renderHook(() => useMoneyReportHeaderStatusBar(REPORT_ID, CHAT_REPORT_ID)); - const passedTransactions = jest.mocked(TransactionUtils.hasDuplicateTransactions).mock.calls[0]?.[6]; + const passedTransactions = jest.mocked(TransactionUtils.hasDuplicateTransactions).mock.calls.at(0)?.at(6) as Transaction[] | undefined; expect(passedTransactions).toHaveLength(2); expect(passedTransactions?.map((transaction) => transaction.transactionID)).toEqual(expect.arrayContaining(['transaction1', 'transaction2'])); }); @@ -111,7 +113,7 @@ describe('useMoneyReportHeaderStatusBar - duplicate transactions', () => { it('does not show the duplicates status bar when the report is already settled, even if duplicates are found', () => { jest.spyOn(TransactionUtils, 'hasDuplicateTransactions').mockReturnValue(true); - jest.spyOn(ReportUtils, 'isSettled').mockReturnValue(true); + jest.spyOn(require('@libs/ReportUtils'), 'isSettled').mockReturnValue(true); const {result} = renderHook(() => useMoneyReportHeaderStatusBar(REPORT_ID, CHAT_REPORT_ID)); From 95580173915fe34c01615fa8b19035440e8f7967 Mon Sep 17 00:00:00 2001 From: Linh Date: Fri, 4 Sep 2026 16:32:50 +0700 Subject: [PATCH 4/5] Avoid unsafe type assertion in useMoneyReportHeaderStatusBar test Array.prototype.at() on a tuple widens to the union of all parameter types, so casting its result back down tripped @typescript-eslint/no-unsafe-type-assertion. Assert the full call args via toHaveBeenCalledWith instead of indexing into mock.calls. --- tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts index 291821c2504d..efefe5c357f8 100644 --- a/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts +++ b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts @@ -97,9 +97,10 @@ describe('useMoneyReportHeaderStatusBar - duplicate transactions', () => { it('passes the report transactions through to hasDuplicateTransactions', () => { renderHook(() => useMoneyReportHeaderStatusBar(REPORT_ID, CHAT_REPORT_ID)); - const passedTransactions = jest.mocked(TransactionUtils.hasDuplicateTransactions).mock.calls.at(0)?.at(6) as Transaction[] | undefined; - expect(passedTransactions).toHaveLength(2); - expect(passedTransactions?.map((transaction) => transaction.transactionID)).toEqual(expect.arrayContaining(['transaction1', 'transaction2'])); + expect(jest.mocked(TransactionUtils.hasDuplicateTransactions)).toHaveBeenCalledWith('test@example.com', 1, mockMoneyRequestReport, undefined, undefined, undefined, [ + mockTransaction1, + mockTransaction2, + ]); }); it('shows the duplicates status bar when hasDuplicateTransactions reports duplicates on an unsettled report', () => { From 215192d8aeabe2da65e55b5cb9f980640676ebe5 Mon Sep 17 00:00:00 2001 From: Linh Date: Fri, 4 Sep 2026 17:00:22 +0700 Subject: [PATCH 5/5] Split semicolon-joined comment into separate sentences Addresses CONSISTENCY-16 review bot feedback on PR #99656. --- tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts index efefe5c357f8..2a57a05b8897 100644 --- a/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts +++ b/tests/unit/hooks/useMoneyReportHeaderStatusBarTest.ts @@ -74,8 +74,8 @@ describe('useMoneyReportHeaderStatusBar - duplicate transactions', () => { jest.spyOn(ReportActionsUtils, 'getOriginalMessage').mockReturnValue(undefined); jest.spyOn(ReportActionsUtils, 'isMoneyRequestAction').mockReturnValue(false); jest.spyOn(ReportPrimaryActionUtils, 'isMarkAsResolvedAction').mockReturnValue(false); - // isPaidGroupPolicy/isPaidGroupPolicyExpenseReport are billing-only and restricted from static import; - // this hook never touches them, but a namespace import can't be statically proven not to, so these + // isPaidGroupPolicy/isPaidGroupPolicyExpenseReport are billing-only and restricted from static import. + // This hook never touches them, but a namespace import can't be statically proven not to, so these // ReportUtils spies go through require() instead (matches other tests spying on this module). jest.spyOn(require('@libs/ReportUtils'), 'hasOnlyHeldExpenses').mockReturnValue(false); jest.spyOn(require('@libs/ReportUtils'), 'isSettled').mockReturnValue(false);