-
Notifications
You must be signed in to change notification settings - Fork 9
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
[Fix] Touch up frontend permissions #12065
base: main
Are you sure you want to change the base?
Conversation
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.
Some questions. Also, I'm still missing some links for roles in the nav menu.
[email protected]
[email protected]
[email protected]
While there is an unauthorized message, maybe we should hide the button entirely for creating a process? 🤔
Also, while it seems the links are correct... I need to select the "community" role which seems weird to need to do as a process operator 😅
@@ -1,30 +1,39 @@ | |||
import { RoleName } from "@gc-digital-talent/auth"; | |||
import { ROLE_NAME, RoleName } from "@gc-digital-talent/auth"; | |||
|
|||
const permissionConstants = () => { |
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.
Does this need to be a function? Seems like we could just export the return value as a plain object.
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.
Sure. I guess a function means you can't accidentally modify it? But I think we can avoid doing that. Updated in 7fdea776a49994f01b1093819188ad5dd9814ea4
apps/web/src/pages/PoolCandidates/AllPoolCandidatesPage/AllPoolCandidatesPage.tsx
Show resolved
Hide resolved
Create pool button hidden if user not authorized to do so: 844c4d3 |
I agree, its not obvious it should be like this, but Process Operators are working under the direction/delegated authority of Community Admins, and we weren't sure how else to organize it. |
Which links are missing? |
I think communities and users. Based on the outline you provided, I think |
They should have those links on their dashboard but not the nav menu, since they shouldn't need to access those pages very often. |
🤖 Resolves #11767
👋 Introduction
This fixes a few places frontend permissions to view different pages, or the links that showed in the new Nav or Dashboard, weren't correct, especially for the newer roles.
🕵️ Details
Where possible, I referenced the same role arrays in permissionConstants to control links appearing in both the nav menu and the dashboard, so ensure they remain in sync. This is a pattern I want to fully move to in #12066 but that involves a larger scope.
🧪 Testing
Log in as [email protected], as [email protected], and as [email protected]. For each, consider the nav menu, the dashboard, and click around to different pages. Check that you can access the pages and functionality described, with the restrictions described:
Community Admins should be able to access:
Community Recruiter should be the same as Community Admin, except...
So... Community Recruiter should be able to access:
Process Operator should be able to access:
Process Operator should see links on their dashboard to:
Platform Admin
Additionally, a Platform Admin should be able to view Talent Requests (accessible from nav or the dashboard as Requests) and Communities and Teams should appear in the System settings dropdown menu in the nav bar.
📸 Screenshot
🚚 Deployment
After deploying this, the new roles should be ready to use. We should instruct Recruitment team (and the ATIP team) to use the Pool Manage Access tab to add people to pools, instead of adding them as Pool Operators within a team.
Additionally, we should run
php artisan app:sync-pool-process-operator
after deploying to ensure Pool Operators are converted to Process Operators.