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.

Sign up for GitHub

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

Jump to bottom

Enable 'set collections' button when user is set as admin #3755

Merged
merged 8 commits into from Sep 26, 2023
Merged

Enable 'set collections' button when user is set as admin #3755

merged 8 commits into from Sep 26, 2023

Conversation

CarolineDenis
Copy link
Contributor

Fixes #2695

@CarolineDenis CarolineDenis mentioned this pull request Jul 10, 2023
Closed
melton-jason
melton-jason requested changes Jul 11, 2023
View reviewed changes
Copy link
Contributor

@melton-jason melton-jason left a comment

Choose a reason for hiding this comment

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

I am not sure that this is a correct fix, especially allowing users (with the right permissions) to modify the Specify 6 collection permissions of administrators (see my comment regarding this).

Besides conceptual considerations, there is a bug related to these changes that should be addressed.

specifyweb/context/views.py Outdated Show resolved Hide resolved
specifyweb/frontend/js_src/lib/components/Security/UserCollections.tsx Show resolved Hide resolved
@grantfitzsimmons
Copy link
Contributor

See my comment

@grantfitzsimmons
Copy link
Contributor

We need to

  • Give the user access to set collections for admins
  • Make sure they can give access to 0 or many collections

melton-jason
melton-jason approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@melton-jason melton-jason left a comment

Choose a reason for hiding this comment

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

This is really what the Specify 6 message displays when a user doesn't have access to collections? 🤦
no_collections

@maxpatiiuk
Copy link
Member

Specify will terminate is not menacing sounding at all 🤖💣💥 ehem, Terminator

almost like Specify will exterminate ...

@CarolineDenis CarolineDenis requested a review from a team July 14, 2023 13:26
@grantfitzsimmons grantfitzsimmons linked an issue Jul 14, 2023 that may be closed by this pull request
Setting a user as admin does not set their 6 collections #2695
Open
grantfitzsimmons
grantfitzsimmons approved these changes Jul 16, 2023
View reviewed changes
Copy link
Contributor

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

Works fine. I am still caught off guard by the fact that the "Set Collections" dialog has its own save button and acts independently of the rest of the options in the user page.

@grantfitzsimmons grantfitzsimmons linked an issue Jul 16, 2023 that may be closed by this pull request
Exceptions on any changes to user access to collection #1174
Closed
@grantfitzsimmons grantfitzsimmons requested a review from a team August 28, 2023 17:18
@grantfitzsimmons grantfitzsimmons added this to the 7.9.1 milestone Aug 28, 2023
Base automatically changed from v7.9-dev to production September 25, 2023 18:42
@CarolineDenis CarolineDenis merged commit b57d3ff into production Sep 26, 2023
9 checks passed
@CarolineDenis CarolineDenis deleted the issue-2695 branch September 26, 2023 14:14
@CarolineDenis CarolineDenis restored the issue-2695 branch March 14, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grantfitzsimmons grantfitzsimmons grantfitzsimmons approved these changes

@melton-jason melton-jason melton-jason approved these changes

Assignees
No one assigned
Labels
None yet
Projects
General Tester Board
Status: ✅Done
Milestone
7.9.1
4 participants
@CarolineDenis @grantfitzsimmons @maxpatiiuk @melton-jason