Skip to content

Commit

Permalink
Support removing members from a group in the UI
Browse files Browse the repository at this point in the history
Add a delete icon button to each row of the UI if the current user is allowed to
remove that member from the group, and the row is not the current user. The
delete action is not available for the current user because handlng this case
involves extra logic (different confirmation prompt, user loses access to
group settings UI after leaving).

When the button is clicked, show a confirmation prompt and then invoke the
`DELETE /api/groups/:pubid/members/:userid` API to remove the member from the
group.
  • Loading branch information
robertknight committed Nov 28, 2024
1 parent d52efda commit d86e789
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 22 deletions.
120 changes: 101 additions & 19 deletions h/static/scripts/group-forms/components/EditGroupMembersForm.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { DataTable, Scroll } from '@hypothesis/frontend-shared';
import {
DataTable,
Scroll,
TrashIcon,
IconButton,
} from '@hypothesis/frontend-shared';
import { useContext, useEffect, useState } from 'preact/hooks';

import { Config } from '../config';
Expand All @@ -8,13 +13,23 @@ import FormContainer from './forms/FormContainer';
import type { GroupMembersResponse } from '../utils/api';
import { callAPI } from '../utils/api';
import GroupFormHeader from './GroupFormHeader';
import WarningDialog from './WarningDialog';

type TableColumn<Row> = {
field: keyof Row;
label: string;
classes?: string;
};

type MemberRow = {
username: string;
userid: string;
showDeleteAction: boolean;
};

async function fetchMembers(
api: APIConfig,
currentUserid: string,
signal: AbortSignal,
): Promise<MemberRow[]> {
const { url, method, headers } = api;
Expand All @@ -24,10 +39,22 @@ async function fetchMembers(
signal,
});
return members.map(member => ({
userid: member.userid,
username: member.username,
showDeleteAction:
member.actions.includes('delete') && member.userid !== currentUserid,
}));
}

async function removeMember(api: APIConfig, userid: string) {
const { url: urlTemplate, method, headers } = api;
const url = urlTemplate.replace(':userid', encodeURIComponent(userid));
await callAPI(url, {
method,
headers,
});
}

export type EditGroupMembersFormProps = {
/** The saved group details. */
group: Group;
Expand All @@ -37,6 +64,7 @@ export default function EditGroupMembersForm({
group,
}: EditGroupMembersFormProps) {
const config = useContext(Config)!;
const userid = config.context.user.userid;

// Fetch group members when the form loads.
const [errorMessage, setErrorMessage] = useState<string | null>(null);
Expand All @@ -48,23 +76,52 @@ export default function EditGroupMembersForm({
}
const abort = new AbortController();
setErrorMessage(null);
fetchMembers(config.api.readGroupMembers, abort.signal)
fetchMembers(config.api.readGroupMembers, userid, abort.signal)
.then(setMembers)
.catch(err => {
setErrorMessage(`Failed to fetch group members: ${err.message}`);
});
return () => {
abort.abort();
};
}, [config.api.readGroupMembers]);
}, [config.api.readGroupMembers, userid]);

