From 3b05dec9bed2bed468a1dac875ac4ad99c201f44 Mon Sep 17 00:00:00 2001 From: Eduardo Date: Wed, 28 May 2025 11:41:26 +0200 Subject: [PATCH 1/3] Improving logic to avoid multiple iterations --- src/hooks/useLoadReportActions.ts | 78 +++++++---- tests/unit/useLoadReportActionsTest.tsx | 179 ++++++++++++++++++++++++ 2 files changed, 233 insertions(+), 24 deletions(-) create mode 100644 tests/unit/useLoadReportActionsTest.tsx diff --git a/src/hooks/useLoadReportActions.ts b/src/hooks/useLoadReportActions.ts index e4af06fd3332..ad419b0ae264 100644 --- a/src/hooks/useLoadReportActions.ts +++ b/src/hooks/useLoadReportActions.ts @@ -44,11 +44,43 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor const newestReportAction = useMemo(() => reportActions?.at(0), [reportActions]); const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]); - const reportActionIDMap = useMemo(() => { - return reportActions.map((action) => ({ - reportActionID: action.reportActionID, - reportID: allReportActionIDs?.includes(action.reportActionID) ? reportID : transactionThreadReport?.reportID, - })); + // Track oldest/newest actions per report in a single pass + const {currentReportOldest, currentReportNewest, transactionThreadOldest, transactionThreadNewest} = useMemo(() => { + let currentReportNewestAction = null; + let currentReportOldestAction = null; + let transactionThreadNewestAction = null; + let transactionThreadOldestAction = null; + + const allReportActionIDsSet = new Set(allReportActionIDs); + + for (const action of reportActions) { + // Determine which report this action belongs to + const isCurrentReport = allReportActionIDsSet.has(action.reportActionID); + const targetReportID = isCurrentReport ? reportID : transactionThreadReport?.reportID; + + // Track newest/oldest per report + if (targetReportID === reportID) { + // Newest = first matching action we encounter + if (!currentReportNewestAction) { + currentReportNewestAction = action; + } + // Oldest = last matching action we encounter + currentReportOldestAction = action; + } else if (targetReportID === transactionThreadReport?.reportID) { + // Same logic for transaction thread + if (!transactionThreadNewestAction) { + transactionThreadNewestAction = action; + } + transactionThreadOldestAction = action; + } + } + + return { + currentReportOldest: currentReportOldestAction, + currentReportNewest: currentReportNewestAction, + transactionThreadOldest: transactionThreadOldestAction, + transactionThreadNewest: transactionThreadNewestAction, + }; }, [reportActions, allReportActionIDs, reportID, transactionThreadReport?.reportID]); /** @@ -70,19 +102,13 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor didLoadOlderChats.current = true; if (!isEmptyObject(transactionThreadReport)) { - // Get older actions based on the oldest reportAction for the current report - const oldestActionCurrentReport = reportActionIDMap.findLast((item) => item.reportID === reportID); - getOlderActions(oldestActionCurrentReport?.reportID, oldestActionCurrentReport?.reportActionID); - - // Get older actions based on the oldest reportAction for the transaction thread report - const oldestActionTransactionThreadReport = reportActionIDMap.findLast((item) => item.reportID === transactionThreadReport.reportID); - getOlderActions(oldestActionTransactionThreadReport?.reportID, oldestActionTransactionThreadReport?.reportActionID); + getOlderActions(reportID, currentReportOldest?.reportActionID); + getOlderActions(transactionThreadReport.reportID, transactionThreadOldest?.reportActionID); } else { - // Retrieve the next REPORT.ACTIONS.LIMIT sized page of comments - getOlderActions(reportID, oldestReportAction.reportActionID); + getOlderActions(reportID, currentReportOldest?.reportActionID); } }, - [isOffline, oldestReportAction, reportID, reportActionIDMap, transactionThreadReport, hasOlderActions], + [isOffline, oldestReportAction, hasOlderActions, transactionThreadReport, reportID, currentReportOldest?.reportActionID, transactionThreadOldest?.reportActionID], ); const loadNewerChats = useCallback( @@ -104,20 +130,24 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor didLoadNewerChats.current = true; - // If this is a one transaction report, ensure we load newer actions for both this report and the report associated with the transaction if (!isEmptyObject(transactionThreadReport)) { - // Get newer actions based on the newest reportAction for the current report - const newestActionCurrentReport = reportActionIDMap.find((item) => item.reportID === reportID); - getNewerActions(newestActionCurrentReport?.reportID, newestActionCurrentReport?.reportActionID); - - // Get newer actions based on the newest reportAction for the transaction thread report - const newestActionTransactionThreadReport = reportActionIDMap.find((item) => item.reportID === transactionThreadReport.reportID); - getNewerActions(newestActionTransactionThreadReport?.reportID, newestActionTransactionThreadReport?.reportActionID); + getNewerActions(reportID, currentReportNewest?.reportActionID); + getNewerActions(transactionThreadReport.reportID, transactionThreadNewest?.reportActionID); } else if (newestReportAction) { getNewerActions(reportID, newestReportAction.reportActionID); } }, - [reportActionID, isFocused, newestReportAction, hasNewerActions, isOffline, transactionThreadReport, reportActionIDMap, reportID], + [ + reportActionID, + isFocused, + newestReportAction, + hasNewerActions, + isOffline, + transactionThreadReport, + reportID, + currentReportNewest?.reportActionID, + transactionThreadNewest?.reportActionID, + ], ); return { diff --git a/tests/unit/useLoadReportActionsTest.tsx b/tests/unit/useLoadReportActionsTest.tsx new file mode 100644 index 000000000000..2f743e78d0cd --- /dev/null +++ b/tests/unit/useLoadReportActionsTest.tsx @@ -0,0 +1,179 @@ +import {renderHook} from '@testing-library/react-native'; +import useLoadReportActions from '@hooks/useLoadReportActions'; +import type Navigation from '@libs/Navigation/Navigation'; + +jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false}))); + +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigationState: () => true, + useRoute: jest.fn(), + useFocusEffect: jest.fn(), + useIsFocused: () => true, + useNavigation: () => ({ + navigate: jest.fn(), + addListener: jest.fn(), + }), + createNavigationContainerRef: jest.fn(), + }; +}); + +describe('useLoadReportActions', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + // Base test data from your example + const baseProps = { + reportID: '6549335221793525', + reportActions: [ + /* your 4 reportActions array here */ + ], + allReportActionIDs: ['8759152536123291182', '2034215190990675144', '186758379215594799'], + transactionThreadReport: undefined, + hasOlderActions: true, + hasNewerActions: false, + }; + + describe('Base cases', () => { + test('correctly identifies current report actions', () => { + jest.doMock('@userActions/Report', () => ({ + getNewerActions: (_: string | undefined, reportActionID: string | undefined) => { + expect(reportActionID).toBe('186758379215594799'); + }, + getOlderActions: (_: string | undefined, reportActionID: string | undefined) => { + expect(reportActionID).toBe('8759152536123291182'); + }, + })); + const {result} = renderHook(() => useLoadReportActions(baseProps)); + + result.current.loadOlderChats(); + result.current.loadNewerChats(); + }); + + test('handles transaction thread report actions', () => { + jest.doMock('@userActions/Report', () => ({ + getNewerActions: (_: string | undefined, reportActionID: string | undefined) => { + expect(reportActionID).toBe('186758379215594799'); + }, + getOlderActions: (_: string | undefined, reportActionID: string | undefined) => { + expect(reportActionID).toBe('2034215190990675144'); + }, + })); + + const propsWithTransaction = { + ...baseProps, + transactionThreadReport: {reportID: '186758379215594798'}, + allReportActionIDs: ['8759152536123291182'], // Only first action belongs to main report + }; + + const {result} = renderHook(() => useLoadReportActions(propsWithTransaction)); + + result.current.loadOlderChats(); + result.current.loadNewerChats(); + }); + }); + + describe('loadOlderChats behavior', () => { + test('loads older actions for current report', () => { + jest.doMock('@userActions/Report', () => ({ + getNewerActions: jest.fn(), + getOlderActions: (reportID: string | undefined, reportActionID: string | undefined) => { + expect(reportID).toBe('6549335221793525'); + expect(reportActionID).toBe('186758379215594799'); + }, + })); + const {result} = renderHook(() => useLoadReportActions(baseProps)); + result.current.loadOlderChats(); + }); + + test('loads actions for both reports when transaction thread exists', () => { + jest.doMock('@userActions/Report', () => ({ + getNewerActions: jest.fn(), + getOlderActions: (reportID: string | undefined, reportActionID: string | undefined) => { + if (reportID !== 'TRANSACTION_THREAD_REPORT') { + expect(reportID).toBe('8759152536123291182'); + expect(reportActionID).toBe('6549335221793525'); + } else { + expect(reportID).toBe('TRANSACTION_THREAD_REPORT'); + expect(reportActionID).toBe('186758379215594799'); + } + }, + })); + const props = { + ...baseProps, + transactionThreadReport: {reportID: 'TRANSACTION_THREAD_REPORT'}, + }; + + const {result} = renderHook(() => useLoadReportActions(props)); + result.current.loadOlderChats(); + }); + }); + + describe('loadNewerChats behavior', () => { + test('loads newer actions when conditions met', () => { + jest.doMock('@userActions/Report', () => ({ + getNewerActions: (reportID: string | undefined, reportActionID: string | undefined) => { + expect(reportID).toBe('6549335221793525'); + expect(reportActionID).toBe('8759152536123291182'); + }, + getOlderActions: jest.fn(), + })); + const props = { + ...baseProps, + hasNewerActions: true, + reportActionID: 'EXISTING_ACTION_ID', + }; + + const {result} = renderHook(() => useLoadReportActions(props)); + result.current.loadNewerChats(); + }); + + test('handles transaction thread newer actions', () => { + jest.doMock('@userActions/Report', () => ({ + getNewerActions: (reportID: string | undefined, reportActionID: string | undefined) => { + if (reportID !== 'TRANSACTION_THREAD_REPORT') { + expect(reportID).toBe('6549335221793525'); + expect(reportActionID).toBe('8759152536123291182'); + } else { + expect(reportID).toBe('TRANSACTION_THREAD_REPORT'); + expect(reportActionID).toBe('2034215190990675144'); + } + }, + getOlderActions: jest.fn(), + })); + const props = { + ...baseProps, + transactionThreadReport: {reportID: 'TRANSACTION_THREAD_REPORT'}, + hasNewerActions: true, + }; + + const {result} = renderHook(() => useLoadReportActions(props)); + result.current.loadNewerChats(); + }); + }); + + describe('Edge cases', () => { + test('handles empty reportActions', () => { + const mockGetOlderActions = jest.fn(); + const mockGetNewerActions = jest.fn(); + jest.doMock('@userActions/Report', () => ({ + getNewerActions: mockGetNewerActions, + getOlderActions: mockGetOlderActions, + })); + const props = { + ...baseProps, + reportActions: [], + }; + + const {result} = renderHook(() => useLoadReportActions(props)); + result.current.loadOlderChats(); + result.current.loadNewerChats(); + + expect(mockGetOlderActions).not.toHaveBeenCalled(); + expect(mockGetNewerActions).not.toHaveBeenCalled(); + }); + }); +}); From 616221ca0c0d389e820d9fb7083228e789306fc6 Mon Sep 17 00:00:00 2001 From: Eduardo Date: Mon, 2 Jun 2025 19:30:27 +0200 Subject: [PATCH 2/3] checking object data --- src/hooks/useLoadReportActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useLoadReportActions.ts b/src/hooks/useLoadReportActions.ts index ad419b0ae264..a4ca06d746a2 100644 --- a/src/hooks/useLoadReportActions.ts +++ b/src/hooks/useLoadReportActions.ts @@ -66,7 +66,7 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor } // Oldest = last matching action we encounter currentReportOldestAction = action; - } else if (targetReportID === transactionThreadReport?.reportID) { + } else if (!isEmptyObject(transactionThreadReport) && transactionThreadReport?.reportID === targetReportID) { // Same logic for transaction thread if (!transactionThreadNewestAction) { transactionThreadNewestAction = action; From e1c674d3b098c3f94ea75836c1370c50834fb1cb Mon Sep 17 00:00:00 2001 From: Eduardo Date: Tue, 3 Jun 2025 14:33:13 +0200 Subject: [PATCH 3/3] Fix eslint hook dependencies --- src/hooks/useLoadReportActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useLoadReportActions.ts b/src/hooks/useLoadReportActions.ts index a4ca06d746a2..14755d8655c2 100644 --- a/src/hooks/useLoadReportActions.ts +++ b/src/hooks/useLoadReportActions.ts @@ -81,7 +81,7 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor transactionThreadOldest: transactionThreadOldestAction, transactionThreadNewest: transactionThreadNewestAction, }; - }, [reportActions, allReportActionIDs, reportID, transactionThreadReport?.reportID]); + }, [reportActions, allReportActionIDs, reportID, transactionThreadReport]); /** * Retrieves the next set of reportActions for the chat once we are nearing the end of what we are currently