From 7a322f41f6304e10ff53e7ae1db6b21466b20a76 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Tue, 14 Jun 2022 20:38:30 +0000 Subject: [PATCH 1/3] add ability to activate users resolves #2254 --- site/src/components/UsersTable/UsersTable.tsx | 12 ++-- site/src/pages/UsersPage/UsersPage.tsx | 29 ++++++++- site/src/pages/UsersPage/UsersPageView.tsx | 3 + site/src/xServices/users/usersXService.ts | 63 ++++++++++++++++++- 4 files changed, 99 insertions(+), 8 deletions(-) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index bd746c65c0265..14ab43418d26a 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -33,6 +33,7 @@ export interface UsersTableProps { canEditUsers?: boolean isLoading?: boolean onSuspendUser: (user: TypesGen.User) => void + onActivateUser: (user: TypesGen.User) => void onResetUserPassword: (user: TypesGen.User) => void onUpdateUserRoles: (user: TypesGen.User, roles: TypesGen.Role["name"][]) => void } @@ -41,6 +42,7 @@ export const UsersTable: FC = ({ users, roles, onSuspendUser, + onActivateUser, onResetUserPassword, onUpdateUserRoles, isUpdatingUserRoles, @@ -115,12 +117,10 @@ export const UsersTable: FC = ({ }, ] : [ - // TODO: Uncomment this and add activate user functionality. - // { - // label: Language.activateMenuItem, - // // eslint-disable-next-line @typescript-eslint/no-empty-function - // onClick: function () {}, - // }, + { + label: Language.activateMenuItem, + onClick: onActivateUser, + }, ] ).concat({ label: Language.resetPasswordMenuItem, diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 0aa095dc46e98..67ae7aa832b80 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -13,15 +13,20 @@ 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 active the user", } export const UsersPage: React.FC = () => { const xServices = useContext(XServiceContext) const [usersState, usersSend] = useActor(xServices.usersXService) const [rolesState, rolesSend] = useActor(xServices.siteRolesXService) - const { users, getUsersError, userIdToSuspend, userIdToResetPassword, newUserPassword } = usersState.context + const { users, getUsersError, userIdToSuspend, userIdToActivate, userIdToResetPassword, newUserPassword } = + usersState.context const navigate = useNavigate() const userToBeSuspended = users?.find((u) => u.id === userIdToSuspend) + const userToBeActivated = users?.find((u) => u.id === userIdToActivate) const userToResetPassword = users?.find((u) => u.id === userIdToResetPassword) const permissions = useSelector(xServices.authXService, selectPermissions) const canEditUsers = permissions && permissions.updateUsers @@ -62,6 +67,9 @@ export const UsersPage: React.FC = () => { onSuspendUser={(user) => { usersSend({ type: "SUSPEND_USER", userId: user.id }) }} + onActivateUser={(user) => { + usersSend({ type: "ACTIVATE_USER", userId: user.id }) + }} onResetUserPassword={(user) => { usersSend({ type: "RESET_USER_PASSWORD", userId: user.id }) }} @@ -99,6 +107,25 @@ export const UsersPage: React.FC = () => { } /> + { + usersSend("CONFIRM_USER_ACTIVATION") + }} + onClose={() => { + usersSend("CANCEL_USER_ACTIVATION") + }} + description={ + <> + {Language.activateDialogMessagePrefix} {userToBeActivated?.username}? + + } + /> + void onSuspendUser: (user: TypesGen.User) => void + onActivateUser: (user: TypesGen.User) => void onResetUserPassword: (user: TypesGen.User) => void onUpdateUserRoles: (user: TypesGen.User, roles: TypesGen.Role["name"][]) => void } @@ -31,6 +32,7 @@ export const UsersPageView: FC = ({ roles, openUserCreationDialog, onSuspendUser, + onActivateUser, onResetUserPassword, onUpdateUserRoles, error, @@ -60,6 +62,7 @@ export const UsersPageView: FC = ({ users={users} roles={roles} onSuspendUser={onSuspendUser} + onActivateUser={onActivateUser} onResetUserPassword={onResetUserPassword} onUpdateUserRoles={onUpdateUserRoles} isUpdatingUserRoles={isUpdatingUserRoles} diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index 91317d584a488..8cf83559488c0 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -16,7 +16,9 @@ export const Language = { createUserSuccess: "Successfully created user.", createUserError: "Error on creating the user.", suspendUserSuccess: "Successfully suspended the user.", - suspendUserError: "Error on suspending the user.", + suspendUserError: "Error suspending 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.", @@ -32,6 +34,9 @@ export interface UsersContext { // Suspend user userIdToSuspend?: TypesGen.User["id"] suspendUserError?: Error | unknown + // Activate user + userIdToActivate?: TypesGen.User["id"] + activateUserError?: Error | unknown // Reset user password userIdToResetPassword?: TypesGen.User["id"] resetUserPasswordError?: Error | unknown @@ -49,6 +54,10 @@ export type UsersEvent = | { type: "SUSPEND_USER"; userId: TypesGen.User["id"] } | { type: "CONFIRM_USER_SUSPENSION" } | { type: "CANCEL_USER_SUSPENSION" } + // Activate events + | { type: "ACTIVATE_USER"; userId: TypesGen.User["id"] } + | { type: "CONFIRM_USER_ACTIVATION" } + | { type: "CANCEL_USER_ACTIVATION" } // Reset password events | { type: "RESET_USER_PASSWORD"; userId: TypesGen.User["id"] } | { type: "CONFIRM_USER_PASSWORD_RESET" } @@ -72,6 +81,9 @@ export const usersMachine = createMachine( suspendUser: { data: TypesGen.User } + activateUser: { + data: TypesGen.User + } updateUserPassword: { data: undefined } @@ -92,6 +104,10 @@ export const usersMachine = createMachine( target: "confirmUserSuspension", actions: ["assignUserIdToSuspend"], }, + ACTIVATE_USER: { + target: "confirmUserActivation", + actions: ["assignUserIdToActivate"], + }, RESET_USER_PASSWORD: { target: "confirmUserPasswordReset", actions: ["assignUserIdToResetPassword", "generateRandomPassword"], @@ -150,6 +166,12 @@ export const usersMachine = createMachine( CANCEL_USER_SUSPENSION: "idle", }, }, + confirmUserActivation: { + on: { + CONFIRM_USER_ACTIVATION: "activatingUser", + CANCEL_USER_ACTIVATION: "idle", + }, + }, suspendingUser: { entry: "clearSuspendUserError", invoke: { @@ -166,6 +188,22 @@ export const usersMachine = createMachine( }, }, }, + activatingUser: { + entry: "clearActivateUserError", + invoke: { + src: "activateUser", + id: "activateUser", + onDone: { + // Update users list + target: "gettingUsers", + actions: ["displayActivateSuccess"], + }, + onError: { + target: "idle", + actions: ["assignActivateUserError", "displayActivatedErrorMessage"], + }, + }, + }, confirmUserPasswordReset: { on: { CONFIRM_USER_PASSWORD_RESET: "resettingUserPassword", @@ -223,6 +261,13 @@ export const usersMachine = createMachine( return API.suspendUser(context.userIdToSuspend) }, + 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") @@ -258,6 +303,9 @@ export const usersMachine = createMachine( assignUserIdToSuspend: assign({ userIdToSuspend: (_, event) => event.userId, }), + assignUserIdToActivate: assign({ + userIdToActivate: (_, event) => event.userId, + }), assignUserIdToResetPassword: assign({ userIdToResetPassword: (_, event) => event.userId, }), @@ -278,6 +326,9 @@ export const usersMachine = createMachine( assignSuspendUserError: assign({ suspendUserError: (_, event) => event.data, }), + assignActivateUserError: assign({ + activateUserError: (_, event) => event.data, + }), assignResetUserPasswordError: assign({ resetUserPasswordError: (_, event) => event.data, }), @@ -292,6 +343,9 @@ export const usersMachine = createMachine( clearSuspendUserError: assign({ suspendUserError: (_) => undefined, }), + clearActivateUserError: assign({ + activateUserError: (_) => undefined, + }), clearResetUserPasswordError: assign({ resetUserPasswordError: (_) => undefined, }), @@ -308,6 +362,13 @@ export const usersMachine = createMachine( const message = getErrorMessage(context.suspendUserError, Language.suspendUserError) displayError(message) }, + displayActivateSuccess: () => { + displaySuccess(Language.activateUserSuccess) + }, + displayActivatedErrorMessage: (context) => { + const message = getErrorMessage(context.activateUserError, Language.activateUserError) + displayError(message) + }, displayResetPasswordSuccess: () => { displaySuccess(Language.resetUserPasswordSuccess) }, From 88643c67e0373a1a426206d03b78ac0f2a7f5c4b Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 15 Jun 2022 15:21:55 +0000 Subject: [PATCH 2/3] added test --- site/src/pages/UsersPage/UsersPage.test.tsx | 81 ++++++++++++++++++++- site/src/testHelpers/entities.ts | 10 +++ site/src/testHelpers/handlers.ts | 2 +- 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index 85ca87b7aced2..3415c8430468f 100644 --- a/site/src/pages/UsersPage/UsersPage.test.tsx +++ b/site/src/pages/UsersPage/UsersPage.test.tsx @@ -6,7 +6,7 @@ import { GlobalSnackbar } from "../../components/GlobalSnackbar/GlobalSnackbar" import { Language as ResetPasswordDialogLanguage } from "../../components/ResetPasswordDialog/ResetPasswordDialog" import { Language as RoleSelectLanguage } from "../../components/RoleSelect/RoleSelect" import { Language as UsersTableLanguage } from "../../components/UsersTable/UsersTable" -import { MockAuditorRole, MockUser, MockUser2, render } from "../../testHelpers/renderHelpers" +import { MockAuditorRole, MockUser, MockUser2, render, SuspendedMockUser } from "../../testHelpers/renderHelpers" import { server } from "../../testHelpers/server" import { permissionsToCheck } from "../../xServices/auth/authXService" import { Language as usersXServiceLanguage } from "../../xServices/users/usersXService" @@ -40,6 +40,35 @@ const suspendUser = async (setupActionSpies: () => void) => { fireEvent.click(confirmButton) } +const activateUser = async (setupActionSpies: () => void) => { + // Get the first user in the table + const users = await screen.findAllByText(/.*@coder.com/) + const firstUserRow = users[2].closest("tr") + if (!firstUserRow) { + throw new Error("Error on get the first user row") + } + + // Click on the "more" button to display the "Activate" option + const moreButton = within(firstUserRow).getByLabelText("more") + fireEvent.click(moreButton) + const menu = screen.getByRole("menu") + const activateButton = within(menu).getByText(UsersTableLanguage.activateMenuItem) + fireEvent.click(activateButton) + + // 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) + fireEvent.click(confirmButton) +} + const resetUserPassword = async (setupActionSpies: () => void) => { // Get the first user in the table const users = await screen.findAllByText(/.*@coder.com/) @@ -99,7 +128,7 @@ describe("Users Page", () => { it("shows users", async () => { render() const users = await screen.findAllByText(/.*@coder.com/) - expect(users.length).toEqual(2) + expect(users.length).toEqual(3) }) it("shows 'Create user' button to an authorized user", () => { @@ -178,6 +207,54 @@ describe("Users Page", () => { }) }) + describe("activate user", () => { + describe("when user is successfully activated", () => { + it("shows a success message and refreshes the page", async () => { + render( + <> + + + , + ) + + await activateUser(() => { + jest.spyOn(API, "activateUser").mockResolvedValueOnce(SuspendedMockUser) + jest + .spyOn(API, "getUsers") + .mockImplementationOnce(() => Promise.resolve([MockUser, MockUser2, SuspendedMockUser])) + }) + + // Check if the success message is displayed + await screen.findByText(usersXServiceLanguage.activateUserSuccess) + + // Check if the API was called correctly + expect(API.activateUser).toBeCalledTimes(1) + expect(API.activateUser).toBeCalledWith(SuspendedMockUser.id) + }) + }) + describe("when activation fails", () => { + it("shows an error message", async () => { + render( + <> + + + , + ) + + await activateUser(() => { + jest.spyOn(API, "activateUser").mockRejectedValueOnce({}) + }) + + // Check if the error message is displayed + await screen.findByText(usersXServiceLanguage.activateUserError) + + // Check if the API was called correctly + expect(API.activateUser).toBeCalledTimes(1) + expect(API.activateUser).toBeCalledWith(SuspendedMockUser.id) + }) + }) + }) + describe("reset user password", () => { describe("when it is success", () => { it("shows a success message", async () => { diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index b37e038529dbc..871369b7b3fde 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -51,6 +51,16 @@ export const MockUser2: TypesGen.User = { roles: [], } +export const SuspendedMockUser: TypesGen.User = { + id: "suspended-mock-user", + username: "SuspendedMockUser", + email: "iamsuspendedsad!@coder.com", + created_at: "", + status: "suspended", + organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"], + roles: [], +} + export const MockOrganization: TypesGen.Organization = { id: "test-org", name: "Test Organization", diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index bfb8e9be0f4ff..f9f130b86b261 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -37,7 +37,7 @@ export const handlers = [ // users rest.get("/api/v2/users", async (req, res, ctx) => { - return res(ctx.status(200), ctx.json([M.MockUser, M.MockUser2])) + return res(ctx.status(200), ctx.json([M.MockUser, M.MockUser2, M.SuspendedMockUser])) }), rest.post("/api/v2/users", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockUser)) From 9a232a8fb8c4928877f62bd33f75053d9a8d0251 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 15 Jun 2022 17:11:37 +0000 Subject: [PATCH 3/3] PR feedback --- site/src/pages/UsersPage/UsersPage.tsx | 3 ++- site/src/xServices/users/usersXService.ts | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 67ae7aa832b80..e379d03bbaa4d 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -15,7 +15,7 @@ export const Language = { suspendDialogMessagePrefix: "Do you want to suspend the user", activateDialogTitle: "Activate user", activateDialogAction: "Activate", - activateDialogMessagePrefix: "Do you want to active the user", + activateDialogMessagePrefix: "Do you want to activate the user", } export const UsersPage: React.FC = () => { @@ -108,6 +108,7 @@ export const UsersPage: React.FC = () => { />