diff --git a/h/static/scripts/group-forms/components/EditGroupMembersForm.tsx b/h/static/scripts/group-forms/components/EditGroupMembersForm.tsx index de84a2ecab7..61246ee8d3e 100644 --- a/h/static/scripts/group-forms/components/EditGroupMembersForm.tsx +++ b/h/static/scripts/group-forms/components/EditGroupMembersForm.tsx @@ -3,15 +3,16 @@ import { Scroll, TrashIcon, IconButton, + Select, } from '@hypothesis/frontend-shared'; import { useContext, useEffect, useState } from 'preact/hooks'; import { Config } from '../config'; import type { APIConfig, Group } from '../config'; -import ErrorNotice from './ErrorNotice'; -import FormContainer from './forms/FormContainer'; -import type { GroupMembersResponse } from '../utils/api'; +import type { GroupMember, GroupMembersResponse, Role } from '../utils/api'; import { callAPI } from '../utils/api'; +import FormContainer from './forms/FormContainer'; +import ErrorNotice from './ErrorNotice'; import GroupFormHeader from './GroupFormHeader'; import WarningDialog from './WarningDialog'; @@ -25,8 +26,40 @@ type MemberRow = { username: string; userid: string; showDeleteAction: boolean; + role: Role; + availableRoles: Role[]; }; +/** + * Mappings between roles and labels. The keys are sorted in descending order + * of permissions. + */ +const roleStrings: Record = { + owner: 'Owner', + admin: 'Admin', + moderator: 'Moderator', + member: 'Member', +}; +const possibleRoles: Role[] = Object.keys(roleStrings) as Role[]; + +function memberToRow(member: GroupMember, currentUserid: string): MemberRow { + const role = member.roles[0] ?? 'member'; + const availableRoles = + member.userid !== currentUserid + ? possibleRoles.filter(role => + member.actions.includes(`updates.roles.${role}`), + ) + : [role]; + return { + userid: member.userid, + username: member.username, + showDeleteAction: + member.actions.includes('delete') && member.userid !== currentUserid, + role, + availableRoles, + }; +} + async function fetchMembers( api: APIConfig, currentUserid: string, @@ -38,12 +71,7 @@ async function fetchMembers( headers, signal, }); - return members.map(member => ({ - userid: member.userid, - username: member.username, - showDeleteAction: - member.actions.includes('delete') && member.userid !== currentUserid, - })); + return members.map(m => memberToRow(m, currentUserid)); } async function removeMember(api: APIConfig, userid: string) { @@ -55,6 +83,57 @@ async function removeMember(api: APIConfig, userid: string) { }); } +async function setMemberRoles( + api: APIConfig, + userid: string, + roles: Role[], +): Promise { + const { url: urlTemplate, method, headers } = api; + const url = urlTemplate.replace(':userid', encodeURIComponent(userid)); + return callAPI(url, { + method, + headers, + json: { + roles, + }, + }); +} + +type RoleSelectProps = { + username: string; + + /** The current role of the member. */ + current: Role; + + /** Ordered list of possible roles that the current user can assign to the member. */ + available: Role[]; + + /** Callback for when the user requests to change the role of the member. */ + onChange: (r: Role) => void; +}; + +function RoleSelect({ + username, + current, + available, + onChange, +}: RoleSelectProps) { + return ( + + ); +} + export type EditGroupMembersFormProps = { /** The saved group details. */ group: Group; @@ -64,7 +143,7 @@ export default function EditGroupMembersForm({ group, }: EditGroupMembersFormProps) { const config = useContext(Config)!; - const userid = config.context.user.userid; + const currentUserid = config.context.user.userid; // Fetch group members when the form loads. const [errorMessage, setErrorMessage] = useState(null); @@ -76,7 +155,7 @@ export default function EditGroupMembersForm({ } const abort = new AbortController(); setErrorMessage(null); - fetchMembers(config.api.readGroupMembers, userid, abort.signal) + fetchMembers(config.api.readGroupMembers, currentUserid, abort.signal) .then(setMembers) .catch(err => { setErrorMessage(`Failed to fetch group members: ${err.message}`); @@ -84,13 +163,17 @@ export default function EditGroupMembersForm({ return () => { abort.abort(); }; - }, [config.api.readGroupMembers, userid]); + }, [config.api.readGroupMembers, currentUserid]); const columns: TableColumn[] = [ { field: 'username', label: 'Username', }, + { + field: 'role', + label: 'Role', + }, { field: 'showDeleteAction', label: '', @@ -122,6 +205,33 @@ export default function EditGroupMembersForm({ } }; + const updateMember = (userid: string, update: Partial) => { + setMembers( + members => + members?.map(m => { + return m.userid === userid ? { ...m, ...update } : m; + }) ?? null, + ); + }; + + const changeRole = async (member: MemberRow, role: Role) => { + updateMember(member.userid, { role }); + try { + const updatedMember = await setMemberRoles( + config.api.editGroupMember!, + member.userid, + [role], + ); + // Update the member row in case the role change affected other columns + // (eg. whether we have permission to delete the user). + updateMember(member.userid, memberToRow(updatedMember, currentUserid)); + } catch (err) { + const prevRole = member.role; + updateMember(member.userid, { role: prevRole }); + setErrorMessage(err.message); + } + }; + const renderRow = (user: MemberRow, field: keyof MemberRow) => { switch (field) { case 'username': @@ -134,6 +244,22 @@ export default function EditGroupMembersForm({ {user.username} ); + case 'role': + if (user.availableRoles.length <= 1) { + return ( + + {roleStrings[user.role]} + + ); + } + return ( + changeRole(user, role)} + /> + ); case 'showDeleteAction': return user.showDeleteAction ? ( { let config; let fakeCallAPI; + const defaultMembers = [ + { + userid: 'acct:bob@localhost', + username: 'bob', + actions: [ + 'delete', + 'updates.roles.admin', + 'updates.roles.moderator', + 'updates.roles.member', + ], + roles: ['admin'], + }, + { + userid: 'acct:johnsmith@localhost', + username: 'johnsmith', + actions: [], + roles: ['owner'], + }, + { + userid: 'acct:jane@localhost', + username: 'jane', + actions: [ + 'delete', + 'updates.roles.admin', + 'updates.roles.moderator', + 'updates.roles.member', + ], + roles: ['admin'], + }, + ]; + beforeEach(() => { const headers = { 'Misc-Header': 'Some-Value' }; config = { @@ -19,6 +51,11 @@ describe('EditGroupMembersForm', () => { method: 'GET', headers, }, + editGroupMember: { + url: '/api/groups/1234/members/:userid', + method: 'PATCH', + headers, + }, removeGroupMember: { url: '/api/groups/1234/members/:userid', method: 'DELETE', @@ -38,24 +75,7 @@ describe('EditGroupMembersForm', () => { fakeCallAPI = sinon.stub(); fakeCallAPI.rejects(new Error('Unknown API call')); - fakeCallAPI.withArgs('/api/groups/1234/members').resolves([ - { - userid: 'acct:bob@localhost', - username: 'bob', - actions: ['delete'], - }, - { - userid: 'acct:johnsmith@localhost', - username: 'johnsmith', - actions: [], - }, - { - userid: 'acct:jane@localhost', - username: 'jane', - actions: ['delete'], - }, - ]); - + fakeCallAPI.withArgs('/api/groups/1234/members').resolves(defaultMembers); fakeCallAPI .withArgs( `/api/groups/1234/members/${encodeURIComponent('acct:bob@localhost')}`, @@ -65,6 +85,21 @@ describe('EditGroupMembersForm', () => { ) .resolves({}); + fakeCallAPI + .withArgs( + `/api/groups/1234/members/${encodeURIComponent('acct:bob@localhost')}`, + sinon.match({ + method: 'PATCH', + }), + ) + .callsFake(async (url, { json }) => { + const updatedMember = { + ...defaultMembers.find(m => m.username === 'bob'), + }; + updatedMember.roles = [...json.roles]; + return updatedMember; + }); + $imports.$mock({ '../utils/api': { callAPI: fakeCallAPI, @@ -97,6 +132,20 @@ describe('EditGroupMembersForm', () => { return wrapper.find(`IconButton[data-testid="remove-${username}"]`); }; + const getRoleSelect = (wrapper, username) => { + return wrapper.find(`Select[data-testid="role-${username}"]`); + }; + + const getOptionValues = selectWrapper => { + return selectWrapper + .find(Select.Option) + .map(option => option.prop('value')); + }; + + const getRoleText = (wrapper, username) => { + return wrapper.find(`span[data-testid="role-${username}"]`); + }; + it('fetches and displays members', async () => { const wrapper = createForm(); assert.calledWith( @@ -114,6 +163,28 @@ describe('EditGroupMembersForm', () => { assert.deepEqual(users, ['bob', 'johnsmith', 'jane']); }); + it('displays error if member fetch fails', async () => { + fakeCallAPI + .withArgs('/api/groups/1234/members') + .rejects(new Error('Permission denied')); + + const wrapper = createForm(); + await waitForError(wrapper); + + const error = wrapper.find('ErrorNotice'); + assert.equal( + error.prop('message'), + 'Failed to fetch group members: Permission denied', + ); + }); + + it('handles member fetch being canceled', () => { + const wrapper = createForm(); + assert.calledWith(fakeCallAPI, '/api/groups/1234/members'); + // Unmount while fetching. This should abort the request. + wrapper.unmount(); + }); + it('displays remove icon if member can be removed', async () => { const wrapper = createForm(); await waitForTable(wrapper); @@ -185,25 +256,89 @@ describe('EditGroupMembersForm', () => { assert.equal(error.prop('message'), 'User not found'); }); - it('displays error if member fetch fails', async () => { - fakeCallAPI - .withArgs('/api/groups/1234/members') - .rejects(new Error('Permission denied')); + it('displays current and available user roles', async () => { + const wrapper = createForm(); + await waitForTable(wrapper); + + // If the user's role can be changed, the current role and available options + // should be displayed in a Select. + const bobRole = getRoleSelect(wrapper, 'bob'); + assert.isTrue(bobRole.exists()); + assert.equal(bobRole.prop('value'), 'admin'); + assert.deepEqual(getOptionValues(bobRole), [ + 'admin', + 'moderator', + 'member', + ]); + + // If the user's role cannot be changed, the current role should be + // displayed as text. + const johnRoleSelect = getRoleSelect(wrapper, 'johnsmith'); + assert.isFalse(johnRoleSelect.exists()); + const johnRoleText = getRoleText(wrapper, 'johnsmith'); + assert.equal(johnRoleText.text(), 'Owner'); + + // The role for the current user should be displayed as text, even if the + // API would allow changing our own role. + const janeRoleSelect = getRoleSelect(wrapper, 'jane'); + assert.isFalse(janeRoleSelect.exists()); + const janeRoleText = getRoleText(wrapper, 'jane'); + assert.equal(janeRoleText.text(), 'Admin'); + }); + it('updates user role when select value is changed', async () => { const wrapper = createForm(); - await waitForError(wrapper); + await waitForTable(wrapper); + let bobRole = getRoleSelect(wrapper, 'bob'); + bobRole.prop('onChange')('moderator'); - const error = wrapper.find('ErrorNotice'); - assert.equal( - error.prop('message'), - 'Failed to fetch group members: Permission denied', + // The displayed role should update immediately when the new role is + // selected. + wrapper.update(); + bobRole = getRoleSelect(wrapper, 'bob'); + assert.equal(bobRole.prop('value'), 'moderator'); + + assert.calledWith( + fakeCallAPI, + `/api/groups/1234/members/${encodeURIComponent('acct:bob@localhost')}`, + sinon.match({ + method: 'PATCH', + json: { + roles: ['moderator'], + }, + }), ); }); - it('handles member fetch being canceled', () => { + it('displays error if changing role fails', async () => { + fakeCallAPI + .withArgs( + `/api/groups/1234/members/${encodeURIComponent('acct:bob@localhost')}`, + sinon.match({ + method: 'PATCH', + }), + ) + .rejects(new Error('Invalid role')); const wrapper = createForm(); - assert.calledWith(fakeCallAPI, '/api/groups/1234/members'); - // Unmount while fetching. This should abort the request. - wrapper.unmount(); + await waitForTable(wrapper); + + // Attempt to change the role + let bobRole = getRoleSelect(wrapper, 'bob'); + const originalRole = bobRole.prop('value'); + bobRole.prop('onChange')('moderator'); + wrapper.update(); + + // The role should briefly change to the new selection. + assert.equal(getRoleSelect(wrapper, 'bob').prop('value'), 'moderator'); + + // Wait for the role change to fail. An error should be displayed. + await waitForError(wrapper); + const error = wrapper.find('ErrorNotice'); + assert.equal(error.prop('message'), 'Invalid role'); + + // The displayed role should revert to the original value. + wrapper.update(); + bobRole = getRoleSelect(wrapper, 'bob'); + assert.equal(bobRole.prop('value'), originalRole); }); }); diff --git a/h/static/scripts/group-forms/utils/api.ts b/h/static/scripts/group-forms/utils/api.ts index 3332e0ebbe4..6f05f9ba331 100644 --- a/h/static/scripts/group-forms/utils/api.ts +++ b/h/static/scripts/group-forms/utils/api.ts @@ -4,7 +4,7 @@ export type GroupType = 'private' | 'restricted' | 'open'; /** Member role within a group. */ -export type Role = 'owner' | 'admin' | 'member'; +export type Role = 'owner' | 'admin' | 'moderator' | 'member'; /** * Request to create or update a group.