From cda4b4f96149d769d49276ec96c9e8a368047c3c Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 28 Sep 2023 21:07:40 +0000 Subject: [PATCH 01/15] Move users loading to react-query --- site/src/api/queries/users.ts | 9 +- site/src/hooks/usePagination.ts | 2 + site/src/pages/UsersPage/UsersPage.tsx | 57 ++++---- .../pages/UsersPage/UsersPageView.stories.tsx | 4 +- site/src/pages/UsersPage/UsersPageView.tsx | 25 +++- site/src/utils/filters.ts | 7 +- site/src/xServices/users/usersXService.ts | 126 ++---------------- 7 files changed, 74 insertions(+), 156 deletions(-) diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index eed556e30942f..3b80c2f8ebfa0 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -1,5 +1,12 @@ 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], + queryFn: () => API.getUsers(req), + }; +}; export const updatePassword = () => { return { diff --git a/site/src/hooks/usePagination.ts b/site/src/hooks/usePagination.ts index d859bc424888a..2feee1ac1f033 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -9,6 +9,7 @@ export const usePagination = ({ const [searchParams, setSearchParams] = searchParamsResult; const page = searchParams.get("page") ? Number(searchParams.get("page")) : 1; const limit = DEFAULT_RECORDS_PER_PAGE; + const offset = page <= 0 ? 0 : (page - 1) * limit; const goToPage = (page: number) => { searchParams.set("page", page.toString()); @@ -19,5 +20,6 @@ export const usePagination = ({ page, limit, goToPage, + offset, }; }; diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index d6756d6663ef7..4b109a86a94ba 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -1,13 +1,10 @@ import { useMachine } from "@xstate/react"; import { User } from "api/typesGenerated"; import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; -import { - getPaginationContext, - nonInitialPage, -} from "components/PaginationWidget/utils"; +import { nonInitialPage } from "components/PaginationWidget/utils"; import { useMe } from "hooks/useMe"; import { usePermissions } from "hooks/usePermissions"; -import { FC, ReactNode, useEffect } from "react"; +import { FC, ReactNode } from "react"; import { Helmet } from "react-helmet-async"; import { useSearchParams, useNavigate } from "react-router-dom"; import { usersMachine } from "xServices/users/usersXService"; @@ -22,6 +19,9 @@ import { useQuery } from "@tanstack/react-query"; import { getAuthMethods } from "api/api"; import { roles } from "api/queries/roles"; import { deploymentConfig } from "api/queries/deployment"; +import { prepareQuery } from "utils/filters"; +import { usePagination } from "hooks"; +import * as UsersQuery from "api/queries/users"; export const Language = { suspendDialogTitle: "Suspend user", @@ -39,28 +39,27 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { const navigate = useNavigate(); const searchParamsResult = useSearchParams(); const { entitlements } = useDashboard(); - const [searchParams, setSearchParams] = searchParamsResult; + const [searchParams] = searchParamsResult; const filter = searchParams.get("filter") ?? ""; - const [usersState, usersSend] = useMachine(usersMachine, { - context: { - filter, - paginationContext: getPaginationContext(searchParams), - }, - actions: { - updateURL: (context, event) => - setSearchParams({ page: event.page, filter: context.filter }), - }, + const [usersState, usersSend] = useMachine(usersMachine); + const pagination = usePagination({ + searchParamsResult, }); + const usersQuery = useQuery( + UsersQuery.users({ + q: prepareQuery(filter), + limit: pagination.limit, + offset: pagination.offset, + }), + ); + const users = usersQuery.data?.users; + const count = usersQuery.data?.count; const { - users, - getUsersError, usernameToDelete, usernameToSuspend, usernameToActivate, userIdToResetPassword, newUserPassword, - paginationRef, - count, } = usersState.context; const { updateUsers: canEditUsers, viewDeploymentValues } = usePermissions(); const rolesQuery = useQuery({ ...roles(), enabled: canEditUsers }); @@ -77,12 +76,9 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { const useFilterResult = useFilter({ searchParamsResult, onUpdate: () => { - usersSend({ type: "UPDATE_PAGE", page: "1" }); + pagination.goToPage(1); }, }); - useEffect(() => { - usersSend({ type: "UPDATE_FILTER", query: useFilterResult.query }); - }, [useFilterResult.query, usersSend]); const statusMenu = useStatusFilterMenu({ value: useFilterResult.values.status, onChange: (option) => @@ -97,13 +93,8 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { return getAuthMethods(); }, }); - // Is loading if - // - users are loading or - // - the user can edit the users but the roles are loading const isLoading = - usersState.matches("gettingUsers") || - rolesQuery.isLoading || - authMethods.isLoading; + usersQuery.isLoading || rolesQuery.isLoading || authMethods.isLoading; return ( <> @@ -115,7 +106,6 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { roles={rolesQuery.data} users={users} authMethods={authMethods.data} - count={count} onListWorkspaces={(user) => { navigate( "/workspaces?filter=" + @@ -162,16 +152,19 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { isLoading={isLoading} canEditUsers={canEditUsers} canViewActivity={entitlements.features.audit_log.enabled} - paginationRef={paginationRef} isNonInitialPage={nonInitialPage(searchParams)} actorID={me.id} filterProps={{ filter: useFilterResult, - error: getUsersError, + error: usersQuery.error, menus: { status: statusMenu, }, }} + count={count} + page={pagination.page} + limit={pagination.limit} + onPageChange={pagination.goToPage} /> = { title: "pages/UsersPageView", component: UsersPageView, args: { - paginationRef: createPaginationRef({ page: 1, limit: 25 }), + page: 1, + limit: 25, isNonInitialPage: false, users: [MockUser, MockUser2], roles: MockAssignableSiteRoles, diff --git a/site/src/pages/UsersPage/UsersPageView.tsx b/site/src/pages/UsersPage/UsersPageView.tsx index 66a87fac8a79f..f09b6895e8c31 100644 --- a/site/src/pages/UsersPage/UsersPageView.tsx +++ b/site/src/pages/UsersPage/UsersPageView.tsx @@ -1,6 +1,4 @@ -import { PaginationWidget } from "components/PaginationWidget/PaginationWidget"; import { ComponentProps, FC } from "react"; -import { PaginationMachineRef } from "xServices/pagination/paginationXService"; import * as TypesGen from "api/typesGenerated"; import { UsersTable } from "./UsersTable/UsersTable"; import { UsersFilter } from "./UsersFilter"; @@ -8,10 +6,10 @@ import { PaginationStatus, TableToolbar, } from "components/TableToolbar/TableToolbar"; +import { PaginationWidgetBase } from "components/PaginationWidget/PaginationWidgetBase"; export interface UsersPageViewProps { users?: TypesGen.User[]; - count?: number; roles?: TypesGen.AssignableRoles[]; isUpdatingUserRoles?: boolean; canEditUsers?: boolean; @@ -30,14 +28,17 @@ export interface UsersPageViewProps { roles: TypesGen.Role["name"][], ) => void; filterProps: ComponentProps; - paginationRef: PaginationMachineRef; isNonInitialPage: boolean; actorID: string; + // Pagination + count?: number; + page: number; + limit: number; + onPageChange: (page: number) => void; } export const UsersPageView: FC> = ({ users, - count, roles, onSuspendUser, onDeleteUser, @@ -52,10 +53,13 @@ export const UsersPageView: FC> = ({ canViewActivity, isLoading, filterProps, - paginationRef, isNonInitialPage, actorID, authMethods, + count, + limit, + onPageChange, + page, }) => { return ( <> @@ -90,7 +94,14 @@ export const UsersPageView: FC> = ({ authMethods={authMethods} /> - + {count !== undefined && ( + + )} ); }; diff --git a/site/src/utils/filters.ts b/site/src/utils/filters.ts index 32a9831d077a4..556e679de959c 100644 --- a/site/src/utils/filters.ts +++ b/site/src/utils/filters.ts @@ -3,12 +3,15 @@ import * as TypesGen from "api/typesGenerated"; export const queryToFilter = ( query?: string, ): TypesGen.WorkspaceFilter | TypesGen.UsersRequest => { - const preparedQuery = query?.trim().replace(/ +/g, " "); return { - q: preparedQuery, + q: prepareQuery(query), }; }; +export const prepareQuery = (query?: string) => { + return query?.trim().replace(/ +/g, " "); +}; + export const workspaceFilterQuery = { me: "owner:me", all: "", diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index f247a74a59af9..ddb1b2686691e 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -1,19 +1,10 @@ -import { getPaginationData } from "components/PaginationWidget/utils"; -import { - PaginationContext, - paginationMachine, - PaginationMachineRef, -} from "xServices/pagination/paginationXService"; -import { assign, createMachine, send, spawn } from "xstate"; +import { assign, createMachine } from "xstate"; import * as API from "api/api"; import { getErrorMessage } from "api/errors"; import * as TypesGen from "api/typesGenerated"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; -import { queryToFilter } from "utils/filters"; import { generateRandomString } from "utils/random"; -const usersPaginationId = "usersPagination"; - export const Language = { getUsersError: "Error getting users.", suspendUserSuccess: "Successfully suspended the user.", @@ -29,10 +20,6 @@ export const Language = { }; export interface UsersContext { - // Get users - users?: TypesGen.User[]; - filter: string; - getUsersError?: unknown; // Suspend user userIdToSuspend?: TypesGen.User["id"]; usernameToSuspend?: TypesGen.User["username"]; @@ -52,13 +39,9 @@ export interface UsersContext { // Update user roles userIdToUpdateRoles?: TypesGen.User["id"]; updateUserRolesError?: unknown; - paginationContext: PaginationContext; - paginationRef: PaginationMachineRef; - count: number; } export type UsersEvent = - | { type: "GET_USERS"; query?: string } // Suspend events | { type: "SUSPEND_USER"; @@ -92,11 +75,7 @@ export type UsersEvent = type: "UPDATE_USER_ROLES"; userId: TypesGen.User["id"]; roles: TypesGen.Role["name"][]; - } - // Filter - | { type: "UPDATE_FILTER"; query: string } - // Pagination - | { type: "UPDATE_PAGE"; page: string }; + }; export const usersMachine = /** @xstate-layout N4IgpgJg5mDOIC5QFdZgE6wMoBcCGOYAdLPujgJYB2UACnlNQRQPZUDEA2gAwC6ioAA4tYFSmwEgAHogC0ARgBMigBxEAnCoCsAdhWrdK9QBYdAGhABPRPO7ruRAMzdj3HYuNbFANm5b1AL4BFqgY2PiERDA4lDQAqmiY7BBsxNQAbiwA1sTRCWE8-EggwqLiVJIyCArc8upOxt6K-o7qjloq3ioW1ghKPkQu6m2KOraaw0EhieEEuWAx1FD5SRjoLOhEggA2BABmGwC2UQsrsIWSpWKsFcVVCqqOGoqOKnUqKrW15lY2ivI6IiNRqOHR6bhdXRTEChTC4OZECgQbZgdhYOJYWgAUQAcgARAD6GKxACULsUruVKnIdLoiPJjPItF42p0fI4ejYtMZjE4ASoxi4dI4lCpobDZpEkSj2HisQAZLEAFSxRKwpPJQhE1wkdxp3nqAJM3i0zm8tPknL63gNRB0TQBTNBHRM4pm8KlyNRAEEAMJKgCSADVvSq1Rq+JdtVS9dUdMYnkN1C9uM0Teofr15CpXEQPt95IXC952m6wh60l72CSseqleGSQTaN6sFgAOoAeRJeM1JWjN2pca8RH+bmar3tdUUVve3me8kcIqU3G0NrLcIilZlcVoeNDquJjZJHcVWF7lIHsdk8d5As6ybeHxLxitym4Dh5b1pYx0LhUjnXSUt1RHc9zDZsAHEsXPftdVAe4GT0IEMz0UYtAhV51BnZwHDsTQDQXZNNEAitESrUD9wJAAxAN5RVMlIwpWDbnguR5BNXlEPUbNmk+Ixp1+PpFHULQgUcbxTH8YTCy8EjNyIABjNg9godBDhWLBUEEMAqFENh2F9DscRokkAFkGwJdFMVxLAAyMmCykvVjqg8eQ82UdQJJ5DxaWZGdmUUDRWi8VM+LGbw5IRJSqBUtSNK0nS9I4X1vRxX0FQsqzsRxWz7MYrVHLg6Q5GMbQgWZbiS3tbhWktQT2P+PMBR0DMM0dLQIuCGF3Xk6LYvUxI8TAFFygMoyTPMw8CTlRUVQcnUWOKlzlEGBlHATPRhn0JkZy6XlSuMUZamEox7UiyI+tUgaMCGkabgM1L0vlCyZuVaD8r7QrFvuYwM3pewRUhTxPkzGx7XqYTGTayTtEUc7iEuuLEm9BTKHSZh9MM4yAzMiy-UDENAzyooCoWwdZBeNQfBccT0NqbkOhnHNAXaPxVHsJpTB0eHFOUq6VhRtGMeSx6Mqm-Hg1DOycXmmNnNkdC51KlqlHaYTRgErM2lvZkDUO37-ELHnYASqgICWFZklSREqEyHISFNiAVllpyltaJ5fscXivd0UqsPq0q3MLFrzRzV5MONx2LcSdg1g2LZdhwA41Id2BtLN52PovIqqhNUTOgI2xF3tUEZy5kcXXNQ6Vx8TrpnLeSIGGhZo4wK2qDSW3smIJuRrATOSc+snY1cD83hC1lPG8OqswkiHGghX8Du0Ywed7lv4hjuPNh2fYjiIdfCAHqMvsHc03PfdpPI6xkF26eqS1E5pUNUbQM1GHm8FRih0diZYY5SB3G2dtiBfyFkfRILsc6IGCiOX8vgmTsTGJ5F89UwQOBDtoU0pgPCry6hKUiYCf7ME3m3beCc94pyIb-fukCs7MTPtPIgCDhJuE+O7bwM49BPBFCYXQoxhjsTFPgnqUU+ZIwwPQWAsAADuGwIAkjgAsMa2NcZTWbK2Ts3YCQ1jrFA76bEPiDFaP+G0UkXhaFfLUNQWhsx6xeM4dweD64bjETFfmiQpGyPkYotAOAHppTFuqRsGj2xdkJLo5U+jya2MBDTWkyhaQGk6K+WwolGjuH+KVaeDIeboCUYsUh6AvFyPQBAduncQFEHyX4lYJT5HRNjKCD2RYRIgyaIoPwM5uJuXEsKDqjwVZ5IKX-OpeBpGlPKeQ3eSd941NOJ48Z3iymNOchJJ4bRFyXy6MmLo3TPIaAFJ0xcug3CMh5sgQQEASH-wwCSFgKJYAVOAd3IglzrkQLuQ8uAqy3adAaHYdiSgOivBnogf4tpWhbQBK4ZQuSRENwRO8m5Kx7mPNjugdYO9E7J2OMiz56A0U-PoafWMJo3I+AZLXcc7FLGCThXObiYxRie2DvDdgFEww0TohGQe2cDHVFsAmekvgTnsV+mkq09gHDOHWs0TpU5OpdSoCwJu8BigEPkqQPA5Alj0EYFQYWJ9h7ywXAyPMdgEzV02bSKVC47SfHfLSX65pSwItcZEaIoyZjGrlktBQSh6guAhI0aejJhQzjcKJA0Xt2g8i8Aqnm0owC+tdghO+eYTRoSLo0DM2F4xEGwZ00wbg1bc3dUBXm7iJHoE0mnRKrt+XkwlcY0wrI6jZlqP5YcbRjQeDsLoM6FbSKI2uugW6LcipNqvNk5hph1rTw6HUPZD8cxAhzEdQNp1nHdURRdcRY7BbEL9dO+WpVo2lSjQKEUjJ2hM25KtNoisvaHXNJHetZtW7oFTdAhAbxmF2HjKCIwIl1b+SZGJES75RimIBGvZu3qMA-oFQKO0edxJVW8kYXazI7ToT9nTAUrph3yWoSixIyHBxlQkrYaDnwOqaFBn0N4c5bGsm0MdcYPNR1jImT4gplGZ0SXpKaAdYJQRtHNFYjwThWoLg8JVESwy-GIeKUsyZgnnIMjcuGuVEIl2mCsamJwXsBR2BeOKuuu6PXEHxV+ol6rSZ+qqCKQYyYMNoRBsKVBvR-j-ITHPcS7DXhWc1XMTT-qATGa4jxDoK5kxWjeL0mqIJ5NczwUEIAA */ @@ -107,9 +86,6 @@ export const usersMachine = context: {} as UsersContext, events: {} as UsersEvent, services: {} as { - getUsers: { - data: TypesGen.GetUsersResponse; - }; createUser: { data: TypesGen.User; }; @@ -132,43 +108,8 @@ export const usersMachine = }, predictableActionArguments: true, id: "usersState", - on: { - UPDATE_FILTER: { - actions: ["assignFilter", "sendResetPage"], - }, - UPDATE_PAGE: { - target: "gettingUsers", - actions: "updateURL", - }, - }, - initial: "startingPagination", + initial: "idle", states: { - startingPagination: { - entry: "assignPaginationRef", - always: { - target: "gettingUsers", - }, - }, - gettingUsers: { - entry: "clearGetUsersError", - invoke: { - src: "getUsers", - id: "getUsers", - onDone: [ - { - target: "idle", - actions: "assignUsers", - }, - ], - onError: [ - { - target: "idle", - actions: ["clearUsers", "assignGetUsersError"], - }, - ], - }, - tags: "loading", - }, idle: { entry: "clearSelectedUser", on: { @@ -234,7 +175,7 @@ export const usersMachine = id: "suspendUser", onDone: [ { - target: "gettingUsers", + target: "idle", actions: "displaySuspendSuccess", }, ], @@ -256,7 +197,7 @@ export const usersMachine = id: "deleteUser", onDone: [ { - target: "gettingUsers", + target: "idle", actions: "displayDeleteSuccess", }, ], @@ -275,7 +216,7 @@ export const usersMachine = id: "activateUser", onDone: [ { - target: "gettingUsers", + target: "idle", actions: "displayActivateSuccess", }, ], @@ -348,17 +289,6 @@ export const usersMachine = }, { services: { - // Passing API.getUsers directly does not invoke the function properly - // when it is mocked. This happen in the UsersPage tests inside of the - // "shows a success message and refresh the page" test case. - getUsers: (context) => { - const { offset, limit } = getPaginationData(context.paginationRef); - return API.getUsers({ - ...queryToFilter(context.filter), - offset, - limit, - }); - }, suspendUser: (context) => { if (!context.userIdToSuspend) { throw new Error("userIdToSuspend is undefined"); @@ -413,16 +343,6 @@ export const usersMachine = userIdToResetPassword: (_) => undefined, userIdToUpdateRoles: (_) => undefined, }), - assignUsers: assign({ - users: (_, event) => event.data.users, - count: (_, event) => event.data.count, - }), - assignFilter: assign({ - filter: (_, event) => event.query, - }), - assignGetUsersError: assign({ - getUsersError: (_, event) => event.data, - }), assignUserToSuspend: assign({ userIdToSuspend: (_, event) => event.userId, usernameToSuspend: (_, event) => event.username, @@ -441,10 +361,6 @@ export const usersMachine = assignUserIdToUpdateRoles: assign({ userIdToUpdateRoles: (_, event) => event.userId, }), - clearGetUsersError: assign((context: UsersContext) => ({ - ...context, - getUsersError: undefined, - })), assignSuspendUserError: assign({ suspendUserError: (_, event) => event.data, }), @@ -460,11 +376,6 @@ export const usersMachine = assignUpdateRolesError: assign({ updateUserRolesError: (_, event) => event.data, }), - clearUsers: assign((context: UsersContext) => ({ - ...context, - users: undefined, - count: undefined, - })), clearSuspendUserError: assign({ suspendUserError: (_) => undefined, }), @@ -531,24 +442,15 @@ export const usersMachine = newUserPassword: (_) => generateRandomString(12), }), updateUserRolesInTheList: assign({ - users: ({ users }, event) => { - if (!users) { - return users; - } - - return users.map((u) => { - return u.id === event.data.id ? event.data : u; - }); - }, - }), - assignPaginationRef: assign({ - paginationRef: (context) => - spawn( - paginationMachine.withContext(context.paginationContext), - usersPaginationId, - ), + // users: ({ users }, event) => { + // if (!users) { + // return users; + // } + // return users.map((u) => { + // return u.id === event.data.id ? event.data : u; + // }); + // }, }), - sendResetPage: send({ type: "RESET_PAGE" }, { to: usersPaginationId }), }, }, ); From 9b3adeac2c09958c59c028a701ff44218fe40919 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 28 Sep 2023 21:12:23 +0000 Subject: [PATCH 02/15] Remove pagination service --- .../PaginationWidget.stories.tsx | 43 ------- .../PaginationWidget.test.tsx | 28 ----- .../PaginationWidget/PaginationWidget.tsx | 110 ------------------ .../PaginationWidgetBase.stories.tsx | 35 ++++++ .../components/PaginationWidget/utils.test.ts | 50 ++++---- site/src/components/PaginationWidget/utils.ts | 43 ------- .../pagination/paginationXService.ts | 66 ----------- 7 files changed, 59 insertions(+), 316 deletions(-) delete mode 100644 site/src/components/PaginationWidget/PaginationWidget.stories.tsx delete mode 100644 site/src/components/PaginationWidget/PaginationWidget.test.tsx delete mode 100644 site/src/components/PaginationWidget/PaginationWidget.tsx create mode 100644 site/src/components/PaginationWidget/PaginationWidgetBase.stories.tsx delete mode 100644 site/src/xServices/pagination/paginationXService.ts diff --git a/site/src/components/PaginationWidget/PaginationWidget.stories.tsx b/site/src/components/PaginationWidget/PaginationWidget.stories.tsx deleted file mode 100644 index 367a2d314c12b..0000000000000 --- a/site/src/components/PaginationWidget/PaginationWidget.stories.tsx +++ /dev/null @@ -1,43 +0,0 @@ -import { PaginationWidget } from "./PaginationWidget"; -import { createPaginationRef } from "./utils"; -import type { Meta, StoryObj } from "@storybook/react"; - -const meta: Meta = { - title: "components/PaginationWidget", - component: PaginationWidget, - args: { - prevLabel: "Previous", - nextLabel: "Next", - paginationRef: createPaginationRef({ page: 1, limit: 12 }), - numRecords: 200, - }, -}; - -export default meta; -type Story = StoryObj; - -export const MoreThan8Pages: Story = {}; - -export const LessThan8Pages: Story = { - args: { - numRecords: 84, - }, -}; - -export const MoreThan7PagesWithActivePageCloseToStart: Story = { - args: { - paginationRef: createPaginationRef({ page: 2, limit: 12 }), - }, -}; - -export const MoreThan7PagesWithActivePageFarFromBoundaries: Story = { - args: { - paginationRef: createPaginationRef({ page: 4, limit: 12 }), - }, -}; - -export const MoreThan7PagesWithActivePageCloseToEnd: Story = { - args: { - paginationRef: createPaginationRef({ page: 17, limit: 12 }), - }, -}; diff --git a/site/src/components/PaginationWidget/PaginationWidget.test.tsx b/site/src/components/PaginationWidget/PaginationWidget.test.tsx deleted file mode 100644 index b1240e4fb8303..0000000000000 --- a/site/src/components/PaginationWidget/PaginationWidget.test.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import { screen } from "@testing-library/react"; -import { render } from "testHelpers/renderHelpers"; -import { PaginationWidget } from "./PaginationWidget"; -import { createPaginationRef } from "./utils"; - -describe("PaginatedList", () => { - it("disables the previous button on the first page", () => { - render( - , - ); - const prevButton = screen.getByLabelText("Previous page"); - expect(prevButton).toBeDisabled(); - }); - - it("disables the next button on the last page", () => { - render( - , - ); - const nextButton = screen.getByLabelText("Next page"); - expect(nextButton).toBeDisabled(); - }); -}); diff --git a/site/src/components/PaginationWidget/PaginationWidget.tsx b/site/src/components/PaginationWidget/PaginationWidget.tsx deleted file mode 100644 index fff86071a2617..0000000000000 --- a/site/src/components/PaginationWidget/PaginationWidget.tsx +++ /dev/null @@ -1,110 +0,0 @@ -import Button from "@mui/material/Button"; -import { makeStyles, useTheme } from "@mui/styles"; -import useMediaQuery from "@mui/material/useMediaQuery"; -import KeyboardArrowLeft from "@mui/icons-material/KeyboardArrowLeft"; -import KeyboardArrowRight from "@mui/icons-material/KeyboardArrowRight"; -import { useActor } from "@xstate/react"; -import { ChooseOne, Cond } from "components/Conditionals/ChooseOne"; -import { CSSProperties } from "react"; -import { PaginationMachineRef } from "xServices/pagination/paginationXService"; -import { PageButton } from "./PageButton"; -import { buildPagedList } from "./utils"; - -export type PaginationWidgetProps = { - prevLabel?: string; - nextLabel?: string; - numRecords?: number; - containerStyle?: CSSProperties; - paginationRef: PaginationMachineRef; -}; - -export const PaginationWidget = ({ - prevLabel = "", - nextLabel = "", - numRecords, - containerStyle, - paginationRef, -}: PaginationWidgetProps): JSX.Element | null => { - const theme = useTheme(); - const isMobile = useMediaQuery(theme.breakpoints.down("md")); - const styles = useStyles(); - const [paginationState, send] = useActor(paginationRef); - - const currentPage = paginationState.context.page; - const numRecordsPerPage = paginationState.context.limit; - - const numPages = numRecords ? Math.ceil(numRecords / numRecordsPerPage) : 0; - const firstPageActive = currentPage === 1 && numPages !== 0; - const lastPageActive = currentPage === numPages && numPages !== 0; - // if beyond page 1, show pagination widget even if there's only one true page, so user can navigate back - const showWidget = numPages > 1 || currentPage > 1; - - if (!showWidget) { - return null; - } - - return ( -
- - - - - - - {buildPagedList(numPages, currentPage).map((page) => - typeof page !== "number" ? ( - - ) : ( - send({ type: "GO_TO_PAGE", page })} - /> - ), - )} - - - -
- ); -}; - -const useStyles = makeStyles((theme) => ({ - defaultContainerStyles: { - justifyContent: "center", - alignItems: "center", - display: "flex", - flexDirection: "row", - padding: "20px", - }, - - prevLabelStyles: { - marginRight: theme.spacing(0.5), - }, -})); diff --git a/site/src/components/PaginationWidget/PaginationWidgetBase.stories.tsx b/site/src/components/PaginationWidget/PaginationWidgetBase.stories.tsx new file mode 100644 index 0000000000000..373dad4dd681b --- /dev/null +++ b/site/src/components/PaginationWidget/PaginationWidgetBase.stories.tsx @@ -0,0 +1,35 @@ +import { PaginationWidgetBase } from "./PaginationWidgetBase"; +import type { Meta, StoryObj } from "@storybook/react"; + +const meta: Meta = { + title: "components/PaginationWidgetBase", + component: PaginationWidgetBase, + args: { + page: 1, + limit: 12, + count: 200, + }, +}; + +export default meta; +type Story = StoryObj; + +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 }, +}; diff --git a/site/src/components/PaginationWidget/utils.test.ts b/site/src/components/PaginationWidget/utils.test.ts index eb3942e22011b..01f79eb0a9f30 100644 --- a/site/src/components/PaginationWidget/utils.test.ts +++ b/site/src/components/PaginationWidget/utils.test.ts @@ -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", () => { diff --git a/site/src/components/PaginationWidget/utils.ts b/site/src/components/PaginationWidget/utils.ts index 1598ce17ded3d..9575396e29595 100644 --- a/site/src/components/PaginationWidget/utils.ts +++ b/site/src/components/PaginationWidget/utils.ts @@ -1,10 +1,3 @@ -import { - PaginationContext, - paginationMachine, - PaginationMachineRef, -} from "xServices/pagination/paginationXService"; -import { spawn } from "xstate"; - /** * Generates a ranged array with an option to step over values. * Shamelessly stolen from: @@ -63,46 +56,10 @@ export const buildPagedList = ( return range(1, numPages); }; -const getInitialPage = (page: string | null): number => - page ? Number(page) : 1; - // pages count from 1 export const getOffset = (page: number, limit: number): number => (page - 1) * limit; -interface PaginationData { - offset: number; - limit: number; -} - -export const getPaginationData = ( - ref: PaginationMachineRef, -): PaginationData => { - const snapshot = ref.getSnapshot(); - if (snapshot) { - const { page, limit } = snapshot.context; - const offset = getOffset(page, limit); - return { offset, limit }; - } else { - throw new Error("No pagination data"); - } -}; - -export const getPaginationContext = ( - searchParams: URLSearchParams, - limit: number = DEFAULT_RECORDS_PER_PAGE, -): PaginationContext => ({ - page: getInitialPage(searchParams.get("page")), - limit, -}); - -// for storybook -export const createPaginationRef = ( - context: PaginationContext, -): PaginationMachineRef => { - return spawn(paginationMachine.withContext(context)); -}; - export const nonInitialPage = (searchParams: URLSearchParams): boolean => { const page = searchParams.get("page"); const numberPage = page ? Number(page) : 1; diff --git a/site/src/xServices/pagination/paginationXService.ts b/site/src/xServices/pagination/paginationXService.ts deleted file mode 100644 index 8ec366048f5d4..0000000000000 --- a/site/src/xServices/pagination/paginationXService.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { ActorRefFrom, createMachine, sendParent, assign } from "xstate"; - -export interface PaginationContext { - page: number; - limit: number; -} - -export type PaginationEvent = - | { type: "NEXT_PAGE" } - | { type: "PREVIOUS_PAGE" } - | { type: "GO_TO_PAGE"; page: number } - | { type: "RESET_PAGE" }; - -export type PaginationMachineRef = ActorRefFrom; - -export const paginationMachine = - /** @xstate-layout N4IgpgJg5mDOIC5QAcCGUCWA7VAXDA9lgLKoDGAFtmAMQByAogBoAqA+gAoCCA4gwNoAGALqIUBWBnxExIAB6IATADZBAOgAcGgCwBWDYMEHBAdm2KANCACeiEwEZ7axboCcbkw8-3XGgL5+VmiYONIk5FRYtBwASgwAagCSAPIAqgDKnLwCIrLIElKEWLIKCAC09hrKarq6AMwarsraGormuiYaVrYIDk4u7q7e3r4BQejYeEWklNQ0PMlsLIvcfEKiSCD5kmElduq69vrKyiaCjqfu3YgauupatYp1eia+o4FbE6HTEXNx6Qx2KschtxDsintyvZqlVzmdGs0tIpbtcENplIoanV7M8jrpFCZntoAh8sAQIHA8l8pkQZpEwGoAE5gVAQHpgwoyTalZSuNTuV6CFzabSCFqdVFmdTPMU+dHnZrKMafEI08KzKJ5Aq7blKJwC1xC3QisUaCU2RDKQ5qGXaOWqaHokl+IA */ - createMachine( - { - tsTypes: {} as import("./paginationXService.typegen").Typegen0, - schema: { - context: {} as PaginationContext, - events: {} as PaginationEvent, - }, - predictableActionArguments: true, - id: "paginationMachine", - initial: "ready", - on: { - NEXT_PAGE: { - actions: ["assignNextPage", "sendUpdatePage"], - }, - PREVIOUS_PAGE: { - actions: ["assignPreviousPage", "sendUpdatePage"], - }, - GO_TO_PAGE: { - actions: ["assignPage", "sendUpdatePage"], - }, - RESET_PAGE: { - actions: ["resetPage", "sendUpdatePage"], - }, - }, - states: { - ready: {}, - }, - }, - { - actions: { - sendUpdatePage: sendParent((context) => ({ - type: "UPDATE_PAGE", - page: context.page.toString(), - })), - assignNextPage: assign({ - page: (context) => context.page + 1, - }), - assignPreviousPage: assign({ - page: (context) => context.page - 1, - }), - assignPage: assign({ - page: (_, event) => event.page, - }), - resetPage: assign({ - page: (_) => 1, - }), - }, - }, - ); From 9db6ec6d290d6fbe88fe909a5151b14b8f210d09 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 11:11:40 +0000 Subject: [PATCH 03/15] Move suspend to react-query --- site/src/api/queries/users.ts | 10 +++ site/src/pages/UsersPage/UsersPage.test.tsx | 4 +- site/src/pages/UsersPage/UsersPage.tsx | 58 ++++++++------- site/src/xServices/users/usersXService.ts | 78 --------------------- 4 files changed, 40 insertions(+), 110 deletions(-) diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 3b80c2f8ebfa0..ffa900f5e0bb0 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -1,3 +1,4 @@ +import { QueryClient } from "@tanstack/react-query"; import * as API from "api/api"; import { UpdateUserPasswordRequest, UsersRequest } from "api/typesGenerated"; @@ -29,3 +30,12 @@ export const createFirstUser = () => { mutationFn: API.createFirstUser, }; }; + +export const suspendUser = (queryClient: QueryClient) => { + return { + mutationFn: API.suspendUser, + onSuccess: async () => { + await queryClient.invalidateQueries(["users"]); + }, + }; +}; diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index cc1e965eb36fd..3114e374980c1 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -176,7 +176,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText(usersXServiceLanguage.suspendUserSuccess); + await screen.findByText("User suspended"); // Check if the API was called correctly expect(API.suspendUser).toBeCalledTimes(1); @@ -195,7 +195,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText(usersXServiceLanguage.suspendUserError); + await screen.findByText("Error suspending user"); // Check if the API was called correctly expect(API.suspendUser).toBeCalledTimes(1); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 4b109a86a94ba..1a2cdbc385ac7 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -4,7 +4,7 @@ import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; import { nonInitialPage } from "components/PaginationWidget/utils"; import { useMe } from "hooks/useMe"; import { usePermissions } from "hooks/usePermissions"; -import { FC, ReactNode } from "react"; +import { FC, ReactNode, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useSearchParams, useNavigate } from "react-router-dom"; import { usersMachine } from "xServices/users/usersXService"; @@ -15,13 +15,15 @@ import { UsersPageView } from "./UsersPageView"; import { useStatusFilterMenu } from "./UsersFilter"; import { useFilter } from "components/Filter/filter"; import { useDashboard } from "components/Dashboard/DashboardProvider"; -import { useQuery } from "@tanstack/react-query"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { getAuthMethods } from "api/api"; import { roles } from "api/queries/roles"; import { deploymentConfig } from "api/queries/deployment"; import { prepareQuery } from "utils/filters"; import { usePagination } from "hooks"; -import * as UsersQuery from "api/queries/users"; +import { users, suspendUser } from "api/queries/users"; +import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; +import { getErrorMessage } from "api/errors"; export const Language = { suspendDialogTitle: "Suspend user", @@ -36,6 +38,7 @@ const getSelectedUser = (id: string, users?: User[]) => users?.find((u) => u.id === id); export const UsersPage: FC<{ children?: ReactNode }> = () => { + const queryClient = useQueryClient(); const navigate = useNavigate(); const searchParamsResult = useSearchParams(); const { entitlements } = useDashboard(); @@ -46,17 +49,14 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { searchParamsResult, }); const usersQuery = useQuery( - UsersQuery.users({ + users({ q: prepareQuery(filter), limit: pagination.limit, offset: pagination.offset, }), ); - const users = usersQuery.data?.users; - const count = usersQuery.data?.count; const { usernameToDelete, - usernameToSuspend, usernameToActivate, userIdToResetPassword, newUserPassword, @@ -95,6 +95,8 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { }); const isLoading = usersQuery.isLoading || rolesQuery.isLoading || authMethods.isLoading; + const [confirmSuspendUser, setConfirmSuspendUser] = useState(); + const suspendUserMutation = useMutation(suspendUser(queryClient)); return ( <> @@ -104,7 +106,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { { navigate( @@ -124,13 +126,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { username: user.username, }); }} - onSuspendUser={(user) => { - usersSend({ - type: "SUSPEND_USER", - userId: user.id, - username: user.username, - }); - }} + onSuspendUser={setConfirmSuspendUser} onActivateUser={(user) => { usersSend({ type: "ACTIVATE_USER", @@ -161,7 +157,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { status: statusMenu, }, }} - count={count} + count={usersQuery.data?.count} page={pagination.page} limit={pagination.limit} onPageChange={pagination.goToPage} @@ -187,24 +183,26 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { { - usersSend("CONFIRM_USER_SUSPENSION"); + open={confirmSuspendUser !== undefined} + confirmLoading={suspendUserMutation.isLoading} + title="Suspend user" + confirmText="Suspend" + onConfirm={async () => { + try { + await suspendUserMutation.mutateAsync(confirmSuspendUser!.id); + setConfirmSuspendUser(undefined); + displaySuccess("User suspended"); + } catch (e) { + displayError(getErrorMessage(e, "Error suspending user")); + } }} onClose={() => { - usersSend("CANCEL_USER_SUSPENSION"); + setConfirmSuspendUser(undefined); }} description={ <> - {Language.suspendDialogMessagePrefix} - {usernameToSuspend && " "} - {usernameToSuspend ?? ""}? + Do you want to suspend the user{" "} + {confirmSuspendUser?.username ?? ""}? } /> @@ -241,7 +239,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { usersState.matches("resettingUserPassword") } loading={usersState.matches("resettingUserPassword")} - user={getSelectedUser(userIdToResetPassword, users)} + user={getSelectedUser(userIdToResetPassword, usersQuery.data?.users)} newPassword={newUserPassword} onClose={() => { usersSend("CANCEL_USER_PASSWORD_RESET"); diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index ddb1b2686691e..b23360e70728d 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -6,9 +6,6 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { generateRandomString } from "utils/random"; export const Language = { - getUsersError: "Error getting users.", - suspendUserSuccess: "Successfully suspended the user.", - suspendUserError: "Error suspending user.", deleteUserSuccess: "Successfully deleted the user.", deleteUserError: "Error deleting user.", activateUserSuccess: "Successfully activated the user.", @@ -20,10 +17,6 @@ export const Language = { }; export interface UsersContext { - // Suspend user - userIdToSuspend?: TypesGen.User["id"]; - usernameToSuspend?: TypesGen.User["username"]; - suspendUserError?: unknown; // Delete user userIdToDelete?: TypesGen.User["id"]; usernameToDelete?: TypesGen.User["username"]; @@ -42,12 +35,6 @@ export interface UsersContext { } export type UsersEvent = - // Suspend events - | { - type: "SUSPEND_USER"; - userId: TypesGen.User["id"]; - username: TypesGen.User["username"]; - } | { type: "CONFIRM_USER_SUSPENSION" } | { type: "CANCEL_USER_SUSPENSION" } // Delete events @@ -113,10 +100,6 @@ export const usersMachine = idle: { entry: "clearSelectedUser", on: { - SUSPEND_USER: { - target: "confirmUserSuspension", - actions: "assignUserToSuspend", - }, DELETE_USER: { target: "confirmUserDeletion", actions: "assignUserToDelete", @@ -138,16 +121,6 @@ export const usersMachine = }, }, }, - confirmUserSuspension: { - on: { - CONFIRM_USER_SUSPENSION: { - target: "suspendingUser", - }, - CANCEL_USER_SUSPENSION: { - target: "idle", - }, - }, - }, confirmUserDeletion: { on: { CONFIRM_USER_DELETE: { @@ -168,28 +141,6 @@ export const usersMachine = }, }, }, - suspendingUser: { - entry: "clearSuspendUserError", - invoke: { - src: "suspendUser", - id: "suspendUser", - onDone: [ - { - target: "idle", - actions: "displaySuspendSuccess", - }, - ], - onError: [ - { - target: "idle", - actions: [ - "assignSuspendUserError", - "displaySuspendedErrorMessage", - ], - }, - ], - }, - }, deletingUser: { entry: "clearDeleteUserError", invoke: { @@ -289,13 +240,6 @@ export const usersMachine = }, { services: { - suspendUser: (context) => { - if (!context.userIdToSuspend) { - throw new Error("userIdToSuspend is undefined"); - } - - return API.suspendUser(context.userIdToSuspend); - }, deleteUser: (context) => { if (!context.userIdToDelete) { throw new Error("userIdToDelete is undefined"); @@ -334,8 +278,6 @@ export const usersMachine = actions: { clearSelectedUser: assign({ - userIdToSuspend: (_) => undefined, - usernameToSuspend: (_) => undefined, userIdToDelete: (_) => undefined, usernameToDelete: (_) => undefined, userIdToActivate: (_) => undefined, @@ -343,10 +285,6 @@ export const usersMachine = userIdToResetPassword: (_) => undefined, userIdToUpdateRoles: (_) => undefined, }), - assignUserToSuspend: assign({ - userIdToSuspend: (_, event) => event.userId, - usernameToSuspend: (_, event) => event.username, - }), assignUserToDelete: assign({ userIdToDelete: (_, event) => event.userId, usernameToDelete: (_, event) => event.username, @@ -361,9 +299,6 @@ export const usersMachine = assignUserIdToUpdateRoles: assign({ userIdToUpdateRoles: (_, event) => event.userId, }), - assignSuspendUserError: assign({ - suspendUserError: (_, event) => event.data, - }), assignDeleteUserError: assign({ deleteUserError: (_, event) => event.data, }), @@ -376,9 +311,6 @@ export const usersMachine = assignUpdateRolesError: assign({ updateUserRolesError: (_, event) => event.data, }), - clearSuspendUserError: assign({ - suspendUserError: (_) => undefined, - }), clearDeleteUserError: assign({ deleteUserError: (_) => undefined, }), @@ -391,16 +323,6 @@ export const usersMachine = clearUpdateUserRolesError: assign({ updateUserRolesError: (_) => undefined, }), - displaySuspendSuccess: () => { - displaySuccess(Language.suspendUserSuccess); - }, - displaySuspendedErrorMessage: (context) => { - const message = getErrorMessage( - context.suspendUserError, - Language.suspendUserError, - ); - displayError(message); - }, displayDeleteSuccess: () => { displaySuccess(Language.deleteUserSuccess); }, From 7fb38589688f92bfaf014d86826afea09c2b8d02 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 11:21:15 +0000 Subject: [PATCH 04/15] Move activate to react-query --- site/src/api/queries/users.ts | 9 +++ site/src/pages/UsersPage/UsersPage.test.tsx | 4 +- site/src/pages/UsersPage/UsersPage.tsx | 44 +++++------- site/src/xServices/users/usersXService.ts | 79 --------------------- 4 files changed, 30 insertions(+), 106 deletions(-) diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index ffa900f5e0bb0..0b862a052b434 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -39,3 +39,12 @@ export const suspendUser = (queryClient: QueryClient) => { }, }; }; + +export const activateUser = (queryClient: QueryClient) => { + return { + mutationFn: API.activateUser, + onSuccess: async () => { + await queryClient.invalidateQueries(["users"]); + }, + }; +}; diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index 3114e374980c1..14b028759aa34 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -301,7 +301,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText(usersXServiceLanguage.activateUserSuccess); + await screen.findByText("User activated"); // Check if the API was called correctly expect(API.activateUser).toBeCalledTimes(1); @@ -317,7 +317,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText(usersXServiceLanguage.activateUserError); + await screen.findByText("Error activating user"); // Check if the API was called correctly expect(API.activateUser).toBeCalledTimes(1); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 1a2cdbc385ac7..1c3e9a7fa8251 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -21,7 +21,7 @@ import { roles } from "api/queries/roles"; import { deploymentConfig } from "api/queries/deployment"; import { prepareQuery } from "utils/filters"; import { usePagination } from "hooks"; -import { users, suspendUser } from "api/queries/users"; +import { users, suspendUser, activateUser } from "api/queries/users"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { getErrorMessage } from "api/errors"; @@ -55,12 +55,8 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { offset: pagination.offset, }), ); - const { - usernameToDelete, - usernameToActivate, - userIdToResetPassword, - newUserPassword, - } = usersState.context; + const { usernameToDelete, userIdToResetPassword, newUserPassword } = + usersState.context; const { updateUsers: canEditUsers, viewDeploymentValues } = usePermissions(); const rolesQuery = useQuery({ ...roles(), enabled: canEditUsers }); const { data: deploymentValues } = useQuery({ @@ -97,6 +93,8 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { usersQuery.isLoading || rolesQuery.isLoading || authMethods.isLoading; const [confirmSuspendUser, setConfirmSuspendUser] = useState(); const suspendUserMutation = useMutation(suspendUser(queryClient)); + const [confirmActivateUser, setConfirmActivateUser] = useState(); + const activateUserMutation = useMutation(activateUser(queryClient)); return ( <> @@ -127,13 +125,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { }); }} onSuspendUser={setConfirmSuspendUser} - onActivateUser={(user) => { - usersSend({ - type: "ACTIVATE_USER", - userId: user.id, - username: user.username, - }); - }} + onActivateUser={setConfirmActivateUser} onResetUserPassword={(user) => { usersSend({ type: "RESET_USER_PASSWORD", userId: user.id }); }} @@ -210,24 +202,26 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { { - usersSend("CONFIRM_USER_ACTIVATION"); + onConfirm={async () => { + try { + await activateUserMutation.mutateAsync(confirmActivateUser!.id); + setConfirmActivateUser(undefined); + displaySuccess("User activated"); + } catch (e) { + displayError(getErrorMessage(e, "Error activating user")); + } }} onClose={() => { - usersSend("CANCEL_USER_ACTIVATION"); + setConfirmActivateUser(undefined); }} description={ <> - {Language.activateDialogMessagePrefix} - {usernameToActivate && " "} - {usernameToActivate ?? ""}? + {Language.activateDialogMessagePrefix}{" "} + {confirmActivateUser?.username ?? ""}? } /> diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index b23360e70728d..547404a7237be 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -8,8 +8,6 @@ import { generateRandomString } from "utils/random"; export const Language = { deleteUserSuccess: "Successfully deleted the user.", deleteUserError: "Error deleting user.", - activateUserSuccess: "Successfully activated the user.", - activateUserError: "Error activating user.", resetUserPasswordSuccess: "Successfully updated the user password.", resetUserPasswordError: "Error on resetting the user password.", updateUserRolesSuccess: "Successfully updated the user roles.", @@ -21,10 +19,6 @@ export interface UsersContext { userIdToDelete?: TypesGen.User["id"]; usernameToDelete?: TypesGen.User["username"]; deleteUserError?: unknown; - // Activate user - userIdToActivate?: TypesGen.User["id"]; - usernameToActivate?: TypesGen.User["username"]; - activateUserError?: unknown; // Reset user password userIdToResetPassword?: TypesGen.User["id"]; resetUserPasswordError?: unknown; @@ -45,14 +39,6 @@ export type UsersEvent = } | { type: "CONFIRM_USER_DELETE" } | { type: "CANCEL_USER_DELETE" } - // Activate events - | { - type: "ACTIVATE_USER"; - userId: TypesGen.User["id"]; - username: TypesGen.User["username"]; - } - | { type: "CONFIRM_USER_ACTIVATION" } - | { type: "CANCEL_USER_ACTIVATION" } // Reset password events | { type: "RESET_USER_PASSWORD"; userId: TypesGen.User["id"] } | { type: "CONFIRM_USER_PASSWORD_RESET" } @@ -104,10 +90,6 @@ export const usersMachine = target: "confirmUserDeletion", actions: "assignUserToDelete", }, - ACTIVATE_USER: { - target: "confirmUserActivation", - actions: "assignUserToActivate", - }, RESET_USER_PASSWORD: { target: "confirmUserPasswordReset", actions: [ @@ -131,16 +113,6 @@ export const usersMachine = }, }, }, - confirmUserActivation: { - on: { - CONFIRM_USER_ACTIVATION: { - target: "activatingUser", - }, - CANCEL_USER_ACTIVATION: { - target: "idle", - }, - }, - }, deletingUser: { entry: "clearDeleteUserError", invoke: { @@ -160,28 +132,6 @@ export const usersMachine = ], }, }, - activatingUser: { - entry: "clearActivateUserError", - invoke: { - src: "activateUser", - id: "activateUser", - onDone: [ - { - target: "idle", - actions: "displayActivateSuccess", - }, - ], - onError: [ - { - target: "idle", - actions: [ - "assignActivateUserError", - "displayActivatedErrorMessage", - ], - }, - ], - }, - }, confirmUserPasswordReset: { on: { CONFIRM_USER_PASSWORD_RESET: { @@ -246,13 +196,6 @@ export const usersMachine = } return API.deleteUser(context.userIdToDelete); }, - activateUser: (context) => { - if (!context.userIdToActivate) { - throw new Error("userIdToActivate is undefined"); - } - - return API.activateUser(context.userIdToActivate); - }, resetUserPassword: (context) => { if (!context.userIdToResetPassword) { throw new Error("userIdToResetPassword is undefined"); @@ -280,8 +223,6 @@ export const usersMachine = clearSelectedUser: assign({ userIdToDelete: (_) => undefined, usernameToDelete: (_) => undefined, - userIdToActivate: (_) => undefined, - usernameToActivate: (_) => undefined, userIdToResetPassword: (_) => undefined, userIdToUpdateRoles: (_) => undefined, }), @@ -289,10 +230,6 @@ export const usersMachine = userIdToDelete: (_, event) => event.userId, usernameToDelete: (_, event) => event.username, }), - assignUserToActivate: assign({ - userIdToActivate: (_, event) => event.userId, - usernameToActivate: (_, event) => event.username, - }), assignUserIdToResetPassword: assign({ userIdToResetPassword: (_, event) => event.userId, }), @@ -302,9 +239,6 @@ export const usersMachine = assignDeleteUserError: assign({ deleteUserError: (_, event) => event.data, }), - assignActivateUserError: assign({ - activateUserError: (_, event) => event.data, - }), assignResetUserPasswordError: assign({ resetUserPasswordError: (_, event) => event.data, }), @@ -314,9 +248,6 @@ export const usersMachine = clearDeleteUserError: assign({ deleteUserError: (_) => undefined, }), - clearActivateUserError: assign({ - activateUserError: (_) => undefined, - }), clearResetUserPasswordError: assign({ resetUserPasswordError: (_) => undefined, }), @@ -333,16 +264,6 @@ export const usersMachine = ); displayError(message); }, - displayActivateSuccess: () => { - displaySuccess(Language.activateUserSuccess); - }, - displayActivatedErrorMessage: (context) => { - const message = getErrorMessage( - context.activateUserError, - Language.activateUserError, - ); - displayError(message); - }, displayResetPasswordSuccess: () => { displaySuccess(Language.resetUserPasswordSuccess); }, From 1a6fcfbd3994f88b3ab526e751347e3dfc1eea03 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 11:21:54 +0000 Subject: [PATCH 05/15] Remove missed suspend events --- site/src/xServices/users/usersXService.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index 547404a7237be..c09e52f40cd0d 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -29,8 +29,6 @@ export interface UsersContext { } export type UsersEvent = - | { type: "CONFIRM_USER_SUSPENSION" } - | { type: "CANCEL_USER_SUSPENSION" } // Delete events | { type: "DELETE_USER"; From 04fb5d31e57f1008ef4a4491dfcaabb8cc2090ee Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 11:31:32 +0000 Subject: [PATCH 06/15] Move delete to react-query --- site/src/api/queries/users.ts | 14 +++- .../pages/CreateUserPage/CreateUserPage.tsx | 5 +- site/src/pages/UsersPage/UsersPage.test.tsx | 4 +- site/src/pages/UsersPage/UsersPage.tsx | 46 ++++++----- site/src/xServices/users/usersXService.ts | 79 +------------------ 5 files changed, 48 insertions(+), 100 deletions(-) diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 0b862a052b434..e1bb6455439ed 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -19,9 +19,12 @@ export const updatePassword = () => { }; }; -export const createUser = () => { +export const createUser = (queryClient: QueryClient) => { return { mutationFn: API.createUser, + onSuccess: async () => { + await queryClient.invalidateQueries(["users"]); + }, }; }; @@ -48,3 +51,12 @@ export const activateUser = (queryClient: QueryClient) => { }, }; }; + +export const deleteUser = (queryClient: QueryClient) => { + return { + mutationFn: API.deleteUser, + onSuccess: async () => { + await queryClient.invalidateQueries(["users"]); + }, + }; +}; diff --git a/site/src/pages/CreateUserPage/CreateUserPage.tsx b/site/src/pages/CreateUserPage/CreateUserPage.tsx index 98ca08a08cf38..5493852d21d45 100644 --- a/site/src/pages/CreateUserPage/CreateUserPage.tsx +++ b/site/src/pages/CreateUserPage/CreateUserPage.tsx @@ -6,7 +6,7 @@ import { CreateUserForm } from "./CreateUserForm"; import { Margins } from "components/Margins/Margins"; import { pageTitle } from "utils/page"; import { getAuthMethods } from "api/api"; -import { useMutation, useQuery } from "@tanstack/react-query"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { createUser } from "api/queries/users"; import { displaySuccess } from "components/GlobalSnackbar/utils"; @@ -17,7 +17,8 @@ export const Language = { export const CreateUserPage: FC = () => { const myOrgId = useOrganizationId(); const navigate = useNavigate(); - const createUserMutation = useMutation(createUser()); + const queryClient = useQueryClient(); + const createUserMutation = useMutation(createUser(queryClient)); // TODO: We should probably place this somewhere else to reduce the number of calls. // This would be called each time this page is loaded. const { data: authMethods } = useQuery({ diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index 14b028759aa34..eb53372804a46 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -252,7 +252,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText(usersXServiceLanguage.deleteUserSuccess); + await screen.findByText("User deleted"); // Check if the API was called correctly expect(API.deleteUser).toBeCalledTimes(1); @@ -274,7 +274,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText(usersXServiceLanguage.deleteUserError); + await screen.findByText("Error deleting user"); // Check if the API was called correctly expect(API.deleteUser).toBeCalledTimes(1); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 1c3e9a7fa8251..6ad1ca55d864e 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -21,7 +21,12 @@ import { roles } from "api/queries/roles"; import { deploymentConfig } from "api/queries/deployment"; import { prepareQuery } from "utils/filters"; import { usePagination } from "hooks"; -import { users, suspendUser, activateUser } from "api/queries/users"; +import { + users, + suspendUser, + activateUser, + deleteUser, +} from "api/queries/users"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { getErrorMessage } from "api/errors"; @@ -55,8 +60,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { offset: pagination.offset, }), ); - const { usernameToDelete, userIdToResetPassword, newUserPassword } = - usersState.context; + const { userIdToResetPassword, newUserPassword } = usersState.context; const { updateUsers: canEditUsers, viewDeploymentValues } = usePermissions(); const rolesQuery = useQuery({ ...roles(), enabled: canEditUsers }); const { data: deploymentValues } = useQuery({ @@ -91,10 +95,15 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { }); const isLoading = usersQuery.isLoading || rolesQuery.isLoading || authMethods.isLoading; + // Suspend const [confirmSuspendUser, setConfirmSuspendUser] = useState(); const suspendUserMutation = useMutation(suspendUser(queryClient)); + // Activate const [confirmActivateUser, setConfirmActivateUser] = useState(); const activateUserMutation = useMutation(activateUser(queryClient)); + // Delete + const [confirmDeleteUser, setConfirmDeleteUser] = useState(); + const deleteUserMutation = useMutation(deleteUser(queryClient)); return ( <> @@ -117,13 +126,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { "/audit?filter=" + encodeURIComponent(`username:${user.username}`), ); }} - onDeleteUser={(user) => { - usersSend({ - type: "DELETE_USER", - userId: user.id, - username: user.username, - }); - }} + onDeleteUser={setConfirmDeleteUser} onSuspendUser={setConfirmSuspendUser} onActivateUser={setConfirmActivateUser} onResetUserPassword={(user) => { @@ -156,19 +159,22 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { /> { - usersSend("CONFIRM_USER_DELETE"); + onConfirm={async () => { + try { + await deleteUserMutation.mutateAsync(confirmDeleteUser!.id); + setConfirmDeleteUser(undefined); + displaySuccess("User deleted"); + } catch (e) { + displayError(getErrorMessage(e, "Error deleting user")); + } }} onCancel={() => { - usersSend("CANCEL_USER_DELETE"); + setConfirmDeleteUser(undefined); }} /> diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index c09e52f40cd0d..86ef72d4e0148 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -6,8 +6,6 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { generateRandomString } from "utils/random"; export const Language = { - deleteUserSuccess: "Successfully deleted the user.", - deleteUserError: "Error deleting user.", resetUserPasswordSuccess: "Successfully updated the user password.", resetUserPasswordError: "Error on resetting the user password.", updateUserRolesSuccess: "Successfully updated the user roles.", @@ -15,10 +13,6 @@ export const Language = { }; export interface UsersContext { - // Delete user - userIdToDelete?: TypesGen.User["id"]; - usernameToDelete?: TypesGen.User["username"]; - deleteUserError?: unknown; // Reset user password userIdToResetPassword?: TypesGen.User["id"]; resetUserPasswordError?: unknown; @@ -29,14 +23,6 @@ export interface UsersContext { } export type UsersEvent = - // Delete events - | { - type: "DELETE_USER"; - userId: TypesGen.User["id"]; - username: TypesGen.User["username"]; - } - | { type: "CONFIRM_USER_DELETE" } - | { type: "CANCEL_USER_DELETE" } // Reset password events | { type: "RESET_USER_PASSWORD"; userId: TypesGen.User["id"] } | { type: "CONFIRM_USER_PASSWORD_RESET" } @@ -84,10 +70,6 @@ export const usersMachine = idle: { entry: "clearSelectedUser", on: { - DELETE_USER: { - target: "confirmUserDeletion", - actions: "assignUserToDelete", - }, RESET_USER_PASSWORD: { target: "confirmUserPasswordReset", actions: [ @@ -101,35 +83,6 @@ export const usersMachine = }, }, }, - confirmUserDeletion: { - on: { - CONFIRM_USER_DELETE: { - target: "deletingUser", - }, - CANCEL_USER_DELETE: { - target: "idle", - }, - }, - }, - deletingUser: { - entry: "clearDeleteUserError", - invoke: { - src: "deleteUser", - id: "deleteUser", - onDone: [ - { - target: "idle", - actions: "displayDeleteSuccess", - }, - ], - onError: [ - { - target: "idle", - actions: ["assignDeleteUserError", "displayDeleteErrorMessage"], - }, - ], - }, - }, confirmUserPasswordReset: { on: { CONFIRM_USER_PASSWORD_RESET: { @@ -188,12 +141,6 @@ export const usersMachine = }, { services: { - deleteUser: (context) => { - if (!context.userIdToDelete) { - throw new Error("userIdToDelete is undefined"); - } - return API.deleteUser(context.userIdToDelete); - }, resetUserPassword: (context) => { if (!context.userIdToResetPassword) { throw new Error("userIdToResetPassword is undefined"); @@ -219,49 +166,31 @@ export const usersMachine = actions: { clearSelectedUser: assign({ - userIdToDelete: (_) => undefined, - usernameToDelete: (_) => undefined, userIdToResetPassword: (_) => undefined, userIdToUpdateRoles: (_) => undefined, }), - assignUserToDelete: assign({ - userIdToDelete: (_, event) => event.userId, - usernameToDelete: (_, event) => event.username, - }), + assignUserIdToResetPassword: assign({ userIdToResetPassword: (_, event) => event.userId, }), assignUserIdToUpdateRoles: assign({ userIdToUpdateRoles: (_, event) => event.userId, }), - assignDeleteUserError: assign({ - deleteUserError: (_, event) => event.data, - }), + assignResetUserPasswordError: assign({ resetUserPasswordError: (_, event) => event.data, }), assignUpdateRolesError: assign({ updateUserRolesError: (_, event) => event.data, }), - clearDeleteUserError: assign({ - deleteUserError: (_) => undefined, - }), + clearResetUserPasswordError: assign({ resetUserPasswordError: (_) => undefined, }), clearUpdateUserRolesError: assign({ updateUserRolesError: (_) => undefined, }), - displayDeleteSuccess: () => { - displaySuccess(Language.deleteUserSuccess); - }, - displayDeleteErrorMessage: (context) => { - const message = getErrorMessage( - context.deleteUserError, - Language.deleteUserError, - ); - displayError(message); - }, + displayResetPasswordSuccess: () => { displaySuccess(Language.resetUserPasswordSuccess); }, From 44d8de64af67217fe3e6659061f243f51a1d328c Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 11:43:18 +0000 Subject: [PATCH 07/15] Move reset password to react-query --- site/src/pages/UsersPage/UsersPage.test.tsx | 4 +- site/src/pages/UsersPage/UsersPage.tsx | 55 ++++++---- site/src/xServices/users/usersXService.ts | 115 +------------------- 3 files changed, 42 insertions(+), 132 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index eb53372804a46..edad0f069dfc4 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -338,7 +338,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText(usersXServiceLanguage.resetUserPasswordSuccess); + await screen.findByText("Password reset"); // Check if the API was called correctly expect(API.updateUserPassword).toBeCalledTimes(1); @@ -357,7 +357,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText(usersXServiceLanguage.resetUserPasswordError); + await screen.findByText("Error resetting password"); // Check if the API was called correctly expect(API.updateUserPassword).toBeCalledTimes(1); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 6ad1ca55d864e..e08f69a409ebd 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -26,9 +26,11 @@ import { suspendUser, activateUser, deleteUser, + updatePassword, } from "api/queries/users"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { getErrorMessage } from "api/errors"; +import { generateRandomString } from "utils/random"; export const Language = { suspendDialogTitle: "Suspend user", @@ -39,9 +41,6 @@ export const Language = { activateDialogMessagePrefix: "Do you want to activate the user", }; -const getSelectedUser = (id: string, users?: User[]) => - users?.find((u) => u.id === id); - export const UsersPage: FC<{ children?: ReactNode }> = () => { const queryClient = useQueryClient(); const navigate = useNavigate(); @@ -60,7 +59,6 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { offset: pagination.offset, }), ); - const { userIdToResetPassword, newUserPassword } = usersState.context; const { updateUsers: canEditUsers, viewDeploymentValues } = usePermissions(); const rolesQuery = useQuery({ ...roles(), enabled: canEditUsers }); const { data: deploymentValues } = useQuery({ @@ -104,6 +102,12 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { // Delete const [confirmDeleteUser, setConfirmDeleteUser] = useState(); const deleteUserMutation = useMutation(deleteUser(queryClient)); + // Reset password + const [confirmResetPassword, setConfirmResetPassword] = useState<{ + user: User; + newPassword: string; + }>(); + const updatePasswordMutation = useMutation(updatePassword()); return ( <> @@ -130,7 +134,10 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { onSuspendUser={setConfirmSuspendUser} onActivateUser={setConfirmActivateUser} onResetUserPassword={(user) => { - usersSend({ type: "RESET_USER_PASSWORD", userId: user.id }); + setConfirmResetPassword({ + user, + newPassword: generateRandomString(12), + }); }} onUpdateUserRoles={(user, roles) => { usersSend({ @@ -232,23 +239,29 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { } /> - {userIdToResetPassword && ( - { + setConfirmResetPassword(undefined); + }} + onConfirm={async () => { + try { + await updatePasswordMutation.mutateAsync({ + userId: confirmResetPassword!.user.id, + password: confirmResetPassword!.newPassword, + old_password: "", + }); + setConfirmResetPassword(undefined); + displaySuccess("Password reset"); + } catch (e) { + displayError(getErrorMessage(e, "Error resetting password")); } - loading={usersState.matches("resettingUserPassword")} - user={getSelectedUser(userIdToResetPassword, usersQuery.data?.users)} - newPassword={newUserPassword} - onClose={() => { - usersSend("CANCEL_USER_PASSWORD_RESET"); - }} - onConfirm={() => { - usersSend("CONFIRM_USER_PASSWORD_RESET"); - }} - /> - )} + }} + /> ); }; diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index 86ef72d4e0148..a9c0ac935b9dc 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -2,37 +2,26 @@ import { assign, createMachine } from "xstate"; import * as API from "api/api"; import { getErrorMessage } from "api/errors"; import * as TypesGen from "api/typesGenerated"; -import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; -import { generateRandomString } from "utils/random"; +import { displayError } from "components/GlobalSnackbar/utils"; export const Language = { - resetUserPasswordSuccess: "Successfully updated the user password.", - resetUserPasswordError: "Error on resetting the user password.", updateUserRolesSuccess: "Successfully updated the user roles.", updateUserRolesError: "Error on updating the user roles.", }; export interface UsersContext { - // Reset user password - userIdToResetPassword?: TypesGen.User["id"]; - resetUserPasswordError?: unknown; - newUserPassword?: string; // Update user roles userIdToUpdateRoles?: TypesGen.User["id"]; updateUserRolesError?: unknown; } export type UsersEvent = - // Reset password events - | { type: "RESET_USER_PASSWORD"; userId: TypesGen.User["id"] } - | { type: "CONFIRM_USER_PASSWORD_RESET" } - | { type: "CANCEL_USER_PASSWORD_RESET" } // Update roles events - | { - type: "UPDATE_USER_ROLES"; - userId: TypesGen.User["id"]; - roles: TypesGen.Role["name"][]; - }; + { + type: "UPDATE_USER_ROLES"; + userId: TypesGen.User["id"]; + roles: TypesGen.Role["name"][]; + }; export const usersMachine = /** @xstate-layout N4IgpgJg5mDOIC5QFdZgE6wMoBcCGOYAdLPujgJYB2UACnlNQRQPZUDEA2gAwC6ioAA4tYFSmwEgAHogC0ARgBMigBxEAnCoCsAdhWrdK9QBYdAGhABPRPO7ruRAMzdj3HYuNbFANm5b1AL4BFqgY2PiERDA4lDQAqmiY7BBsxNQAbiwA1sTRCWE8-EggwqLiVJIyCArc8upOxt6K-o7qjloq3ioW1ghKPkQu6m2KOraaw0EhieEEuWAx1FD5SRjoLOhEggA2BABmGwC2UQsrsIWSpWKsFcVVCqqOGoqOKnUqKrW15lY2ivI6IiNRqOHR6bhdXRTEChTC4OZECgQbZgdhYOJYWgAUQAcgARAD6GKxACULsUruVKnIdLoiPJjPItF42p0fI4ejYtMZjE4ASoxi4dI4lCpobDZpEkSj2HisQAZLEAFSxRKwpPJQhE1wkdxp3nqAJM3i0zm8tPknL63gNRB0TQBTNBHRM4pm8KlyNRAEEAMJKgCSADVvSq1Rq+JdtVS9dUdMYnkN1C9uM0Teofr15CpXEQPt95IXC952m6wh60l72CSseqleGSQTaN6sFgAOoAeRJeM1JWjN2pca8RH+bmar3tdUUVve3me8kcIqU3G0NrLcIilZlcVoeNDquJjZJHcVWF7lIHsdk8d5As6ybeHxLxitym4Dh5b1pYx0LhUjnXSUt1RHc9zDZsAHEsXPftdVAe4GT0IEMz0UYtAhV51BnZwHDsTQDQXZNNEAitESrUD9wJAAxAN5RVMlIwpWDbnguR5BNXlEPUbNmk+Ixp1+PpFHULQgUcbxTH8YTCy8EjNyIABjNg9godBDhWLBUEEMAqFENh2F9DscRokkAFkGwJdFMVxLAAyMmCykvVjqg8eQ82UdQJJ5DxaWZGdmUUDRWi8VM+LGbw5IRJSqBUtSNK0nS9I4X1vRxX0FQsqzsRxWz7MYrVHLg6Q5GMbQgWZbiS3tbhWktQT2P+PMBR0DMM0dLQIuCGF3Xk6LYvUxI8TAFFygMoyTPMw8CTlRUVQcnUWOKlzlEGBlHATPRhn0JkZy6XlSuMUZamEox7UiyI+tUgaMCGkabgM1L0vlCyZuVaD8r7QrFvuYwM3pewRUhTxPkzGx7XqYTGTayTtEUc7iEuuLEm9BTKHSZh9MM4yAzMiy-UDENAzyooCoWwdZBeNQfBccT0NqbkOhnHNAXaPxVHsJpTB0eHFOUq6VhRtGMeSx6Mqm-Hg1DOycXmmNnNkdC51KlqlHaYTRgErM2lvZkDUO37-ELHnYASqgICWFZklSREqEyHISFNiAVllpyltaJ5fscXivd0UqsPq0q3MLFrzRzV5MONx2LcSdg1g2LZdhwA41Id2BtLN52PovIqqhNUTOgI2xF3tUEZy5kcXXNQ6Vx8TrpnLeSIGGhZo4wK2qDSW3smIJuRrATOSc+snY1cD83hC1lPG8OqswkiHGghX8Du0Ywed7lv4hjuPNh2fYjiIdfCAHqMvsHc03PfdpPI6xkF26eqS1E5pUNUbQM1GHm8FRih0diZYY5SB3G2dtiBfyFkfRILsc6IGCiOX8vgmTsTGJ5F89UwQOBDtoU0pgPCry6hKUiYCf7ME3m3beCc94pyIb-fukCs7MTPtPIgCDhJuE+O7bwM49BPBFCYXQoxhjsTFPgnqUU+ZIwwPQWAsAADuGwIAkjgAsMa2NcZTWbK2Ts3YCQ1jrFA76bEPiDFaP+G0UkXhaFfLUNQWhsx6xeM4dweD64bjETFfmiQpGyPkYotAOAHppTFuqRsGj2xdkJLo5U+jya2MBDTWkyhaQGk6K+WwolGjuH+KVaeDIeboCUYsUh6AvFyPQBAduncQFEHyX4lYJT5HRNjKCD2RYRIgyaIoPwM5uJuXEsKDqjwVZ5IKX-OpeBpGlPKeQ3eSd941NOJ48Z3iymNOchJJ4bRFyXy6MmLo3TPIaAFJ0xcug3CMh5sgQQEASH-wwCSFgKJYAVOAd3IglzrkQLuQ8uAqy3adAaHYdiSgOivBnogf4tpWhbQBK4ZQuSRENwRO8m5Kx7mPNjugdYO9E7J2OMiz56A0U-PoafWMJo3I+AZLXcc7FLGCThXObiYxRie2DvDdgFEww0TohGQe2cDHVFsAmekvgTnsV+mkq09gHDOHWs0TpU5OpdSoCwJu8BigEPkqQPA5Alj0EYFQYWJ9h7ywXAyPMdgEzV02bSKVC47SfHfLSX65pSwItcZEaIoyZjGrlktBQSh6guAhI0aejJhQzjcKJA0Xt2g8i8Aqnm0owC+tdghO+eYTRoSLo0DM2F4xEGwZ00wbg1bc3dUBXm7iJHoE0mnRKrt+XkwlcY0wrI6jZlqP5YcbRjQeDsLoM6FbSKI2uugW6LcipNqvNk5hph1rTw6HUPZD8cxAhzEdQNp1nHdURRdcRY7BbEL9dO+WpVo2lSjQKEUjJ2hM25KtNoisvaHXNJHetZtW7oFTdAhAbxmF2HjKCIwIl1b+SZGJES75RimIBGvZu3qMA-oFQKO0edxJVW8kYXazI7ToT9nTAUrph3yWoSixIyHBxlQkrYaDnwOqaFBn0N4c5bGsm0MdcYPNR1jImT4gplGZ0SXpKaAdYJQRtHNFYjwThWoLg8JVESwy-GIeKUsyZgnnIMjcuGuVEIl2mCsamJwXsBR2BeOKuuu6PXEHxV+ol6rSZ+qqCKQYyYMNoRBsKVBvR-j-ITHPcS7DXhWc1XMTT-qATGa4jxDoK5kxWjeL0mqIJ5NczwUEIAA */ @@ -43,21 +32,6 @@ export const usersMachine = context: {} as UsersContext, events: {} as UsersEvent, services: {} as { - createUser: { - data: TypesGen.User; - }; - suspendUser: { - data: TypesGen.User; - }; - deleteUser: { - data: undefined; - }; - activateUser: { - data: TypesGen.User; - }; - updateUserPassword: { - data: undefined; - }; updateUserRoles: { data: TypesGen.User; }; @@ -70,51 +44,12 @@ export const usersMachine = idle: { entry: "clearSelectedUser", on: { - RESET_USER_PASSWORD: { - target: "confirmUserPasswordReset", - actions: [ - "assignUserIdToResetPassword", - "generateRandomPassword", - ], - }, UPDATE_USER_ROLES: { target: "updatingUserRoles", actions: "assignUserIdToUpdateRoles", }, }, }, - confirmUserPasswordReset: { - on: { - CONFIRM_USER_PASSWORD_RESET: { - target: "resettingUserPassword", - }, - CANCEL_USER_PASSWORD_RESET: { - target: "idle", - }, - }, - }, - resettingUserPassword: { - entry: "clearResetUserPasswordError", - invoke: { - src: "resetUserPassword", - id: "resetUserPassword", - onDone: [ - { - target: "idle", - actions: "displayResetPasswordSuccess", - }, - ], - onError: [ - { - target: "idle", - actions: [ - "assignResetUserPasswordError", - "displayResetPasswordErrorMessage", - ], - }, - ], - }, - }, updatingUserRoles: { entry: "clearUpdateUserRolesError", invoke: { @@ -141,20 +76,6 @@ export const usersMachine = }, { services: { - resetUserPassword: (context) => { - if (!context.userIdToResetPassword) { - throw new Error("userIdToResetPassword is undefined"); - } - - if (!context.newUserPassword) { - throw new Error("newUserPassword not generated"); - } - - return API.updateUserPassword(context.userIdToResetPassword, { - password: context.newUserPassword, - old_password: "", - }); - }, updateUserRoles: (context, event) => { if (!context.userIdToUpdateRoles) { throw new Error("userIdToUpdateRoles is undefined"); @@ -166,41 +87,20 @@ export const usersMachine = actions: { clearSelectedUser: assign({ - userIdToResetPassword: (_) => undefined, userIdToUpdateRoles: (_) => undefined, }), - assignUserIdToResetPassword: assign({ - userIdToResetPassword: (_, event) => event.userId, - }), assignUserIdToUpdateRoles: assign({ userIdToUpdateRoles: (_, event) => event.userId, }), - assignResetUserPasswordError: assign({ - resetUserPasswordError: (_, event) => event.data, - }), assignUpdateRolesError: assign({ updateUserRolesError: (_, event) => event.data, }), - clearResetUserPasswordError: assign({ - resetUserPasswordError: (_) => undefined, - }), clearUpdateUserRolesError: assign({ updateUserRolesError: (_) => undefined, }), - - displayResetPasswordSuccess: () => { - displaySuccess(Language.resetUserPasswordSuccess); - }, - displayResetPasswordErrorMessage: (context) => { - const message = getErrorMessage( - context.resetUserPasswordError, - Language.resetUserPasswordError, - ); - displayError(message); - }, displayUpdateRolesErrorMessage: (context) => { const message = getErrorMessage( context.updateUserRolesError, @@ -208,9 +108,6 @@ export const usersMachine = ); displayError(message); }, - generateRandomPassword: assign({ - newUserPassword: (_) => generateRandomString(12), - }), updateUserRolesInTheList: assign({ // users: ({ users }, event) => { // if (!users) { From 269f67fe7cbae0eb7955af9d83bfeedb89d57d82 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 12:22:33 +0000 Subject: [PATCH 08/15] Move update roles to react-query --- site/src/api/queries/users.ts | 10 ++ site/src/pages/UsersPage/UsersPage.test.tsx | 6 +- site/src/pages/UsersPage/UsersPage.tsx | 24 ++-- .../UsersPage/UsersTable/EditRolesButton.tsx | 2 +- site/src/xServices/users/usersXService.ts | 123 ------------------ 5 files changed, 26 insertions(+), 139 deletions(-) delete mode 100644 site/src/xServices/users/usersXService.ts diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index e1bb6455439ed..9bb1a43d2050e 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -60,3 +60,13 @@ export const deleteUser = (queryClient: QueryClient) => { }, }; }; + +export const updateRoles = (queryClient: QueryClient) => { + return { + mutationFn: ({ userId, roles }: { userId: string; roles: string[] }) => + API.updateUserRoles(roles, userId), + onSuccess: async () => { + await queryClient.invalidateQueries(["users"]); + }, + }; +}; diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index edad0f069dfc4..e1b5eedf5a57a 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -8,7 +8,6 @@ import { MockAuditorRole, MockOwnerRole, } from "testHelpers/entities"; -import { Language as usersXServiceLanguage } from "xServices/users/usersXService"; import * as API from "api/api"; import { Role } from "api/typesGenerated"; import { Language as ResetPasswordDialogLanguage } from "./ResetPasswordDialog"; @@ -406,10 +405,7 @@ describe("UsersPage", () => { }, MockAuditorRole); // Check if the error message is displayed - const errorMessage = await screen.findByText( - usersXServiceLanguage.updateUserRolesError, - ); - await waitFor(() => expect(errorMessage).toBeDefined()); + await waitFor(() => expect("Error updating user roles").toBeDefined()); // Check if the API was called correctly const currentRoles = MockUser.roles.map((r) => r.name); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index e08f69a409ebd..a44fbd6e80654 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -1,4 +1,3 @@ -import { useMachine } from "@xstate/react"; import { User } from "api/typesGenerated"; import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; import { nonInitialPage } from "components/PaginationWidget/utils"; @@ -7,7 +6,6 @@ import { usePermissions } from "hooks/usePermissions"; import { FC, ReactNode, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useSearchParams, useNavigate } from "react-router-dom"; -import { usersMachine } from "xServices/users/usersXService"; import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog"; import { ResetPasswordDialog } from "./ResetPasswordDialog"; import { pageTitle } from "utils/page"; @@ -27,6 +25,7 @@ import { activateUser, deleteUser, updatePassword, + updateRoles, } from "api/queries/users"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { getErrorMessage } from "api/errors"; @@ -48,7 +47,6 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { const { entitlements } = useDashboard(); const [searchParams] = searchParamsResult; const filter = searchParams.get("filter") ?? ""; - const [usersState, usersSend] = useMachine(usersMachine); const pagination = usePagination({ searchParamsResult, }); @@ -108,6 +106,8 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { newPassword: string; }>(); const updatePasswordMutation = useMutation(updatePassword()); + // Update roles + const updateRolesMutation = useMutation(updateRoles(queryClient)); return ( <> @@ -139,14 +139,18 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { newPassword: generateRandomString(12), }); }} - onUpdateUserRoles={(user, roles) => { - usersSend({ - type: "UPDATE_USER_ROLES", - userId: user.id, - roles, - }); + onUpdateUserRoles={async (user, roles) => { + try { + await updateRolesMutation.mutateAsync({ + userId: user.id, + roles, + }); + displaySuccess("User roles updated"); + } catch (e) { + displayError(getErrorMessage(e, "Error updating user roles")); + } }} - isUpdatingUserRoles={usersState.matches("updatingUserRoles")} + isUpdatingUserRoles={updateRolesMutation.isLoading} isLoading={isLoading} canEditUsers={canEditUsers} canViewActivity={entitlements.features.audit_log.enabled} diff --git a/site/src/pages/UsersPage/UsersTable/EditRolesButton.tsx b/site/src/pages/UsersPage/UsersTable/EditRolesButton.tsx index 739f42eb6948a..be5e38f9302a4 100644 --- a/site/src/pages/UsersPage/UsersTable/EditRolesButton.tsx +++ b/site/src/pages/UsersPage/UsersTable/EditRolesButton.tsx @@ -188,7 +188,7 @@ const useStyles = makeStyles((theme) => ({ padding: 0, "&:disabled": { - opacity: 0, + opacity: 0.5, }, }, options: { diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts deleted file mode 100644 index a9c0ac935b9dc..0000000000000 --- a/site/src/xServices/users/usersXService.ts +++ /dev/null @@ -1,123 +0,0 @@ -import { assign, createMachine } from "xstate"; -import * as API from "api/api"; -import { getErrorMessage } from "api/errors"; -import * as TypesGen from "api/typesGenerated"; -import { displayError } from "components/GlobalSnackbar/utils"; - -export const Language = { - updateUserRolesSuccess: "Successfully updated the user roles.", - updateUserRolesError: "Error on updating the user roles.", -}; - -export interface UsersContext { - // Update user roles - userIdToUpdateRoles?: TypesGen.User["id"]; - updateUserRolesError?: unknown; -} - -export type UsersEvent = - // Update roles events - { - type: "UPDATE_USER_ROLES"; - userId: TypesGen.User["id"]; - roles: TypesGen.Role["name"][]; - }; - -export const usersMachine = - /** @xstate-layout N4IgpgJg5mDOIC5QFdZgE6wMoBcCGOYAdLPujgJYB2UACnlNQRQPZUDEA2gAwC6ioAA4tYFSmwEgAHogC0ARgBMigBxEAnCoCsAdhWrdK9QBYdAGhABPRPO7ruRAMzdj3HYuNbFANm5b1AL4BFqgY2PiERDA4lDQAqmiY7BBsxNQAbiwA1sTRCWE8-EggwqLiVJIyCArc8upOxt6K-o7qjloq3ioW1ghKPkQu6m2KOraaw0EhieEEuWAx1FD5SRjoLOhEggA2BABmGwC2UQsrsIWSpWKsFcVVCqqOGoqOKnUqKrW15lY2ivI6IiNRqOHR6bhdXRTEChTC4OZECgQbZgdhYOJYWgAUQAcgARAD6GKxACULsUruVKnIdLoiPJjPItF42p0fI4ejYtMZjE4ASoxi4dI4lCpobDZpEkSj2HisQAZLEAFSxRKwpPJQhE1wkdxp3nqAJM3i0zm8tPknL63gNRB0TQBTNBHRM4pm8KlyNRAEEAMJKgCSADVvSq1Rq+JdtVS9dUdMYnkN1C9uM0Teofr15CpXEQPt95IXC952m6wh60l72CSseqleGSQTaN6sFgAOoAeRJeM1JWjN2pca8RH+bmar3tdUUVve3me8kcIqU3G0NrLcIilZlcVoeNDquJjZJHcVWF7lIHsdk8d5As6ybeHxLxitym4Dh5b1pYx0LhUjnXSUt1RHc9zDZsAHEsXPftdVAe4GT0IEMz0UYtAhV51BnZwHDsTQDQXZNNEAitESrUD9wJAAxAN5RVMlIwpWDbnguR5BNXlEPUbNmk+Ixp1+PpFHULQgUcbxTH8YTCy8EjNyIABjNg9godBDhWLBUEEMAqFENh2F9DscRokkAFkGwJdFMVxLAAyMmCykvVjqg8eQ82UdQJJ5DxaWZGdmUUDRWi8VM+LGbw5IRJSqBUtSNK0nS9I4X1vRxX0FQsqzsRxWz7MYrVHLg6Q5GMbQgWZbiS3tbhWktQT2P+PMBR0DMM0dLQIuCGF3Xk6LYvUxI8TAFFygMoyTPMw8CTlRUVQcnUWOKlzlEGBlHATPRhn0JkZy6XlSuMUZamEox7UiyI+tUgaMCGkabgM1L0vlCyZuVaD8r7QrFvuYwM3pewRUhTxPkzGx7XqYTGTayTtEUc7iEuuLEm9BTKHSZh9MM4yAzMiy-UDENAzyooCoWwdZBeNQfBccT0NqbkOhnHNAXaPxVHsJpTB0eHFOUq6VhRtGMeSx6Mqm-Hg1DOycXmmNnNkdC51KlqlHaYTRgErM2lvZkDUO37-ELHnYASqgICWFZklSREqEyHISFNiAVllpyltaJ5fscXivd0UqsPq0q3MLFrzRzV5MONx2LcSdg1g2LZdhwA41Id2BtLN52PovIqqhNUTOgI2xF3tUEZy5kcXXNQ6Vx8TrpnLeSIGGhZo4wK2qDSW3smIJuRrATOSc+snY1cD83hC1lPG8OqswkiHGghX8Du0Ywed7lv4hjuPNh2fYjiIdfCAHqMvsHc03PfdpPI6xkF26eqS1E5pUNUbQM1GHm8FRih0diZYY5SB3G2dtiBfyFkfRILsc6IGCiOX8vgmTsTGJ5F89UwQOBDtoU0pgPCry6hKUiYCf7ME3m3beCc94pyIb-fukCs7MTPtPIgCDhJuE+O7bwM49BPBFCYXQoxhjsTFPgnqUU+ZIwwPQWAsAADuGwIAkjgAsMa2NcZTWbK2Ts3YCQ1jrFA76bEPiDFaP+G0UkXhaFfLUNQWhsx6xeM4dweD64bjETFfmiQpGyPkYotAOAHppTFuqRsGj2xdkJLo5U+jya2MBDTWkyhaQGk6K+WwolGjuH+KVaeDIeboCUYsUh6AvFyPQBAduncQFEHyX4lYJT5HRNjKCD2RYRIgyaIoPwM5uJuXEsKDqjwVZ5IKX-OpeBpGlPKeQ3eSd941NOJ48Z3iymNOchJJ4bRFyXy6MmLo3TPIaAFJ0xcug3CMh5sgQQEASH-wwCSFgKJYAVOAd3IglzrkQLuQ8uAqy3adAaHYdiSgOivBnogf4tpWhbQBK4ZQuSRENwRO8m5Kx7mPNjugdYO9E7J2OMiz56A0U-PoafWMJo3I+AZLXcc7FLGCThXObiYxRie2DvDdgFEww0TohGQe2cDHVFsAmekvgTnsV+mkq09gHDOHWs0TpU5OpdSoCwJu8BigEPkqQPA5Alj0EYFQYWJ9h7ywXAyPMdgEzV02bSKVC47SfHfLSX65pSwItcZEaIoyZjGrlktBQSh6guAhI0aejJhQzjcKJA0Xt2g8i8Aqnm0owC+tdghO+eYTRoSLo0DM2F4xEGwZ00wbg1bc3dUBXm7iJHoE0mnRKrt+XkwlcY0wrI6jZlqP5YcbRjQeDsLoM6FbSKI2uugW6LcipNqvNk5hph1rTw6HUPZD8cxAhzEdQNp1nHdURRdcRY7BbEL9dO+WpVo2lSjQKEUjJ2hM25KtNoisvaHXNJHetZtW7oFTdAhAbxmF2HjKCIwIl1b+SZGJES75RimIBGvZu3qMA-oFQKO0edxJVW8kYXazI7ToT9nTAUrph3yWoSixIyHBxlQkrYaDnwOqaFBn0N4c5bGsm0MdcYPNR1jImT4gplGZ0SXpKaAdYJQRtHNFYjwThWoLg8JVESwy-GIeKUsyZgnnIMjcuGuVEIl2mCsamJwXsBR2BeOKuuu6PXEHxV+ol6rSZ+qqCKQYyYMNoRBsKVBvR-j-ITHPcS7DXhWc1XMTT-qATGa4jxDoK5kxWjeL0mqIJ5NczwUEIAA */ - createMachine( - { - tsTypes: {} as import("./usersXService.typegen").Typegen0, - schema: { - context: {} as UsersContext, - events: {} as UsersEvent, - services: {} as { - updateUserRoles: { - data: TypesGen.User; - }; - }, - }, - predictableActionArguments: true, - id: "usersState", - initial: "idle", - states: { - idle: { - entry: "clearSelectedUser", - on: { - UPDATE_USER_ROLES: { - target: "updatingUserRoles", - actions: "assignUserIdToUpdateRoles", - }, - }, - }, - updatingUserRoles: { - entry: "clearUpdateUserRolesError", - invoke: { - src: "updateUserRoles", - id: "updateUserRoles", - onDone: [ - { - target: "idle", - actions: "updateUserRolesInTheList", - }, - ], - onError: [ - { - target: "idle", - actions: [ - "assignUpdateRolesError", - "displayUpdateRolesErrorMessage", - ], - }, - ], - }, - }, - }, - }, - { - services: { - updateUserRoles: (context, event) => { - if (!context.userIdToUpdateRoles) { - throw new Error("userIdToUpdateRoles is undefined"); - } - - return API.updateUserRoles(event.roles, context.userIdToUpdateRoles); - }, - }, - - actions: { - clearSelectedUser: assign({ - userIdToUpdateRoles: (_) => undefined, - }), - - assignUserIdToUpdateRoles: assign({ - userIdToUpdateRoles: (_, event) => event.userId, - }), - - assignUpdateRolesError: assign({ - updateUserRolesError: (_, event) => event.data, - }), - - clearUpdateUserRolesError: assign({ - updateUserRolesError: (_) => undefined, - }), - displayUpdateRolesErrorMessage: (context) => { - const message = getErrorMessage( - context.updateUserRolesError, - Language.updateUserRolesError, - ); - displayError(message); - }, - updateUserRolesInTheList: assign({ - // users: ({ users }, event) => { - // if (!users) { - // return users; - // } - // return users.map((u) => { - // return u.id === event.data.id ? event.data : u; - // }); - // }, - }), - }, - }, - ); From b4b7a77ddd02575bf3c89b0c33a9e857e2fe3874 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 12:29:49 +0000 Subject: [PATCH 09/15] Update copy and remove language object --- site/src/pages/UsersPage/UsersPage.test.tsx | 40 ++++++++++----------- site/src/pages/UsersPage/UsersPage.tsx | 39 +++++++++----------- 2 files changed, 35 insertions(+), 44 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index e1b5eedf5a57a..0f16891b8cb71 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -13,7 +13,7 @@ import { Role } from "api/typesGenerated"; import { Language as ResetPasswordDialogLanguage } from "./ResetPasswordDialog"; import { renderWithAuth } from "testHelpers/renderHelpers"; import { server } from "testHelpers/server"; -import { Language as UsersPageLanguage, UsersPage } from "./UsersPage"; +import { UsersPage } from "./UsersPage"; const renderPage = () => { return renderWithAuth(); @@ -34,17 +34,14 @@ const suspendUser = async (setupActionSpies: () => void) => { // Check if the confirm message is displayed const confirmDialog = await screen.findByRole("dialog"); - expect(confirmDialog).toHaveTextContent( - `${UsersPageLanguage.suspendDialogMessagePrefix} ${MockUser.username}?`, - ); // Setup spies to check the actions after setupActionSpies(); // Click on the "Confirm" button - const confirmButton = await within(confirmDialog).findByText( - UsersPageLanguage.suspendDialogAction, - ); + const confirmButton = await within(confirmDialog).findByText("suspend", { + exact: false, + }); await user.click(confirmButton); }; @@ -93,17 +90,14 @@ const activateUser = async (setupActionSpies: () => void) => { // Check if the confirm message is displayed const confirmDialog = screen.getByRole("dialog"); - expect(confirmDialog).toHaveTextContent( - `${UsersPageLanguage.activateDialogMessagePrefix} ${SuspendedMockUser.username}?`, - ); // Setup spies to check the actions after setupActionSpies(); // Click on the "Confirm" button - const confirmButton = within(confirmDialog).getByText( - UsersPageLanguage.activateDialogAction, - ); + const confirmButton = within(confirmDialog).getByText("activate", { + exact: false, + }); fireEvent.click(confirmButton); }; @@ -175,7 +169,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText("User suspended"); + await screen.findByText("Successfully suspended the user."); // Check if the API was called correctly expect(API.suspendUser).toBeCalledTimes(1); @@ -194,7 +188,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText("Error suspending user"); + await screen.findByText("Error suspending user."); // Check if the API was called correctly expect(API.suspendUser).toBeCalledTimes(1); @@ -251,7 +245,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText("User deleted"); + await screen.findByText("Successfully deleted the user."); // Check if the API was called correctly expect(API.deleteUser).toBeCalledTimes(1); @@ -273,7 +267,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText("Error deleting user"); + await screen.findByText("Error deleting user."); // Check if the API was called correctly expect(API.deleteUser).toBeCalledTimes(1); @@ -300,7 +294,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText("User activated"); + await screen.findByText("Successfully activated the user."); // Check if the API was called correctly expect(API.activateUser).toBeCalledTimes(1); @@ -316,7 +310,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText("Error activating user"); + await screen.findByText("Error activating user."); // Check if the API was called correctly expect(API.activateUser).toBeCalledTimes(1); @@ -337,7 +331,7 @@ describe("UsersPage", () => { }); // Check if the success message is displayed - await screen.findByText("Password reset"); + await screen.findByText("Successfully updated the user password."); // Check if the API was called correctly expect(API.updateUserPassword).toBeCalledTimes(1); @@ -356,7 +350,7 @@ describe("UsersPage", () => { }); // Check if the error message is displayed - await screen.findByText("Error resetting password"); + await screen.findByText("Error on resetting the user password."); // Check if the API was called correctly expect(API.updateUserPassword).toBeCalledTimes(1); @@ -405,7 +399,9 @@ describe("UsersPage", () => { }, MockAuditorRole); // Check if the error message is displayed - await waitFor(() => expect("Error updating user roles").toBeDefined()); + await waitFor(() => + expect("Error on updating the user roles.").toBeDefined(), + ); // Check if the API was called correctly const currentRoles = MockUser.roles.map((r) => r.name); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index a44fbd6e80654..4edab1507c4d1 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -31,15 +31,6 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { getErrorMessage } from "api/errors"; import { generateRandomString } from "utils/random"; -export const Language = { - suspendDialogTitle: "Suspend user", - suspendDialogAction: "Suspend", - suspendDialogMessagePrefix: "Do you want to suspend the user", - activateDialogTitle: "Activate user", - activateDialogAction: "Activate", - activateDialogMessagePrefix: "Do you want to activate the user", -}; - export const UsersPage: FC<{ children?: ReactNode }> = () => { const queryClient = useQueryClient(); const navigate = useNavigate(); @@ -145,9 +136,11 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { userId: user.id, roles, }); - displaySuccess("User roles updated"); + displaySuccess("Successfully updated the user roles."); } catch (e) { - displayError(getErrorMessage(e, "Error updating user roles")); + displayError( + getErrorMessage(e, "Error on updating the user roles."), + ); } }} isUpdatingUserRoles={updateRolesMutation.isLoading} @@ -179,9 +172,9 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { try { await deleteUserMutation.mutateAsync(confirmDeleteUser!.id); setConfirmDeleteUser(undefined); - displaySuccess("User deleted"); + displaySuccess("Successfully deleted the user."); } catch (e) { - displayError(getErrorMessage(e, "Error deleting user")); + displayError(getErrorMessage(e, "Error deleting user.")); } }} onCancel={() => { @@ -200,9 +193,9 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { try { await suspendUserMutation.mutateAsync(confirmSuspendUser!.id); setConfirmSuspendUser(undefined); - displaySuccess("User suspended"); + displaySuccess("Successfully suspended the user."); } catch (e) { - displayError(getErrorMessage(e, "Error suspending user")); + displayError(getErrorMessage(e, "Error suspending user.")); } }} onClose={() => { @@ -221,15 +214,15 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { hideCancel={false} open={confirmActivateUser !== undefined} confirmLoading={activateUserMutation.isLoading} - title={Language.activateDialogTitle} - confirmText={Language.activateDialogAction} + title="Activate user" + confirmText="Activate" onConfirm={async () => { try { await activateUserMutation.mutateAsync(confirmActivateUser!.id); setConfirmActivateUser(undefined); - displaySuccess("User activated"); + displaySuccess("Successfully activated the user."); } catch (e) { - displayError(getErrorMessage(e, "Error activating user")); + displayError(getErrorMessage(e, "Error activating user.")); } }} onClose={() => { @@ -237,7 +230,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { }} description={ <> - {Language.activateDialogMessagePrefix}{" "} + Do you want to activate the user{" "} {confirmActivateUser?.username ?? ""}? } @@ -260,9 +253,11 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { old_password: "", }); setConfirmResetPassword(undefined); - displaySuccess("Password reset"); + displaySuccess("Successfully updated the user password."); } catch (e) { - displayError(getErrorMessage(e, "Error resetting password")); + displayError( + getErrorMessage(e, "Error on resetting the user password."), + ); } }} /> From 76e94fb4e46bf99d6657f0d5d81ee4fbb368fb2b Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 12:35:18 +0000 Subject: [PATCH 10/15] Move auth methods queries to the users query module --- site/src/api/queries/users.ts | 9 +++++++++ site/src/pages/CreateUserPage/CreateUserPage.tsx | 12 +++--------- .../UserSettingsPage/SecurityPage/SecurityPage.tsx | 13 +++++-------- site/src/pages/UsersPage/UsersPage.tsx | 13 ++++--------- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 9bb1a43d2050e..1eac49771fc16 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -70,3 +70,12 @@ export const updateRoles = (queryClient: QueryClient) => { }, }; }; + +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, + }; +}; diff --git a/site/src/pages/CreateUserPage/CreateUserPage.tsx b/site/src/pages/CreateUserPage/CreateUserPage.tsx index 5493852d21d45..a81c4c6b4ab08 100644 --- a/site/src/pages/CreateUserPage/CreateUserPage.tsx +++ b/site/src/pages/CreateUserPage/CreateUserPage.tsx @@ -5,9 +5,8 @@ import { useNavigate } from "react-router-dom"; import { CreateUserForm } from "./CreateUserForm"; import { Margins } from "components/Margins/Margins"; import { pageTitle } from "utils/page"; -import { getAuthMethods } from "api/api"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { createUser } from "api/queries/users"; +import { authMethods, createUser } from "api/queries/users"; import { displaySuccess } from "components/GlobalSnackbar/utils"; export const Language = { @@ -19,12 +18,7 @@ export const CreateUserPage: FC = () => { const navigate = useNavigate(); const queryClient = useQueryClient(); const createUserMutation = useMutation(createUser(queryClient)); - // TODO: We should probably place this somewhere else to reduce the number of calls. - // This would be called each time this page is loaded. - const { data: authMethods } = useQuery({ - queryKey: ["authMethods"], - queryFn: getAuthMethods, - }); + const authMethodsQuery = useQuery(authMethods()); return ( @@ -34,7 +28,7 @@ export const CreateUserPage: FC = () => { { await createUserMutation.mutateAsync(user); displaySuccess("Successfully created user."); diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx index ac1e4338b2ed9..0589f26c041f7 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx @@ -3,30 +3,27 @@ import { ComponentProps, FC } from "react"; import { Section } from "components/SettingsLayout/Section"; import { SecurityForm } from "./SettingsSecurityForm"; import { useMutation, useQuery } from "@tanstack/react-query"; -import { getAuthMethods, getUserLoginType } from "api/api"; +import { getUserLoginType } from "api/api"; import { SingleSignOnSection, useSingleSignOnSection, } from "./SingleSignOnSection"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; -import { updatePassword } from "api/queries/users"; +import { authMethods, updatePassword } from "api/queries/users"; import { displaySuccess } from "components/GlobalSnackbar/utils"; export const SecurityPage: FC = () => { const me = useMe(); const updatePasswordMutation = useMutation(updatePassword()); - const { data: authMethods } = useQuery({ - queryKey: ["authMethods"], - queryFn: getAuthMethods, - }); + const authMethodsQuery = useQuery(authMethods()); const { data: userLoginType } = useQuery({ queryKey: ["loginType"], queryFn: getUserLoginType, }); const singleSignOnSection = useSingleSignOnSection(); - if (!authMethods || !userLoginType) { + if (!authMethodsQuery.data || !userLoginType) { return ; } @@ -51,7 +48,7 @@ export const SecurityPage: FC = () => { }} oidc={{ section: { - authMethods, + authMethods: authMethodsQuery.data, userLoginType, ...singleSignOnSection, }, diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 4edab1507c4d1..44bf8c2911202 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -14,7 +14,6 @@ import { useStatusFilterMenu } from "./UsersFilter"; import { useFilter } from "components/Filter/filter"; import { useDashboard } from "components/Dashboard/DashboardProvider"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { getAuthMethods } from "api/api"; import { roles } from "api/queries/roles"; import { deploymentConfig } from "api/queries/deployment"; import { prepareQuery } from "utils/filters"; @@ -26,6 +25,7 @@ import { deleteUser, updatePassword, updateRoles, + authMethods, } from "api/queries/users"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { getErrorMessage } from "api/errors"; @@ -74,14 +74,9 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { status: option?.value, }), }); - const authMethods = useQuery({ - queryKey: ["authMethods"], - queryFn: () => { - return getAuthMethods(); - }, - }); + const authMethodsQuery = useQuery(authMethods()); const isLoading = - usersQuery.isLoading || rolesQuery.isLoading || authMethods.isLoading; + usersQuery.isLoading || rolesQuery.isLoading || authMethodsQuery.isLoading; // Suspend const [confirmSuspendUser, setConfirmSuspendUser] = useState(); const suspendUserMutation = useMutation(suspendUser(queryClient)); @@ -109,7 +104,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { oidcRoleSyncEnabled={oidcRoleSyncEnabled} roles={rolesQuery.data} users={usersQuery.data?.users} - authMethods={authMethods.data} + authMethods={authMethodsQuery.data} onListWorkspaces={(user) => { navigate( "/workspaces?filter=" + From 50c861ddea776251e6f2e5149965b50ebe701883 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 12:48:55 +0000 Subject: [PATCH 11/15] Add mock to deployment config and fix fragment warning --- site/src/components/Filter/filter.tsx | 26 ++++++++++----------- site/src/pages/UsersPage/UsersPage.test.tsx | 2 +- site/src/testHelpers/handlers.ts | 4 ++++ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/site/src/components/Filter/filter.tsx b/site/src/components/Filter/filter.tsx index f3312ea862eb5..1824554721b13 100644 --- a/site/src/components/Filter/filter.tsx +++ b/site/src/components/Filter/filter.tsx @@ -353,20 +353,18 @@ const PresetMenu = ({ View advanced filtering {learnMoreLink2 && learnMoreLabel2 && ( - <> - { - setIsOpen(false); - }} - > - - {learnMoreLabel2} - - + { + setIsOpen(false); + }} + > + + {learnMoreLabel2} + )} diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index 0f16891b8cb71..c6bc89bdd11ad 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -302,7 +302,7 @@ describe("UsersPage", () => { }); }); describe("when activation fails", () => { - it("shows an error message", async () => { + it.only("shows an error message", async () => { renderPage(); await activateUser(() => { diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index c1deb90054929..6ae40e2e0c09d 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -341,6 +341,10 @@ export const handlers = [ return res(ctx.status(200), ctx.json(M.MockDeploymentStats)); }), + rest.get("/api/v2/deployment/config", (_, res, ctx) => { + return res(ctx.status(200), ctx.json(M.MockDeploymentConfig)); + }), + rest.get( "/api/v2/workspacebuilds/:workspaceBuildId/parameters", (_, res, ctx) => { From 93e4e805435c585b5a4a2a3bb72291fda9dc2495 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 29 Sep 2023 13:50:56 +0000 Subject: [PATCH 12/15] Fix tests --- site/src/pages/UsersPage/UsersPage.test.tsx | 282 ++++++++------------ 1 file changed, 107 insertions(+), 175 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index c6bc89bdd11ad..1097d2cfcb62c 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, screen, waitFor, within } from "@testing-library/react"; +import { fireEvent, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { rest } from "msw"; import { @@ -6,7 +6,6 @@ import { MockUser2, SuspendedMockUser, MockAuditorRole, - MockOwnerRole, } from "testHelpers/entities"; import * as API from "api/api"; import { Role } from "api/typesGenerated"; @@ -19,33 +18,26 @@ const renderPage = () => { return renderWithAuth(); }; -const suspendUser = async (setupActionSpies: () => void) => { +const suspendUser = async () => { const user = userEvent.setup(); // Get the first user in the table const moreButtons = await screen.findAllByLabelText("more"); const firstMoreButton = moreButtons[0]; - await user.click(firstMoreButton); const menu = await screen.findByRole("menu"); const suspendButton = within(menu).getByText(/Suspend/); - await user.click(suspendButton); // Check if the confirm message is displayed const confirmDialog = await screen.findByRole("dialog"); - - // Setup spies to check the actions after - setupActionSpies(); - - // Click on the "Confirm" button - const confirmButton = await within(confirmDialog).findByText("suspend", { - exact: false, + const confirmButton = await within(confirmDialog).findByRole("button", { + name: "Suspend", }); await user.click(confirmButton); }; -const deleteUser = async (setupActionSpies: () => void) => { +const deleteUser = async () => { const user = userEvent.setup(); // Click on the "more" button to display the "Delete" option // Needs to await fetching users and fetching permissions, because they're needed to see the more button @@ -71,15 +63,12 @@ const deleteUser = async (setupActionSpies: () => void) => { const dialog = screen.getByRole("dialog"); await user.type(textField, MockUser2.username); - // Setup spies to check the actions after - setupActionSpies(); - // Click on the "Confirm" button const confirmButton = within(dialog).getByRole("button", { name: "Delete" }); await user.click(confirmButton); }; -const activateUser = async (setupActionSpies: () => void) => { +const activateUser = async () => { const moreButtons = await screen.findAllByLabelText("more"); const suspendedMoreButton = moreButtons[2]; fireEvent.click(suspendedMoreButton); @@ -91,12 +80,9 @@ const activateUser = async (setupActionSpies: () => void) => { // Check if the confirm message is displayed const confirmDialog = screen.getByRole("dialog"); - // Setup spies to check the actions after - setupActionSpies(); - // Click on the "Confirm" button - const confirmButton = within(confirmDialog).getByText("activate", { - exact: false, + const confirmButton = within(confirmDialog).getByRole("button", { + name: "Activate", }); fireEvent.click(confirmButton); }; @@ -129,7 +115,7 @@ const resetUserPassword = async (setupActionSpies: () => void) => { fireEvent.click(confirmButton); }; -const updateUserRole = async (setupActionSpies: () => void, role: Role) => { +const updateUserRole = async (role: Role) => { // Get the first user in the table const users = await screen.findAllByText(/.*@coder.com/); const userRow = users[0].closest("tr"); @@ -141,9 +127,6 @@ const updateUserRole = async (setupActionSpies: () => void, role: Role) => { const editButton = within(userRow).getByTitle("Edit user roles"); fireEvent.click(editButton); - // Setup spies to check the actions after - setupActionSpies(); - // Click on the role option const fieldset = await screen.findByTitle("Available roles"); const auditorOption = within(fieldset).getByText(role.display_name); @@ -157,121 +140,93 @@ const updateUserRole = async (setupActionSpies: () => void, role: Role) => { describe("UsersPage", () => { describe("suspend user", () => { describe("when it is success", () => { - it("shows a success message and refresh the page", async () => { + it("shows a success message", async () => { renderPage(); - await suspendUser(() => { - jest.spyOn(API, "suspendUser").mockResolvedValueOnce(MockUser); - jest.spyOn(API, "getUsers").mockResolvedValueOnce({ - users: [SuspendedMockUser, MockUser2], - count: 2, - }); - }); + server.use( + rest.put( + `/api/v2/users/${MockUser.id}/status/suspend`, + async (req, res, ctx) => { + return res(ctx.status(200), ctx.json(SuspendedMockUser)); + }, + ), + ); + + await suspendUser(); // Check if the success message is displayed await screen.findByText("Successfully suspended the user."); - - // Check if the API was called correctly - expect(API.suspendUser).toBeCalledTimes(1); - expect(API.suspendUser).toBeCalledWith(MockUser.id); - - // Check if the users list was reload - await waitFor(() => expect(API.getUsers).toBeCalledTimes(1)); }); }); + describe("when it fails", () => { it("shows an error message", async () => { renderPage(); - await suspendUser(() => { - jest.spyOn(API, "suspendUser").mockRejectedValueOnce({}); - }); + server.use( + rest.put( + `/api/v2/users/${MockUser.id}/status/suspend`, + async (req, res, ctx) => { + return res( + ctx.status(400), + ctx.json({ + message: "Error suspending user.", + }), + ); + }, + ), + ); + + await suspendUser(); // Check if the error message is displayed await screen.findByText("Error suspending user."); - - // Check if the API was called correctly - expect(API.suspendUser).toBeCalledTimes(1); - expect(API.suspendUser).toBeCalledWith(MockUser.id); }); }); }); - describe("pagination", () => { - it("goes to next and previous page", async () => { - const { container } = renderPage(); - const user = userEvent.setup(); - - const mock = jest - .spyOn(API, "getUsers") - .mockResolvedValueOnce({ users: [MockUser, MockUser2], count: 26 }); - - const nextButton = await screen.findByLabelText("Next page"); - expect(nextButton).toBeEnabled(); - const previousButton = await screen.findByLabelText("Previous page"); - expect(previousButton).toBeDisabled(); - await user.click(nextButton); - - await waitFor(() => - expect(API.getUsers).toBeCalledWith({ offset: 25, limit: 25, q: "" }), - ); - - mock.mockClear(); - await user.click(previousButton); - - await waitFor(() => - expect(API.getUsers).toBeCalledWith({ offset: 0, limit: 25, q: "" }), - ); - - const pageButtons = container.querySelectorAll( - `button[name="Page button"]`, - ); - // count handler says there are 2 pages of results - expect(pageButtons.length).toBe(2); - }); - }); - describe("delete user", () => { describe("when it is success", () => { - it("shows a success message and refresh the page", async () => { + it("shows a success message", async () => { renderPage(); - await deleteUser(() => { - jest.spyOn(API, "deleteUser").mockResolvedValueOnce(undefined); - jest.spyOn(API, "getUsers").mockResolvedValueOnce({ - users: [MockUser, SuspendedMockUser], - count: 2, - }); - }); + server.use( + rest.delete( + `/api/v2/users/${MockUser2.id}`, + async (req, res, ctx) => { + return res(ctx.status(200), ctx.json(MockUser2)); + }, + ), + ); + + await deleteUser(); // Check if the success message is displayed await screen.findByText("Successfully deleted the user."); - - // Check if the API was called correctly - expect(API.deleteUser).toBeCalledTimes(1); - expect(API.deleteUser).toBeCalledWith(MockUser2.id); - - // Check if the users list was reloaded - await waitFor(() => { - const users = screen.getAllByLabelText("more"); - expect(users.length).toEqual(2); - }); }); }); describe("when it fails", () => { it("shows an error message", async () => { renderPage(); - await deleteUser(() => { - jest.spyOn(API, "deleteUser").mockRejectedValueOnce({}); - }); + server.use( + rest.delete( + `/api/v2/users/${MockUser2.id}`, + async (req, res, ctx) => { + return res( + ctx.status(400), + ctx.json({ + message: "Error deleting user.", + }), + ); + }, + ), + ); + + await deleteUser(); // Check if the error message is displayed await screen.findByText("Error deleting user."); - - // Check if the API was called correctly - expect(API.deleteUser).toBeCalledTimes(1); - expect(API.deleteUser).toBeCalledWith(MockUser2.id); }); }); }); @@ -281,40 +236,43 @@ describe("UsersPage", () => { it("shows a success message and refreshes the page", async () => { renderPage(); - await activateUser(() => { - jest - .spyOn(API, "activateUser") - .mockResolvedValueOnce(SuspendedMockUser); - jest.spyOn(API, "getUsers").mockImplementationOnce(() => - Promise.resolve({ - users: [MockUser, MockUser2, SuspendedMockUser], - count: 3, - }), - ); - }); + server.use( + rest.put( + `/api/v2/users/${SuspendedMockUser.id}/status/activate`, + async (req, res, ctx) => { + return res(ctx.status(200), ctx.json(MockUser)); + }, + ), + ); + + await activateUser(); // Check if the success message is displayed await screen.findByText("Successfully activated the user."); - - // Check if the API was called correctly - expect(API.activateUser).toBeCalledTimes(1); - expect(API.activateUser).toBeCalledWith(SuspendedMockUser.id); }); }); describe("when activation fails", () => { - it.only("shows an error message", async () => { + it("shows an error message", async () => { renderPage(); - await activateUser(() => { - jest.spyOn(API, "activateUser").mockRejectedValueOnce({}); - }); + server.use( + rest.put( + `/api/v2/users/${SuspendedMockUser.id}/status/activate`, + async (req, res, ctx) => { + return res( + ctx.status(400), + ctx.json({ + message: "Error activating user.", + }), + ); + }, + ), + ); + + await activateUser(); // Check if the error message is displayed await screen.findByText("Error activating user."); - - // Check if the API was called correctly - expect(API.activateUser).toBeCalledTimes(1); - expect(API.activateUser).toBeCalledWith(SuspendedMockUser.id); }); }); }); @@ -367,26 +325,24 @@ describe("UsersPage", () => { it("updates the roles", async () => { renderPage(); - const { userRow } = await updateUserRole(() => { - jest.spyOn(API, "updateUserRoles").mockResolvedValueOnce({ - ...MockUser, - roles: [...MockUser.roles, MockAuditorRole], - }); - }, MockAuditorRole); + server.use( + rest.put( + `/api/v2/users/${MockUser.id}/roles`, + async (req, res, ctx) => { + return res( + ctx.status(200), + ctx.json({ + ...MockUser, + roles: [...MockUser.roles, MockAuditorRole], + }), + ); + }, + ), + ); - // Check if the select text was updated with the Auditor role - await waitFor(() => { - expect(userRow).toHaveTextContent(MockOwnerRole.display_name); - }); - expect(userRow).toHaveTextContent(MockAuditorRole.display_name); + await updateUserRole(MockAuditorRole); - // Check if the API was called correctly - const currentRoles = MockUser.roles.map((r) => r.name); - expect(API.updateUserRoles).toBeCalledTimes(1); - expect(API.updateUserRoles).toBeCalledWith( - [...currentRoles, MockAuditorRole.name], - MockUser.id, - ); + await screen.findByText("Successfully updated the user roles."); }); }); @@ -394,43 +350,19 @@ describe("UsersPage", () => { it("shows an error message", async () => { renderPage(); - await updateUserRole(() => { - jest.spyOn(API, "updateUserRoles").mockRejectedValueOnce({}); - }, MockAuditorRole); - - // Check if the error message is displayed - await waitFor(() => - expect("Error on updating the user roles.").toBeDefined(), - ); - - // Check if the API was called correctly - const currentRoles = MockUser.roles.map((r) => r.name); - - expect(API.updateUserRoles).toBeCalledTimes(1); - expect(API.updateUserRoles).toBeCalledWith( - [...currentRoles, MockAuditorRole.name], - MockUser.id, - ); - }); - it("shows an error from the backend", async () => { - renderPage(); - server.use( rest.put(`/api/v2/users/${MockUser.id}/roles`, (req, res, ctx) => { return res( ctx.status(400), - ctx.json({ message: "message from the backend" }), + ctx.json({ message: "Error on updating the user roles." }), ); }), ); - await updateUserRole(() => null, MockAuditorRole); + await updateUserRole(MockAuditorRole); // Check if the error message is displayed - const errorMessage = await screen.findByText( - "message from the backend", - ); - expect(errorMessage).toBeDefined(); + await screen.findByText("Error on updating the user roles."); }); }); }); From 579cb1135f566ed1eaae8e6c07c54b1568c99ea6 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 2 Oct 2023 08:01:25 -0300 Subject: [PATCH 13/15] Update UsersPage.tsx --- site/src/pages/UsersPage/UsersPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 44bf8c2911202..d997c0ce3ffe0 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -225,7 +225,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { }} description={ <> - Do you want to activate the user{" "} + Do you want to activate{" "} {confirmActivateUser?.username ?? ""}? } From 5eb2321430763d226d260c105ddcd0b9edc6e002 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 2 Oct 2023 08:02:17 -0300 Subject: [PATCH 14/15] Update UsersPageView.tsx --- site/src/pages/UsersPage/UsersPageView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/UsersPage/UsersPageView.tsx b/site/src/pages/UsersPage/UsersPageView.tsx index f09b6895e8c31..56cd39e85d446 100644 --- a/site/src/pages/UsersPage/UsersPageView.tsx +++ b/site/src/pages/UsersPage/UsersPageView.tsx @@ -94,7 +94,7 @@ export const UsersPageView: FC> = ({ authMethods={authMethods} /> - {count !== undefined && ( + {count && ( Date: Mon, 2 Oct 2023 11:08:49 +0000 Subject: [PATCH 15/15] Remove comments --- site/src/pages/UsersPage/UsersPage.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index d997c0ce3ffe0..42fb7e7269cf2 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -77,22 +77,22 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { const authMethodsQuery = useQuery(authMethods()); const isLoading = usersQuery.isLoading || rolesQuery.isLoading || authMethodsQuery.isLoading; - // Suspend + const [confirmSuspendUser, setConfirmSuspendUser] = useState(); const suspendUserMutation = useMutation(suspendUser(queryClient)); - // Activate + const [confirmActivateUser, setConfirmActivateUser] = useState(); const activateUserMutation = useMutation(activateUser(queryClient)); - // Delete + const [confirmDeleteUser, setConfirmDeleteUser] = useState(); const deleteUserMutation = useMutation(deleteUser(queryClient)); - // Reset password + const [confirmResetPassword, setConfirmResetPassword] = useState<{ user: User; newPassword: string; }>(); const updatePasswordMutation = useMutation(updatePassword()); - // Update roles + const updateRolesMutation = useMutation(updateRoles(queryClient)); return (