const columns = [
const columns: TableColumn<MemberRow>[] = [
{
field: 'username' as keyof MemberRow,
field: 'username',
label: 'Username',
},
{
field: 'showDeleteAction',
label: '',
classes: 'w-[55px]',
},
];

const [pendingRemoval, setPendingRemoval] = useState<string | null>(null);

const removeUserFromGroup = async (username: string) => {
// istanbul ignore next
if (!members || !config.api.removeGroupMember) {
return;
}
const member = members.find(m => m.username === username);
// istanbul ignore next
if (!member) {
return;
}
setPendingRemoval(null);

try {
await removeMember(config.api.removeGroupMember, member.userid);
setMembers(members =>
members ? members.filter(m => m.userid !== member.userid) : null,
);
} catch (err) {
setErrorMessage(err.message);
}
};

const renderRow = (user: MemberRow, field: keyof MemberRow) => {
switch (field) {
case 'username':
Expand All @@ -77,26 +134,51 @@ export default function EditGroupMembersForm({
{user.username}
</div>
);
case 'showDeleteAction':
return user.showDeleteAction ? (
<IconButton
icon={TrashIcon}
title="Remove member"
data-testid={`remove-${user.username}`}
onClick={() => setPendingRemoval(user.username)}
/>
) : null;
// istanbul ignore next
default:
return null;
}
};

return (
<FormContainer title="Edit group members">
<GroupFormHeader group={group} />
<ErrorNotice message={errorMessage} />
<div className="w-full">
<Scroll>
<DataTable
title="Group members"
rows={members ?? []}
columns={columns}
renderItem={renderRow}
/>
</Scroll>
</div>
</FormContainer>
<>
<FormContainer title="Edit group members">
<GroupFormHeader group={group} />
<ErrorNotice message={errorMessage} />
<div className="w-full">
<Scroll>
<DataTable
title="Group members"
rows={members ?? []}
columns={columns}
renderItem={renderRow}
/>
</Scroll>
</div>
</FormContainer>
{pendingRemoval && (
<WarningDialog
title="Remove member?"
confirmAction="Remove member"
message={
<p>
Are you sure you want to remove <b>{pendingRemoval}</b> from the
group?
</p>
}
onConfirm={() => removeUserFromGroup(pendingRemoval)}
onCancel={() => setPendingRemoval(null)}
/>
)}
</>
);
}
3 changes: 2 additions & 1 deletion h/static/scripts/group-forms/components/WarningDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { ModalDialog, Button } from '@hypothesis/frontend-shared';
import type { ComponentChildren } from 'preact';

export type WarningDialogProps = {
/** Title of the dialog. */
title: string;

/** Message displayed in the dialog. */
message: string;
message: ComponentChildren;

/** Label for the confirmation button. */
confirmAction: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,60 @@ describe('EditGroupMembersForm', () => {
let fakeCallAPI;

beforeEach(() => {
const headers = { 'Misc-Header': 'Some-Value' };
config = {
api: {
readGroupMembers: {
url: '/api/groups/1234/members',
method: 'GET',
headers: { 'Misc-Header': 'Some-Value' },
headers,
},
removeGroupMember: {
url: '/api/groups/1234/members/:userid',
method: 'DELETE',
headers,
},
},
context: {
group: {
pubid: '1234',
name: 'Test group',
},
user: {
userid: 'acct:jane@localhost',
},
},
};

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/${encodeURIComponent('acct:bob@localhost')}`,
sinon.match({
method: 'DELETE',
}),
)
.resolves({});

$imports.$mock({
'../utils/api': {
callAPI: fakeCallAPI,
Expand Down Expand Up @@ -65,6 +93,10 @@ describe('EditGroupMembersForm', () => {
return wrapper.find('[data-testid="username"]').map(node => node.text());
};

const getRemoveUserButton = (wrapper, username) => {
return wrapper.find(`IconButton[data-testid="remove-${username}"]`);
};

it('fetches and displays members', async () => {
const wrapper = createForm();
assert.calledWith(
Expand All @@ -79,7 +111,78 @@ describe('EditGroupMembersForm', () => {
await waitForTable(wrapper);

const users = getRenderedUsers(wrapper);
assert.deepEqual(users, ['bob', 'johnsmith']);
assert.deepEqual(users, ['bob', 'johnsmith', 'jane']);
});

it('displays remove icon if member can be removed', async () => {
const wrapper = createForm();
await waitForTable(wrapper);

const expectRemovable = {
bob: true, // `delete` action present in `actions`
johnsmith: false, // `delete` action present in `actions`
jane: false, // `delete` action present, but this is the current user
};

for (const [username, removable] of Object.entries(expectRemovable)) {
const removeButton = getRemoveUserButton(wrapper, username);
assert.equal(removeButton.exists(), removable);
}
});

it('removes user with confirmation if remove button is clicked', async () => {
const wrapper = createForm();
await waitForTable(wrapper);
const removeBob = getRemoveUserButton(wrapper, 'bob');

removeBob.prop('onClick')();
wrapper.update();

// Confirmation prompt should be shown.
const warning = wrapper.find('WarningDialog');
assert.isTrue(warning.exists());

// Canceling the prompt should hide the dialog.
warning.prop('onCancel')();
wrapper.update();
assert.isFalse(wrapper.exists('WarningDialog'));

// Confirming the prompt should hide the dialog and trigger a call to
// remove the user.
removeBob.prop('onClick')();
wrapper.update();
warning.prop('onConfirm')();
wrapper.update();
assert.isFalse(wrapper.exists('WarningDialog'));

// Once the user is removed, their row should disappear.
await waitFor(() => {
wrapper.update();
return !getRemoveUserButton(wrapper, 'bob').exists();
});
});

it('shows error if removing user fails', async () => {
const userid = 'acct:bob@localhost';
fakeCallAPI
.withArgs(
`/api/groups/1234/members/${encodeURIComponent(userid)}`,
sinon.match({
method: 'DELETE',
}),
)
.rejects(new Error('User not found'));

const wrapper = createForm();
await waitForTable(wrapper);
const removeBob = getRemoveUserButton(wrapper, 'bob');
removeBob.prop('onClick')();
wrapper.update();
wrapper.find('WarningDialog').prop('onConfirm')();

await waitForError(wrapper);
const error = wrapper.find('ErrorNotice');
assert.equal(error.prop('message'), 'User not found');
});

it('displays error if member fetch fails', async () => {
Expand Down
5 changes: 5 additions & 0 deletions h/static/scripts/group-forms/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ export type ConfigObject = {
createGroup: APIConfig;
updateGroup?: APIConfig;
readGroupMembers?: APIConfig;
editGroupMember?: APIConfig;
removeGroupMember?: APIConfig;
};
context: {
group: Group | null;
user: {
userid: string;
};
};
features: {
group_members: boolean;
Expand Down

0 comments on commit d86e789

Please sign in to comment.