Skip to content

chore(site): remove users and pagination services #9932

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Oct 2, 2023
Merged

chore(site): remove users and pagination services #9932

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
cda4b4f
Move users loading to react-query
BrunoQuaresma Sep 28, 2023
9b3adea
Remove pagination service
BrunoQuaresma Sep 28, 2023
9db6ec6
Move suspend to react-query
BrunoQuaresma Sep 29, 2023
7fb3858
Move activate to react-query
BrunoQuaresma Sep 29, 2023
1a6fcfb
Remove missed suspend events
BrunoQuaresma Sep 29, 2023
04fb5d3
Move delete to react-query
BrunoQuaresma Sep 29, 2023
44d8de6
Move reset password to react-query
BrunoQuaresma Sep 29, 2023
269f67f
Move update roles to react-query
BrunoQuaresma Sep 29, 2023
b4b7a77
Update copy and remove language object
BrunoQuaresma Sep 29, 2023
76e94fb
Move auth methods queries to the users query module
BrunoQuaresma Sep 29, 2023
50c861d
Add mock to deployment config and fix fragment warning
BrunoQuaresma Sep 29, 2023
93e4e80
Fix tests
BrunoQuaresma Sep 29, 2023
579cb11
Update UsersPage.tsx
BrunoQuaresma Oct 2, 2023
5eb2321
Update UsersPageView.tsx
BrunoQuaresma Oct 2, 2023
735fde8
Merge branch 'main' of https://github.com/coder/coder into bq/refacto…
BrunoQuaresma Oct 2, 2023
57bb614
Merge branch 'bq/refactor-users-service' of https://github.com/coder/…
BrunoQuaresma Oct 2, 2023
23517e3
Remove comments
BrunoQuaresma Oct 2, 2023
d2830ff
Merge branch 'main' of https://github.com/coder/coder into bq/refacto…
BrunoQuaresma Oct 2, 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
61 changes: 59 additions & 2 deletions site/src/api/queries/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import { QueryClient } from "@tanstack/react-query";
import * as API from "api/api";
import { UpdateUserPasswordRequest } from "api/typesGenerated";
import { UpdateUserPasswordRequest, UsersRequest } from "api/typesGenerated";

export const users = (req: UsersRequest) => {
return {
queryKey: ["users", req],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we really be using req as a key here? not req.id or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Req is basically the page, offset and filter params so in this case I think makes sense to use them together. Reference: https://tanstack.com/query/v4/docs/react/guides/query-keys#query-keys-are-hashed-deterministically

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's not always needed, but I've seen the pattern where the payload for the query is treated as the last argument of the query key. It also makes it possible for the query function to access it via the queryKey property on its options argument, even if you extract the query function outside the hook

queryFn: () => API.getUsers(req),
};
};

export const updatePassword = () => {
return {
Expand All @@ -11,9 +19,12 @@ export const updatePassword = () => {
};
};

export const createUser = () => {
export const createUser = (queryClient: QueryClient) => {
return {
mutationFn: API.createUser,
onSuccess: async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 24, we don't have a closure like we do on line 8. Do you know why? Referencing this convo in Slack the other day.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is because one is a query and another one a mutation so when it is a mutation, you can pass the args on the call like createUser.mutateAsync(...)

await queryClient.invalidateQueries(["users"]);
},
};
};

Expand All @@ -22,3 +33,49 @@ export const createFirstUser = () => {
mutationFn: API.createFirstUser,
};
};

export const suspendUser = (queryClient: QueryClient) => {
return {
mutationFn: API.suspendUser,
onSuccess: async () => {
await queryClient.invalidateQueries(["users"]);
},
};
};

export const activateUser = (queryClient: QueryClient) => {
return {
mutationFn: API.activateUser,
onSuccess: async () => {
await queryClient.invalidateQueries(["users"]);
},
};
};

export const deleteUser = (queryClient: QueryClient) => {
return {
mutationFn: API.deleteUser,
onSuccess: async () => {
await queryClient.invalidateQueries(["users"]);
},
};
};

export const updateRoles = (queryClient: QueryClient) => {
return {
mutationFn: ({ userId, roles }: { userId: string; roles: string[] }) =>
API.updateUserRoles(roles, userId),
onSuccess: async () => {
await queryClient.invalidateQueries(["users"]);
},
};
};

