Skip to content

Commit 087424f

Browse files
mjasikowskimjasikowski
authored
Merge pull request Expensify#62928 from callstack-internal/gedu07/useLoadReportActions_improvement
Improving logic to avoid multiple iterations
2 parents 649eff9 + e1c674d commit 087424f

2 files changed

Lines changed: 234 additions & 25 deletions

File tree

‎src/hooks/useLoadReportActions.ts‎

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,44 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor
4444
const newestReportAction = useMemo(() => reportActions?.at(0), [reportActions]);
4545
const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]);
4646

47-
const reportActionIDMap = useMemo(() => {
48-
return reportActions.map((action) => ({
49-
reportActionID: action.reportActionID,
50-
reportID: allReportActionIDs?.includes(action.reportActionID) ? reportID : transactionThreadReport?.reportID,
51-
}));
52-
}, [reportActions, allReportActionIDs, reportID, transactionThreadReport?.reportID]);
47+
// Track oldest/newest actions per report in a single pass
48+
const {currentReportOldest, currentReportNewest, transactionThreadOldest, transactionThreadNewest} = useMemo(() => {
49+
let currentReportNewestAction = null;
50+
let currentReportOldestAction = null;
51+
let transactionThreadNewestAction = null;
52+
let transactionThreadOldestAction = null;
53+
54+
const allReportActionIDsSet = new Set(allReportActionIDs);
55+
56+
for (const action of reportActions) {
57+
// Determine which report this action belongs to
58+
const isCurrentReport = allReportActionIDsSet.has(action.reportActionID);
59+
const targetReportID = isCurrentReport ? reportID : transactionThreadReport?.reportID;
60+
61+
// Track newest/oldest per report
62+
if (targetReportID === reportID) {
63+
// Newest = first matching action we encounter
64+
if (!currentReportNewestAction) {
65+
currentReportNewestAction = action;
66+
}
67+
// Oldest = last matching action we encounter
68+
currentReportOldestAction = action;
69+
} else if (!isEmptyObject(transactionThreadReport) && transactionThreadReport?.reportID === targetReportID) {
70+
// Same logic for transaction thread
71+
if (!transactionThreadNewestAction) {
72+
transactionThreadNewestAction = action;
73+
}
74+
transactionThreadOldestAction = action;
75+
}
76+
}
77+
78+
return {
79+
currentReportOldest: currentReportOldestAction,
80+
currentReportNewest: currentReportNewestAction,
81+
transactionThreadOldest: transactionThreadOldestAction,
82+
transactionThreadNewest: transactionThreadNewestAction,
83+
};
84+
}, [reportActions, allReportActionIDs, reportID, transactionThreadReport]);
5385

