diff --git a/src/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index 918831f27545..a32d4f7d3dd0 100644 --- a/src/hooks/usePaginatedReportActions.ts +++ b/src/hooks/usePaginatedReportActions.ts @@ -18,9 +18,13 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { }); const [reportActionPages] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_PAGES}${reportIDWithDefault}`); - const reportActions = useMemo(() => { + const { + data: reportActions, + hasNextPage, + hasPreviousPage, + } = useMemo(() => { if (!sortedAllReportActions?.length) { - return []; + return {data: [], hasNextPage: false, hasPreviousPage: false}; } return PaginationUtils.getContinuousChain(sortedAllReportActions, reportActionPages ?? [], (reportAction) => reportAction.reportActionID, reportActionID); }, [reportActionID, reportActionPages, sortedAllReportActions]); @@ -34,6 +38,8 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { reportActions, linkedAction, sortedAllReportActions, + hasOlderActions: hasNextPage, + hasNewerActions: hasPreviousPage, }; } diff --git a/src/libs/Middleware/Pagination.ts b/src/libs/Middleware/Pagination.ts index 80f36725611a..bfa8183ac03b 100644 --- a/src/libs/Middleware/Pagination.ts +++ b/src/libs/Middleware/Pagination.ts @@ -17,7 +17,6 @@ type PaginationCommonConfig Array>; getItemID: (item: PagedResource) => string; - isLastItem: (item: PagedResource) => boolean; }; type PaginationConfig = PaginationCommonConfig & { @@ -85,7 +84,7 @@ const Pagination: Middleware = (requestResponse, request) => { return requestResponse; } - const {resourceCollectionKey, pageCollectionKey, sortItems, getItemID, isLastItem, type} = paginationConfig; + const {resourceCollectionKey, pageCollectionKey, sortItems, getItemID, type} = paginationConfig; const {resourceID, cursorID} = request; return requestResponse.then((response) => { if (!response?.onyxData) { @@ -106,13 +105,10 @@ const Pagination: Middleware = (requestResponse, request) => { const newPage = sortedPageItems.map((item) => getItemID(item)); - // Detect if we are at the start of the list. This will always be the case for the initial request with no cursor. - // For previous requests we check that no new data is returned. Ideally the server would return that info. - if ((type === 'initial' && !cursorID) || (type === 'next' && newPage.length === 1 && newPage.at(0) === cursorID)) { + if (response.hasNewerActions === false || (type === 'initial' && !cursorID)) { newPage.unshift(CONST.PAGINATION_START_ID); } - const pageItem = sortedPageItems.at(-1); - if (pageItem && isLastItem(pageItem)) { + if (response.hasOlderActions === false) { newPage.push(CONST.PAGINATION_END_ID); } diff --git a/src/libs/PaginationUtils.ts b/src/libs/PaginationUtils.ts index 2a214aaae19b..1cb3e7295c27 100644 --- a/src/libs/PaginationUtils.ts +++ b/src/libs/PaginationUtils.ts @@ -161,13 +161,19 @@ function mergeAndSortContinuousPages(sortedItems: TResource[], pages: /** * Returns the page of items that contains the item with the given ID, or the first page if null. + * Also returns whether next / previous pages can be fetched. * See unit tests for example of inputs and expected outputs. * * Note: sortedItems should be sorted in descending order. */ -function getContinuousChain(sortedItems: TResource[], pages: Pages, getID: (item: TResource) => string, id?: string): TResource[] { +function getContinuousChain( + sortedItems: TResource[], + pages: Pages, + getID: (item: TResource) => string, + id?: string, +): {data: TResource[]; hasNextPage: boolean; hasPreviousPage: boolean} { if (pages.length === 0) { - return id ? [] : sortedItems; + return {data: id ? [] : sortedItems, hasNextPage: false, hasPreviousPage: false}; } const pagesWithIndexes = getPagesWithIndexes(sortedItems, pages, getID); @@ -185,7 +191,7 @@ function getContinuousChain(sortedItems: TResource[], pages: Pages, g // If we are linking to an action that doesn't exist in Onyx, return an empty array if (index === -1) { - return []; + return {data: [], hasNextPage: false, hasPreviousPage: false}; } const linkedPage = pagesWithIndexes.find((pageIndex) => index >= pageIndex.firstIndex && index <= pageIndex.lastIndex); @@ -193,7 +199,7 @@ function getContinuousChain(sortedItems: TResource[], pages: Pages, g const item = sortedItems.at(index); // If we are linked to an action in a gap return it by itself if (!linkedPage && item) { - return [item]; + return {data: [item], hasNextPage: false, hasPreviousPage: false}; } if (linkedPage) { @@ -206,7 +212,11 @@ function getContinuousChain(sortedItems: TResource[], pages: Pages, g } } - return page ? sortedItems.slice(page.firstIndex, page.lastIndex + 1) : sortedItems; + if (!page) { + return {data: sortedItems, hasNextPage: false, hasPreviousPage: false}; + } + + return {data: sortedItems.slice(page.firstIndex, page.lastIndex + 1), hasNextPage: page.lastID !== CONST.PAGINATION_END_ID, hasPreviousPage: page.firstID !== CONST.PAGINATION_START_ID}; } export default {mergeAndSortContinuousPages, getContinuousChain}; diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index f313ecbb3f48..51b33f09a204 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -276,7 +276,6 @@ registerPaginationConfig({ pageCollectionKey: ONYXKEYS.COLLECTION.REPORT_ACTIONS_PAGES, sortItems: (reportActions) => ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true), getItemID: (reportAction) => reportAction.reportActionID, - isLastItem: (reportAction) => reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED, }); function clearGroupChat() { diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index dd38a0716377..7ce9114553f5 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -222,7 +222,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro const [isLinkingToMessage, setIsLinkingToMessage] = useState(!!reportActionIDFromRoute); const [currentUserAccountID = -1] = useOnyx(ONYXKEYS.SESSION, {selector: (value) => value?.accountID}); - const {reportActions, linkedAction, sortedAllReportActions} = usePaginatedReportActions(reportID, reportActionIDFromRoute); + const {reportActions, linkedAction, sortedAllReportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID, reportActionIDFromRoute); const [isBannerVisible, setIsBannerVisible] = useState(true); const [scrollPosition, setScrollPosition] = useState({}); @@ -756,6 +756,8 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro {!shouldShowSkeleton && report && ( { - if (isLoadingNewerReportActions || isLoadingInitialReportActions || (reportActionID && isOffline)) { + if (!hasNewerActions || isLoadingNewerReportActions || isLoadingInitialReportActions || (reportActionID && isOffline)) { return; } @@ -270,7 +278,7 @@ function ReportActionsView({ Report.getNewerActions(reportID, newestReportAction.reportActionID); } }, - [isLoadingNewerReportActions, isLoadingInitialReportActions, reportActionID, isOffline, transactionThreadReport, reportActionIDMap, reportID], + [isLoadingNewerReportActions, isLoadingInitialReportActions, reportActionID, isOffline, transactionThreadReport, reportActionIDMap, reportID, hasNewerActions], ); const hasMoreCached = reportActions.length < combinedReportActions.length; @@ -279,7 +287,6 @@ function ReportActionsView({ const hasCachedActionOnFirstRender = useInitialValue(() => reportActions.length > 0); const hasNewestReportAction = reportActions.at(0)?.created === report.lastVisibleActionCreated || reportActions.at(0)?.created === transactionThreadReport?.lastVisibleActionCreated; const oldestReportAction = useMemo(() => reportActions?.at(-1), [reportActions]); - const hasCreatedAction = oldestReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED; useEffect(() => { const wasLoginChangedDetected = prevAuthTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS && !session?.authTokenType; @@ -343,7 +350,7 @@ function ReportActionsView({ } // Don't load more chats if we're already at the beginning of the chat history - if (!oldestReportAction || hasCreatedAction) { + if (!oldestReportAction || !hasOlderActions) { return; } @@ -367,11 +374,11 @@ function ReportActionsView({ isLoadingOlderReportActions, isLoadingInitialReportActions, oldestReportAction, - hasCreatedAction, reportID, reportActionIDMap, transactionThreadReport, hasLoadingOlderReportActionsError, + hasOlderActions, ], ); @@ -524,6 +531,14 @@ function arePropsEqual(oldProps: ReportActionsViewProps, newProps: ReportActions return false; } + if (oldProps.hasNewerActions !== newProps.hasNewerActions) { + return false; + } + + if (oldProps.hasOlderActions !== newProps.hasOlderActions) { + return false; + } + return lodashIsEqual(oldProps.report, newProps.report); } diff --git a/src/types/onyx/Response.ts b/src/types/onyx/Response.ts index 3c12611ee841..d9eb5a7dfd2d 100644 --- a/src/types/onyx/Response.ts +++ b/src/types/onyx/Response.ts @@ -74,6 +74,12 @@ type Response = { /** The email of the user */ email?: string; + + /** If there is older data to load for pagination commands */ + hasOlderActions?: boolean; + + /** If there is newer data to load for pagination commands */ + hasNewerActions?: boolean; }; export default Response; diff --git a/tests/ui/PaginationTest.tsx b/tests/ui/PaginationTest.tsx index a7ce0632b606..186a5f5d4483 100644 --- a/tests/ui/PaginationTest.tsx +++ b/tests/ui/PaginationTest.tsx @@ -128,47 +128,60 @@ function buildReportComments(count: number, initialID: string, reverse = false) } function mockOpenReport(messageCount: number, initialID: string) { - fetchMock.mockAPICommand('OpenReport', ({reportID}) => - reportID === REPORT_ID - ? [ - { - onyxMethod: 'merge', - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, - value: buildReportComments(messageCount, initialID), - }, - ] - : [], - ); + fetchMock.mockAPICommand('OpenReport', ({reportID}) => { + const comments = buildReportComments(messageCount, initialID); + return { + onyxData: + reportID === REPORT_ID + ? [ + { + onyxMethod: 'merge', + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, + value: comments, + }, + ] + : [], + hasOlderActions: !comments['1'], + hasNewerActions: !!reportID, + }; + }); } function mockGetOlderActions(messageCount: number) { - fetchMock.mockAPICommand('GetOlderActions', ({reportID, reportActionID}) => - reportID === REPORT_ID - ? [ - { - onyxMethod: 'merge', - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, - // The API also returns the action that was requested with the reportActionID. - value: buildReportComments(messageCount + 1, reportActionID), - }, - ] - : [], - ); + fetchMock.mockAPICommand('GetOlderActions', ({reportID, reportActionID}) => { + // The API also returns the action that was requested with the reportActionID. + const comments = buildReportComments(messageCount + 1, reportActionID); + return { + onyxData: + reportID === REPORT_ID + ? [ + { + onyxMethod: 'merge', + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, + value: comments, + }, + ] + : [], + hasOlderActions: comments['1'] != null, + }; + }); } function mockGetNewerActions(messageCount: number) { - fetchMock.mockAPICommand('GetNewerActions', ({reportID, reportActionID}) => - reportID === REPORT_ID - ? [ - { - onyxMethod: 'merge', - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, - // The API also returns the action that was requested with the reportActionID. - value: buildReportComments(messageCount + 1, reportActionID, true), - }, - ] - : [], - ); + fetchMock.mockAPICommand('GetNewerActions', ({reportID, reportActionID}) => ({ + onyxData: + reportID === REPORT_ID + ? [ + { + onyxMethod: 'merge', + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, + // The API also returns the action that was requested with the reportActionID. + value: buildReportComments(messageCount + 1, reportActionID, true), + }, + ] + : [], + hasNewerActions: messageCount > 0, + })); } /** diff --git a/tests/unit/PaginationUtilsTest.ts b/tests/unit/PaginationUtilsTest.ts index 09323e10a416..e5f4ce7675e8 100644 --- a/tests/unit/PaginationUtilsTest.ts +++ b/tests/unit/PaginationUtilsTest.ts @@ -59,7 +59,9 @@ describe('PaginationUtils', () => { ]; for (const targetID of targetIDs) { const result = PaginationUtils.getContinuousChain(input, pages, getID, targetID); - expect(result).toStrictEqual(expectedOutput); + expect(result.data).toStrictEqual(expectedOutput); + expect(result.hasPreviousPage).toBe(true); + expect(result.hasNextPage).toBe(true); } }); @@ -95,7 +97,9 @@ describe('PaginationUtils', () => { // Expect these sortedItems const expectedResult: Item[] = []; const result = PaginationUtils.getContinuousChain(input, pages, getID, '8'); - expect(result).toStrictEqual(expectedResult); + expect(result.data).toStrictEqual(expectedResult); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(false); }); it('given an input ID of an action in a gap it will return only that action', () => { @@ -132,7 +136,9 @@ describe('PaginationUtils', () => { '8', ]); const result = PaginationUtils.getContinuousChain(input, pages, getID, '8'); - expect(result).toStrictEqual(expectedResult); + expect(result.data).toStrictEqual(expectedResult); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(false); }); it('given an empty input ID and the report only contains pending actions, it will return all actions', () => { @@ -152,7 +158,9 @@ describe('PaginationUtils', () => { // Expect these sortedItems const expectedResult = [...input]; const result = PaginationUtils.getContinuousChain(input, pages, getID, ''); - expect(result).toStrictEqual(expectedResult); + expect(result.data).toStrictEqual(expectedResult); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(false); }); it('given an input ID and the report only contains pending actions, it will return an empty array', () => { @@ -172,7 +180,9 @@ describe('PaginationUtils', () => { // Expect these sortedItems const expectedResult: Item[] = []; const result = PaginationUtils.getContinuousChain(input, pages, getID, '4'); - expect(result).toStrictEqual(expectedResult); + expect(result.data).toStrictEqual(expectedResult); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(false); }); it('does not include actions outside of pages', () => { @@ -212,7 +222,9 @@ describe('PaginationUtils', () => { '9', ]); const result = PaginationUtils.getContinuousChain(input, pages, getID, '10'); - expect(result).toStrictEqual(expectedResult); + expect(result.data).toStrictEqual(expectedResult); + expect(result.hasPreviousPage).toBe(true); + expect(result.hasNextPage).toBe(true); }); it('given a page with an empty firstItemID include actions until the start', () => { @@ -237,7 +249,9 @@ describe('PaginationUtils', () => { '14', ]); const result = PaginationUtils.getContinuousChain(input, pages, getID, ''); - expect(result).toStrictEqual(expectedResult); + expect(result.data).toStrictEqual(expectedResult); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(true); }); it('given a page with null lastItemID include actions to the end', () => { @@ -262,7 +276,9 @@ describe('PaginationUtils', () => { '14', ]); const result = PaginationUtils.getContinuousChain(input, pages, getID, ''); - expect(result).toStrictEqual(expectedResult); + expect(result.data).toStrictEqual(expectedResult); + expect(result.hasPreviousPage).toBe(true); + expect(result.hasNextPage).toBe(false); }); }); diff --git a/tests/utils/TestHelper.ts b/tests/utils/TestHelper.ts index 576fe4e38de1..c44710f5f466 100644 --- a/tests/utils/TestHelper.ts +++ b/tests/utils/TestHelper.ts @@ -19,7 +19,7 @@ type MockFetch = jest.MockedFn & { fail: () => void; succeed: () => void; resume: () => Promise; - mockAPICommand: (command: TCommand, responseHandler: (params: ApiRequestCommandParameters[TCommand]) => OnyxResponse['onyxData']) => void; + mockAPICommand: (command: TCommand, responseHandler: (params: ApiRequestCommandParameters[TCommand]) => OnyxResponse) => void; }; type QueueItem = { @@ -183,7 +183,7 @@ function signOutTestUser() { function getGlobalFetchMock(): typeof fetch { let queue: QueueItem[] = []; // eslint-disable-next-line @typescript-eslint/no-explicit-any - let responses = new Map OnyxResponse['onyxData']>(); + let responses = new Map OnyxResponse>(); let isPaused = false; let shouldFail = false; @@ -202,7 +202,7 @@ function getGlobalFetchMock(): typeof fetch { const responseHandler = command ? responses.get(command) : null; if (responseHandler) { const requestData = options?.body instanceof FormData ? Object.fromEntries(options.body) : {}; - return Promise.resolve({jsonCode: 200, onyxData: responseHandler(requestData)}); + return Promise.resolve({jsonCode: 200, ...responseHandler(requestData)}); } return Promise.resolve({jsonCode: 200}); @@ -236,7 +236,7 @@ function getGlobalFetchMock(): typeof fetch { }; mockFetch.fail = () => (shouldFail = true); mockFetch.succeed = () => (shouldFail = false); - mockFetch.mockAPICommand = (command: TCommand, responseHandler: (params: ApiRequestCommandParameters[TCommand]) => OnyxResponse['onyxData']): void => { + mockFetch.mockAPICommand = (command: TCommand, responseHandler: (params: ApiRequestCommandParameters[TCommand]) => OnyxResponse): void => { responses.set(command, responseHandler); }; return mockFetch as typeof fetch;