-
Notifications
You must be signed in to change notification settings - Fork 13
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
[Stage users][Settings][Kebab] Add 'Activate' option ('Stage users') #225
Conversation
8c99a6f
to
8437d26
Compare
64c06d2
to
970e7e2
Compare
970e7e2
to
f70fec8
Compare
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.
The code works. I have only readability comments. Also I tried to avoid to comment the code which was not change here.
But that said, from my PoV, one of the biggest readability issues are the interfaces of props of ActivateStageUsers
and UsersDisplayTable
.
From my PoV they should be something like:
interface ActivateUsersModalProps {
isVisible: boolean;
onToggle: (visible) => void;
uidsToActivate: string[];
onSuccess?: () => void;
}
interface UserDisplayTableProps {
uids: string[];
from: "active-users" | "stage-users" | "preserved-users";
}
One issue that I just notice is that when user is |
f70fec8
to
ebf781c
Compare
ebf781c
to
0c728ea
Compare
e01a9ac
to
a88c2a3
Compare
@pvoborni - The code has been adapted to use the |
The 'Activate' option enables the account of a given user, moving it from the 'Stage users' to the 'Active users' page. This option is only accessible form the 'Stage useres' page. The solution has adapted the API call made. Instead of using a generalistic batch endpoint a new specific endpoint has been created: `useActivateUserMutation`. This is being used from the main page and the kebab options located in the 'Settings' page. Signed-off-by: Carla Martinez <carlmart@redhat.com>
a88c2a3
to
363ab86
Compare
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.
LGTM
The 'Activate' option enables the account of a given user, moving it from the 'Stage users' to the 'Active users' page. This option is only accessible form the 'Stage users' page.
The solution has adapted the API call made. Instead of using a generalistic batch endpoint a new specific endpoint has been created:
useActivateUserMutation
. This is being used from the main page and the kebab options located in the'Settings' page.