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

[Is a member of][User groups] 'Refresh' button functionality #297

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

carma12
Copy link
Collaborator

@carma12 carma12 commented Feb 27, 2024

The 'Refresh' button should re-fetch the data related to the user groups of a given user.

The solution has been implemented by taking the refetch functionality retrieved from the useGetUserByUidQuery hook and the button is disabled during the operation.

This PR depends on this one to be merged: #293

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.

Almost LGTM, I think we can drop one state var and save a render. See the comment.

src/components/MemberOf/MemberOfUserGroups.tsx Outdated Show resolved Hide resolved
@carma12 carma12 force-pushed the user-groups-refresh-button-2 branch from e10e4d9 to 53892d3 Compare March 7, 2024 07:54
@carma12
Copy link
Collaborator Author

carma12 commented Mar 7, 2024

@pvoborni - The code has been modified based on your last comment.

The 'Refresh' button should refetch the
data related to the user groups of a given
user. The solution has been implemented
taking the `refetch` functionality.

Signed-off-by: Carla Martinez <[email protected]>
@carma12 carma12 force-pushed the user-groups-refresh-button-2 branch from 53892d3 to 9252c62 Compare March 7, 2024 08:16
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 c3cc12f into freeipa:main Mar 7, 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