- Notifications
You must be signed in to change notification settings - Fork928
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -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", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should this beactivate in place ofactive?
| suspendUserError: "Error on suspending the user.", | ||
| suspendUserError: "Error suspending user.", | ||
| activateUserSuccess: "Successfully activated the user", | ||
| activateUserError: "Error activating user", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit: Add periods (.).
| @@ -99,6 +107,25 @@ export const UsersPage: React.FC = () => { | |||
| } | |||
| /> | |||
| <ConfirmDialog | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We have asuccess type forConfirmDialog which uses the green button. Maybe we can use that for this use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
good idea!
| } | ||
| 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 |
AbhineetJainJun 15, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
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.contextThoughts?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 thedefault is 80 andwe've got it set to 120. This would be a good thing to chat about in FE variety if you'd like.
resolves#2254