5486
/**
5587
* Retrieves the next set of reportActions for the chat once we are nearing the end of what we are currently
@@ -70,19 +102,13 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor
70102
didLoadOlderChats.current = true;
71103

72104
if (!isEmptyObject(transactionThreadReport)) {
73-
// Get older actions based on the oldest reportAction for the current report
74-
const oldestActionCurrentReport = reportActionIDMap.findLast((item) => item.reportID === reportID);
75-
getOlderActions(oldestActionCurrentReport?.reportID, oldestActionCurrentReport?.reportActionID);
76-
77-
// Get older actions based on the oldest reportAction for the transaction thread report
78-
const oldestActionTransactionThreadReport = reportActionIDMap.findLast((item) => item.reportID === transactionThreadReport.reportID);
79-
getOlderActions(oldestActionTransactionThreadReport?.reportID, oldestActionTransactionThreadReport?.reportActionID);
105+
getOlderActions(reportID, currentReportOldest?.reportActionID);
106+
getOlderActions(transactionThreadReport.reportID, transactionThreadOldest?.reportActionID);
80107
} else {
81-
// Retrieve the next REPORT.ACTIONS.LIMIT sized page of comments
82-
getOlderActions(reportID, oldestReportAction.reportActionID);
108+
getOlderActions(reportID, currentReportOldest?.reportActionID);
83109
}
84110
},
85-
[isOffline, oldestReportAction, reportID, reportActionIDMap, transactionThreadReport, hasOlderActions],
111+
[isOffline, oldestReportAction, hasOlderActions, transactionThreadReport, reportID, currentReportOldest?.reportActionID, transactionThreadOldest?.reportActionID],
86112
);
87113

88114
const loadNewerChats = useCallback(
@@ -104,20 +130,24 @@ function useLoadReportActions({reportID, reportActionID, reportActions, allRepor
104130

105131
didLoadNewerChats.current = true;
106132

107-
// If this is a one transaction report, ensure we load newer actions for both this report and the report associated with the transaction
108133
if (!isEmptyObject(transactionThreadReport)) {
109-
// Get newer actions based on the newest reportAction for the current report
110-
const newestActionCurrentReport = reportActionIDMap.find((item) => item.reportID === reportID);
111-
getNewerActions(newestActionCurrentReport?.reportID, newestActionCurrentReport?.reportActionID);
112-
113-
// Get newer actions based on the newest reportAction for the transaction thread report
114-
const newestActionTransactionThreadReport = reportActionIDMap.find((item) => item.reportID === transactionThreadReport.reportID);
115-
getNewerActions(newestActionTransactionThreadReport?.reportID, newestActionTransactionThreadReport?.reportActionID);
134+
getNewerActions(reportID, currentReportNewest?.reportActionID);
135+
getNewerActions(transactionThreadReport.reportID, transactionThreadNewest?.reportActionID);
116136
} else if (newestReportAction) {
117137
getNewerActions(reportID, newestReportAction.reportActionID);
118138
}
119139
},
120-
[reportActionID, isFocused, newestReportAction, hasNewerActions, isOffline, transactionThreadReport, reportActionIDMap, reportID],
140+
[
141+
reportActionID,
142+
isFocused,
143+
newestReportAction,
144+
hasNewerActions,
145+
isOffline,
146+
transactionThreadReport,
147+
reportID,
148+
currentReportNewest?.reportActionID,
149+
transactionThreadNewest?.reportActionID,
150+
],
121151
);
122152

123153
return {

‎tests/unit/useLoadReportActionsTest.tsx‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import {renderHook} from '@testing-library/react-native';
2+
import useLoadReportActions from '@hooks/useLoadReportActions';
3+
import type Navigation from '@libs/Navigation/Navigation';
4+
5+
jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false})));
6+
7+
jest.mock('@react-navigation/native', () => {
8+
const actualNav = jest.requireActual<typeof Navigation>('@react-navigation/native');
9+
return {
10+
...actualNav,
11+
useNavigationState: () => true,
12+
useRoute: jest.fn(),
13+
useFocusEffect: jest.fn(),
14+
useIsFocused: () => true,
15+
useNavigation: () => ({
16+
navigate: jest.fn(),
17+
addListener: jest.fn(),
18+
}),
19+
createNavigationContainerRef: jest.fn(),
20+
};
21+
});
22+
23+
describe('useLoadReportActions', () => {
24+
beforeEach(() => {
25+
jest.clearAllMocks();
26+
});
27+
28+
// Base test data from your example
29+
const baseProps = {
30+
reportID: '6549335221793525',
31+
reportActions: [
32+
/* your 4 reportActions array here */
33+
],
34+
allReportActionIDs: ['8759152536123291182', '2034215190990675144', '186758379215594799'],
35+
transactionThreadReport: undefined,
36+
hasOlderActions: true,
37+
hasNewerActions: false,
38+
};
39+
40+
describe('Base cases', () => {
41+
test('correctly identifies current report actions', () => {
42+
jest.doMock('@userActions/Report', () => ({
43+
getNewerActions: (_: string | undefined, reportActionID: string | undefined) => {
44+
expect(reportActionID).toBe('186758379215594799');
45+
},
46+
getOlderActions: (_: string | undefined, reportActionID: string | undefined) => {
47+
expect(reportActionID).toBe('8759152536123291182');
48+
},
49+
}));
50+
const {result} = renderHook(() => useLoadReportActions(baseProps));
51+
52+
result.current.loadOlderChats();
53+
result.current.loadNewerChats();
54+
});
55+
56+
test('handles transaction thread report actions', () => {
57+
jest.doMock('@userActions/Report', () => ({
58+
getNewerActions: (_: string | undefined, reportActionID: string | undefined) => {
59+
expect(reportActionID).toBe('186758379215594799');
60+
},
61+
getOlderActions: (_: string | undefined, reportActionID: string | undefined) => {
62+
expect(reportActionID).toBe('2034215190990675144');
63+
},
64+
}));
65+
66+
const propsWithTransaction = {
67+
...baseProps,
68+
transactionThreadReport: {reportID: '186758379215594798'},
69+
allReportActionIDs: ['8759152536123291182'], // Only first action belongs to main report
70+
};
71+
72+
const {result} = renderHook(() => useLoadReportActions(propsWithTransaction));
73+
74+
result.current.loadOlderChats();
75+
result.current.loadNewerChats();
76+
});
77+
});
78+
79+
describe('loadOlderChats behavior', () => {
80+
test('loads older actions for current report', () => {
81+
jest.doMock('@userActions/Report', () => ({
82+
getNewerActions: jest.fn(),
83+
getOlderActions: (reportID: string | undefined, reportActionID: string | undefined) => {
84+
expect(reportID).toBe('6549335221793525');
85+
expect(reportActionID).toBe('186758379215594799');
86+
},
87+
}));
88+
const {result} = renderHook(() => useLoadReportActions(baseProps));
89+
result.current.loadOlderChats();
90+
});
91+
92+
test('loads actions for both reports when transaction thread exists', () => {
93+
jest.doMock('@userActions/Report', () => ({
94+
getNewerActions: jest.fn(),
95+
getOlderActions: (reportID: string | undefined, reportActionID: string | undefined) => {
96+
if (reportID !== 'TRANSACTION_THREAD_REPORT') {
97+
expect(reportID).toBe('8759152536123291182');
98+
expect(reportActionID).toBe('6549335221793525');
99+
} else {
100+
expect(reportID).toBe('TRANSACTION_THREAD_REPORT');
101+
expect(reportActionID).toBe('186758379215594799');
102+
}
103+
},
104+
}));
105+
const props = {
106+
...baseProps,
107+
transactionThreadReport: {reportID: 'TRANSACTION_THREAD_REPORT'},
108+
};
109+
110+
const {result} = renderHook(() => useLoadReportActions(props));
111+
result.current.loadOlderChats();
112+
});
113+
});
114+
115+
describe('loadNewerChats behavior', () => {
116+
test('loads newer actions when conditions met', () => {
117+
jest.doMock('@userActions/Report', () => ({
118+
getNewerActions: (reportID: string | undefined, reportActionID: string | undefined) => {
119+
expect(reportID).toBe('6549335221793525');
120+
expect(reportActionID).toBe('8759152536123291182');
121+
},
122+
getOlderActions: jest.fn(),
123+
}));
124+
const props = {
125+
...baseProps,
126+
hasNewerActions: true,
127+
reportActionID: 'EXISTING_ACTION_ID',
128+
};
129+
130+
const {result} = renderHook(() => useLoadReportActions(props));
131+
result.current.loadNewerChats();
132+
});
133+
134+
test('handles transaction thread newer actions', () => {
135+
jest.doMock('@userActions/Report', () => ({
136+
getNewerActions: (reportID: string | undefined, reportActionID: string | undefined) => {
137+
if (reportID !== 'TRANSACTION_THREAD_REPORT') {
138+
expect(reportID).toBe('6549335221793525');
139+
expect(reportActionID).toBe('8759152536123291182');
140+
} else {
141+
expect(reportID).toBe('TRANSACTION_THREAD_REPORT');
142+
expect(reportActionID).toBe('2034215190990675144');
143+
}
144+
},
145+
getOlderActions: jest.fn(),
146+
}));
147+
const props = {
148+
...baseProps,
149+
transactionThreadReport: {reportID: 'TRANSACTION_THREAD_REPORT'},
150+
hasNewerActions: true,
151+
};
152+
153+
const {result} = renderHook(() => useLoadReportActions(props));
154+
result.current.loadNewerChats();
155+
});
156+
});
157+
158+
describe('Edge cases', () => {
159+
test('handles empty reportActions', () => {
160+
const mockGetOlderActions = jest.fn();
161+
const mockGetNewerActions = jest.fn();
162+
jest.doMock('@userActions/Report', () => ({
163+
getNewerActions: mockGetNewerActions,
164+
getOlderActions: mockGetOlderActions,
165+
}));
166+
const props = {
167+
...baseProps,
168+
reportActions: [],
169+
};
170+
171+
const {result} = renderHook(() => useLoadReportActions(props));
172+
result.current.loadOlderChats();
173+
result.current.loadNewerChats();
174+
175+
expect(mockGetOlderActions).not.toHaveBeenCalled();
176+
expect(mockGetNewerActions).not.toHaveBeenCalled();
177+
});
178+
});
179+
});

0 commit comments

Comments
 (0)