Skip to content

Commit

Permalink
Persist group details changes when switching tabs
Browse files Browse the repository at this point in the history
When switching from the "Settings" tab to "Members" and back, persist changes to
group settings.

This is done by extracting the saved group details from the config into a
state value in the `AppRoot` component which is then passed down to the current
route as a prop. The `CreateEditGroupForm` component then updates this value via
a callback after changes are saved.
  • Loading branch information
robertknight committed Nov 20, 2024
1 parent e0a8a3c commit 0a78569
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 33 deletions.
12 changes: 8 additions & 4 deletions h/static/scripts/group-forms/components/AppRoot.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Route, Switch } from 'wouter-preact';
import { useMemo } from 'preact/hooks';
import { useMemo, useState } from 'preact/hooks';

import CreateEditGroupForm from './CreateEditGroupForm';
import EditGroupMembersForm from './EditGroupMembersForm';
Expand All @@ -25,20 +25,24 @@ export default function AppRoot({ config }: AppRootProps) {
[config],
);

// Saved group details. Extracted out of `config` so we can update them after
// saving changes to the backend.
const [group, setGroup] = useState(config.context.group);

return (
<>
{stylesheetLinks}
<Config.Provider value={config}>
<Router>
<Switch>
<Route path={routes.groups.new}>
<CreateEditGroupForm />
<CreateEditGroupForm group={group} onUpdateGroup={setGroup} />
</Route>
<Route path={routes.groups.edit}>
<CreateEditGroupForm />
<CreateEditGroupForm group={group} onUpdateGroup={setGroup} />
</Route>
<Route path={routes.groups.editMembers}>
<EditGroupMembersForm />
<EditGroupMembersForm group={group!} />
</Route>
<Route>
<h1 data-testid="unknown-route">Page not found</h1>
Expand Down
17 changes: 15 additions & 2 deletions h/static/scripts/group-forms/components/CreateEditGroupForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useContext, useEffect, useId, useState } from 'preact/hooks';

