From 6baa769520c86aac6cee0830d1d2f95c8dd8e06b Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Fri, 21 Aug 2026 19:05:29 -0500 Subject: [PATCH] =?UTF-8?q?feat(frontend):=20edit=20the=20customer=20email?= =?UTF-8?q?s=20from=20Admin=20=E2=86=92=20Settings=20(#92)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A card per email under the existing settings screen: subject, body, the placeholders it understands, and which of them it cannot lose. Each card starts from the copy that is actually in use — the stored version if there is one, the built-in default otherwise — rather than an empty box, so editing means changing words rather than writing the email from scratch. A badge distinguishes customised from default, which is why the API reports an unedited template as null rather than as its default text: the two are different states and the screen has to be able to tell them apart. Restore default is offered only when there is something to restore, so it is never a button that looks like it did something and did not. It removes the stored rows rather than writing the defaults into them, which is what keeps the badge honest afterwards. The server's refusal is shown verbatim. When a body drops a placeholder it needs, the message names which one, and that message is the entire value of the validation — replacing it with a generic failure would leave an admin guessing at which of five templates and which of three placeholders they broke. A textarea rather than the markdown editor already used for item descriptions. That editor is a heavy dependency to load into the settings screen for five short bodies, and its preview would render markdown as the browser shows it rather than as the email renderer will — a preview that quietly disagrees with the output is worse than none. Worth revisiting if the copy gets longer. Two things the end-to-end spec found rather than assumed. The refusal assertion first matched three elements, because the placeholder appears as the required marker, as an available tag, and inside the error — it now asserts the whole sentence. And the four tests raced each other: the suite runs fully parallel and they all edit one shared stored template, so one asserted a template was unset while another had just saved it. That describe block now runs serially, which is the honest fix for tests that mutate shared server state rather than making the assertions vaguer. Verified: 99 end-to-end tests passing on a freshly created container, up from 95, with the whole suite run rather than the new spec alone — precisely because these tests write state other suites read. Build clean, lint unchanged at 27 warnings. Refs #92 Co-Authored-By: Claude Opus 5 --- frontend/src/admin/EmailTemplateCard.tsx | 115 +++++++++++++++++++++ frontend/src/admin/Settings.tsx | 31 ++++++ frontend/src/admin/emailTemplatesApi.ts | 56 ++++++++++ frontend/tests/e2e/email-templates.spec.ts | 96 +++++++++++++++++ 4 files changed, 298 insertions(+) create mode 100644 frontend/src/admin/EmailTemplateCard.tsx create mode 100644 frontend/src/admin/emailTemplatesApi.ts create mode 100644 frontend/tests/e2e/email-templates.spec.ts diff --git a/frontend/src/admin/EmailTemplateCard.tsx b/frontend/src/admin/EmailTemplateCard.tsx new file mode 100644 index 0000000..e1a1992 --- /dev/null +++ b/frontend/src/admin/EmailTemplateCard.tsx @@ -0,0 +1,115 @@ +import { useState } from 'react'; +import Card from 'antd/es/card'; +import Input from 'antd/es/input'; +import Button from 'antd/es/button'; +import Space from 'antd/es/space'; +import Tag from 'antd/es/tag'; +import Typography from 'antd/es/typography'; +import message from 'antd/es/message'; +import { EmailTemplate, saveEmailTemplate, resetEmailTemplate } from './emailTemplatesApi'; + +const { Text, Paragraph } = Typography; + +type Props = Readonly<{ + template: EmailTemplate; + onChanged: (updated: EmailTemplate) => void; +}>; + +export default function EmailTemplateCard({ template, onChanged }: Props) { + // Falls back to the default so the editor starts from the real copy rather + // than an empty box. `subject`/`body` being null means "never customised", + // which is why the badge below can say so. + const [subject, setSubject] = useState(template.subject ?? template.defaultSubject); + const [body, setBody] = useState(template.body ?? template.defaultBody); + const [saving, setSaving] = useState(false); + + const customised = template.subject !== null || template.body !== null; + + async function handleSave() { + setSaving(true); + try { + const updated = await saveEmailTemplate(template.key, subject, body); + onChanged(updated); + message.success(`${template.label} saved`); + } catch (err) { + // The server's refusal names the placeholder that is missing, which is + // the only useful thing to say here — so it is shown rather than replaced + // with something generic. + message.error((err as Error).message); + } finally { + setSaving(false); + } + } + + async function handleReset() { + setSaving(true); + try { + const restored = await resetEmailTemplate(template.key); + setSubject(template.defaultSubject); + setBody(template.defaultBody); + onChanged(restored); + message.success(`${template.label} restored to the default`); + } catch (err) { + message.error((err as Error).message); + } finally { + setSaving(false); + } + } + + return ( + + {template.label} + {customised ? Customised : Default} + + } + > + + Markdown. Placeholders are replaced when the email is sent + {template.required.length > 0 && ( + <> + {' '}— {template.required.map((n) => `{{${n}}}`).join(' and ')}{' '} + {template.required.length === 1 ? 'is' : 'are'} required and cannot be removed + + )} + . + + + + {template.available.map((name) => ( + {`{{${name}}}`} + ))} + + + setSubject(e.target.value)} + placeholder="Subject" + style={{ marginBottom: 8 }} + /> + setBody(e.target.value)} + autoSize={{ minRows: 6, maxRows: 16 }} + style={{ fontFamily: 'monospace', marginBottom: 12 }} + /> + + + + {/* Only offered when there is something to restore, so the button is + not a no-op that looks like it did something. */} + {customised && ( + + )} + + + ); +} diff --git a/frontend/src/admin/Settings.tsx b/frontend/src/admin/Settings.tsx index 8b4c8b7..eec5490 100644 --- a/frontend/src/admin/Settings.tsx +++ b/frontend/src/admin/Settings.tsx @@ -6,12 +6,15 @@ import Typography from 'antd/es/typography'; import message from 'antd/es/message'; import Card from 'antd/es/card'; import { fetchAdminSettings, updateAdminSettings } from './adminSettingsApi'; +import { EmailTemplate, fetchEmailTemplates } from './emailTemplatesApi'; +import EmailTemplateCard from './EmailTemplateCard'; const { Title, Text } = Typography; export default function Settings() { const [form] = Form.useForm(); const [loading, setLoading] = useState(true); + const [templates, setTemplates] = useState([]); useEffect(() => { fetchAdminSettings() @@ -23,6 +26,22 @@ export default function Settings() { .finally(() => setLoading(false)); }, [form]); + useEffect(() => { + fetchEmailTemplates() + .then(setTemplates) + // Reported rather than swallowed: an empty list would otherwise read as + // "there are no templates" instead of "they could not be loaded". + .catch(() => message.error('Could not load the email templates')); + }, []); + + // Replaces the one that changed so the Customised badge and the Restore + // button reflect what the server now holds, without refetching the rest. + function handleTemplateChanged(updated: EmailTemplate) { + setTemplates(current => + current.map(t => (t.key === updated.key ? { ...t, subject: updated.subject, body: updated.body } : t)) + ); + } + async function handleSave() { const values = await form.validateFields(); await updateAdminSettings(values); @@ -30,6 +49,7 @@ export default function Settings() { } return ( +
Cart Settings @@ -42,5 +62,16 @@ export default function Settings() { + + Customer emails + + The wording customers receive. Leave one alone and it sends the built-in copy. + +
+ {templates.map(template => ( + + ))} +
+
); } diff --git a/frontend/src/admin/emailTemplatesApi.ts b/frontend/src/admin/emailTemplatesApi.ts new file mode 100644 index 0000000..2a6d86b --- /dev/null +++ b/frontend/src/admin/emailTemplatesApi.ts @@ -0,0 +1,56 @@ +export interface EmailTemplate { + key: string; + /** Human name for the card, so it is identifiable without reading the body. */ + label: string; + /** Placeholders the body must keep. Saving without one is refused. */ + required: string[]; + /** Every placeholder this template understands. */ + available: string[]; + defaultSubject: string; + defaultBody: string; + /** Null when never customised, which is distinct from "same as the default". */ + subject: string | null; + body: string | null; +} + +async function json(res: Response): Promise { + return (await res.json()) as T; +} + +// The server's refusals say which placeholder is missing, and that message is +// the whole point of the validation — so it is surfaced rather than replaced +// with something generic. +async function expectOk(res: Response, action: string): Promise { + if (res.ok) return res; + const detail = await res.json().catch(() => null); + throw new Error(detail?.error ? String(detail.error) : action); +} + +export async function fetchEmailTemplates(): Promise { + const res = await expectOk(await fetch('/api/admin/email-templates'), 'could not load templates'); + return json(res); +} + +export async function saveEmailTemplate( + key: string, + subject: string, + body: string +): Promise { + const res = await expectOk( + await fetch(`/api/admin/email-templates/${key}`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ subject, body }) + }), + 'could not save the template' + ); + return json(res); +} + +export async function resetEmailTemplate(key: string): Promise { + const res = await expectOk( + await fetch(`/api/admin/email-templates/${key}`, { method: 'DELETE' }), + 'could not restore the default' + ); + return json(res); +} diff --git a/frontend/tests/e2e/email-templates.spec.ts b/frontend/tests/e2e/email-templates.spec.ts new file mode 100644 index 0000000..daaaed2 --- /dev/null +++ b/frontend/tests/e2e/email-templates.spec.ts @@ -0,0 +1,96 @@ +import { test, expect } from './fixtures'; + +const suffix = () => `t${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`; + +// Each test leaves the templates as it found them, because they are stored in +// admin_settings and would otherwise change the copy a later test reads. +async function restore(page: import('@playwright/test').Page, key: string) { + await page.request.delete(`/api/admin/email-templates/${key}`); +} + +async function openSettings(page: import('@playwright/test').Page) { + await page.goto('/admin'); + await page.getByRole('tab', { name: 'Settings' }).click(); + await expect(page.getByRole('heading', { name: 'Customer emails' })).toBeVisible(); +} + +// Serial: these edit one shared stored template, and the suite runs fully +// parallel by default — so run concurrently they would race, one asserting a +// template is unset while another has just saved it. +test.describe.configure({ mode: 'serial' }); + +test.describe('Editing the customer emails', () => { + test.afterEach(async ({ page }) => { + await restore(page, 'passwordReset'); + }); + + test('shows every template, marked default until it is edited', async ({ page }) => { + await openSettings(page); + + for (const label of [ + 'Email verification', + 'Password reset', + 'Favorited item sold', + 'Favorited item withdrawn', + 'Cart reminder' + ]) { + await expect(page.getByText(label, { exact: true })).toBeVisible(); + } + }); + + test('saves a replacement subject and body', async ({ page }) => { + const subject = `Reset ${suffix()}`; + await openSettings(page); + + await page.getByLabel('Password reset subject').fill(subject); + await page + .getByLabel('Password reset body') + .fill('Fresh wording. [Choose a new password]({{resetUrl}}).'); + + const card = page.locator('.ant-card').filter({ hasText: 'Password reset' }); + await card.getByRole('button', { name: 'Save', exact: true }).click(); + + await expect(page.getByText('Password reset saved')).toBeVisible(); + + // Persisted, not merely accepted by the form. + const stored = await (await page.request.get('/api/admin/email-templates')).json(); + const reset = stored.find((t: { key: string }) => t.key === 'passwordReset'); + expect(reset.subject).toBe(subject); + }); + + // The assertion that matters. A body without its link still sends and still + // looks fine in the log, so the save has to be refused rather than warned + // about — and the admin has to be told which placeholder is missing. + test('refuses a body that drops the required placeholder, and says which', async ({ page }) => { + await openSettings(page); + + await page.getByLabel('Password reset body').fill('Just click the thing in your email.'); + + const card = page.locator('.ant-card').filter({ hasText: 'Password reset' }); + await card.getByRole('button', { name: 'Save', exact: true }).click(); + + await expect(page.getByText('the body must keep {{resetUrl}}')).toBeVisible(); + + // And nothing was stored. + const stored = await (await page.request.get('/api/admin/email-templates')).json(); + const reset = stored.find((t: { key: string }) => t.key === 'passwordReset'); + expect(reset.body).toBeNull(); + }); + + test('restores the built-in copy', async ({ page }) => { + await page.request.put('/api/admin/email-templates/passwordReset', { + data: { subject: 'Temporary', body: 'Temporary [link]({{resetUrl}}).' } + }); + + await openSettings(page); + const card = page.locator('.ant-card').filter({ hasText: 'Password reset' }); + await card.getByRole('button', { name: 'Restore default' }).click(); + + await expect(page.getByText('Password reset restored to the default')).toBeVisible(); + + const stored = await (await page.request.get('/api/admin/email-templates')).json(); + const reset = stored.find((t: { key: string }) => t.key === 'passwordReset'); + expect(reset.subject).toBeNull(); + expect(reset.body).toBeNull(); + }); +});