Skip to content

fix: create centralized Pagination component #10911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 92 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
92 commits
Select commit Hold shift + click to select a range
b4211f3
wip: commit current progress on usePaginatedQuery
Parkreiner Nov 13, 2023
173e823
chore: add cacheTime to users query
Parkreiner Nov 13, 2023
d715d73
chore: update cache logic for UsersPage usersQuery
Parkreiner Nov 13, 2023
8a199b7
wip: commit progress on Pagination
Parkreiner Nov 13, 2023
98ae96d
chore: add function overloads to prepareQuery
Parkreiner Nov 13, 2023
37ea50b
wip: commit progress on usePaginatedQuery
Parkreiner Nov 14, 2023
2c1e9e3
docs: add clarifying comment about implementation
Parkreiner Nov 14, 2023
b3a9ab4
chore: remove optional prefetch property from query options
Parkreiner Nov 15, 2023
39a2ced
chore: redefine queryKey
Parkreiner Nov 15, 2023
4dcfd29
refactor: consolidate how queryKey/queryFn are called
Parkreiner Nov 15, 2023
86f8437
refactor: clean up pagination code more
Parkreiner Nov 15, 2023
f612d8f
fix: remove redundant properties
Parkreiner Nov 15, 2023
5878326
refactor: clean up code
Parkreiner Nov 15, 2023
5cc1c2d
wip: commit progress on usePaginatedQuery
Parkreiner Nov 15, 2023
0138f21
wip: commit current pagination progress
Parkreiner Nov 15, 2023
a38fd30
docs: clean up comments for clarity
Parkreiner Nov 17, 2023
22d6c24
wip: get type signatures compatible (breaks runtime logic slightly)
Parkreiner Nov 20, 2023
e717da1
refactor: clean up type definitions
Parkreiner Nov 20, 2023
7624d94
chore: add support for custom onInvalidPage functions
Parkreiner Nov 20, 2023
2623131
refactor: clean up type definitions more for clarity reasons
Parkreiner Nov 20, 2023
29242e9
chore: delete Pagination component (separate PR)
Parkreiner Nov 20, 2023
5a9aa2d
chore: remove cacheTime fixes (to be resolved in future PR)
Parkreiner Nov 20, 2023
3d67304
docs: add clarifying/intellisense comments for DX
Parkreiner Nov 20, 2023
d977060
refactor: link users queries to same queryKey implementation
Parkreiner Nov 20, 2023
9224624
docs: remove misleading comment
Parkreiner Nov 20, 2023
540c779
docs: more comments
Parkreiner Nov 20, 2023
4b42b6f
chore: update onInvalidPage params for more flexibility
Parkreiner Nov 20, 2023
5bc43c9
fix: remove explicit any
Parkreiner Nov 20, 2023
bd146a1
refactor: clean up type definitions
Parkreiner Nov 20, 2023
983b83f
refactor: rename query params for consistency
Parkreiner Nov 20, 2023
a43e294
refactor: clean up input validation for page changes
Parkreiner Nov 20, 2023
db03f3e
refactor/fix: update hook to be aware of async data
Parkreiner Nov 21, 2023
1b825f1
chore: add contravariance to dictionary
Parkreiner Nov 21, 2023
9631a27
refactor: increase type-safety of usePaginatedQuery
Parkreiner Nov 21, 2023
4ea0cba
docs: more comments
Parkreiner Nov 21, 2023
3f63ec7
chore: move usePaginatedQuery file
Parkreiner Nov 21, 2023
614e4ff
fix: add back cacheTime
Parkreiner Nov 21, 2023
b0c8d48
Merge branch 'main' into mes/pagination-2
Parkreiner Nov 21, 2023
e994532
chore: swap in usePaginatedQuery for users table
Parkreiner Nov 21, 2023
9502044
chore: add goToFirstPage to usePaginatedQuery
Parkreiner Nov 21, 2023
8dbbfd3
fix: make page redirects work properly
Parkreiner Nov 21, 2023
88d0a5f
refactor: clean up clamp logic
Parkreiner Nov 21, 2023
132f5b1
chore: swap in usePaginatedQuery for Audits table
Parkreiner Nov 21, 2023
33bf4e8
refactor: move dependencies around
Parkreiner Nov 21, 2023
218da68
fix: remove deprecated properties from hook
Parkreiner Nov 21, 2023
54f01f1
refactor: clean up code more
Parkreiner Nov 21, 2023
ede2abc
docs: add todo comment
Parkreiner Nov 21, 2023
9ecab16
chore: update testing fixtures
Parkreiner Nov 21, 2023
0c81d1c
wip: commit current progress for tests
Parkreiner Nov 21, 2023
3aa714c
fix: update useEffectEvent to sync via layout effects
Parkreiner Nov 21, 2023
2893630
wip: commit more progress on tests
Parkreiner Nov 21, 2023
ef900d4
wip: stub out all expected test cases
Parkreiner Nov 21, 2023
23dc583
wip: more test progress
Parkreiner Nov 22, 2023
24361d1
wip: more test progress
Parkreiner Nov 22, 2023
755f8c0
wip: commit more test progress
Parkreiner Nov 22, 2023
8bff0d3
wip: AHHHHHHHH
Parkreiner Nov 22, 2023
1fae773
chore: finish two more test cases
Parkreiner Nov 22, 2023
be82728
wip: add in all tests (still need to investigate prefetching
Parkreiner Nov 22, 2023
848fa0f
refactor: clean up code slightly
Parkreiner Nov 22, 2023
c89e8e3
fix: remove math bugs when calculating pages
Parkreiner Nov 22, 2023
3624a6d
fix: wrap up all testing and clean up cases
Parkreiner Nov 22, 2023
13e2f30
docs: update comments for clarity
Parkreiner Nov 22, 2023
8f83673
fix: update error-handling for invalid page handling
Parkreiner Nov 24, 2023
2b111a6
wip: commit progress for auto-scroll container
Parkreiner Nov 24, 2023
e9a99d4
chore: integrate pagination into users table
Parkreiner Nov 24, 2023
5bf5b9c
wip: commit current progress on scroll logic
Parkreiner Nov 24, 2023
9eebb20
wip: more attempts
Parkreiner Nov 25, 2023
3d39835
wip: more progress
Parkreiner Nov 25, 2023
d817cc5
wip: more progress
Parkreiner Nov 25, 2023
d36dca7
refactor: clean up scroll sync logic
Parkreiner Nov 27, 2023
3961ec1
docs: add big comment explainign syncScrollChange
Parkreiner Nov 27, 2023
c7e94dd
fix: make sure effects run properly
Parkreiner Nov 27, 2023
66fc6ba
fix: finalize effect sync logic
Parkreiner Nov 27, 2023
7d660a4
fix: add on-mount logic for effects
Parkreiner Nov 27, 2023
a03e167
fix: remove autoScroll property (not a good enough use case)
Parkreiner Nov 27, 2023
4ffacfb
refactor: clean up code
Parkreiner Nov 27, 2023
e98ab1b
fix: make disabled messages have consistent verbage
Parkreiner Nov 27, 2023
8fb2703
refactor: quarantine useEffect evil
Parkreiner Nov 27, 2023
1fea4bc
refactor: revise Pagination implementation
Parkreiner Nov 27, 2023
12b3790
chore: get Pagination component integrated into users table
Parkreiner Nov 27, 2023
b736e0d
refactor: make updated stories happy
Parkreiner Nov 27, 2023
71cd29f
fix: update renderComponent to support proper re-renders
Parkreiner Nov 28, 2023
d0bd797
wip: commit current test progress
Parkreiner Nov 28, 2023
07097b1
chore: finish Pagination tests
Parkreiner Nov 28, 2023
43cb93a
fix: beef up test to check for every possible user event
Parkreiner Nov 28, 2023
80c88bb
fix: remove stray line of code
Parkreiner Nov 28, 2023
b4abf11
refactor: change timing for Pagination.test (just to improve assurity)
Parkreiner Nov 28, 2023
983915b
fix: update logic for calculating disabled statuses
Parkreiner Nov 28, 2023
beccc68
refactor: split off PaginationHeader into separate file
Parkreiner Nov 28, 2023
982a8c4
fix: switch AuditPage to use Pagination component
Parkreiner Nov 28, 2023
73c24a0
fix: update stories for audit page
Parkreiner Nov 28, 2023
a5fc929
fix: update vertical spacing for workspaces table
Parkreiner Nov 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"coderdenttest",
"coderdtest",
"codersdk",
"contravariance",
"cronstrue",
"databasefake",
"dbmem",
Expand Down
23 changes: 23 additions & 0 deletions site/src/api/queries/audits.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { getAuditLogs } from "api/api";
import { type AuditLogResponse } from "api/typesGenerated";
import { useFilterParamsKey } from "components/Filter/filter";
import { type UsePaginatedQueryOptions } from "hooks/usePaginatedQuery";

export function paginatedAudits(searchParams: URLSearchParams) {
return {
searchParams,
queryPayload: () => searchParams.get(useFilterParamsKey) ?? "",
queryKey: ({ payload, pageNumber }) => {
return ["auditLogs", payload, pageNumber] as const;
},
queryFn: ({ payload, limit, offset }) => {
return getAuditLogs({
offset,
limit,
q: payload,
});
},

cacheTime: 5 * 1000 * 60,
} as const satisfies UsePaginatedQueryOptions<AuditLogResponse, string>;
}
27 changes: 25 additions & 2 deletions site/src/api/queries/users.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { QueryClient, type UseQueryOptions } from "react-query";
import * as API from "api/api";
import {
import type {
AuthorizationRequest,
GetUsersResponse,
UpdateUserPasswordRequest,
Expand All @@ -10,11 +10,34 @@ import {
} from "api/typesGenerated";
import { getAuthorizationKey } from "./authCheck";
import { getMetadataAsJSON } from "utils/metadata";
import { type UsePaginatedQueryOptions } from "hooks/usePaginatedQuery";
import { prepareQuery } from "utils/filters";

export function usersKey(req: UsersRequest) {
return ["users", req] as const;
}

export function paginatedUsers() {
return {
queryPayload: ({ limit, offset, searchParams }) => {
return {
limit,
offset,
q: prepareQuery(searchParams.get("filter") ?? ""),
};
},

queryKey: ({ payload }) => usersKey(payload),
queryFn: ({ payload, signal }) => API.getUsers(payload, signal),
cacheTime: 5 * 1000 * 60,
} as const satisfies UsePaginatedQueryOptions<GetUsersResponse, UsersRequest>;
}

export const users = (req: UsersRequest): UseQueryOptions<GetUsersResponse> => {
return {
queryKey: ["users", req],
queryKey: usersKey(req),
queryFn: ({ signal }) => API.getUsers(req, signal),
cacheTime: 5 * 1000 * 60,
};
};

Expand Down
2 changes: 1 addition & 1 deletion site/src/components/Filter/filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type UseFilterConfig = {
onUpdate?: (newValue: string) => void;
};

const useFilterParamsKey = "filter";
export const useFilterParamsKey = "filter";

export const useFilter = ({
fallbackFilter = "",
Expand Down
243 changes: 243 additions & 0 deletions site/src/components/PaginationWidget/Pagination.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
import { type ComponentProps, type HTMLAttributes } from "react";
import { type PaginationResult, Pagination } from "./Pagination";

import { renderComponent } from "testHelpers/renderHelpers";
import { fireEvent, waitFor } from "@testing-library/react";

beforeAll(() => {
jest.useFakeTimers();
});

afterAll(() => {
jest.clearAllMocks();
jest.useRealTimers();
});

type ResultBase = Omit<
PaginationResult,
"isPreviousData" | "currentChunk" | "totalRecords" | "totalPages"
>;

const mockPaginationResult: ResultBase = {
isSuccess: false,
currentPage: 1,
limit: 25,
hasNextPage: false,
hasPreviousPage: false,
goToPreviousPage: () => {},
goToNextPage: () => {},
goToFirstPage: () => {},
onPageChange: () => {},
};

const initialRenderResult: PaginationResult = {
...mockPaginationResult,
isSuccess: false,
isPreviousData: false,
currentChunk: undefined,
hasNextPage: false,
hasPreviousPage: false,
totalRecords: undefined,
totalPages: undefined,
};

const successResult: PaginationResult = {
...mockPaginationResult,
isSuccess: true,
isPreviousData: false,
currentChunk: 1,
totalPages: 1,
totalRecords: 4,
};

type TestProps = Omit<
ComponentProps<typeof Pagination>,
keyof HTMLAttributes<HTMLDivElement>
>;

const mockUnitLabel = "ducks";

function render(props: TestProps) {
return renderComponent(<Pagination {...props} />);
}

function assertNoScroll(mockScroll: jest.SpyInstance) {
setTimeout(() => {
expect(mockScroll).not.toBeCalled();
}, 5000);

return jest.runAllTimersAsync();
}

async function mountWithSuccess(mockScroll: jest.SpyInstance) {
// eslint-disable-next-line testing-library/render-result-naming-convention -- Forced destructuring just makes this awkward
const result = render({
paginationUnitLabel: mockUnitLabel,
paginationResult: successResult,
});

await assertNoScroll(mockScroll);
return result;
}

/**
* Expected state transitions:
*
* 1. Initial render - isPreviousData is false, while currentPage can be any
* number (but will usually be 1)
* 1. Re-render from first-ever page loading in - currentPage stays the same,
* while isPreviousData stays false (data changes elsewhere in the app,
* though)
* 2. Re-render from user changing the page - currentPage becomes the new page,
* while isPreviousData depends on cache state
* 1. Change to page that's already been fetched - isPreviousData is false
* 2. Change to new page - isPreviousData is true during the transition
* 3. Re-render fetch for new page succeeding - currentPage stays the same, but
* isPreviousData flips from true to false
*/
describe(`${Pagination.name}`, () => {
describe("Initial render", () => {
it("Does absolutely nothing - should not scroll on component mount because that will violently hijack the user's browser", async () => {
const mockScroll = jest.spyOn(window, "scrollTo");

render({
paginationUnitLabel: mockUnitLabel,
paginationResult: initialRenderResult,
});

await assertNoScroll(mockScroll);
});
});

describe("Responding to page changes", () => {
it("Triggers scroll immediately if currentPage changes and isPreviousData is immediately false (previous query is cached)", async () => {
const mockScroll = jest.spyOn(window, "scrollTo");
const { rerender } = await mountWithSuccess(mockScroll);

rerender(
<Pagination
paginationUnitLabel={mockUnitLabel}
paginationResult={{
...successResult,
currentPage: 2,
isPreviousData: false,
}}
/>,
);

await waitFor(() => expect(mockScroll).toBeCalled());
});

it("Does nothing observable if page changes and isPreviousData is true (scroll will get queued, but will not be processed)", async () => {
const mockScroll = jest.spyOn(window, "scrollTo");
const { rerender } = await mountWithSuccess(mockScroll);

rerender(
<Pagination
paginationUnitLabel={mockUnitLabel}
paginationResult={{
...successResult,
currentPage: 2,
isPreviousData: true,
}}
/>,
);

await assertNoScroll(mockScroll);
});
});

describe("Responding to changes in React Query's isPreviousData", () => {
it("Does nothing when isPreviousData flips from false to true while currentPage stays the same (safety net for 'impossible' case)", async () => {
const mockScroll = jest.spyOn(window, "scrollTo");

const { rerender } = render({
paginationUnitLabel: mockUnitLabel,
paginationResult: initialRenderResult,
});

rerender(
<Pagination
paginationUnitLabel={mockUnitLabel}
paginationResult={{ ...successResult, isPreviousData: true }}
/>,
);

await assertNoScroll(mockScroll);
});

it("Triggers scroll if scroll has been queued while waiting for isPreviousData to flip from true to false", async () => {
const mockScroll = jest.spyOn(window, "scrollTo");
const { rerender } = await mountWithSuccess(mockScroll);

rerender(
<Pagination
paginationUnitLabel={mockUnitLabel}
paginationResult={{
...successResult,
currentPage: 2,
isPreviousData: true,
}}
/>,
);

rerender(
<Pagination
paginationUnitLabel={mockUnitLabel}
paginationResult={{
...successResult,
currentPage: 2,
isPreviousData: false,
}}
/>,
);

await waitFor(() => expect(mockScroll).toBeCalled());
});

it("Cancels a scroll if user interacts with the browser in any way before isPreviousData flips from true to false", async () => {
const mockScroll = jest.spyOn(window, "scrollTo");

// Values are based on (keyof WindowEventMap), but frustratingly, the
// native events aren't camel-case, while the fireEvent properties are
const userInteractionEvents = [
"click",
"scroll",
"pointerEnter",
"touchStart",
"keyDown",
] as const;

for (const event of userInteractionEvents) {
const { rerender, unmount } = await mountWithSuccess(mockScroll);

rerender(
<Pagination
paginationUnitLabel={mockUnitLabel}
paginationResult={{
...successResult,
currentPage: 2,
isPreviousData: true,
}}
/>,
);

fireEvent[event](window);

rerender(
<Pagination
paginationUnitLabel={mockUnitLabel}
paginationResult={{
...successResult,
currentPage: 2,
isPreviousData: false,
}}
/>,
);

await assertNoScroll(mockScroll);
unmount();
}
});
});
});
Loading