Skip to content
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

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

carma12
Copy link
Collaborator

@carma12 carma12 commented Dec 15, 2023

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.

@carma12 carma12 force-pushed the kebab-activate-option branch 3 times, most recently from 8c99a6f to 8437d26 Compare December 19, 2023 08:21
@carma12 carma12 force-pushed the kebab-activate-option branch 6 times, most recently from 64c06d2 to 970e7e2 Compare January 12, 2024 12:21
@pvoborni pvoborni self-assigned this Jan 16, 2024
@carma12 carma12 force-pushed the kebab-activate-option branch from 970e7e2 to f70fec8 Compare January 17, 2024 08:23
Copy link
Member

@pvoborni pvoborni left a 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";
}

src/services/rpc.ts Outdated Show resolved Hide resolved
src/services/rpc.ts Outdated Show resolved Hide resolved
src/components/modals/ActivateStageUsers.tsx Outdated Show resolved Hide resolved
@pvoborni
Copy link
Member

One issue that I just notice is that when user is activated the current page stays to be in stage users instead of expected active users.

@carma12 carma12 force-pushed the kebab-activate-option branch 2 times, most recently from e01a9ac to a88c2a3 Compare January 19, 2024 09:03
@carma12
Copy link
Collaborator Author

carma12 commented Jan 19, 2024

@pvoborni - The code has been adapted to use the onSuccess function for redirecting from both pages ('Stage users' and 'Settings') and the props mentioned above have been amended.

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>
@carma12 carma12 force-pushed the kebab-activate-option branch from a88c2a3 to 363ab86 Compare January 19, 2024 10:26
Copy link
Member

@pvoborni pvoborni left a comment

Choose a reason for hiding this comment

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

LGTM

@carma12 carma12 merged commit acf201e into freeipa:main Jan 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants