-
Notifications
You must be signed in to change notification settings - Fork 5
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
Move isUserStaffWithRestricted at the user context level #11378
Conversation
Size Change: +40 B (0%) Total Size: 1.01 MB
ℹ️ View Unchanged
|
I'm no longer seeing the view item link on the works page for restricted items, when I'm logged in |
There was a typo in the role name 😄 ( |
That's what reviews are for 😃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good and is working ok locally. Thanks
Think you just need to merge in main to fix the broken e2e tests |
What does this change?
Makes use of the User Context and adds a check of whether or not the user has the
StaffWithRestricted
role. We did this check in various places, so I thought it might be good to have it at a higher level. This is very much a suggestion though, as I was going through the files and wondering how we could use Contexts for the item viewer. So we can just close this if it's not wanted!How to test
Ensure behaviour doesn't change in affected files.
How can we measure success?
No need for repetitive code for the check no more
Have we considered potential risks?
If we QA this and the conditions make sense, risk should be very low.