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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 15, 2022
Merged

feat: ability to activate suspended users #2344

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a322f4
add ability to activate users
Kira-Pilot Jun 14, 2022
88643c6
added test
Kira-Pilot Jun 15, 2022
26ace52
Merge remote-tracking branch 'origin/main' into ability-to-activate/k…
Kira-Pilot Jun 15, 2022
9a232a8
PR feedback
Kira-Pilot Jun 15, 2022
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
Next Next commit
add ability to activate users 
resolves #2254
  • Loading branch information
@Kira-Pilot
Kira-Pilot committed Jun 14, 2022
commit 7a322f41f6304e10ff53e7ae1db6b21466b20a76
12 changes: 6 additions & 6 deletions site/src/components/UsersTable/UsersTable.tsx
View file
Open in desktop
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
29 changes: 28 additions & 1 deletion site/src/pages/UsersPage/UsersPage.tsx
View file
Open in desktop
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 active the user",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be activate in place of active?

}

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 •
edited
Loading

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,25 @@ 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!

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
View file
Open in desktop
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
63 changes: 62 additions & 1 deletion site/src/xServices/users/usersXService.ts
View file
Open in desktop
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",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Add periods (.).

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