Skip to content

feat: ability to activate suspended users #2344

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions site/src/components/UsersTable/UsersTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -41,6 +42,7 @@ export const UsersTable: FC<UsersTableProps> = ({
users,
roles,
onSuspendUser,
onActivateUser,
onResetUserPassword,
onUpdateUserRoles,
isUpdatingUserRoles,
Expand Down Expand Up @@ -115,12 +117,10 @@ export const UsersTable: FC<UsersTableProps> = ({
},
]
: [
// 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,
Expand Down
81 changes: 79 additions & 2 deletions site/src/pages/UsersPage/UsersPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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/)
Expand Down Expand Up @@ -99,7 +128,7 @@ describe("Users Page", () => {
it("shows users", async () => {
render(<UsersPage />)
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", () => {
Expand Down Expand Up @@ -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(
<>
<UsersPage />
<GlobalSnackbar />
</>,
)

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(
<>
<UsersPage />
<GlobalSnackbar />
</>,
)

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 () => {
Expand Down
30 changes: 29 additions & 1 deletion site/src/pages/UsersPage/UsersPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 activate 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
Copy link
Contributor

@AbhineetJain AbhineetJain Jun 15, 2022

Choose a reason for hiding this comment

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

Wondering what the ideal linting should be here. Something like:

const {
    users,
    getUsersError,
    userIdToSuspend,
    userIdToActivate, 
    userIdToResetPassword, 
    newUserPassword,
} = usersState.context

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, sorry I missed this! I agree, that is nicer. I left it up to the formatter and it didn't print it as clearly. I can update on my next UI PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I missed it in my initial review too. Next PR sounds good! I wonder if we can add something to the formatter though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yeah, just tried it out and we'd have to adjust the prettier rule. I think the default is 80 and we've got it set to 120. This would be a good thing to chat about in FE variety if you'd like.

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
Expand Down Expand Up @@ -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 })
}}
Expand Down Expand Up @@ -99,6 +107,26 @@ export const UsersPage: React.FC = () => {
}
/>

<ConfirmDialog
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a success type for ConfirmDialog which uses the green button. Maybe we can use that for this use case.

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea!

type="success"
hideCancel={false}
open={usersState.matches("confirmUserActivation")}
confirmLoading={usersState.matches("activatingUser")}
title={Language.activateDialogTitle}
confirmText={Language.activateDialogAction}
onConfirm={() => {
usersSend("CONFIRM_USER_ACTIVATION")
}}
onClose={() => {
usersSend("CANCEL_USER_ACTIVATION")
}}
description={
<>
{Language.activateDialogMessagePrefix} <strong>{userToBeActivated?.username}</strong>?
</>
}
/>

<ResetPasswordDialog
loading={usersState.matches("resettingUserPassword")}
user={userToResetPassword}
Expand Down
3 changes: 3 additions & 0 deletions site/src/pages/UsersPage/UsersPageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface UsersPageViewProps {
isLoading?: boolean
openUserCreationDialog: () => 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
}
Expand All @@ -31,6 +32,7 @@ export const UsersPageView: FC<UsersPageViewProps> = ({
roles,
openUserCreationDialog,
onSuspendUser,
onActivateUser,
onResetUserPassword,
onUpdateUserRoles,
error,
Expand Down Expand Up @@ -60,6 +62,7 @@ export const UsersPageView: FC<UsersPageViewProps> = ({
users={users}
roles={roles}
onSuspendUser={onSuspendUser}
onActivateUser={onActivateUser}
onResetUserPassword={onResetUserPassword}
onUpdateUserRoles={onUpdateUserRoles}
isUpdatingUserRoles={isUpdatingUserRoles}
Expand Down
10 changes: 10 additions & 0 deletions site/src/testHelpers/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion site/src/testHelpers/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
63 changes: 62 additions & 1 deletion site/src/xServices/users/usersXService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand All @@ -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
Expand All @@ -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" }
Expand All @@ -72,6 +81,9 @@ export const usersMachine = createMachine(
suspendUser: {
data: TypesGen.User
}
activateUser: {
data: TypesGen.User
}
updateUserPassword: {
data: undefined
}
Expand All @@ -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"],
Expand Down Expand Up @@ -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: {
Expand All @@ -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",
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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,
}),
Expand All @@ -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,
}),
Expand All @@ -292,6 +343,9 @@ export const usersMachine = createMachine(
clearSuspendUserError: assign({
suspendUserError: (_) => undefined,
}),
clearActivateUserError: assign({
activateUserError: (_) => undefined,
}),
clearResetUserPasswordError: assign({
resetUserPasswordError: (_) => undefined,
}),
Expand All @@ -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)
},
Expand Down