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

Bug/wp 47 fix shared workspace manage team modal #922

Merged
merged 10 commits into from
Dec 22, 2023

Conversation

van-go
Copy link
Contributor

@van-go van-go commented Dec 18, 2023

Overview

In 'Shared Workspace':

  1. if user is a GUEST, disable 'Rename' and 'Trash' buttons
  2. change button style for 'Update' button to match toolbar

Related

Changes

  1. In DataFilesToolbar.jsx: created an 'isGuest' field which uses the authenticatedUserQuery to determine if a user is a guest. canRename and canTrash check isGuest; if that is True, then those buttons are disabled.
  2. In SystemRoleSelector.jsx: changed the button import to come from _common. Added a left margin to the css (DataFilesProjectMembers.module.scss)

Testing

  1. Login as a guest, go to a Data Files > Shared Workspace, verify that the 'Rename' and 'Trash' buttons are disabled.
  2. Login normally, edit the Team Members for a Shared Workspace, verify the button is matches the buttons in the toolbar and turns purple when rolled over.

UI

Disable 'Rename' and 'Trash' for Guest
https://github.com/TACC/Core-Portal/assets/35277477/b41396be-86bc-4d2a-80ae-fadeaff350c5

changes to 'Update' button
Screen Shot 2023-12-18 at 3 49 09 PM 1

Notes

Copy link

codecov bot commented Dec 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7fd106f) 63.70% compared to head (14acb22) 63.71%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #922      +/-   ##
==========================================
+ Coverage   63.70%   63.71%   +0.01%     
==========================================
  Files         431      431              
  Lines       12433    12438       +5     
  Branches     2594     2597       +3     
==========================================
+ Hits         7920     7925       +5     
  Misses       4292     4292              
  Partials      221      221              
Flag Coverage Δ
javascript 69.83% <100.00%> (+0.02%) ⬆️
unittests 57.48% <ø> (ø)

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

Files Coverage Δ
...aFilesProjectMembers/_cells/SystemRoleSelector.jsx 71.11% <ø> (ø)
...ts/DataFiles/DataFilesToolbar/DataFilesToolbar.jsx 76.85% <100.00%> (+1.12%) ⬆️

Copy link
Contributor

@shayanaijaz shayanaijaz left a comment

Choose a reason for hiding this comment

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

Functionality works as expected. Currently the move task also fails as a guest user. I think the move button also needs to be disabled since a user cannot do a move operation when they don't have modify permissions on files.

Copy link
Contributor

@edmondsgarrett edmondsgarrett 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 updating those unit tests as well

@van-go
Copy link
Contributor Author

van-go commented Dec 22, 2023

@shayanaijaz I added the same 'isGuest' check to the move function. Guest users can no longer move files. The button is disabled for Guest users.

Copy link
Contributor

@shayanaijaz shayanaijaz 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 additions

@chandra-tacc chandra-tacc merged commit 7335dcb into main Dec 22, 2023
6 checks passed
@chandra-tacc chandra-tacc deleted the bug/WP-47-fix-shared-workspace-manage-team-modal branch December 22, 2023 18:05
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.

4 participants