import { Button, RadioGroup } from '@hypothesis/frontend-shared';
import { Config } from '../config';
import type { Group } from '../config';
import { callAPI } from '../utils/api';
import type {
CreateUpdateGroupAPIRequest,
Expand Down Expand Up @@ -82,9 +83,19 @@ function GroupTypeChangeWarning({
);
}

export default function CreateEditGroupForm() {
export type CreateEditGroupFormProps = {
/** The saved group details, or `null` if the group has not been saved yet. */
group: Group | null;

/** Update the saved group details in the frontend's local state. */
onUpdateGroup?: (g: Group) => void;
};

export default function CreateEditGroupForm({
group,
onUpdateGroup,
}: CreateEditGroupFormProps) {
const config = useContext(Config)!;
const group = config.context.group;

const [name, setName] = useState(group?.name ?? '');
const [description, setDescription] = useState(group?.description ?? '');
Expand Down Expand Up @@ -178,6 +189,8 @@ export default function CreateEditGroupForm() {

// Mark form as saved, unless user edited it while saving.
setSaveState(state => (state === 'saving' ? 'saved' : state));

onUpdateGroup?.({ ...group!, name, description, type: groupType });
} catch (apiError) {
setSaveState('unsaved');
setErrorMessage(apiError.message);
Expand Down
14 changes: 9 additions & 5 deletions h/static/scripts/group-forms/components/EditGroupMembersForm.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { DataTable, Scroll } from '@hypothesis/frontend-shared';
import { useContext, useEffect, useState } from 'preact/hooks';
import { useEffect, useState } from 'preact/hooks';

import { Config } from '../config';
import { routes } from '../routes';
import type { Group } from '../config';
import ErrorNotice from './ErrorNotice';
import FormContainer from './forms/FormContainer';
import type { GroupMembersResponse } from '../utils/api';
Expand All @@ -25,10 +25,14 @@ async function fetchMembers(
}));
}

export default function EditGroupMembersForm() {
const config = useContext(Config)!;
const group = config.context.group!;
export type EditGroupMembersFormProps = {
/** The saved group details. */
group: Group;
};

export default function EditGroupMembersForm({
group,
}: EditGroupMembersFormProps) {
// Fetch group members when the form loads.
const [errorMessage, setErrorMessage] = useState<string | null>(null);
const [members, setMembers] = useState<MemberRow[] | null>(null);
Expand Down
51 changes: 40 additions & 11 deletions h/static/scripts/group-forms/components/test/AppRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { Config } from '../../config';

describe('AppRoot', () => {
let configContext;
const config = { styles: [] };

const config = { context: { group: { pubid: '1234' } }, styles: [] };

beforeEach(() => {
const mockComponent = name => {
Expand Down Expand Up @@ -38,14 +39,44 @@ describe('AppRoot', () => {
assert.equal(links.at(0).prop('href'), '/static/styles/foo.css');
});

it('passes config to route', () => {
history.pushState({}, null, '/groups/new');
/** Navigate to `path`, run `callback` and then reset the location. */
function navigate(path, callback) {
history.pushState({}, null, path);
try {
mount(<AppRoot config={config} />);
assert.strictEqual(configContext, config);
callback();
} finally {
history.back();
}
}

it('passes config to route', () => {
navigate('/groups/new', () => {
mount(<AppRoot config={config} />);
assert.strictEqual(configContext, config);
});
});

it('passes saved group to group settings route', () => {
navigate('/groups/1234/edit', () => {
const wrapper = mount(<AppRoot config={config} />);
assert.equal(
wrapper.find('CreateEditGroupForm').prop('group'),
config.context.group,
);
});
});

it('passes updated group to group settings route', () => {
navigate('/groups/1234/edit', () => {
const wrapper = mount(<AppRoot config={config} />);
const updatedGroup = { name: 'foobar' };
wrapper.find('CreateEditGroupForm').prop('onUpdateGroup')(updatedGroup);
wrapper.update();
assert.equal(
wrapper.find('CreateEditGroupForm').prop('group'),
updatedGroup,
);
});
});

[
Expand All @@ -67,13 +98,11 @@ describe('AppRoot', () => {
},
].forEach(({ path, selector }) => {
it(`renders expected component for URL (${path})`, () => {
history.pushState({}, '', path);
try {
navigate(path, () => {
const wrapper = mount(<AppRoot config={config} />);
assert.isTrue(wrapper.exists(selector));
} finally {
history.back();
}
const component = wrapper.find(selector);
assert.isTrue(component.exists());
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ describe('CreateEditGroupForm', () => {

let wrappers;

const createWrapper = () => {
const createWrapper = (props = {}) => {
const wrapper = mount(
<Config.Provider value={config}>
<CreateEditGroupForm />
<CreateEditGroupForm group={config.context.group} {...props} />
</Config.Provider>,
);
wrappers.push(wrapper);
Expand Down Expand Up @@ -249,16 +249,18 @@ describe('CreateEditGroupForm', () => {
});

it('displays an edit-group form', async () => {
const { wrapper, elements } = createWrapper();
const group = {
name: 'Original name',
description: 'Original description',
type: 'restricted', // Non-default value
};
const { wrapper, elements } = createWrapper({ group });
const { header, nameField, descriptionField, submitButton } = elements;

assert.equal(header.text(), 'Edit group');
assert.equal(nameField.prop('value'), config.context.group.name);
assert.equal(
descriptionField.prop('value'),
config.context.group.description,
);
assert.equal(getSelectedGroupType(wrapper), config.context.group.type);
assert.equal(nameField.prop('value'), group.name);
assert.equal(descriptionField.prop('value'), group.description);
assert.equal(getSelectedGroupType(wrapper), group.type);
assert.equal(submitButton.text(), 'Save changes');
assert.isTrue(wrapper.exists('[data-testid="back-link"]'));
assert.isFalse(wrapper.exists('[data-testid="error-message"]'));
Expand Down Expand Up @@ -315,7 +317,8 @@ describe('CreateEditGroupForm', () => {
});

it('updates the group', async () => {
const { wrapper, elements } = createWrapper();
const onUpdateGroup = sinon.stub();
const { wrapper, elements } = createWrapper({ onUpdateGroup });
const { nameField, descriptionField } = elements;

const name = 'Edited Group Name';
Expand All @@ -342,6 +345,18 @@ describe('CreateEditGroupForm', () => {
},
}),
);

// Once changes are saved, the edited group details should be persisted.
await assertInLoadingState(wrapper, false);
assert.calledWith(
onUpdateGroup,
sinon.match({
...config.context.group,
name,
description,
type: newGroupType,
}),
);
});

it('shows a loading state when the update-group API request is in-flight', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('EditGroupMembersForm', () => {
const createForm = (props = {}) => {
return mount(
<Config.Provider value={config}>
<EditGroupMembersForm {...props} />
<EditGroupMembersForm group={config.context.group} {...props} />
</Config.Provider>,
);
};
Expand Down

0 comments on commit 0a78569

Please sign in to comment.