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

♻️ Update the updateUserRoles mutation #10351

Closed
2 tasks done
Tracked by #10361 ...
tristan-orourke opened this issue May 10, 2024 · 3 comments · Fixed by #10791
Closed
2 tasks done
Tracked by #10361 ...

♻️ Update the updateUserRoles mutation #10351

tristan-orourke opened this issue May 10, 2024 · 3 comments · Fixed by #10791
Assignees
Labels
debt Refactor or improve existing code.

Comments

@tristan-orourke
Copy link
Member

tristan-orourke commented May 10, 2024

♻️ Debt/Refactor

We're creating new permissions which allow for assigning of specific roles, instead of the previous permission which allow assigning any role, or any team role. The mutation used to assign roles must be updated to check the new permissions.

🕵️ Details

Now, in the implementation of the updateUserRoles mutation, we will have to carefully check if the user has the appropriate permission for EACH role which is being attached or detached from the user. This will be particularly complex in the case of sync, where you have to inspect the user's current roles and compare them to the sync array, to see which ones are being removed.

QUESTION: It may actually be easier to remove the sync operation entirely from the mutation, and update the client code to use attach and detach exclusively. (sync also makes it dangerously easy to remove roles by accident.)

I think we can also deprecate/remove updateUserTeamRoles mutation and code, since that was created mostly to make permission checking easier, and we'll use a totally new strategy now.

QUESTION: Depending on how we implemented #10302, we may also need to update how the team is passed into a role assignment. If Team is being abstracted away in the schema, then we can't get the teamId of a Pool or Community to pass in. Do we instead let it accept a Teamable id? or add a teamId field to Teamable interface?

🙋‍♀️ Proposed Solution

Note: Awkwardly, the update-team-processOperatorMembership permission has to give someone in a Community-team permission to assign someone to a Pool-team, if the Pool belongs to the Community.

✅ Acceptance Criteria

A set of assumptions which, when tested, verify that the debt was addressed and expected functionality has not been affected.

  • Assigning roles through the /admin/teams/:id/members page continues to function (though api may change behind the scenes). It can be used by Platform Admins and Community Managers.
  • Assigning roles through the /admin/users/:id/edit page continues to function (though api may change behind the scenes). Community Managers can assign or remove (legacy) team-based roles, and Platform Admins can assign or remove all (legacy) roles. (New Community Admin, Community Recruiter, and Process Operator roles can be ignored here for now.)
  • Role assignment uses the new update-any/team-<role>Membership permissions
    • adding or removing Platform Admin role requires update-any-platformAdminMembership
    • adding or removing Community Admin role requires update-any-communityAdminMembership or update-team-communityAdminMembership in the correct community team
    • adding or removing Community Recruiter role requires update-any-communityRecruiterMembership or update-team-communityRecruiterMembership in the appropriate community team
    • adding or removing Process Operator membership requires update-any-processOperatorMembership or update-team-processOperatorMembership in the pool's team OR the pool's community's team
    • adding or removing Community Manager or Request Responder roles (legacy roles) require assign-any-role permission (will be removed in cleanup)
    • adding or removing Pool Operator legacy role requires assign-any-teamRole permission (will be removed in cleanup)
  • Its possible to use a mutation to assign new Community and Pool based roles
  • phpunit tests

🛑 Blockers

Issues which must be completed before this one.

Blocked By

  1. 2 of 2
    feature
    JamesHuf
  2. deployment feature
    tristan-orourke
@tristan-orourke tristan-orourke added the debt Refactor or improve existing code. label May 10, 2024
@tristan-orourke tristan-orourke added the review in scrum Ready to be looked at and pulled into "ready to dev" label May 30, 2024
Copy link

github-actions bot commented May 30, 2024

Status: Ready to merge ✔️

Issues blocking this PR:


This comment was automatically written by the Blocking Issues bot, and this PR will be monitored for further progress.

@petertgiles petertgiles removed the review in scrum Ready to be looked at and pulled into "ready to dev" label May 31, 2024
@vd1992 vd1992 self-assigned this Jun 25, 2024
@brindasasi
Copy link
Contributor

brindasasi commented Jul 12, 2024

Tests

  • Assigning roles through the /admin/teams/:id/members page continues to function (though api may change behind the scenes). It can be used by Platform Admins and Community Managers

    • Verify Applicant users can't use the above page
  • Assigning roles through the /admin/users/:id/edit page continues to function

  • Community Managers can assign or remove (legacy) team-based roles,

  • Platform Admins can assign or remove all (legacy) roles.

  • New Community Admin, Community Recruiter, and Process Operator roles can be ignored here for now.

  • adding or removing Platform Admin role requires update-any-platformAdminMembership

  • adding or removing Community Admin role requires update-any-communityAdminMembership or update-team-communityAdminMembership in the correct community team

  • adding or removing Community Recruiter role requires update-any-communityRecruiterMembership or update-team-communityRecruiterMembership in the appropriate community team

  • adding or removing Process Operator membership requires update-any-processOperatorMembership or update-team-processOperatorMembership in the pool's team OR the pool's community's team

  • adding or removing Community Manager or Request Responder roles (legacy roles) require assign-any-role permission (will be removed in cleanup)

  • adding or removing Pool Operator legacy role requires assign-any-teamRole permission (will be removed in cleanup)

  • Its possible to use a mutation to assign new Community and Pool based roles

@vd1992
Copy link
Contributor

vd1992 commented Jul 12, 2024

I think you might've meant to put that in the PR?

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: ✅ Done
Development

Successfully merging a pull request may close this issue.

4 participants