Pagination View fixes for DSync User/Group Lists (#2572)

* Handle pagination query params correctly and set response header for pageToken

* Support for pageToken

* Revert tokenmap change for and add comment

* Exclude `log_webhook_events` checkbox while creating

* `pageToken` handling for WebhookLogs

* `pageToken` handling in API route

* Fix unit tests

* Fix test

* Update tokenmap using effect

components/dsync/CreateDirectory.tsx

@@ -47,7 +47,7 @@ const CreateDirectory = ({
           excludeFields={
             setupLinkToken
               ? ['name', 'tenant', 'product', 'webhook_url', 'webhook_secret', 'log_webhook_events']
-              : undefined
+              : ['log_webhook_events']
           }
           urls={{
             post: setupLinkToken

internal-ui/src/dsync/DirectoryGroups.tsx

@@ -1,13 +1,14 @@
 import useSWR from 'swr';
 import { useTranslation } from 'next-i18next';
 import EyeIcon from '@heroicons/react/24/outline/EyeIcon';
-import type { Group } from '../types';
+import type { ApiSuccess, Group } from '../types';
 import { fetcher, addQueryParamsToPath } from '../utils';
 import { DirectoryTab } from '../dsync';
 import { usePaginate, useDirectory } from '../hooks';
 import { TableBodyType } from '../shared/Table';
 import { Loading, Table, EmptyState, Error, Pagination, PageHeader, pageLimit } from '../shared';
 import { useRouter } from '../hooks';
+import { useEffect } from 'react';
 
 export const DirectoryGroups = ({
   urls,
@@ -18,7 +19,7 @@ export const DirectoryGroups = ({
 }) => {
   const { router } = useRouter();
   const { t } = useTranslation('common');
-  const { paginate, setPaginate, pageTokenMap } = usePaginate(router!);
+  const { paginate, setPaginate, pageTokenMap, setPageTokenMap } = usePaginate(router!);
 
   const params = {
     pageOffset: paginate.offset,
@@ -26,6 +27,7 @@ export const DirectoryGroups = ({
   };
 
   // For DynamoDB
+  // Use the (next)pageToken mapped to the previous page offset to get the current page
   if (paginate.offset > 0 && pageTokenMap[paginate.offset - pageLimit]) {
     params['pageToken'] = pageTokenMap[paginate.offset - pageLimit];
   }
@@ -33,7 +35,15 @@ export const DirectoryGroups = ({
   const getUrl = addQueryParamsToPath(urls.getGroups, params);
 
   const { directory, isLoadingDirectory, directoryError } = useDirectory(urls.getDirectory);
-  const { data, isLoading, error } = useSWR<{ data: Group[] }>(getUrl, fetcher);
+  const { data, isLoading, error } = useSWR<ApiSuccess<Group[]>>(getUrl, fetcher);
+
+  const nextPageToken = data?.pageToken;
+
+  useEffect(() => {
+    if (nextPageToken) {
+      setPageTokenMap((tokenMap) => ({ ...tokenMap, [paginate.offset]: nextPageToken }));
+    }
+  }, [nextPageToken, paginate.offset]);
 
   if (isLoading || isLoadingDirectory) {
     return <Loading />;

internal-ui/src/dsync/DirectoryUsers.tsx

@@ -1,13 +1,14 @@
 import useSWR from 'swr';
 import { useTranslation } from 'next-i18next';
 import EyeIcon from '@heroicons/react/24/outline/EyeIcon';
-import type { User } from '../types';
+import type { ApiSuccess, User } from '../types';
 import { addQueryParamsToPath, fetcher } from '../utils';
 import { DirectoryTab } from '../dsync';
 import { usePaginate, useDirectory } from '../hooks';
 import { TableBodyType } from '../shared/Table';
 import { Loading, Table, EmptyState, Error, Pagination, PageHeader, pageLimit } from '../shared';
 import { useRouter } from '../hooks';
+import { useEffect } from 'react';
 
 export const DirectoryUsers = ({
   urls,
@@ -18,7 +19,7 @@ export const DirectoryUsers = ({
 }) => {
   const { router } = useRouter();
   const { t } = useTranslation('common');
-  const { paginate, setPaginate, pageTokenMap } = usePaginate(router!);
+  const { paginate, setPaginate, pageTokenMap, setPageTokenMap } = usePaginate(router!);
 
   const params = {
     pageOffset: paginate.offset,
@@ -26,6 +27,7 @@ export const DirectoryUsers = ({
   };
 
   // For DynamoDB
+  // Use the (next)pageToken mapped to the previous page offset to get the current page
   if (paginate.offset > 0 && pageTokenMap[paginate.offset - pageLimit]) {
     params['pageToken'] = pageTokenMap[paginate.offset - pageLimit];
   }
@@ -33,7 +35,15 @@ export const DirectoryUsers = ({
   const getUrl = addQueryParamsToPath(urls.getUsers, params);
 
   const { directory, isLoadingDirectory, directoryError } = useDirectory(urls.getDirectory);
-  const { data, isLoading, error } = useSWR<{ data: User[] }>(getUrl, fetcher);
+  const { data, isLoading, error } = useSWR<ApiSuccess<User[]>>(getUrl, fetcher);
+
+  const nextPageToken = data?.pageToken;
+
+  useEffect(() => {
+    if (nextPageToken) {
+      setPageTokenMap((tokenMap) => ({ ...tokenMap, [paginate.offset]: nextPageToken }));
+    }
+  }, [nextPageToken, paginate.offset]);
 
   if (isLoading || isLoadingDirectory) {
     return <Loading />;

internal-ui/src/dsync/DirectoryWebhookLogs.tsx

@@ -1,8 +1,8 @@
 import useSWR from 'swr';
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
 import { useTranslation } from 'next-i18next';
 import EyeIcon from '@heroicons/react/24/outline/EyeIcon';
-import type { WebhookEventLog } from '../types';
+import { ApiSuccess, type WebhookEventLog } from '../types';
 import { fetcher, addQueryParamsToPath } from '../utils';
 import { DirectoryTab } from '../dsync';
 import { usePaginate, useDirectory } from '../hooks';
@@ -32,7 +32,7 @@ export const DirectoryWebhookLogs = ({
   const { router } = useRouter();
   const { t } = useTranslation('common');
   const [delModalVisible, setDelModalVisible] = useState(false);
-  const { paginate, setPaginate, pageTokenMap } = usePaginate(router!);
+  const { paginate, setPaginate, pageTokenMap, setPageTokenMap } = usePaginate(router!);
 
   const params = {
     pageOffset: paginate.offset,
@@ -40,6 +40,7 @@ export const DirectoryWebhookLogs = ({
   };
 
   // For DynamoDB
+  // Use the (next)pageToken mapped to the previous page offset to get the current page
   if (paginate.offset > 0 && pageTokenMap[paginate.offset - pageLimit]) {
     params['pageToken'] = pageTokenMap[paginate.offset - pageLimit];
   }
@@ -47,7 +48,15 @@ export const DirectoryWebhookLogs = ({
   const getUrl = addQueryParamsToPath(urls.getEvents, params);
 
   const { directory, isLoadingDirectory, directoryError } = useDirectory(urls.getDirectory);
-  const { data, isLoading, error } = useSWR<{ data: WebhookEventLog[] }>(getUrl, fetcher);
+  const { data, isLoading, error } = useSWR<ApiSuccess<WebhookEventLog[]>>(getUrl, fetcher);
+
+  const nextPageToken = data?.pageToken;
+
+  useEffect(() => {
+    if (nextPageToken) {
+      setPageTokenMap((tokenMap) => ({ ...tokenMap, [paginate.offset]: nextPageToken }));
+    }
+  }, [nextPageToken, paginate.offset]);
 
   if (isLoading || isLoadingDirectory) {
     return <Loading />;

internal-ui/src/federated-saml/FederatedSAMLApps.tsx

@@ -17,6 +17,7 @@ import { TableBodyType } from '../shared/Table';
 import { pageLimit } from '../shared/Pagination';
 import { usePaginate } from '../hooks';
 import { useRouter } from '../hooks';
+import { useEffect } from 'react';
 
 type ExcludeFields = keyof Pick<SAMLFederationApp, 'product'>;
 
@@ -35,7 +36,7 @@ export const FederatedSAMLApps = ({
 }) => {
   const { router } = useRouter();
   const { t } = useTranslation('common');
-  const { paginate, setPaginate, pageTokenMap } = usePaginate(router!);
+  const { paginate, setPaginate, pageTokenMap, setPageTokenMap } = usePaginate(router!);
 
   let getAppsUrl = `${urls.getApps}?pageOffset=${paginate.offset}&pageLimit=${pageLimit}`;
 
@@ -44,7 +45,18 @@ export const FederatedSAMLApps = ({
     getAppsUrl += `&pageToken=${pageTokenMap[paginate.offset - pageLimit]}`;
   }
 
-  const { data, isLoading, error } = useSWR<{ data: SAMLFederationApp[] }>(getAppsUrl, fetcher);
+  const { data, isLoading, error } = useSWR<{ data: SAMLFederationApp[]; pageToken?: string }>(
+    getAppsUrl,
+    fetcher
+  );
+
+  const nextPageToken = data?.pageToken;
+
+  useEffect(() => {
+    if (nextPageToken) {
+      setPageTokenMap((tokenMap) => ({ ...tokenMap, [paginate.offset]: nextPageToken }));
+    }
+  }, [nextPageToken, paginate.offset]);
 
   if (isLoading) {
     return <Loading />;

internal-ui/src/setup-link/SetupLinks.tsx

@@ -1,5 +1,5 @@
 import useSWR from 'swr';
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
 import { useTranslation } from 'next-i18next';
 import EyeIcon from '@heroicons/react/24/outline/EyeIcon';
 import TrashIcon from '@heroicons/react/24/outline/TrashIcon';
@@ -51,7 +51,7 @@ export const SetupLinks = ({
   const [showSetupLink, setShowSetupLink] = useState(false);
   const [showRegenModal, setShowRegenModal] = useState(false);
   const [setupLink, setSetupLink] = useState<SetupLink | null>(null);
-  const { paginate, setPaginate, pageTokenMap } = usePaginate(router!);
+  const { paginate, setPaginate, pageTokenMap, setPageTokenMap } = usePaginate(router!);
 
   const params = {
     pageOffset: paginate.offset,
@@ -65,7 +65,18 @@ export const SetupLinks = ({
   }
 
   const getLinksUrl = addQueryParamsToPath(urls.getLinks, params);
-  const { data, isLoading, error, mutate } = useSWR<{ data: SetupLink[] }>(getLinksUrl, fetcher);
+  const { data, isLoading, error, mutate } = useSWR<{ data: SetupLink[]; pageToken?: string }>(
+    getLinksUrl,
+    fetcher
+  );
+
+  const nextPageToken = data?.pageToken;
+
+  useEffect(() => {
+    if (nextPageToken) {
+      setPageTokenMap((tokenMap) => ({ ...tokenMap, [paginate.offset]: nextPageToken }));
+    }
+  }, [nextPageToken, paginate.offset]);
 
   if (isLoading) {
     return <Loading />;

npm/src/directory-sync/scim/Groups.ts

@@ -1,6 +1,13 @@
 import { randomUUID } from 'crypto';
 
-import type { Group, DatabaseStore, PaginationParams, Response, GroupMembership } from '../../typings';
+import type {
+  Group,
+  DatabaseStore,
+  PaginationParams,
+  Response,
+  GroupMembership,
+  Records,
+} from '../../typings';
 import * as dbutils from '../../db/utils';
 import { apiError, JacksonError } from '../../controller/error';
 import { Base } from './Base';
@@ -238,10 +245,10 @@ export class Groups extends Base {
       directoryId?: string;
     }
   ): Promise<Response<Group[]>> {
-    const { pageOffset, pageLimit, directoryId } = params;
+    const { pageOffset, pageLimit, pageToken, directoryId } = params;
 
     try {
-      let groups: Group[] = [];
+      let result: Records;
 
       // Filter by directoryId
       if (directoryId) {
@@ -250,12 +257,12 @@ export class Groups extends Base {
           value: directoryId,
         };
 
-        groups = (await this.store('groups').getByIndex(index, pageOffset, pageLimit)).data;
+        result = await this.store('groups').getByIndex(index, pageOffset, pageLimit, pageToken);
       } else {
-        groups = (await this.store('groups').getAll(pageOffset, pageLimit)).data;
+        result = await this.store('groups').getAll(pageOffset, pageLimit, pageToken);
       }
 
-      return { data: groups, error: null };
+      return { data: result.data, error: null, pageToken: result.pageToken };
     } catch (err: any) {
       return apiError(err);
     }

npm/src/directory-sync/scim/Users.ts

@@ -1,4 +1,4 @@
-import type { User, DatabaseStore, PaginationParams, Response } from '../../typings';
+import type { User, DatabaseStore, PaginationParams, Response, Records } from '../../typings';
 import { apiError, JacksonError } from '../../controller/error';
 import { Base } from './Base';
 import { keyFromParts } from '../../db/utils';
@@ -180,30 +180,29 @@ export class Users extends Base {
   public async getAll({
     pageOffset,
     pageLimit,
+    pageToken,
     directoryId,
   }: PaginationParams & {
     directoryId?: string;
   } = {}): Promise<Response<User[]>> {
     try {
-      let users: User[] = [];
-
+      let result: Records;
       // Filter by directoryId
       if (directoryId) {
-        users = (
-          await this.store('users').getByIndex(
-            {
-              name: indexNames.directoryId,
-              value: directoryId,
-            },
-            pageOffset,
-            pageLimit
-          )
-        ).data as User[];
+        result = await this.store('users').getByIndex(
+          {
+            name: indexNames.directoryId,
+            value: directoryId,
+          },
+          pageOffset,
+          pageLimit,
+          pageToken
+        );
       } else {
-        users = (await this.store('users').getAll(pageOffset, pageLimit)).data;
+        result = await this.store('users').getAll(pageOffset, pageLimit, pageToken);
       }
 
-      return { data: users, error: null };
+      return { data: result.data, error: null, pageToken: result.pageToken };
     } catch (err: any) {
       return apiError(err);
     }

npm/src/directory-sync/scim/WebhookEventsLogger.ts

@@ -6,6 +6,7 @@ import type {
   WebhookEventLog,
   DirectorySyncEvent,
   PaginationParams,
+  Records,
 } from '../../typings';
 import { Base } from './Base';
 import { webhookLogsTTL } from '../utils';
@@ -125,9 +126,9 @@ export class WebhookEventsLogger extends Base {
    */
   // Get the event logs for a directory paginated
   public async getAll(params: GetAllParams = {}) {
-    const { pageOffset, pageLimit, directoryId } = params;
+    const { pageOffset, pageLimit, pageToken, directoryId } = params;
 
-    let eventLogs: WebhookEventLog[] = [];
+    let result: Records<WebhookEventLog>;
 
     if (directoryId) {
       const index = {
@@ -135,12 +136,12 @@ export class WebhookEventsLogger extends Base {
         value: directoryId,
       };
 
-      eventLogs = (await this.eventStore().getByIndex(index, pageOffset, pageLimit)).data;
+      result = await this.eventStore().getByIndex(index, pageOffset, pageLimit, pageToken);
     } else {
-      eventLogs = (await this.eventStore().getAll(pageOffset, pageLimit)).data;
+      result = await this.eventStore().getAll(pageOffset, pageLimit, pageToken);
     }
 
-    return eventLogs;
+    return { data: result.data, pageToken: result.pageToken };
   }
 
   public async delete(id: string) {

npm/src/directory-sync/types.ts

@@ -178,7 +178,7 @@ export type GroupMembership = {
   user_id: string;
 };
 
-export type Response<T> = { data: T; error: null } | { data: null; error: ApiError };
+export type Response<T> = { data: T; error: null; pageToken?: string } | { data: null; error: ApiError };
 
 export type EventCallback = (event: DirectorySyncEvent) => Promise<void>;
 

npm/test/dsync/batch/webhooks.test.ts

@@ -140,7 +140,7 @@ tap.test('Event batching', async (t) => {
   });
 
   t.test('Should log the webhook events if logging is enabled', async (t) => {
-    const logs = await directorySync.webhookLogs
+    const { data: logs } = await directorySync.webhookLogs
       .setTenantAndProduct(directory1Payload.tenant, directory1Payload.product)
       .getAll();
 
@@ -157,7 +157,7 @@ tap.test('Event batching', async (t) => {
   });
 
   t.test('Should not log the webhook events if logging is disabled', async (t) => {
-    const logs = await directorySync.webhookLogs
+    const { data: logs } = await directorySync.webhookLogs
       .setTenantAndProduct(directory2Payload.tenant, directory2Payload.product)
       .getAll();
 

npm/test/dsync/webhooks.test.ts

@@ -78,7 +78,7 @@ tap.test('Webhook Events /', async (t) => {
       // Create a user
       await directorySync.requests.handle(usersRequest.create(directory, users[0]));
 
-      const events = await directorySync.webhookLogs.getAll();
+      const { data: events } = await directorySync.webhookLogs.getAll();
 
       t.equal(events.length, 0);
 
@@ -100,7 +100,7 @@ tap.test('Webhook Events /', async (t) => {
       // Create a user
       await directorySync.requests.handle(usersRequest.create(directory, users[0]));
 
-      const events = await directorySync.webhookLogs.getAll();
+      const { data: events } = await directorySync.webhookLogs.getAll();
 
       t.equal(events.length, 0);
 
@@ -114,7 +114,7 @@ tap.test('Webhook Events /', async (t) => {
       // Create a user
       await directorySync.requests.handle(usersRequest.create(directory, users[0]));
 
-      const logs = await directorySync.webhookLogs.getAll();
+      const { data: logs } = await directorySync.webhookLogs.getAll();
 
       const log = await directorySync.webhookLogs.get(logs[0].id);
 
@@ -144,7 +144,7 @@ tap.test('Webhook Events /', async (t) => {
       mock.verify();
       mock.restore();
 
-      const logs = await directorySync.webhookLogs.getAll();
+      const { data: logs } = await directorySync.webhookLogs.getAll();
 
       t.ok(logs);
       t.equal(logs.length, 3);
@@ -193,7 +193,7 @@ tap.test('Webhook Events /', async (t) => {
       mock.verify();
       mock.restore();
 
-      const logs = await directorySync.webhookLogs.getAll();
+      const { data: logs } = await directorySync.webhookLogs.getAll();
 
       t.ok(logs);
       t.equal(logs.length, 3);
@@ -251,7 +251,7 @@ tap.test('Webhook Events /', async (t) => {
     mock.verify();
     mock.restore();
 
-    const logs = await directorySync.webhookLogs.getAll();
+    const { data: logs } = await directorySync.webhookLogs.getAll();
 
     t.ok(logs);
     t.equal(logs.length, 4);

pages/api/admin/directory-sync/[directoryId]/events/index.ts

@@ -2,6 +2,7 @@ import type { NextApiRequest, NextApiResponse } from 'next';
 import jackson from '@lib/jackson';
 import { defaultHandler } from '@lib/api';
 import { ApiError } from '@lib/error';
+import { parsePaginateApiParams } from '@lib/utils';
 
 const handler = async (req: NextApiRequest, res: NextApiResponse) => {
   await defaultHandler(req, res, {
@@ -14,7 +15,9 @@ const handler = async (req: NextApiRequest, res: NextApiResponse) => {
 const handleGET = async (req: NextApiRequest, res: NextApiResponse) => {
   const { directorySyncController } = await jackson();
 
-  const { directoryId, offset, limit } = req.query as { directoryId: string; offset: string; limit: string };
+  const { directoryId } = req.query as { directoryId: string };
+
+  const { pageOffset, pageLimit, pageToken } = parsePaginateApiParams(req.query);
 
   const { data: directory, error } = await directorySyncController.directories.get(directoryId);
 
@@ -22,18 +25,20 @@ const handleGET = async (req: NextApiRequest, res: NextApiResponse) => {
     throw new ApiError(error.message, error.code);
   }
 
-  const pageOffset = parseInt(offset);
-  const pageLimit = parseInt(limit);
-
-  const events = await directorySyncController.webhookLogs
+  const result = await directorySyncController.webhookLogs
     .setTenantAndProduct(directory.tenant, directory.product)
     .getAll({
       pageOffset,
       pageLimit,
+      pageToken,
       directoryId,
     });
 
-  res.json({ data: events });
+  if (result.pageToken) {
+    res.setHeader('jackson-pagetoken', result.pageToken);
+  }
+
+  res.json({ data: result.data });
 };
 
 // Delete all events

pages/api/admin/directory-sync/[directoryId]/groups/index.ts

@@ -2,6 +2,7 @@ import type { NextApiRequest, NextApiResponse } from 'next';
 import jackson from '@lib/jackson';
 import { defaultHandler } from '@lib/api';
 import { ApiError } from '@lib/error';
+import { parsePaginateApiParams } from '@lib/utils';
 
 const handler = async (req: NextApiRequest, res: NextApiResponse) => {
   await defaultHandler(req, res, {
@@ -13,7 +14,9 @@ const handler = async (req: NextApiRequest, res: NextApiResponse) => {
 const handleGET = async (req: NextApiRequest, res: NextApiResponse) => {
   const { directorySyncController } = await jackson();
 
-  const { directoryId, offset, limit } = req.query as { directoryId: string; offset: string; limit: string };
+  const { directoryId } = req.query as { directoryId: string };
+
+  const { pageOffset, pageLimit, pageToken } = parsePaginateApiParams(req.query);
 
   const { data: directory, error } = await directorySyncController.directories.get(directoryId);
 
@@ -21,18 +24,17 @@ const handleGET = async (req: NextApiRequest, res: NextApiResponse) => {
     throw new ApiError(error.message, error.code);
   }
 
-  const pageOffset = parseInt(offset);
-  const pageLimit = parseInt(limit);
-
-  const { data: groups, error: groupsError } = await directorySyncController.groups
+  const result = await directorySyncController.groups
     .setTenantAndProduct(directory.tenant, directory.product)
-    .getAll({ pageOffset, pageLimit, directoryId });
+    .getAll({ pageOffset, pageLimit, pageToken, directoryId });
 
-  if (groupsError) {
-    throw new ApiError(groupsError.message, groupsError.code);
+  if (result.error) {
+    throw new ApiError(result.error.message, result.error.code);
+  } else if (result.pageToken) {
+    res.setHeader('jackson-pagetoken', result.pageToken);
   }
 
-  res.json({ data: groups });
+  res.json({ data: result.data });
 };
 
 export default handler;

pages/api/admin/directory-sync/[directoryId]/users/index.ts

@@ -2,6 +2,7 @@ import type { NextApiRequest, NextApiResponse } from 'next';
 import jackson from '@lib/jackson';
 import { defaultHandler } from '@lib/api';
 import { ApiError } from '@lib/error';
+import { parsePaginateApiParams } from '@lib/utils';
 
 const handler = async (req: NextApiRequest, res: NextApiResponse) => {
   await defaultHandler(req, res, {
@@ -13,7 +14,11 @@ const handler = async (req: NextApiRequest, res: NextApiResponse) => {
 const handleGET = async (req: NextApiRequest, res: NextApiResponse) => {
   const { directorySyncController } = await jackson();
 
-  const { directoryId, offset, limit } = req.query as { directoryId: string; offset: string; limit: string };
+  const { directoryId } = req.query as {
+    directoryId: string;
+  };
+
+  const { pageOffset, pageLimit, pageToken } = parsePaginateApiParams(req.query);
 
   const { data: directory, error } = await directorySyncController.directories.get(directoryId);
 
@@ -21,18 +26,17 @@ const handleGET = async (req: NextApiRequest, res: NextApiResponse) => {
     throw new ApiError(error.message, error.code);
   }
 
-  const pageOffset = parseInt(offset);
-  const pageLimit = parseInt(limit);
-
-  const { data: users, error: usersError } = await directorySyncController.users
+  const result = await directorySyncController.users
     .setTenantAndProduct(directory.tenant, directory.product)
-    .getAll({ pageOffset, pageLimit, directoryId });
+    .getAll({ pageOffset, pageLimit, pageToken, directoryId });
 
-  if (usersError) {
-    throw new ApiError(usersError.message, usersError.code);
+  if (result.error) {
+    throw new ApiError(result.error.message, result.error.code);
+  } else if (result.pageToken) {
+    res.setHeader('jackson-pagetoken', result.pageToken);
   }
 
-  res.json({ data: users });
+  res.json({ data: result.data });
 };
 
 export default handler;