export const authMethods = () => {
return {
// Even the endpoint being /users/authmethods we don't want to revalidate it
// when users change so its better add a unique query key
queryKey: ["authMethods"],
queryFn: API.getAuthMethods,
};
};
26 changes: 12 additions & 14 deletions site/src/components/Filter/filter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,20 +353,18 @@ const PresetMenu = ({
View advanced filtering
</MenuItem>
{learnMoreLink2 && learnMoreLabel2 && (
<>
<MenuItem
component="a"
href={learnMoreLink2}
target="_blank"
sx={{ fontSize: 13, fontWeight: 500 }}
onClick={() => {
setIsOpen(false);
}}
>
<OpenInNewOutlined sx={{ fontSize: "14px !important" }} />
{learnMoreLabel2}
</MenuItem>
</>
Comment on lines -356 to -369
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice bit of cleanup!

<MenuItem
component="a"
href={learnMoreLink2}
target="_blank"
sx={{ fontSize: 13, fontWeight: 500 }}
onClick={() => {
setIsOpen(false);
}}
>
<OpenInNewOutlined sx={{ fontSize: "14px !important" }} />
{learnMoreLabel2}
</MenuItem>
)}
</Menu>
</>
Expand Down
43 changes: 0 additions & 43 deletions site/src/components/PaginationWidget/PaginationWidget.stories.tsx
View file
Open in desktop

This file was deleted.

28 changes: 0 additions & 28 deletions site/src/components/PaginationWidget/PaginationWidget.test.tsx
View file
Open in desktop

This file was deleted.

110 changes: 0 additions & 110 deletions site/src/components/PaginationWidget/PaginationWidget.tsx
View file
Open in desktop

This file was deleted.

35 changes: 35 additions & 0 deletions site/src/components/PaginationWidget/PaginationWidgetBase.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { PaginationWidgetBase } from "./PaginationWidgetBase";
import type { Meta, StoryObj } from "@storybook/react";

const meta: Meta<typeof PaginationWidgetBase> = {
title: "components/PaginationWidgetBase",
component: PaginationWidgetBase,
args: {
page: 1,
limit: 12,
count: 200,
},
};

export default meta;
type Story = StoryObj<typeof PaginationWidgetBase>;

export const MoreThan8Pages: Story = {};

export const LessThan8Pages: Story = {
args: {
count: 84,
},
};

export const MoreThan7PagesWithActivePageCloseToStart: Story = {
args: { page: 2, limit: 12 },
};

export const MoreThan7PagesWithActivePageFarFromBoundaries: Story = {
args: { page: 4, limit: 12 },
};

export const MoreThan7PagesWithActivePageCloseToEnd: Story = {
args: { page: 17, limit: 12 },
};
50 changes: 24 additions & 26 deletions site/src/components/PaginationWidget/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,29 @@
import { buildPagedList, getOffset } from "./utils";

describe("unit/PaginationWidget", () => {
describe("buildPagedList", () => {
it.each<{
numPages: number;
activePage: number;
expected: (string | number)[];
}>([
{ numPages: 7, activePage: 1, expected: [1, 2, 3, 4, 5, 6, 7] },
{ numPages: 17, activePage: 1, expected: [1, 2, 3, 4, 5, "right", 17] },
{
numPages: 17,
activePage: 9,
expected: [1, "left", 8, 9, 10, "right", 17],
},
{
numPages: 17,
activePage: 17,
expected: [1, "left", 13, 14, 15, 16, 17],
},
])(
`buildPagedList($numPages, $activePage)`,
({ numPages, activePage, expected }) => {
expect(buildPagedList(numPages, activePage)).toEqual(expected);
},
);
});
describe("buildPagedList", () => {
it.each<{
numPages: number;
activePage: number;
expected: (string | number)[];
}>([
{ numPages: 7, activePage: 1, expected: [1, 2, 3, 4, 5, 6, 7] },
{ numPages: 17, activePage: 1, expected: [1, 2, 3, 4, 5, "right", 17] },
{
numPages: 17,
activePage: 9,
expected: [1, "left", 8, 9, 10, "right", 17],
},
{
numPages: 17,
activePage: 17,
expected: [1, "left", 13, 14, 15, 16, 17],
},
])(
`buildPagedList($numPages, $activePage)`,
({ numPages, activePage, expected }) => {
expect(buildPagedList(numPages, activePage)).toEqual(expected);
},
);
});

describe("getOffset", () => {
Expand Down
Loading