-
Notifications
You must be signed in to change notification settings - Fork 6
#2225 - updates permissions for reviews link #2467
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
#2225 - updates permissions for reviews link #2467
Conversation
jackkeller
commented
Jun 3, 2024
- updates permissions for Feedback > Reviews ( removes isAdmin, adds hasReportPermissions)
web-ui/src/components/menu/Menu.jsx
Outdated
| ]; | ||
|
|
||
| const getFeedbackLinks = (isAdmin, isPDL, isSupervisor) => { | ||
| const getFeedbackLinks = (isAdmin, isPDL, isSupervisor, hasReportPermission) => { |
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.
So, in looking at this again...this isn't actually the right permission set. I'll identify it and drop it to you shortly.
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.
Okay, I'll work in the changes at that point.
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.
These feedback links should be accessible as follows:
- FEEDBACK -> Any of
CAN_VIEW_FEEDBACK_ANSWER,CAN_VIEW_FEEDBACK_REQUEST,CAN_VIEW_REVIEW_PERIOD- View Feedback ->
CAN_VIEW_FEEDBACK_ANSWER - Received Requests ->
CAN_VIEW_FEEDBACK_REQUEST - Reviews ->
CAN_VIEW_REVIEW_PERIOD - Self-Reviews ->
CAN_VIEW_REVIEW_PERIOD
- View Feedback ->
Implementing this depends on PR #2476 being merged
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.
That PR is merged now.
|
Aside from |