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

fix: users can be removed from all groups in Web UI #9259

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

maxrussell
Copy link
Contributor

@maxrussell maxrussell commented Apr 26, 2024

Ticket

DET-10246

Description

Validation was keeping us from removing all groups from a user in the Web UI. This fixes that.
image

Test Plan

  1. Turn on RBAC or use a cluster with RBAC enabled
  2. Create a user
  3. Add the user to one group (can be a new group if you'd like)
  4. While logged in as an admin in the Web UI, navigate to the Admin Settings page (/admin/user-management)
  5. Use the kebab menu next to your new user and select "Manage Groups"
  6. Remove all groups from the list of groups to which the user should belong
  7. The app should let you click "Save," removing the user from all groups
  8. Verify the user isn't in the group in the CLI by using det user-group describe GroupName (substituting the real group name you chose into the command)

Checklist

  • Changes have been manually QA'd

Co-authored-by: Amanda Vialva <amanda.vialva@hpe.com>
@maxrussell maxrussell requested a review from a team as a code owner April 26, 2024 22:55
@cla-bot cla-bot bot added the cla-signed label Apr 26, 2024
Copy link

netlify bot commented Apr 26, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit ede6398
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/662fef8eb407320007ac3cf2
😎 Deploy Preview https://deploy-preview-9259--determined-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Apr 26, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 37.95%. Comparing base (7aa5d5d) to head (ede6398).
Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9259      +/-   ##
==========================================
- Coverage   44.69%   37.95%   -6.75%     
==========================================
  Files        1270      946     -324     
  Lines      155186   115571   -39615     
  Branches     2435     2436       +1     
==========================================
- Hits        69368    43866   -25502     
+ Misses      85582    71469   -14113     
  Partials      236      236              
Flag Coverage Δ
harness ?
web 35.18% <33.33%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
webui/react/src/components/ManageGroupsModal.tsx 47.32% <33.33%> (-21.20%) ⬇️

... and 324 files with indirect coverage changes

@carolinaecalderon carolinaecalderon added the to-cherry-pick Pull requests that need to be cherry-picked into the current release label Apr 29, 2024
@carolinaecalderon carolinaecalderon requested review from a team and removed request for EmilyBonar and a team April 29, 2024 13:18
Copy link
Contributor

@johnkim-det johnkim-det left a comment

Choose a reason for hiding this comment

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

could we do this with something like disabled: _.isEqual(userGroups, groupsValue) to still prevent submitting when no changes are made while allowing an empty array value?

@maxrussell
Copy link
Contributor Author

Good idea, @johnkim-det. Updated.

Copy link
Contributor

@johnkim-det johnkim-det left a comment

Choose a reason for hiding this comment

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

LGTM thanks for the change

@maxrussell maxrussell enabled auto-merge (squash) April 29, 2024 19:06
@maxrussell maxrussell merged commit 86328cb into main Apr 29, 2024
87 of 99 checks passed
@maxrussell maxrussell deleted the amanda+max/remove-user-from-groups branch April 29, 2024 19:16
github-actions bot pushed a commit that referenced this pull request Apr 29, 2024
Co-authored-by: Amanda Vialva <amanda.vialva@hpe.com>
(cherry picked from commit 86328cb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed to-cherry-pick Pull requests that need to be cherry-picked into the current release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants