Skip to content

Commit

Permalink
Use pagination metadata for report actions
Browse files Browse the repository at this point in the history
  • Loading branch information
janicduplessis committed Oct 3, 2024
1 parent ba9af8f commit 7463dd4
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 69 deletions.
10 changes: 8 additions & 2 deletions src/hooks/usePaginatedReportActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -34,6 +38,8 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) {
reportActions,
linkedAction,
sortedAllReportActions,
hasOlderActions: hasNextPage,
hasNewerActions: hasPreviousPage,
};
}

Expand Down
12 changes: 4 additions & 8 deletions src/libs/Middleware/Pagination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ type PaginationCommonConfig<TResourceKey extends OnyxCollectionKey = OnyxCollect
pageCollectionKey: TPageKey;
sortItems: (items: OnyxValues[TResourceKey]) => Array<PagedResource<TResourceKey>>;
getItemID: (item: PagedResource<TResourceKey>) => string;
isLastItem: (item: PagedResource<TResourceKey>) => boolean;
};

type PaginationConfig<TResourceKey extends OnyxCollectionKey, TPageKey extends OnyxPagesKey> = PaginationCommonConfig<TResourceKey, TPageKey> & {
Expand Down Expand Up @@ -85,8 +84,8 @@ const Pagination: Middleware = (requestResponse, request) => {
return requestResponse;
}

const {resourceCollectionKey, pageCollectionKey, sortItems, getItemID, isLastItem, type} = paginationConfig;
const {resourceID, cursorID} = request;
const {resourceCollectionKey, pageCollectionKey, sortItems, getItemID} = paginationConfig;
const {resourceID} = request;
return requestResponse.then((response) => {
if (!response?.onyxData) {
return Promise.resolve(response);
Expand All @@ -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) {
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);
}

Expand Down
20 changes: 15 additions & 5 deletions src/libs/PaginationUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,19 @@ function mergeAndSortContinuousPages<TResource>(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<TResource>(sortedItems: TResource[], pages: Pages, getID: (item: TResource) => string, id?: string): TResource[] {
function getContinuousChain<TResource>(
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);
Expand All @@ -185,15 +191,15 @@ function getContinuousChain<TResource>(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);

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) {
Expand All @@ -206,7 +212,11 @@ function getContinuousChain<TResource>(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};
1 change: 0 additions & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 3 additions & 1 deletion src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<ScrollPosition>({});
Expand Down Expand Up @@ -756,6 +756,8 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro
{!shouldShowSkeleton && report && (
<ReportActionsView
reportActions={reportActions}
hasNewerActions={hasNewerActions}
hasOlderActions={hasOlderActions}
report={report}
parentReportAction={parentReportAction}
isLoadingInitialReportActions={reportMetadata?.isLoadingInitialReportActions}
Expand Down
25 changes: 20 additions & 5 deletions src/pages/home/report/ReportActionsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ type ReportActionsViewProps = {
/** The reportID of the transaction thread report associated with this current report, if any */
// eslint-disable-next-line react/no-unused-prop-types
transactionThreadReportID?: string | null;

/** If the report has newer actions to load */
hasNewerActions: boolean;

/** If the report has older actions to load */
hasOlderActions: boolean;
};

let listOldID = Math.round(Math.random() * 100);
Expand All @@ -76,6 +82,8 @@ function ReportActionsView({
isLoadingNewerReportActions = false,
hasLoadingNewerReportActionsError = false,
transactionThreadReportID,
hasNewerActions,
hasOlderActions,
}: ReportActionsViewProps) {
useCopySelectionHelper();
const reactionListRef = useContext(ReactionListContext);
Expand Down Expand Up @@ -253,7 +261,7 @@ function ReportActionsView({
*/
const fetchNewerAction = useCallback(
(newestReportAction: OnyxTypes.ReportAction) => {
if (isLoadingNewerReportActions || isLoadingInitialReportActions || (reportActionID && isOffline)) {
if (!hasNewerActions || isLoadingNewerReportActions || isLoadingInitialReportActions || (reportActionID && isOffline)) {
return;
}

Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -367,11 +374,11 @@ function ReportActionsView({
isLoadingOlderReportActions,
isLoadingInitialReportActions,
oldestReportAction,
hasCreatedAction,
reportID,
reportActionIDMap,
transactionThreadReport,
hasLoadingOlderReportActionsError,
hasOlderActions,
],
);

Expand Down Expand Up @@ -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);
}

Expand Down
6 changes: 6 additions & 0 deletions src/types/onyx/Response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ type Response = {
/** Base64 key to decrypt messages from Pusher encrypted channels */
// eslint-disable-next-line @typescript-eslint/naming-convention
shared_secret?: 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;
83 changes: 48 additions & 35 deletions tests/ui/PaginationTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
}

/**
Expand Down
Loading

0 comments on commit 7463dd4

Please sign in to comment.