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

♻️ Consolidate how frontend permissions are stored and checked. #12066

Open
5 tasks
tristan-orourke opened this issue Nov 21, 2024 · 0 comments · May be fixed by #12007
Open
5 tasks

♻️ Consolidate how frontend permissions are stored and checked. #12066

tristan-orourke opened this issue Nov 21, 2024 · 0 comments · May be fixed by #12007
Assignees
Labels
debt Refactor or improve existing code.

Comments

@tristan-orourke
Copy link
Member

tristan-orourke commented Nov 21, 2024

♻️ Debt/Refactor

Instead of defining the same lists of roles all accross our front-end code, we should define them in a commom place (permissionConstants) and always reference them from there.

Also, we have ended up with multiple versions a role-checking util function (utils.hasRole, teamUtils.checkRole, and communityUtils.checkRole). Lets consolidate them into util.hasRole and delete the others.

🕵️ Details

We check user roles in more and more places on the frontend, mostly as a property for the RequireAuth component, but in other cases as well. The list of roles is maintained separately in the code in each location, even though they reflect a smaller number of conceptual permission groups.

For example, we need to check the same list of roles to see if a user can access a page component wrapped in RequireAuth, if they can see a link to that page in the dashboard, and if they can see a link to that page in the nav menu. So we should define the list of roles once in permissionConstants and refer to it everywhere else.

🙋‍♀️ Proposed Solution

  1. Add functionality to utils.hasRole to match teamUtils.checkRole and communityUtils.checkRole. Specifically, if an empty list of permissions is passed in, it always returns true, and it accepts an optional third arg, teamableId, which is compared to roleAssignment.teamable.id.
  2. Delete the checkRole functions and replace with calls to hasRole.
  3. Replace basically every instance of a reference to ROLE_NAME with a reference to permissionConstants, adding groups to permission constants as needed. (There may be a few places where it makes sense to do otherwise.)

🌎 Localization

✅ Acceptance Criteria

  • Add functionality to utils.hasRole to match teamUtils.checkRole and communityUtils.checkRole
    • if an empty list of permissions is passed in, it always returns true
    • it accepts an optional third arg, teamableId, which is compared to roleAssignment.teamable.id
  • Delete the checkRole functions and replace with calls to hasRole
  • Replace basically every instance of a reference to ROLE_NAME with a reference to permissionConstants, adding groups to permission constants as needed. (There may be a few places where it makes sense to do otherwise.)

🛑 Blockers

Blocked By

No tasks being tracked yet.
@tristan-orourke tristan-orourke added the debt Refactor or improve existing code. label Nov 21, 2024
@tristan-orourke tristan-orourke moved this to 🏗 In progress in GC Digital Talent Nov 21, 2024
@tristan-orourke tristan-orourke self-assigned this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Refactor or improve existing code.
Projects
Status: 🏗 In progress
Development

Successfully merging a pull request may close this issue.

1 participant