From ace2cc5ce16c0ab7f559d213e7964f9e4bf94989 Mon Sep 17 00:00:00 2001 From: Aubin Date: Tue, 18 Feb 2025 18:53:17 +0100 Subject: [PATCH 1/6] remove the offset-based pagination altogether, replace with cursor-based pagination --- front/lib/api/data_source_view.ts | 16 ++------ front/lib/api/pagination.ts | 37 +++++++------------ .../[dsvId]/content-nodes.ts | 4 +- .../[dsvId]/content-nodes.ts | 4 +- .../data_source_views/[dsvId]/tables/index.ts | 4 +- 5 files changed, 22 insertions(+), 43 deletions(-) diff --git a/front/lib/api/data_source_view.ts b/front/lib/api/data_source_view.ts index 3c87c04afa50..b450ebb2c8d6 100644 --- a/front/lib/api/data_source_view.ts +++ b/front/lib/api/data_source_view.ts @@ -14,11 +14,7 @@ import { FOLDERS_TO_HIDE_IF_EMPTY_MIME_TYPES, getContentNodeFromCoreNode, } from "@app/lib/api/content_nodes"; -import type { - CursorPaginationParams, - OffsetPaginationParams, -} from "@app/lib/api/pagination"; -import { isCursorPaginationParams } from "@app/lib/api/pagination"; +import type { CursorPaginationParams } from "@app/lib/api/pagination"; import type { Authenticator } from "@app/lib/auth"; import type { DustError } from "@app/lib/error"; import { DataSourceViewResource } from "@app/lib/resources/data_source_view_resource"; @@ -70,8 +66,7 @@ export function filterAndCropContentNodesByView( interface GetContentNodesForDataSourceViewParams { internalIds?: string[]; parentId?: string; - // TODO(nodes-core): remove offset pagination upon project cleanup - pagination?: CursorPaginationParams | OffsetPaginationParams; + pagination?: CursorPaginationParams; viewType: ContentNodesViewType; } @@ -164,12 +159,7 @@ export async function getContentNodesForDataSourceView( ? undefined : ROOT_PARENT_ID); - // TODO(nodes-core): remove offset pagination upon project cleanup - let nextPageCursor: string | null = pagination - ? isCursorPaginationParams(pagination) - ? pagination.cursor - : null - : null; + let nextPageCursor: string | null = pagination ? pagination.cursor : null; let resultNodes: CoreAPIContentNode[] = []; do { diff --git a/front/lib/api/pagination.ts b/front/lib/api/pagination.ts index df5d5c93a6a6..ec4f299c0d79 100644 --- a/front/lib/api/pagination.ts +++ b/front/lib/api/pagination.ts @@ -84,41 +84,41 @@ export function getPaginationParams( return new Ok(queryValidation.right); } -// Offset pagination. +// Cursor pagination. -const OffsetPaginationParamsCodec = t.type({ +const CursorPaginationParamsCodec = t.type({ limit: LimitCodec, - offset: t.number, + cursor: t.string, }); -export interface OffsetPaginationParams { +export interface CursorPaginationParams { limit: number; - offset: number; + cursor: string; } -export function getOffsetPaginationParams( +export function getCursorPaginationParams( req: NextApiRequest -): Result { - const { limit, offset } = req.query; +): Result { + const { limit, cursor } = req.query; if (!req.query.limit) { return new Ok(undefined); } - if (typeof limit !== "string" || (offset && typeof offset !== "string")) { + if (typeof limit !== "string" || (cursor && typeof cursor !== "string")) { return new Err( new InvalidPaginationParamsError( "Invalid pagination parameters", - "limit and offset must be strings" + "limit and cursor must be strings" ) ); } const rawParams = { - limit: parseInt(limit), - offset: offset ? parseInt(offset) : 0, + limit: parseInt(limit, 10), + cursor, }; - const queryValidation = OffsetPaginationParamsCodec.decode(rawParams); + const queryValidation = CursorPaginationParamsCodec.decode(rawParams); // Validate and decode the raw parameters. if (isLeft(queryValidation)) { @@ -134,14 +134,3 @@ export function getOffsetPaginationParams( return new Ok(queryValidation.right); } - -export interface CursorPaginationParams { - limit: number; - cursor: string; -} - -export function isCursorPaginationParams( - pagination: CursorPaginationParams | OffsetPaginationParams -): pagination is CursorPaginationParams { - return "cursor" in pagination; -} diff --git a/front/pages/api/poke/workspaces/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts b/front/pages/api/poke/workspaces/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts index 29cc41ce14ee..ed4ca4dff019 100644 --- a/front/pages/api/poke/workspaces/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts +++ b/front/pages/api/poke/workspaces/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts @@ -10,7 +10,7 @@ import type { NextApiRequest, NextApiResponse } from "next"; import { withSessionAuthentication } from "@app/lib/api/auth_wrappers"; import { getContentNodesForDataSourceView } from "@app/lib/api/data_source_view"; -import { getOffsetPaginationParams } from "@app/lib/api/pagination"; +import { getCursorPaginationParams } from "@app/lib/api/pagination"; import { Authenticator } from "@app/lib/auth"; import type { SessionWithUser } from "@app/lib/iam/provider"; import { DataSourceViewResource } from "@app/lib/resources/data_source_view_resource"; @@ -122,7 +122,7 @@ async function handler( }); } - const paginationRes = getOffsetPaginationParams(req); + const paginationRes = getCursorPaginationParams(req); if (paginationRes.isErr()) { return apiError(req, res, { status_code: 400, diff --git a/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts b/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts index c81125606a42..a1f8fdba6f47 100644 --- a/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts +++ b/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts @@ -10,7 +10,7 @@ import type { NextApiRequest, NextApiResponse } from "next"; import { withSessionAuthenticationForWorkspace } from "@app/lib/api/auth_wrappers"; import { getContentNodesForDataSourceView } from "@app/lib/api/data_source_view"; -import { getOffsetPaginationParams } from "@app/lib/api/pagination"; +import { getCursorPaginationParams } from "@app/lib/api/pagination"; import { withResourceFetchingFromRoute } from "@app/lib/api/resource_wrappers"; import type { Authenticator } from "@app/lib/auth"; import type { DataSourceViewResource } from "@app/lib/resources/data_source_view_resource"; @@ -82,7 +82,7 @@ async function handler( }); } - const paginationRes = getOffsetPaginationParams(req); + const paginationRes = getCursorPaginationParams(req); if (paginationRes.isErr()) { return apiError(req, res, { status_code: 400, diff --git a/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/tables/index.ts b/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/tables/index.ts index 84092e695d60..2a6bbab3f206 100644 --- a/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/tables/index.ts +++ b/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/tables/index.ts @@ -6,7 +6,7 @@ import type { NextApiRequest, NextApiResponse } from "next"; import { withSessionAuthenticationForWorkspace } from "@app/lib/api/auth_wrappers"; import { getContentNodesForDataSourceView } from "@app/lib/api/data_source_view"; -import { getOffsetPaginationParams } from "@app/lib/api/pagination"; +import { getCursorPaginationParams } from "@app/lib/api/pagination"; import { withResourceFetchingFromRoute } from "@app/lib/api/resource_wrappers"; import type { Authenticator } from "@app/lib/auth"; import type { DataSourceViewResource } from "@app/lib/resources/data_source_view_resource"; @@ -34,7 +34,7 @@ async function handler( switch (req.method) { case "GET": - const paginationRes = getOffsetPaginationParams(req); + const paginationRes = getCursorPaginationParams(req); if (paginationRes.isErr()) { return apiError(req, res, { status_code: 400, From 0e5670ae959372437fb46869ded9c7541103ad42 Mon Sep 17 00:00:00 2001 From: Aubin Date: Tue, 18 Feb 2025 20:47:19 +0100 Subject: [PATCH 2/6] add a few missing types --- front/lib/api/data_source_view.ts | 1 + front/lib/api/pagination.ts | 2 +- .../spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/front/lib/api/data_source_view.ts b/front/lib/api/data_source_view.ts index b450ebb2c8d6..90abdb9bd64e 100644 --- a/front/lib/api/data_source_view.ts +++ b/front/lib/api/data_source_view.ts @@ -73,6 +73,7 @@ interface GetContentNodesForDataSourceViewParams { interface GetContentNodesForDataSourceViewResult { nodes: DataSourceViewContentNode[]; total: number; + nextPageCursor: string | null; } function filterNodesByViewType( diff --git a/front/lib/api/pagination.ts b/front/lib/api/pagination.ts index ec4f299c0d79..d81b5476802c 100644 --- a/front/lib/api/pagination.ts +++ b/front/lib/api/pagination.ts @@ -93,7 +93,7 @@ const CursorPaginationParamsCodec = t.type({ export interface CursorPaginationParams { limit: number; - cursor: string; + cursor: string | null; } export function getCursorPaginationParams( diff --git a/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts b/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts index a1f8fdba6f47..e99b6e771ca6 100644 --- a/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts +++ b/front/pages/api/w/[wId]/spaces/[spaceId]/data_source_views/[dsvId]/content-nodes.ts @@ -25,6 +25,7 @@ const GetContentNodesOrChildrenRequestBody = t.type({ export type GetDataSourceViewContentNodes = { nodes: DataSourceViewContentNode[]; total: number; + nextPageCursor: string | null; }; // This endpoints serves two purposes: From 49dd91b38b6e08601173fd691d57de8650e448d3 Mon Sep 17 00:00:00 2001 From: Aubin Date: Tue, 18 Feb 2025 21:10:47 +0100 Subject: [PATCH 3/6] remove exhaustion of the cursor in getContentNodesForDataSourceView --- front/lib/api/data_source_view.ts | 34 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/front/lib/api/data_source_view.ts b/front/lib/api/data_source_view.ts index 90abdb9bd64e..ba8937fb1695 100644 --- a/front/lib/api/data_source_view.ts +++ b/front/lib/api/data_source_view.ts @@ -163,27 +163,25 @@ export async function getContentNodesForDataSourceView( let nextPageCursor: string | null = pagination ? pagination.cursor : null; let resultNodes: CoreAPIContentNode[] = []; - do { - const coreRes = await coreAPI.searchNodes({ - filter: { - data_source_views: [makeCoreDataSourceViewFilter(dataSourceView)], - node_ids, - parent_id, - }, - options: { limit, cursor: nextPageCursor ?? undefined }, - }); + const coreRes = await coreAPI.searchNodes({ + filter: { + data_source_views: [makeCoreDataSourceViewFilter(dataSourceView)], + node_ids, + parent_id, + }, + options: { limit, cursor: nextPageCursor ?? undefined }, + }); - if (coreRes.isErr()) { - return new Err(new Error(coreRes.error.message)); - } + if (coreRes.isErr()) { + return new Err(new Error(coreRes.error.message)); + } - const filteredNodes = removeCatchAllFoldersIfEmpty( - filterNodesByViewType(coreRes.value.nodes, viewType) - ); + const filteredNodes = removeCatchAllFoldersIfEmpty( + filterNodesByViewType(coreRes.value.nodes, viewType) + ); - resultNodes = [...resultNodes, ...filteredNodes].slice(0, limit); - nextPageCursor = coreRes.value.next_page_cursor; - } while (nextPageCursor && resultNodes.length < limit); + resultNodes = [...resultNodes, ...filteredNodes].slice(0, limit); + nextPageCursor = coreRes.value.next_page_cursor; return new Ok({ nodes: resultNodes.map((node) => From e5ba49b2a175cce444cf90c16329c308e2bf7de8 Mon Sep 17 00:00:00 2001 From: Aubin Date: Fri, 21 Feb 2025 17:13:31 +0100 Subject: [PATCH 4/6] update the hook to use cursor-based pagination --- front/lib/swr/data_source_views.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/front/lib/swr/data_source_views.ts b/front/lib/swr/data_source_views.ts index 65b1139d5f25..b359b2dd887a 100644 --- a/front/lib/swr/data_source_views.ts +++ b/front/lib/swr/data_source_views.ts @@ -3,12 +3,11 @@ import type { DataSourceViewType, LightWorkspaceType, } from "@dust-tt/types"; -import type { PaginationState } from "@tanstack/react-table"; import { useMemo } from "react"; import type { Fetcher, KeyedMutator, SWRConfiguration } from "swr"; +import type { CursorPaginationParams } from "@app/lib/api/pagination"; import { - appendPaginationParams, fetcher, fetcherMultiple, fetcherWithBody, @@ -113,7 +112,7 @@ export function useDataSourceViewContentNodes({ dataSourceView?: DataSourceViewType; internalIds?: string[]; parentId?: string; - pagination?: PaginationState; + pagination?: CursorPaginationParams; viewType?: ContentNodesViewType; disabled?: boolean; swrOptions?: SWRConfiguration; @@ -125,9 +124,15 @@ export function useDataSourceViewContentNodes({ mutateRegardlessOfQueryParams: KeyedMutator; nodes: GetDataSourceViewContentNodes["nodes"]; totalNodesCount: number; + nextPageCursor: string | null; } { const params = new URLSearchParams(); - appendPaginationParams(params, pagination); + if (pagination?.cursor) { + params.append("cursor", pagination.cursor); + } + if (pagination?.limit) { + params.append("limit", pagination.limit.toString()); + } const url = dataSourceView ? `/api/w/${owner.sId}/spaces/${dataSourceView.spaceId}/data_source_views/${dataSourceView.sId}/content-nodes?${params}` @@ -169,6 +174,7 @@ export function useDataSourceViewContentNodes({ mutateRegardlessOfQueryParams, nodes: useMemo(() => (data ? data.nodes : []), [data]), totalNodesCount: data ? data.total : 0, + nextPageCursor: data?.nextPageCursor || null, }; } From 1c79d5fcf0f9a415c8fd56b96be29c4f239c9ef8 Mon Sep 17 00:00:00 2001 From: Aubin Date: Fri, 21 Feb 2025 18:23:12 +0100 Subject: [PATCH 5/6] add a hook useCursorPaginationFromUrl in sparkle --- sparkle/src/hooks/usePaginationFromUrl.ts | 63 +++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/sparkle/src/hooks/usePaginationFromUrl.ts b/sparkle/src/hooks/usePaginationFromUrl.ts index ddc4339a23dd..ebf9d4072965 100644 --- a/sparkle/src/hooks/usePaginationFromUrl.ts +++ b/sparkle/src/hooks/usePaginationFromUrl.ts @@ -44,3 +44,66 @@ export const usePaginationFromUrl = ({ return res; }; + +interface CursorPaginationState { + cursor: string | null; + pageSize: number; +} + +interface UseCursorPaginationFromUrlProps { + urlPrefix?: string; + initialPageSize?: number; + defaultHistory?: HistoryOptions; +} + +export function useCursorPaginationFromUrl({ + urlPrefix = "", + initialPageSize = 25, + defaultHistory = "push", +}: UseCursorPaginationFromUrlProps = {}) { + const [cursorParam, setCursorParam] = useHashParam( + urlPrefix ? `${urlPrefix}Cursor` : "cursor" + ); + const [pageSizeParam, setPageSizeParam] = useHashParam( + urlPrefix ? `${urlPrefix}PageSize` : "pageSize" + ); + + const cursor = cursorParam || null; + const pageSize = pageSizeParam ? parseInt(pageSizeParam) : initialPageSize; + + return useMemo(() => { + const pagination: CursorPaginationState = { cursor, pageSize }; + + const setPagination = ( + newValue: CursorPaginationState, + history?: HistoryOptions + ) => { + if (newValue.cursor !== cursor || newValue.pageSize !== pageSize) { + if (newValue.cursor) { + setCursorParam(newValue.cursor, { + history: history ?? defaultHistory, + }); + } else { + setCursorParam(undefined, { + history: history ?? defaultHistory, + }); + } + + if (newValue.pageSize !== initialPageSize) { + setPageSizeParam(newValue.pageSize.toString()); + } else { + setPageSizeParam(undefined); + } + } + }; + + return { pagination, setPagination }; + }, [ + cursor, + pageSize, + initialPageSize, + setCursorParam, + setPageSizeParam, + defaultHistory, + ]); +} From 3eb36f80700cb75828c10a6cabc62aef489ef6f4 Mon Sep 17 00:00:00 2001 From: Aubin Date: Fri, 21 Feb 2025 18:23:39 +0100 Subject: [PATCH 6/6] hack around the pagination in the DataTable by keeping 2 states --- .../spaces/SpaceDataSourceViewContentList.tsx | 73 ++++++++++++++----- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/front/components/spaces/SpaceDataSourceViewContentList.tsx b/front/components/spaces/SpaceDataSourceViewContentList.tsx index 66e27c9c350c..6895c21ce9fe 100644 --- a/front/components/spaces/SpaceDataSourceViewContentList.tsx +++ b/front/components/spaces/SpaceDataSourceViewContentList.tsx @@ -1,4 +1,4 @@ -import type { MenuItem } from "@dust-tt/sparkle"; +import type { HistoryOptions, MenuItem } from "@dust-tt/sparkle"; import { Button, cn, @@ -11,7 +11,6 @@ import { SearchInput, Spinner, useHashParam, - usePaginationFromUrl, useSendNotification, } from "@dust-tt/sparkle"; import type { @@ -32,6 +31,7 @@ import { import type { CellContext, ColumnDef, + PaginationState, SortingState, } from "@tanstack/react-table"; import { useRouter } from "next/router"; @@ -175,8 +175,8 @@ function useStaticDataSourceViewHasContent({ owner, internalIds: parentId ? [parentId] : undefined, pagination: { - pageIndex: 0, - pageSize: 1, + cursor: null, + limit: 1, }, viewType, }); @@ -227,10 +227,16 @@ export const SpaceDataSourceViewContentList = ({ const [sorting, setSorting] = React.useState([]); const contentActionsRef = useRef(null); - const { pagination, setPagination } = usePaginationFromUrl({ + // State for cursor pagination (in URL). + const { cursorPagination, setCursorPagination } = useCursorPaginationFromUrl({ urlPrefix: "table", initialPageSize: 25, }); + // State for DataTable pagination. + const [tablePagination, setTablePagination] = useState({ + pageIndex: cursorPagination.cursor ? 1 : 0, + pageSize: cursorPagination.pageSize, + }); const [viewType, setViewType] = useHashParam( "viewType", DEFAULT_VIEW_TYPE @@ -248,14 +254,14 @@ export const SpaceDataSourceViewContentList = ({ const handleViewTypeChange = useCallback( (newViewType: ContentNodesViewType) => { if (newViewType !== viewType) { - setPagination( - { pageIndex: 0, pageSize: pagination.pageSize }, + setCursorPagination( + { cursor: null, pageSize: cursorPagination.pageSize }, "replace" ); setViewType(newViewType); } }, - [setPagination, setViewType, viewType, pagination.pageSize] + [setCursorPagination, setViewType, viewType, cursorPagination.pageSize] ); const { searchResultNodes, isSearchLoading, isSearchValidating } = @@ -266,10 +272,6 @@ export const SpaceDataSourceViewContentList = ({ search: debouncedSearch, }); - // TODO(20250127, nodes-core): turn to true and remove when implementing pagination - const isServerPagination = false; - // isFolder(dataSourceView.dataSource) && !dataSourceSearch; - const columns = useMemo( () => getTableColumns(showSpaceUsage), [showSpaceUsage] @@ -279,12 +281,16 @@ export const SpaceDataSourceViewContentList = ({ isNodesLoading, mutateRegardlessOfQueryParams: mutateContentNodes, nodes: childrenNodes, + nextPageCursor, totalNodesCount, } = useDataSourceViewContentNodes({ dataSourceView, owner, parentId, - pagination: isServerPagination ? pagination : undefined, + pagination: { + cursor: cursorPagination.cursor, + limit: cursorPagination.pageSize, + }, viewType: isValidContentNodesViewType(viewType) ? viewType : DEFAULT_VIEW_TYPE, @@ -325,6 +331,37 @@ export const SpaceDataSourceViewContentList = ({ viewType: "tables", }); + const handlePaginationChange = useCallback( + (newTablePagination: PaginationState) => { + if (newTablePagination.pageSize !== tablePagination.pageSize) { + // Handle page size change + setTablePagination(newTablePagination); + setCursorPagination({ + cursor: null, + pageSize: newTablePagination.pageSize, + }); + } else if ( + newTablePagination.pageIndex > tablePagination.pageIndex && + nextPageCursor + ) { + // Next page - use cursor + setTablePagination(newTablePagination); + setCursorPagination({ + cursor: nextPageCursor, + pageSize: tablePagination.pageSize, + }); + } else if (newTablePagination.pageIndex < tablePagination.pageIndex) { + // Previous page - reset cursor + setTablePagination(newTablePagination); + setCursorPagination({ + cursor: null, + pageSize: tablePagination.pageSize, + }); + } + }, + [tablePagination, nextPageCursor, setCursorPagination] + ); + const isDataSourceManaged = isManaged(dataSourceView.dataSource); const addToSpace = useCallback( @@ -569,8 +606,8 @@ export const SpaceDataSourceViewContentList = ({ placeholder="Search (Name)" value={dataSourceSearch} onChange={(s) => { - setPagination( - { pageIndex: 0, pageSize: pagination.pageSize }, + setCursorPagination( + { cursor: null, pageSize: cursorPagination.pageSize }, "replace" ); setDataSourceSearch(s); @@ -684,10 +721,10 @@ export const SpaceDataSourceViewContentList = ({ )} sorting={sorting} setSorting={setSorting} - totalRowCount={isServerPagination ? totalNodesCount : undefined} + totalRowCount={totalNodesCount} rowCountIsCapped={totalNodesCount === ROWS_COUNT_CAPPED} - pagination={pagination} - setPagination={setPagination} + pagination={tablePagination} + setPagination={handlePaginationChange} columnsBreakpoints={columnsBreakpoints} /> )}