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

As an admin user, I want to be able to change the roles of other staff and admin users #615

Closed
Ndshah82 opened this issue Apr 1, 2024 · 10 comments · Fixed by #711
Closed
Assignees
Labels

Comments

@Ndshah82
Copy link
Collaborator

Ndshah82 commented Apr 1, 2024

Admin users should be able to change the role of other staff between staff and admin for their organization.

Acceptance Criteria
Backend

  • Add a new controller UserRolesController (scoped under Organization) with separate actions (and routes) for changing roles on another User e.g., if I am changing a role from staff to admin we have an action that will delete the staff role on the user and create an admin role on the user, and another action for the opposite. Each request should be authorized.
  • Add a new policy class UserRolePolicy that has the general pre-checks (org and active staff) and checks the User making the request is an admin (i.e. has Admin role, and the Admin permissions include a new permission change_user_role. The policy should also prevent a user from making the request on themself (see StaffAccountPolicy for reference).

UI

  • If I am an Admin I should be able to see the staff index table, and in the Role column, see a select that displays the current role for that staff, and if I click the select, I can select the other role. This saves using Turbo upon selection.
  • The role select on my own row in the table is disabled.
  • User submitting the action is notified of change, or failure of change, via flash

image

@Ndshah82
Copy link
Collaborator Author

Ndshah82 commented Apr 1, 2024

@kasugaijin Can admin bump other admin down to staff?

@kasugaijin
Copy link
Collaborator

kasugaijin commented Apr 1, 2024

Yes we only have the two roles (admin and staff) at the moment. So an admin can bump another down to staff. We toyed with the idea of super admin but don’t see it as necessary for MVP. Generally an org owner is only going to give admin status to someone they trust to do that role properly so shouldn’t be an issue.

@kasugaijin kasugaijin added Ready Make a comment to get assigned. and removed Not Ready labels Apr 18, 2024
@mononoken mononoken added Not Ready and removed Ready Make a comment to get assigned. labels Apr 19, 2024
@mononoken
Copy link
Collaborator

@kasugaijin I think we can close this issue or need to change it up.

In regards to the above, I don't think admin are actually currently able to change roles. At least, I don't recall myself implementing any role interactions for users. They are able to deactivate each other, which is maybe what you meant. Do we want to implement the ability for admins to change roles directly? Currently, the only way to change roles is via the console.

@kasugaijin
Copy link
Collaborator

kasugaijin commented Apr 19, 2024

@mononoken i think ideally admin do have the ability to change role. The thinking behind it was to allow others to take on admin roles in the future if current admins are leaving the org.

This will require additional auth policy won’t it?

@mononoken
Copy link
Collaborator

Ah okay I misunderstood. Yes, we would definitely want to setup a policy for these actions however we decide to implement it. Roles are records with a has_and_belongs_to_many association with Users, so I think we would want a UsersRolesController with create and destroy actions for the user role association. We might want separate actions for each role, since creating and destroying an admin role is not the same as creating and destroying an adopter or staff role.

@kasugaijin
Copy link
Collaborator

@mononoken thanks for the clarification.
I think separating this out into its own controller and policy makes more sense and is more future proof than trying to lump it into e.g., StaffController and StaffAccountPolicy.

I have updated the ticket description. It could be split into two tasks, but it's also a doable chunk of work for someone experienced (cough cough Githubbers 👯 )

@mononoken
Copy link
Collaborator

@kasugaijin, I think we can move this issue back to ready. Is that correct?

I think one thing that would be good to add to the requirements is some kind of visual to confirm with the user that the select change was registered. That would also help alert a user if they accidentally changed a select. I think a flash would be fine for that.

@kasugaijin
Copy link
Collaborator

@mononoken yes you’re right we can move this to ready. I agree with the user notification.

@kasugaijin kasugaijin added Ready Make a comment to get assigned. and removed Not Ready labels May 8, 2024
@jmilljr24
Copy link
Collaborator

I can work on this.

@jmilljr24 jmilljr24 self-assigned this May 11, 2024
@jmilljr24 jmilljr24 added Assigned and removed Ready Make a comment to get assigned. labels May 11, 2024
@kasugaijin
Copy link
Collaborator

thanks @jmilljr24

This was referenced May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants