scientist-softserv / atla-hyku

Other
0 stars 0 forks source link

Limit importers and exporters by user #201

Closed laritakr closed 7 months ago

laritakr commented 7 months ago

Story

Refs

Prior work limited the ability to view the importer and exporter pages by user role, but did not limit what importers and exporters could be seen.

With this work, only admin users can see all importers and exporters, while other users can only see importers and exporters they have created.

Expected Behavior Before Changes

Users with advanced depositor role can see all importers and exporters.

Expected Behavior After Changes

Only admin can see all importers and exporters. Advanced depositors can only see what they have created.

Screenshots / Video

![Screenshot 2023-11-28 at 5 34 02 PM](https://github.com/scientist-softserv/atla-hyku/assets/17851674/ce644725-fe98-47af-a0b7-c903bbe4a874) ![Screenshot 2023-11-28 at 5 24 52 PM](https://github.com/scientist-softserv/atla-hyku/assets/17851674/df97260c-a039-416f-a6f3-29b267fc6052) ![Screenshot 2023-11-28 at 5 26 12 PM](https://github.com/scientist-softserv/atla-hyku/assets/17851674/1d5c32ed-45fd-42ff-8e79-81e9d9125690) ![Screenshot 2023-11-28 at 5 27 32 PM](https://github.com/scientist-softserv/atla-hyku/assets/17851674/a73a963f-df1d-499c-8259-c3868a37f1ed)

Notes

jeremyf commented 7 months ago

Since this PR introduces complex permissions surround importers and exporters, I think writing CanCanCan rules is probably the way we want to go (as opposed to the current #check_permissions overrides). Especially if this feature gets pushed back to Hyku.

In my head, this would look like:

  1. Creating a #bulkrax_permissions method somewhere (either in Bulkrax itself, app/models/ability.rb, or an ability concern like app/models/concerns/hyrax/ability/collection_ability.rb in Hyku)
  2. Define can rules pertaining to importers and exporters1
  3. Add :bulkrax_permissions to Ability.ability_logic

In principle I really do agree with this. And this was our initial assumed approach. But once we articulated that approach, it felt like a lot of work, so we looked at how we could shorten that time. However, there are a few complicating facts:

  1. The already shipped code breaks the CanCan pattern by asking a question (e.g. current_ability#can_export_works?)
  2. To incorporate the other pattern is a more costly undertaking (in that we'd need to coordinate effort with Bulkrax and Atla, create a version upgrade which would impact our current work across Adventist, Atla, UTK, and Pals)
  3. This implementation cleaves closest to the existing Bulkrax implementation.

I believe the correct approach is to file an issue surrounding this behavior. Further complicating the entire fact is the index action's filter of Exporters and Importers is something that should be bulkrax configurable (e.g. a custom lambda) because CanCan does not understand filtering rules for query sets.

ShanaLMoore commented 7 months ago
rake hyku:roles:create_default_roles_and_groups