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

Manager of an Admin Set does not see Admin Set menu on dashboard #1110

Closed
hannahfrost opened this issue May 12, 2017 · 15 comments
Closed

Manager of an Admin Set does not see Admin Set menu on dashboard #1110

hannahfrost opened this issue May 12, 2017 · 15 comments

Comments

@hannahfrost
Copy link

Expected behavior

I am a Manager of the default admin set. I was granted this role by the Repository Admin.

I should see the Admin Set menu under "Repository Contents" on the dashboard when I am logged in, so that I can help manage the Admin Set.

Actual behavior

When I am logged in and looking at the admin dashboard, I don't see the Admin Set menu under "Repository Contents", so I can't help manage the admin set.

@hannahfrost
Copy link
Author

Related to samvera/hyrax#853

@mjgiarlo
Copy link
Member

When this user gets to the Admin Set menu, I assume they should:

  1. Only see AdminSets they manage
  2. Not be able to created new Admin Sets
  3. Be able to edit the AdminSets they manage (change title/description/participants/release/visibility/workflow)
  4. Be able to delete the AdminSets they manage

What say you, @hannahfrost?

@hannahfrost
Copy link
Author

@mjgiarlo I say yes to all points listed above, thank you very much.

@dazza-codes
Copy link
Contributor

dazza-codes commented May 15, 2017

This begs the question of the group/role definitions and how they should differ for admin vs. manager vs. user and how those groups/roles apply to an AdminSet (or any collection)? That is, the implementation details depend on the definition and assignment to user groups/roles.

@carrickr
Copy link

screen shot 2017-05-15 at 11 03 00 am

Screenshot of adding a generic user as a manager to admin set.

@mjgiarlo
Copy link
Member

@darrenleeweber I believe we want to display this menu to any user who is an admin or who has the 'manage' role on any AdminSet (well, technically on its PermissionTemplate). We should be able to do this with logic in our Ability module. There's some code in there already that can be used or adapted for this purpose: https://github.com/projecthydra-labs/hyrax/blob/0eb1dad61fcbb8ab44ed2738bbcd2ee2256593b0/app/models/concerns/hyrax/ability.rb#L38-L49

@carrickr
Copy link

=> {"system_create_dtsi"=>"2017-05-15T18:01:13Z", "system_modified_dtsi"=>"2017-05-15T18:01:13Z", "has_model_ssim"=>["AdminSet"], :id=>"8p58pc92q", "accessControl_ssim"=>["6a2fc871-d8a2-41b9-add8-c4bd2576e8e1"], "title_tesim"=>["Testing Set"], "title_sim"=>["Testing Set"], "description_tesim"=>["For Carrick"], "creator_ssim"=>["archivist1@example.com"], "thumbnail_path_ss"=>"/assets/default-f936e9c3ea7a38e2c2092099586a71380b11258697b37fb4df376704495a849a.png", "generic_type_sim"=>["Admin Set"], "read_access_group_ssim"=>["public"], "edit_access_person_ssim"=>["carrickr@umich.edu", "archivist1@example.com"], "human_readable_type_sim"=>"Admin Set", "human_readable_type_tesim"=>"Admin Set"}

Manage role is reflected in edit_access_person_ssim just for reference

@dazza-codes
Copy link
Contributor

We've replicated this bug in hyrax (without hyku).

@carrickr
Copy link

carrickr commented May 15, 2017

Darren, I think we want to roughly use this:

samvera/hyrax@0cd90ae

I just grabbed the code Mike highlighted and added a function to get a list of just the admin sets a user can manage.

So we want to:

  • DRY up the code for this function admin_set_ids_for_deposit in ability.rb (helper function that takes roles as a param?)
  • figure out where we determine if adminset shows up in the sidebar and if you can edit and delete it, set those locations to use this new function
  • adjust ability_spec.rb to expect managers to be able to edit, delete admin sets they can manage
  • ensure managers of admin sets cannot see other admin sets they don't manage and cannot create admin sets

Thoughts?

@carrickr
Copy link

@mjgiarlo @hannahfrost

So I have code up in the bugfix/1110 branch that fixes it, but the problem is that to hack it in I need to grant global access to the dashboard, because the dashboard doesn't appear to be granular enough I can give folks with manager rights access to just the admin sets views. Am I missing something with the admin namespace? Or is dashboard all or nothing (comments in the code seem to suggest all or nothing).

If that's the case when we have a larger ticket about breaking up the dashboard in addition to this ticket and talk about how we'll do it.

@mjgiarlo
Copy link
Member

@carrickr You'll want to confab with @jcoyne, the architect of the consolidated dashboard, about this.

@jcoyne
Copy link
Member

jcoyne commented May 15, 2017

@carrickr prior to this ticket, there was no differences in the dashboard for "admin set managers" than a regular user. In order to resolve this ticket this must be changed.

@carrickr
Copy link

Looking through this we may not need additional ability.rb tests, in that there seem to be tests for the abilities

https://github.com/projecthydra-labs/hyrax/blob/master/spec/abilities/ability_spec.rb#L119-L131

we just need view tests

@jcoyne
Copy link
Member

jcoyne commented May 16, 2017

@carrickr I think you should add a test for :manage_any

@dazza-codes
Copy link
Contributor

dazza-codes commented May 16, 2017

RE Participants in https://docs.google.com/document/d/1oCVnyzamH1sD3mSaqPooknfRTDYKYAsAdEx61hBm6v4/edit

There is a short-term goal to “better align the AdminSet Participants with the Roles defined for a Workflow”. What does this mean? While coding the implementation details for the ability of a Participant-Manager, we are trying to use the ability code for User-Role and User-Group. However, that may not provide the desired result(s) because the mapping from a User or Role to a Participant is not clear yet? It seems as though the only source of authority for the Participant access details is in the AdminSet object itself (it should have metadata to identify manager participants by their user-id), which does not depend in any way on user roles or groups. If that is true, the changes to Ability might be misdirected? If not, then we need a clear alignment between user role/group and AdminSet-participants.

In short, we may be confusing a rails-cancan permission/ability based on user role/group data with something different, an FCrepo object "participant".

For example, see samvera/hyrax@9e829b8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants