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

Add UI for changing a member's role within a group #9124

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Conversation

robertknight
Copy link
Member

@robertknight robertknight commented Nov 25, 2024

Add a "Role" column to the Members tab of the group settings UI. This displays a
dropdown with the different roles that the current user can assign to the target
user within the group. If the current user cannot change the role, the column
displays the current role as plain text.

We currently prevent the user from changing their own role since this has added
complexity. It may alter the user the actions the user can take on other users
and they could lose access to the group settings page entirely if they demote
themselves to a "Member".

Example group as user robert with owner role:

edit-role-as-first-user

Same group as user robert2 with admin role:

edit-role-as-second-user

TODO:

  • Confirm how we should handle demoting yourself. If we decide this action should be possible, then we should show a confirmation prompt and refresh the member list afterwards, since the action will affect the other actions you can perform. If you downgrade yourself to a member, you also currently lose access to the settings page entirely. Alternatively we could decide to disable this action entirely, which will require a change on the backend to the available actions.

@robertknight robertknight force-pushed the remove-group-member-ui branch from bbea36d to 83c581f Compare November 28, 2024 07:11
@robertknight robertknight force-pushed the role-edit-ui branch 2 times, most recently from 4fb2246 to c53d4df Compare November 28, 2024 07:43
@robertknight robertknight marked this pull request as ready for review November 28, 2024 07:49
@robertknight robertknight requested a review from acelaya November 28, 2024 08:01
Copy link
Contributor

@acelaya acelaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

const possibleRoles: Role[] = Object.keys(roleStrings) as Role[];

function memberToRow(member: GroupMember, currentUserid: string): MemberRow {
const role = member.roles[0] ?? 'member';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment explaining why we pick only the first item in the member.roles list and ignore the rest?

IIRC, this was designed as a list to be more future-proof, allowing to add multiple roles without breaking the API contract, but for now all members will have only one role in that list.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UI design will need to change if we support more roles, and it isn't clear exactly how we'd present and enable selection of multiple roles. To keep things simple, we just assume a single role for the moment and will update this UI as necessary if we support multiple roles later.

@robertknight robertknight force-pushed the remove-group-member-ui branch from 83c581f to b64771a Compare November 28, 2024 09:55
Base automatically changed from remove-group-member-ui to main November 28, 2024 10:03
Add a "Role" column to the Members tab of the group settings UI. This displays a
dropdown with the different roles that the current user can assign to the target
user within the group. If the current user cannot change the role, the column
displays the current role as plain text.

We currently prevent the user from changing their own role since this has added
complexity. It may alter the user the actions the user can take on other users
and they could lose access to the group settings page entirely if they demote
themselves to a "Member".
@robertknight robertknight merged commit a4865c4 into main Nov 28, 2024
9 checks passed
@robertknight robertknight deleted the role-edit-ui branch November 28, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants