Skip to content

Commit e34ebbb

Browse files
presleyppull[bot]
presleyp
authored and
pull[bot]
committed
feat: paginating Users page (#4792)
* Extract PageButton * Fix import * Extract utils * Format * Separate pagination - wip * Spawn pagination machine - buggy filter * Make labels optional * Layout, fix send reset bug * Format * Fix refresh data bug * Remove debugging line * Fix url updates setSearchParams overwrites all search params, rather than merging * Update Audit Page * Simplify pagination widget * Fix workspaces story * Fix Audit story * Fix pagination story and pagebutton highlight * Fix pagination tests * Add to utils tests * Format * Add tests * Start adding pagination - type error * Tweak machine * Refactor paginated api calls * Show pagination when count is undefined * fix stories * Fix api helper * Add test * Format * Make widget show all the time to avoid blink
1 parent 60be476 commit e34ebbb

File tree

8 files changed

+536
-449
lines changed

8 files changed

+536
-449
lines changed

‎site/src/api/api.ts

Lines changed: 29 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,10 @@ export const getApiKey = async (): Promise<TypesGen.GenerateAPIKeyResponse> => {
132132
}
133133

134134
export const getUsers = async (
135-
filter?: TypesGen.UsersRequest,
135+
options: TypesGen.UsersRequest,
136136
): Promise<TypesGen.User[]> => {
137-
const url = getURLWithSearchParams("/api/v2/users", filter)
138-
const response = await axios.get<TypesGen.User[]>(url)
137+
const url = getURLWithSearchParams("/api/v2/users", options)
138+
const response = await axios.get<TypesGen.User[]>(url.toString())
139139
return response.data
140140
}
141141

@@ -264,51 +264,43 @@ export const watchWorkspace = (workspaceId: string): EventSource => {
264264
)
265265
}
266266

267+
interface SearchParamOptions extends TypesGen.Pagination {
268+
q?: string
269+
}
270+
267271
export const getURLWithSearchParams = (
268272
basePath: string,
269-
filter?: TypesGen.WorkspaceFilter | TypesGen.UsersRequest,
273+
options?: SearchParamOptions,
270274
): string => {
271-
const searchParams = new URLSearchParams()
272-
273-
if (filter?.q && filter.q !== "") {
274-
searchParams.append("q", filter.q)
275+
if (options) {
276+
const searchParams = new URLSearchParams()
277+
const keys = Object.keys(options) as (keyof SearchParamOptions)[]
278+
keys.forEach((key) => {
279+
const value = options[key]
280+
if (value !== undefined && value !== "") {
281+
searchParams.append(key, value.toString())
282+
}
283+
})
284+
const searchString = searchParams.toString()
285+
return searchString ? `${basePath}?${searchString}` : basePath
286+
} else {
287+
return basePath
275288
}
276-
277-
const searchString = searchParams.toString()
278-
279-
return searchString ? `${basePath}?${searchString}` : basePath
280289
}
281290

282291
export const getWorkspaces = async (
283292
options: TypesGen.WorkspacesRequest,
284293
): Promise<TypesGen.Workspace[]> => {
285-
const searchParams = new URLSearchParams()
286-
if (options.limit) {
287-
searchParams.set("limit", options.limit.toString())
288-
}
289-
if (options.offset) {
290-
searchParams.set("offset", options.offset.toString())
291-
}
292-
if (options.q) {
293-
searchParams.set("q", options.q)
294-
}
295-
296-
const response = await axios.get<TypesGen.Workspace[]>(
297-
`/api/v2/workspaces?${searchParams.toString()}`,
298-
)
294+
const url = getURLWithSearchParams("/api/v2/workspaces", options)
295+
const response = await axios.get<TypesGen.Workspace[]>(url)
299296
return response.data
300297
}
301298

302299
export const getWorkspacesCount = async (
303300
options: TypesGen.WorkspaceCountRequest,
304301
): Promise<TypesGen.WorkspaceCountResponse> => {
305-
const searchParams = new URLSearchParams()
306-
if (options.q) {
307-
searchParams.set("q", options.q)
308-
}
309-
const response = await axios.get(
310-
`/api/v2/workspaces/count?${searchParams.toString()}`,
311-
)
302+
const url = getURLWithSearchParams("/api/v2/workspaces/count", options)
303+
const response = await axios.get(url)
312304
return response.data
313305
}
314306

@@ -555,31 +547,16 @@ export const getEntitlements = async (): Promise<TypesGen.Entitlements> => {
555547
export const getAuditLogs = async (
556548
options: TypesGen.AuditLogsRequest,
557549
): Promise<TypesGen.AuditLogResponse> => {
558-
const searchParams = new URLSearchParams()
559-
if (options.limit) {
560-
searchParams.set("limit", options.limit.toString())
561-
}
562-
if (options.offset) {
563-
searchParams.set("offset", options.offset.toString())
564-
}
565-
if (options.q) {
566-
searchParams.set("q", options.q)
567-
}
568-
569-
const response = await axios.get(`/api/v2/audit?${searchParams.toString()}`)
550+
const url = getURLWithSearchParams("/api/v2/audit", options)
551+
const response = await axios.get(url)
570552
return response.data
571553
}
572554

573555
export const getAuditLogsCount = async (
574556
options: TypesGen.AuditLogCountRequest = {},
575557
): Promise<TypesGen.AuditLogCountResponse> => {
576-
const searchParams = new URLSearchParams()
577-
if (options.q) {
578-
searchParams.set("q", options.q)
579-
}
580-
const response = await axios.get(
581-
`/api/v2/audit/count?${searchParams.toString()}`,
582-
)
558+
const url = getURLWithSearchParams("/api/v2/audit/count", options)
559+
const response = await axios.get(url)
583560
return response.data
584561
}
585562

‎site/src/components/PaginationWidget/PaginationWidget.tsx

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,12 @@ export const PaginationWidget = ({
3434
const currentPage = paginationState.context.page
3535
const numRecordsPerPage = paginationState.context.limit
3636

37-
const numPages = numRecords ? Math.ceil(numRecords / numRecordsPerPage) : 0
37+
const numPages = numRecords
38+
? Math.ceil(numRecords / numRecordsPerPage)
39+
: undefined
3840
const firstPageActive = currentPage === 1 && numPages !== 0
3941
const lastPageActive = currentPage === numPages && numPages !== 0
4042

41-
// No need to display any pagination if we know the number of pages is 1 or 0
42-
if (numPages <= 1 || numRecords === 0) {
43-
return null
44-
}
45-
4643
return (
4744
<div style={containerStyle} className={styles.defaultContainerStyles}>
4845
<Button
@@ -54,7 +51,7 @@ export const PaginationWidget = ({
5451
<KeyboardArrowLeft />
5552
<div>{prevLabel}</div>
5653
</Button>
57-
<Maybe condition={numPages > 0}>
54+
<Maybe condition={numPages !== undefined}>
5855
<ChooseOne>
5956
<Cond condition={isMobile}>
6057
<PageButton
@@ -64,24 +61,25 @@ export const PaginationWidget = ({
6461
/>
6562
</Cond>
6663
<Cond>
67-
{buildPagedList(numPages, currentPage).map((page) =>
68-
typeof page !== "number" ? (
69-
<PageButton
70-
key={`Page${page}`}
71-
activePage={currentPage}
72-
placeholder="..."
73-
disabled
74-
/>
75-
) : (
76-
<PageButton
77-
key={`Page${page}`}
78-
activePage={currentPage}
79-
page={page}
80-
numPages={numPages}
81-
onPageClick={() => send({ type: "GO_TO_PAGE", page })}
82-
/>
83-
),
84-
)}
64+
{numPages &&
65+
buildPagedList(numPages, currentPage).map((page) =>
66+
typeof page !== "number" ? (
67+
<PageButton
68+
key={`Page${page}`}
69+
activePage={currentPage}
70+
placeholder="..."
71+
disabled
72+
/>
73+
) : (
74+
<PageButton
75+
key={`Page${page}`}
76+
activePage={currentPage}
77+
page={page}
78+
numPages={numPages}
79+
onPageClick={() => send({ type: "GO_TO_PAGE", page })}
80+
/>
81+
),
82+
)}
8583
</Cond>
8684
</ChooseOne>
8785
</Maybe>

‎site/src/pages/UsersPage/UsersPage.test.tsx

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,32 @@ describe("UsersPage", () => {
238238
})
239239
})
240240

241+
describe("pagination", () => {
242+
it("goes to next and previous page", async () => {
243+
renderPage()
244+
const user = userEvent.setup()
245+
246+
const mock = jest
247+
.spyOn(API, "getUsers")
248+
.mockResolvedValueOnce([MockUser, MockUser2])
249+
250+
const nextButton = await screen.findByLabelText("Next page")
251+
await user.click(nextButton)
252+
253+
await waitFor(() =>
254+
expect(API.getUsers).toBeCalledWith({ offset: 25, limit: 25, q: "" }),
255+
)
256+
257+
mock.mockClear()
258+
const previousButton = await screen.findByLabelText("Previous page")
259+
await user.click(previousButton)
260+
261+
await waitFor(() =>
262+
expect(API.getUsers).toBeCalledWith({ offset: 0, limit: 25, q: "" }),
263+
)
264+
})
265+
})
266+
241267
describe("delete user", () => {
242268
describe("when it is success", () => {
243269
it("shows a success message and refresh the page", async () => {

‎site/src/pages/UsersPage/UsersPage.tsx

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useActor, useMachine } from "@xstate/react"
22
import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"
3+
import { getPaginationContext } from "components/PaginationWidget/utils"
34
import { usePermissions } from "hooks/usePermissions"
45
import { FC, ReactNode, useContext, useEffect } from "react"
56
import { Helmet } from "react-helmet-async"
@@ -25,10 +26,15 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
2526
const xServices = useContext(XServiceContext)
2627
const navigate = useNavigate()
2728
const [searchParams, setSearchParams] = useSearchParams()
28-
const filter = searchParams.get("filter") ?? undefined
29+
const filter = searchParams.get("filter") ?? ""
2930
const [usersState, usersSend] = useMachine(usersMachine, {
3031
context: {
3132
filter,
33+
paginationContext: getPaginationContext(searchParams),
34+
},
35+
actions: {
36+
updateURL: (context, event) =>
37+
setSearchParams({ page: event.page, filter: context.filter }),
3238
},
3339
})
3440
const {
@@ -39,6 +45,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
3945
userIdToActivate,
4046
userIdToResetPassword,
4147
newUserPassword,
48+
paginationRef,
4249
} = usersState.context
4350

4451
const userToBeSuspended = users?.find((u) => u.id === userIdToSuspend)
@@ -56,14 +63,6 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
5663
usersState.matches("gettingUsers") ||
5764
(canEditUsers && rolesState.matches("gettingRoles"))
5865

59-
// Fetch users on component mount
60-
useEffect(() => {
61-
usersSend({
62-
type: "GET_USERS",
63-
query: filter,
64-
})
65-
}, [filter, usersSend])
66-
6766
// Fetch roles on component mount
6867
useEffect(() => {
6968
// Only fetch the roles if the user has permission for it
@@ -113,9 +112,9 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
113112
canEditUsers={canEditUsers}
114113
filter={usersState.context.filter}
115114
onFilter={(query) => {
116-
searchParams.set("filter", query)
117-
setSearchParams(searchParams)
115+
usersSend({ type: "UPDATE_FILTER", query })
118116
}}
117+
paginationRef={paginationRef}
119118
/>
120119

121120
{userToBeDeleted && (

‎site/src/pages/UsersPage/UsersPageView.stories.tsx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ComponentMeta, Story } from "@storybook/react"
2+
import { createPaginationRef } from "components/PaginationWidget/utils"
23
import {
3-
MockSiteRoles,
4+
MockAssignableSiteRoles,
45
MockUser,
56
MockUser2,
67
} from "../../testHelpers/renderHelpers"
@@ -9,6 +10,11 @@ import { UsersPageView, UsersPageViewProps } from "./UsersPageView"
910
export default {
1011
title: "pages/UsersPageView",
1112
component: UsersPageView,
13+
argTypes: {
14+
paginationRef: {
15+
defaultValue: createPaginationRef({ page: 1, limit: 25 }),
16+
},
17+
},
1218
} as ComponentMeta<typeof UsersPageView>
1319

1420
const Template: Story<UsersPageViewProps> = (args) => (
@@ -18,8 +24,7 @@ const Template: Story<UsersPageViewProps> = (args) => (
1824
export const Admin = Template.bind({})
1925
Admin.args = {
2026
users: [MockUser, MockUser2],
21-
roles: MockSiteRoles,
22-
canCreateUser: true,
27+
roles: MockAssignableSiteRoles,
2328
canEditUsers: true,
2429
}
2530

@@ -32,7 +37,7 @@ SmallViewport.parameters = {
3237
}
3338

3439
export const Member = Template.bind({})
35-
Member.args = { ...Admin.args, canCreateUser: false, canEditUsers: false }
40+
Member.args = { ...Admin.args, canEditUsers: false }
3641

3742
export const Empty = Template.bind({})
3843
Empty.args = { ...Admin.args, users: [] }

‎site/src/pages/UsersPage/UsersPageView.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import { PaginationWidget } from "components/PaginationWidget/PaginationWidget"
12
import { FC } from "react"
3+
import { PaginationMachineRef } from "xServices/pagination/paginationXService"
24
import * as TypesGen from "../../api/typesGenerated"
35
import { SearchBarWithFilter } from "../../components/SearchBarWithFilter/SearchBarWithFilter"
46
import { UsersTable } from "../../components/UsersTable/UsersTable"
@@ -26,6 +28,7 @@ export interface UsersPageViewProps {
2628
roles: TypesGen.Role["name"][],
2729
) => void
2830
onFilter: (query: string) => void
31+
paginationRef: PaginationMachineRef
2932
}
3033

3134
export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
@@ -43,6 +46,7 @@ export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
4346
isLoading,
4447
filter,
4548
onFilter,
49+
paginationRef,
4650
}) => {
4751
const presetFilters = [
4852
{ query: userFilterQuery.active, name: Language.activeUsersFilterName },
@@ -71,6 +75,8 @@ export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
7175
canEditUsers={canEditUsers}
7276
isLoading={isLoading}
7377
/>
78+
79+
<PaginationWidget paginationRef={paginationRef} />
7480
</>
7581
)
7682
}

‎site/src/testHelpers/entities.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ export const MockAuditorRole: TypesGen.Role = {
4848
display_name: "Auditor",
4949
}
5050

51-
export const MockSiteRoles = [MockUserAdminRole, MockAuditorRole]
52-
5351
// assignableRole takes a role and a boolean. The boolean implies if the
5452
// actor can assign (add/remove) the role from other users.
5553
export function assignableRole(
@@ -62,6 +60,12 @@ export function assignableRole(
6260
}
6361
}
6462

63+
export const MockSiteRoles = [MockUserAdminRole, MockAuditorRole]
64+
export const MockAssignableSiteRoles = [
65+
assignableRole(MockUserAdminRole, true),
66+
assignableRole(MockAuditorRole, true),
67+
]
68+
6569
export const MockMemberPermissions = {
6670
viewAuditLog: false,
6771
}

0 commit comments

Comments
 (0)