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

Role masking for Site Admin #2180

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ives1227
Copy link

@ives1227 ives1227 commented Jul 2, 2019

This allows Site Admin users to toggle between Exhibit Admin and Curator roles to see different views and permissions without having to create separate users. This is particularly useful when using an authentication system outside of Spotlight that does not permit the creation of mock users.

A migration was added to add a role_mask field to the spotlight_roles table. The Can Can ability.rb file has been edited to take into account the new role mask.

When a Site Administrator is logged in, a simple 'View As' menu appears on all screens under the navigation:

ViewAs

When a mask is in place, a notification appears in the same place with an option to clear:

Clear

The Exhibit Admin and Curator masks do not limit visibility to specific exhibits so all exhibits will still be available under the masks.

@jkeck jkeck requested a review from ggeisler July 2, 2019 21:43
@jkeck
Copy link
Contributor

jkeck commented Jul 2, 2019

Thanks for the contribution @ives1227!

I've requested one of the designers (@ggeisler) to review this feature/implementation from the UI/UX perspective before we get too deep into code review (as it may change some of the underlying mechanics).

Copy link
Contributor

@ggeisler ggeisler left a comment

Choose a reason for hiding this comment

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

Hi @ives1227. I've looked at your branch locally and have a few questions about the goals of this PR. I'm just having a little trouble understanding the motivation for this and exactly how it adds value. Here's what I mean:

  • At the site level (features outside of an individual exhibit), the role masking feature doesn't seem relevant, since all site-level features are accessible only to a superadmin (what you referred to as the "Site Admin"), not an exhibit admin or a curator.

  • Within an individual exhibit, there are the administration pages (accessible via the "Exhibit dashboard" link) and the non-admin pages. For the non-admin pages, I don't think there is any difference between accessing those pages as an Exhibit admin versus a Curator. Basically you can just edit the home, feature, and about pages, and you have the same power and features to do that whether you are an Exhibit admin or a Curator. So the role masking feature doesn't seem relevant there, either.

  • The administration section is structured so that an Exhibit admin user sees and has access to everything (i.e. links under both the "Configuration" and "Curation" sidebar headings), and a Curator user sees and has access to only the links under the "Curation" heading. So using the role masking feature here will basically just show or hide the Configuration section, depending on which role you select.

So, unless I'm missing something, the only value this feature adds is to enable the superadmin user to toggle between seeing both the Configuration and Curation sections in the admin section when viewing in the Exhibit admin role, and seeing just the Curation section when viewing in the Curator role. Adding a new feature, with a new UI element on every page of the application, just to do this seems kind of overkill. I get that new superadmins might not intuitively understand the difference between the Configuration and Curation sections in the admin section and how they relate to the two exhibit roles, but it seems much simpler to just point them to documentation such as this for a quick explanation: https://exhibits.stanford.edu/exhibits-documentation/feature/managing-users. I believe the distinction between the two roles is pretty clear once you understand the first paragraph of that page.

But perhaps you have use cases for this feature that aren't covered by the points above? If so, please let us know in a bit more detail what they are. I don't mean to dismiss this feature idea at all, I just want to be sure it has enough value to justify the added code and UX complexity and can't be solved in a less intrusive way, such as documentation for new superadmins.

@ives1227
Copy link
Author

@ggeisler - I was waiting for feedback from our main administrator and trainer. The biggest use of this feature is for training and testing purposes without needing separate accounts. This is what she said:

"From what I gathered on a community call once, I think Stanford sets up all their exhibit owners with the Exhibit Admin role and they don't really use the Curator role. We've done the opposite where we only set up exhibit owners with the Curator role and they aren't even aware of the Exhibit Admin role and the functionality it contains since we take care of those pieces for them. So the masking feature would only be relevant to institutions who use the Spotlight roles like we do.

For us, it enables Super Admins to train exhibit owners with the Curator mask on (without having a separate user account) so we are all looking at the same admin UI during training. It also enables Super Admins to test Curator functionality (without having a separate user account). For example, the masking option came in handy when we found and created a fix for the Curator role not being able to upload thumbnails even though this functionality appears in the admin UI under that role. And we'll continue to use the masking feature as we make tweaks to the user roles in the future."

@ggeisler
Copy link
Contributor

@ives1227: Thanks for passing on the feedback from your Spotlight administrator and trainer. I discussed this with a couple of the developers at Stanford back in the summer and neglected to write up our response to this. Apologies for that.

As your trainer mentions, you use Spotlight differently than we do in that you don't give exhibit owners admin access, only curator access. It sounds like this PR is primarily a solution intended to make training easier given your approach to restricting exhibit owners to curator-only access to their exhibits.

Our concern is that this solution adds a UI element that is somewhat obtrusive (in that site admins will see it on every page of the application) but at the same time not really useful for any institution that doesn't follow the same approach to Spotlight exhibit administration and training as you do (after five years and nearly 100 exhibits we haven't had a need for it). There's also quite a bit of code being added to the codebase for something that, again, might be of limited use beyond Harvard.

At Stanford we've added a fair amount of local Spotlight customization in our Exhibits repo (some examples also listed in this community roadmap spreadsheet), rather than contributing to the Spotlight repo, for features that we're not sure will be useful beyond Stanford. I wonder if this PR fits into that category, and should just be something that you maintain in your institutional Spotlight repo? If over time we discover that the larger Spotlight community is really looking for a feature like this, we could do some work to understand the details of what the community is looking for and revisit your implementation to see if it matches the broader need.

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.

3 participants