From 4ed9513ad23f91e175e6ae6165ef7773bfdfa77f Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Fri, 21 Aug 2026 18:57:52 -0500 Subject: [PATCH] feat(backend): make the five customer emails editable copy (#92) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every customer email was a template literal in the route that sent it, so changing a word meant a code change, a review and a deploy. All five now render from markdown that an admin can edit: verification, password reset, favorite sold, favorite withdrawn, and the cart reminder. Five, not the four the issue counted — the favorite alerts have separate copy for sold and withdrawn. markdown-it runs with html disabled, which is its default and the reason for choosing it over marked. Raw HTML in a stored body is escaped rather than passed through, so editing copy cannot put script into a customer's inbox. That is a stronger guarantee than sanitising output, because there is no output to sanitise. Values are substituted into the markdown before it renders, which means a value that should become a list has to arrive as markdown. The cart reminder previously built li elements by hand; those would now be escaped and shown to the customer as literal angle brackets, so it emits a markdown list instead. The greeting is one placeholder rather than a bare name, so a template author writes {{greeting}} instead of "Hi {{firstName}}," — which reads as "Hi ," for anyone who registered before first names were required. Saving is refused when a body has dropped a placeholder it needs, naming all of them rather than the first. This is the rule that separates a convenience from a way to break password resets from a settings screen: a reset email with no link still sends, still looks correct in the log, and is useless to everyone who receives it. The favorite alerts' consent sentence is appended by the server and is not editable. It explains why the customer is receiving the mail, which is a compliance artifact rather than copy, and editing wording should not be able to delete it. Unset templates fall back to the built-in defaults, so an install that never touches the settings screen behaves exactly as it did. The API reports an uncustomised template as null rather than as its default text, so "never edited" stays distinguishable from "edited to something identical", and DELETE restores the default by forgetting the row rather than writing the default into it. Two problems surfaced during verification, both worth recording. Five favorite-alert tests failed with no error and no mail. The cause was not this code: resetDb does not truncate admin_settings, so a subject of "Gone" stored by the new template tests survived into a later suite and changed the mail it was asserting on. Cleaning up inside the template tests would have fixed only that pairing, so resetDb now clears stored templates for every suite — template rows are test data like any other, and one outliving the suite that wrote it makes a failure appear somewhere unrelated. The withdrawal notification then failed on timing. Loading copy from the database made the sender async, and the removal path was fire-and-forget, so the response could beat the mail out of the door. Dispatch was previously synchronous even though the sends themselves were not awaited; that is now restored by awaiting it. Verified: 197 unit and 195 integration passing, lint unchanged at 4 warnings. The admin screen for editing these follows in the next commit. Refs #92 Co-Authored-By: Claude Opus 5 --- backend/package-lock.json | 119 ++++++++++++ backend/package.json | 2 + backend/src/app.ts | 2 + backend/src/emailTemplates.ts | 172 +++++++++++++++++ backend/src/favoriteAlerts.ts | 46 ++--- backend/src/routes/admin.ts | 2 +- backend/src/routes/adminEmailTemplates.ts | 112 +++++++++++ backend/src/routes/customers.ts | 26 +-- backend/src/server.ts | 28 ++- .../emailTemplates.integration.test.ts | 177 ++++++++++++++++++ backend/tests/integration/setup/testDb.ts | 8 + backend/tests/unit/emailTemplates.test.ts | 148 +++++++++++++++ 12 files changed, 797 insertions(+), 45 deletions(-) create mode 100644 backend/src/emailTemplates.ts create mode 100644 backend/src/routes/adminEmailTemplates.ts create mode 100644 backend/tests/integration/emailTemplates.integration.test.ts create mode 100644 backend/tests/unit/emailTemplates.test.ts diff --git a/backend/package-lock.json b/backend/package-lock.json index 4854063..4653eda 100755 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -8,10 +8,12 @@ "name": "redefined-designs-backend", "version": "1.0.0", "dependencies": { + "@types/markdown-it": "^14.2.0", "bcryptjs": "^2.4.3", "cookie-parser": "^1.4.6", "express": "^4.19.2", "express-rate-limit": "^8.6.2", + "markdown-it": "^15.0.0", "multer": "^1.4.5-lts.1", "node-cron": "^3.0.3", "node-pg-migrate": "^7.6.1", @@ -1944,6 +1946,28 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/linkify-it": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/@types/linkify-it/-/linkify-it-5.0.0.tgz", + "integrity": "sha512-sVDA58zAw4eWAffKOaQH5/5j3XeayukzDk+ewSsnv3p4yJEZHCCzMDiZM8e0OUrRvmpGZ85jf4yDHkHsgBNr9Q==", + "license": "MIT" + }, + "node_modules/@types/markdown-it": { + "version": "14.2.0", + "resolved": "https://registry.npmjs.org/@types/markdown-it/-/markdown-it-14.2.0.tgz", + "integrity": "sha512-NoQ2yGlLWj4wpxMs+TYmRKk3thDrQ97agr7sFqfLsAlvoS8SNQuTrlObhFqG9iugdTtgOE9jpJ6FNM4ZGsa5xQ==", + "license": "MIT", + "dependencies": { + "@types/linkify-it": "^5", + "@types/mdurl": "^2" + } + }, + "node_modules/@types/mdurl": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@types/mdurl/-/mdurl-2.0.0.tgz", + "integrity": "sha512-RGdgjQUZba5p6QEFAVx2OGb8rQDL/cPRG7GiedRzMcJ1tYnUANBncjbSB1NRGwbvjcPeikRABz2nshyPk1bhWg==", + "license": "MIT" + }, "node_modules/@types/methods": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@types/methods/-/methods-1.1.4.tgz", @@ -3401,6 +3425,18 @@ "node": ">= 0.8" } }, + "node_modules/entities": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/entities/-/entities-8.0.0.tgz", + "integrity": "sha512-zwfzJecQ/Uej6tusMqwAqU/6KL2XaB2VZ2Jg54Je6ahNBGNH6Ek6g3jjNCF0fG9EWQKGZNddNjU5F1ZQn/sBnA==", + "license": "BSD-2-Clause", + "engines": { + "node": ">=20.19.0" + }, + "funding": { + "url": "https://github.com/fb55/entities?sponsor=1" + } + }, "node_modules/error-ex": { "version": "1.3.4", "resolved": "https://registry.npmjs.org/error-ex/-/error-ex-1.3.4.tgz", @@ -5562,6 +5598,25 @@ "dev": true, "license": "MIT" }, + "node_modules/linkify-it": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/linkify-it/-/linkify-it-6.1.0.tgz", + "integrity": "sha512-wJ/TwpSDTLepCrQoYWYIExIKg5Zchex2Nn5yk2mFnB+6PtdkHtyLx742md9csRjjOnGkKIS/RrbY7l8D6gT9Vw==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/markdown-it" + } + ], + "license": "MIT", + "dependencies": { + "uc.micro": "^3.0.0" + } + }, "node_modules/locate-path": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-5.0.0.tgz", @@ -5645,6 +5700,49 @@ "tmpl": "1.0.5" } }, + "node_modules/markdown-it": { + "version": "15.0.0", + "resolved": "https://registry.npmjs.org/markdown-it/-/markdown-it-15.0.0.tgz", + "integrity": "sha512-Lf8ajvVNdRpzSNB4VegxNy7gjs8gU35l4b4+ET49LrQC5PKYwLZ72u60LeJ9gv3qiaesuYjJWCyVeQmv/QWKQw==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/markdown-it" + } + ], + "license": "MIT", + "dependencies": { + "argparse": "^3.0.0", + "entities": "^8.0.0", + "linkify-it": "^6.0.0", + "mdurl": "^2.1.0", + "punycode.js": "^2.3.1", + "uc.micro": "^3.0.0" + }, + "bin": { + "markdown-it": "bin/markdown-it.mjs" + } + }, + "node_modules/markdown-it/node_modules/argparse": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/argparse/-/argparse-3.0.0.tgz", + "integrity": "sha512-BOp5NMrHqKxmq/OLr+clzzrRxgOKSLkcjmkWuChp7Irqwn4s74WjOBPIgWfA/HMcBnVkZ5XEuf9uUqzlpfCQ6A==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/nodeca" + } + ], + "license": "Python-2.0" + }, "node_modules/math-intrinsics": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/math-intrinsics/-/math-intrinsics-1.1.0.tgz", @@ -5654,6 +5752,12 @@ "node": ">= 0.4" } }, + "node_modules/mdurl": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/mdurl/-/mdurl-2.1.0.tgz", + "integrity": "sha512-1+HBaOx0zi/dQWht8rNv9MYf9qqpqL/kxI0hXImU6Y547zM6Sni8BQibt7ifgMcYtQg41ao3Ivd6cnSM86inpg==", + "license": "MIT" + }, "node_modules/media-typer": { "version": "0.3.0", "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", @@ -6472,6 +6576,15 @@ "node": ">=6" } }, + "node_modules/punycode.js": { + "version": "2.3.1", + "resolved": "https://registry.npmjs.org/punycode.js/-/punycode.js-2.3.1.tgz", + "integrity": "sha512-uxFIHU0YlHYhDQtV4R9J6a52SLx28BCjT+4ieh7IGbgwVJWO+km431c4yRlREUAsAmt/uMjQUyQHNEPf0M39CA==", + "license": "MIT", + "engines": { + "node": ">=6" + } + }, "node_modules/pure-rand": { "version": "6.1.0", "resolved": "https://registry.npmjs.org/pure-rand/-/pure-rand-6.1.0.tgz", @@ -7428,6 +7541,12 @@ "typescript": ">=4.8.4 <6.1.0" } }, + "node_modules/uc.micro": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/uc.micro/-/uc.micro-3.0.0.tgz", + "integrity": "sha512-U3PppEkleoTnIfi8BozMx3yju3qc/L6SwqWo2Sw+54PX+PX0q9I+r1Um5HCmqD7n9VDX5/v3vQH/AjA6deDdtw==", + "license": "MIT" + }, "node_modules/uglify-js": { "version": "3.19.3", "resolved": "https://registry.npmjs.org/uglify-js/-/uglify-js-3.19.3.tgz", diff --git a/backend/package.json b/backend/package.json index fe3da80..64bc930 100755 --- a/backend/package.json +++ b/backend/package.json @@ -22,10 +22,12 @@ "migrate:create": "node-pg-migrate create --migration-file-language js" }, "dependencies": { + "@types/markdown-it": "^14.2.0", "bcryptjs": "^2.4.3", "cookie-parser": "^1.4.6", "express": "^4.19.2", "express-rate-limit": "^8.6.2", + "markdown-it": "^15.0.0", "multer": "^1.4.5-lts.1", "node-cron": "^3.0.3", "node-pg-migrate": "^7.6.1", diff --git a/backend/src/app.ts b/backend/src/app.ts index f674ee1..e55c9ce 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -6,6 +6,7 @@ import { router as cartCheckoutRouter, webhookRouter as cartCheckoutWebhookRoute import adminRouter from './routes/admin'; import adminCustomersRouter from './routes/adminCustomers'; import adminSettingsRouter from './routes/adminSettings'; +import adminEmailTemplatesRouter from './routes/adminEmailTemplates'; import adminCategoriesRouter from './routes/adminCategories'; import adminTagsRouter from './routes/adminTags'; import filtersRouter from './routes/filters'; @@ -54,6 +55,7 @@ app.use('/api/checkout/cart', cartCheckoutRouter); // instead of being quietly public. See middleware/adminGate.ts and #63. app.use('/api/admin/customers', requireAdminGate, adminCustomersRouter); app.use('/api/admin/settings', requireAdminGate, adminSettingsRouter); +app.use('/api/admin/email-templates', requireAdminGate, adminEmailTemplatesRouter); app.use('/api/admin/categories', requireAdminGate, adminCategoriesRouter); app.use('/api/admin/tags', requireAdminGate, adminTagsRouter); app.use('/api/admin', requireAdminGate, adminRouter); diff --git a/backend/src/emailTemplates.ts b/backend/src/emailTemplates.ts new file mode 100644 index 0000000..136a44f --- /dev/null +++ b/backend/src/emailTemplates.ts @@ -0,0 +1,172 @@ +import MarkdownIt from 'markdown-it'; + +/** + * The five customer emails, their default copy, and the rules for editing it. + * + * Bodies are markdown rather than HTML. `html: false` is markdown-it's default + * and is the point of choosing it: raw HTML in a stored body is escaped, not + * passed through, so editing copy from the settings screen cannot put script + * into a customer's inbox. That is a stronger guarantee than sanitising output + * afterwards, because there is no output to sanitise. + */ +const md = new MarkdownIt({ html: false, linkify: true }); + +export type TemplateKey = + | 'verification' + | 'passwordReset' + | 'favoriteSold' + | 'favoriteWithdrawn' + | 'cartReminder'; + +export interface TemplateDefinition { + /** Shown in the admin so a card is identifiable without reading its body. */ + label: string; + /** + * Placeholders a body must contain. Saving without one is refused: a reset + * email with no link still sends, still looks fine in the log, and is useless + * to everyone who receives it. + */ + required: readonly string[]; + /** Every placeholder this template understands, for the admin to see. */ + available: readonly string[]; + defaultSubject: string; + defaultBody: string; + /** + * Appended after rendering and deliberately not editable. The favorite alerts + * carry a consent notice explaining why the customer is receiving them, which + * is a compliance artifact rather than copy — editing wording should not be + * able to delete the sentence that makes the email lawful to send. + */ + footer?: string; +} + +const FAVORITE_CONSENT_FOOTER = + '

You are receiving this because you asked to be told when a favorited item becomes ' + + 'unavailable. You can turn these off on your account page.

'; + +export const TEMPLATES: Record = { + verification: { + label: 'Email verification', + required: ['verifyUrl'], + available: ['greeting', 'verifyUrl'], + defaultSubject: 'Confirm your email address', + defaultBody: + '{{greeting}}\n\n' + + 'Please confirm this address so we know we can reach you.\n\n' + + '[Confirm my email]({{verifyUrl}})\n\n' + + 'This link expires in 24 hours.' + }, + + passwordReset: { + label: 'Password reset', + required: ['resetUrl'], + available: ['greeting', 'resetUrl'], + defaultSubject: 'Reset your Redefined Designs password', + defaultBody: + 'Someone asked to reset the password for this account.\n\n' + + '[Choose a new password]({{resetUrl}}). This link expires in one hour.\n\n' + + "If this wasn't you, you can ignore this email — your password has not changed." + }, + + favoriteSold: { + label: 'Favorited item sold', + required: ['itemName'], + available: ['itemName', 'siteUrl'], + defaultSubject: '"{{itemName}}" has been sold', + defaultBody: + 'An item you favorited has been sold to another customer, so it is no longer available.\n\n' + + '**{{itemName}}**\n\n' + + 'Every piece is one of a kind, so this one will not be restocked. You can browse what is ' + + 'still available at [Redefined Designs]({{siteUrl}}).', + footer: FAVORITE_CONSENT_FOOTER + }, + + favoriteWithdrawn: { + label: 'Favorited item withdrawn', + required: ['itemName'], + available: ['itemName', 'siteUrl'], + defaultSubject: '"{{itemName}}" is no longer available', + defaultBody: + 'An item you favorited has been withdrawn and is no longer available.\n\n' + + '**{{itemName}}**\n\n' + + 'You can browse what is still available at [Redefined Designs]({{siteUrl}}).', + footer: FAVORITE_CONSENT_FOOTER + }, + + cartReminder: { + label: 'Cart reminder', + required: ['itemList', 'cartUrl'], + available: ['greeting', 'itemList', 'cartUrl'], + defaultSubject: 'Items waiting in your cart', + defaultBody: + '{{greeting}}\n\n' + + 'You still have items in your cart at Redefined Designs:\n\n' + + '{{itemList}}\n\n' + + '[View your cart]({{cartUrl}}) before your reservation expires.' + } +}; + +/** + * The `{{greeting}}` value: "Hi Thom," when a first name is known, "Hi," + * otherwise. + * + * One placeholder rather than a bare name, so a template author writes + * `{{greeting}}` on its own line instead of `Hi {{firstName}},` — which reads + * as "Hi ," for anyone who registered before first names were required (#106). + */ +export function greeting(firstName: string | null | undefined): string { + const name = (firstName ?? '').trim(); + return name ? `Hi ${name},` : 'Hi,'; +} + +/** Matches `{{name}}`, tolerating whitespace inside the braces. */ +const PLACEHOLDER = /\{\{\s*(\w+)\s*\}\}/g; + +/** + * Which of a template's required placeholders a candidate body is missing. + * + * Returns all of them rather than the first, so a save that dropped two says so + * once instead of over two attempts. + */ +export function missingPlaceholders(key: TemplateKey, body: string): string[] { + const present = new Set(); + for (const match of body.matchAll(PLACEHOLDER)) { + present.add(match[1]); + } + return TEMPLATES[key].required.filter((name) => !present.has(name)); +} + +function substitute(text: string, values: Record): string { + return text.replace(PLACEHOLDER, (whole, name: string) => + Object.prototype.hasOwnProperty.call(values, name) ? values[name] : whole + ); +} + +export interface StoredTemplate { + subject?: string | null; + body?: string | null; +} + +/** + * Produces the subject and HTML for one email. + * + * Values are substituted into the markdown *before* rendering, which is why a + * value that should become a list has to arrive as markdown — emitting HTML + * here would be escaped and shown to the customer as literal tags. + * + * An absent or blank stored value falls back to the built-in default, so an + * unconfigured install behaves exactly as it did before any of this existed. + */ +export function renderTemplate( + key: TemplateKey, + stored: StoredTemplate, + values: Record +): { subject: string; html: string } { + const definition = TEMPLATES[key]; + const subjectSource = stored.subject?.trim() ? stored.subject : definition.defaultSubject; + const bodySource = stored.body?.trim() ? stored.body : definition.defaultBody; + + const html = md.render(substitute(bodySource, values)) + (definition.footer ?? ''); + + return { subject: substitute(subjectSource, values), html }; +} diff --git a/backend/src/favoriteAlerts.ts b/backend/src/favoriteAlerts.ts index 227bedb..b9f2c36 100644 --- a/backend/src/favoriteAlerts.ts +++ b/backend/src/favoriteAlerts.ts @@ -1,5 +1,7 @@ import { pool } from './db'; import { sendMail } from './mailer'; +import { renderTemplate, TemplateKey } from './emailTemplates'; +import { loadStoredTemplate } from './routes/adminEmailTemplates'; // Shown to the customer when they opt in, and stored verbatim against their // consent so the record says what they actually agreed to — the same pattern @@ -42,15 +44,24 @@ export async function collectFavoriteRecipients( // thing the customer asked to hear about. Sent independently so one bad // address cannot stop the rest — and whatever prompted this has already // happened regardless of whether the mail goes out. -function send(recipients: FavoriteRecipient[], subject: (name: string) => string, body: (name: string) => string): void { +async function send(recipients: FavoriteRecipient[], key: TemplateKey): Promise { + if (!recipients.length) return; + + // Loaded once for the batch rather than per recipient: the copy is the same + // for everyone, only the item name differs. + const stored = await loadStoredTemplate(key); + const siteUrl = process.env.PUBLIC_URL ?? ''; + for (const recipient of recipients) { - sendMail(recipient.email, subject(recipient.item_name), body(recipient.item_name)) + const { subject, html } = renderTemplate(key, stored, { + itemName: recipient.item_name, + siteUrl + }); + sendMail(recipient.email, subject, html) .catch(err => console.error('favorite alert failed', err)); } } -const FOOTER = `

You are receiving this because you asked to be told when a favorited item becomes - unavailable. You can turn these off on your account page.

`; // Called *after* the sale has been committed, never inside the transaction. // Emailing about a sale that then rolled back would be worse than a late @@ -60,27 +71,16 @@ const FOOTER = `

You are receiving this because you asked to be told when a fa // longer available reads as a bug. export async function notifyFavoritersOfSale(itemIds: number[], buyerId: number | null): Promise { const recipients = await collectFavoriteRecipients(itemIds, buyerId); - send( - recipients, - name => `"${name}" has been sold`, - name => `

An item you favorited has been sold to another customer, so it is no longer available.

-

${name}

-

Every piece is one of a kind, so this one will not be restocked. You can browse what is - still available at Redefined Designs.

- ${FOOTER}` - ); + await send(recipients, 'favoriteSold'); } // Sent when an item is withdrawn from sale rather than sold. Recipients must be // collected before the delete, since the favorites rows cascade with the item. -export function notifyFavoritersOfRemoval(recipients: FavoriteRecipient[]): void { - send( - recipients, - name => `"${name}" is no longer available`, - name => `

An item you favorited has been withdrawn and is no longer available.

-

${name}

-

You can browse what is still available at - Redefined Designs.

- ${FOOTER}` - ); +// Async now that the copy is loaded from the database before rendering. It was +// previously synchronous in dispatch — the sends were fire-and-forget, but they +// were *started* before the caller returned. Leaving it fire-and-forget would +// mean the response can beat the mail out of the door, which is a behaviour +// change nobody asked for and which the withdrawal test caught. +export async function notifyFavoritersOfRemoval(recipients: FavoriteRecipient[]): Promise { + await send(recipients, 'favoriteWithdrawn'); } diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index 4a04fc7..48261fe 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -346,7 +346,7 @@ router.delete('/items/:id', asyncRoute(async (req: Request, res: Response) => { // Sent only once the delete has succeeded, so nobody hears about a withdrawal // that did not happen. - notifyFavoritersOfRemoval(recipients); + await notifyFavoritersOfRemoval(recipients); res.status(204).end(); })); diff --git a/backend/src/routes/adminEmailTemplates.ts b/backend/src/routes/adminEmailTemplates.ts new file mode 100644 index 0000000..e239e84 --- /dev/null +++ b/backend/src/routes/adminEmailTemplates.ts @@ -0,0 +1,112 @@ +import { Router, Request, Response } from 'express'; +import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { TEMPLATES, TemplateKey, StoredTemplate, missingPlaceholders } from '../emailTemplates'; + +const router = Router(); + +const KEYS = Object.keys(TEMPLATES) as TemplateKey[]; + +// Stored in admin_settings rather than a table of their own: it is already a +// key/value store with a settled read/write shape, and five templates is not a +// schema. +const settingKey = (key: TemplateKey, part: 'subject' | 'body') => `email_${key}_${part}`; + +function isTemplateKey(value: unknown): value is TemplateKey { + return typeof value === 'string' && (KEYS as string[]).includes(value); +} + +export async function loadStoredTemplate(key: TemplateKey): Promise { + const { rows } = await pool.query(`SELECT key, value FROM admin_settings WHERE key = ANY($1)`, [ + [settingKey(key, 'subject'), settingKey(key, 'body')] + ]); + const stored: StoredTemplate = {}; + for (const row of rows) { + if (row.key === settingKey(key, 'subject')) stored.subject = row.value; + if (row.key === settingKey(key, 'body')) stored.body = row.value; + } + return stored; +} + +// Returns the definitions alongside whatever is stored, so the admin screen can +// show the placeholders a template accepts and which of them it must keep, +// rather than the editor having to know. +router.get('/', asyncRoute(async (_req: Request, res: Response) => { + const { rows } = await pool.query( + `SELECT key, value FROM admin_settings WHERE key LIKE 'email\\_%'` + ); + const stored = new Map(rows.map((r) => [r.key, r.value])); + + res.json( + KEYS.map((key) => ({ + key, + label: TEMPLATES[key].label, + required: TEMPLATES[key].required, + available: TEMPLATES[key].available, + defaultSubject: TEMPLATES[key].defaultSubject, + defaultBody: TEMPLATES[key].defaultBody, + // Null rather than the default, so the admin can tell "not customised" + // from "customised to exactly the default text". + subject: stored.get(settingKey(key, 'subject')) ?? null, + body: stored.get(settingKey(key, 'body')) ?? null + })) + ); +})); + +router.put('/:key', asyncRoute(async (req: Request, res: Response) => { + const key = req.params.key; + if (!isTemplateKey(key)) { + return res.status(404).json({ error: 'unknown template' }); + } + + const subject = typeof req.body?.subject === 'string' ? req.body.subject.trim() : ''; + const body = typeof req.body?.body === 'string' ? req.body.body.trim() : ''; + + if (!subject) { + return res.status(400).json({ error: 'a subject is required' }); + } + if (!body) { + return res.status(400).json({ error: 'a body is required' }); + } + + // The rule that makes this feature safe rather than a way to break password + // resets from a settings screen. A body without its link still sends, still + // looks correct in the log, and is useless to everyone who receives it — so + // the save is refused rather than warned about. + const missing = missingPlaceholders(key, body); + if (missing.length) { + const named = missing.map((name) => '{{' + name + '}}').join(' and '); + return res.status(400).json({ error: `the body must keep ${named}` }); + } + + for (const [part, value] of [ + ['subject', subject], + ['body', body] + ] as const) { + await pool.query( + `INSERT INTO admin_settings (key, value, updated_at) VALUES ($1, $2, now()) + ON CONFLICT (key) DO UPDATE SET value = $2, updated_at = now()`, + [settingKey(key, part), value] + ); + } + + res.json({ key, subject, body }); +})); + +// Restores the built-in copy by removing the stored rows, rather than by +// writing the default into them — so "not customised" stays distinguishable +// from "customised back to the original wording". +router.delete('/:key', asyncRoute(async (req: Request, res: Response) => { + const key = req.params.key; + if (!isTemplateKey(key)) { + return res.status(404).json({ error: 'unknown template' }); + } + + await pool.query(`DELETE FROM admin_settings WHERE key = ANY($1)`, [ + [settingKey(key, 'subject'), settingKey(key, 'body')] + ]); + + res.json({ key, subject: null, body: null }); +})); + +export default router; diff --git a/backend/src/routes/customers.ts b/backend/src/routes/customers.ts index 444ddce..d27f9e4 100755 --- a/backend/src/routes/customers.ts +++ b/backend/src/routes/customers.ts @@ -4,6 +4,8 @@ import crypto from 'node:crypto'; import { pool } from '../db'; import { requireCustomer } from '../middleware/customerAuth'; import { sendMail } from '../mailer'; +import { renderTemplate, greeting } from '../emailTemplates'; +import { loadStoredTemplate } from './adminEmailTemplates'; import { MARKETING_CONSENT_TEXT, isValidEmail } from '../utils'; import { FAVORITE_ALERTS_CONSENT_TEXT } from '../favoriteAlerts'; import { asyncRoute } from '../asyncRoute'; @@ -103,11 +105,12 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => { [verifyToken, customer.id, new Date(Date.now() + 24 * 60 * 60 * 1000)] ); const verifyUrl = `${process.env.PUBLIC_URL}/verify-email?token=${verifyToken}`; - sendMail( - customer.email, - 'Verify your Redefined Designs account', - `

Welcome! Please verify your email to finish setting up your account.

` - ).catch(err => console.error('verify email send failed', err)); + const verifyTemplate = renderTemplate('verification', await loadStoredTemplate('verification'), { + greeting: greeting(customer.first_name), + verifyUrl + }); + sendMail(customer.email, verifyTemplate.subject, verifyTemplate.html) + .catch(err => console.error('verify email send failed', err)); const sessionToken = await createSession(customer.id); setSessionCookie(res, sessionToken); @@ -154,13 +157,12 @@ router.post('/request-password-reset', passwordResetRequestLimiter, asyncRoute(a ); const resetUrl = `${process.env.PUBLIC_URL}/reset-password?token=${token}`; - sendMail( - customer.email, - 'Reset your Redefined Designs password', - `

Someone asked to reset the password for this account.

-

Choose a new password. This link expires in one hour.

-

If this wasn't you, you can ignore this email — your password has not changed.

` - ).catch(err => console.error('password reset email send failed', err)); + const resetTemplate = renderTemplate('passwordReset', await loadStoredTemplate('passwordReset'), { + greeting: greeting(customer.first_name), + resetUrl + }); + sendMail(customer.email, resetTemplate.subject, resetTemplate.html) + .catch(err => console.error('password reset email send failed', err)); } res.json({ status: 'sent' }); diff --git a/backend/src/server.ts b/backend/src/server.ts index d04004e..397207f 100755 --- a/backend/src/server.ts +++ b/backend/src/server.ts @@ -2,6 +2,8 @@ import cron from 'node-cron'; import app from './app'; import { pool } from './db'; import { sendMail } from './mailer'; +import { renderTemplate, greeting } from './emailTemplates'; +import { loadStoredTemplate } from './routes/adminEmailTemplates'; import { validateEnv } from './envValidation'; // Release cart holds whose expiry has passed. @@ -39,16 +41,24 @@ async function sendCartReminders(): Promise { byEmail.get(row.email)!.items.push({ name: row.item_name, expiresAt: row.expires_at, cartItemId: row.cart_item_id }); } + // Loaded once rather than per recipient: the copy is shared, only the + // greeting and the item list differ. + const stored = await loadStoredTemplate('cartReminder'); + for (const [email, data] of byEmail) { - const itemList = data.items.map(i => `
  • ${i.name} — reserved until ${i.expiresAt.toLocaleString()}
    • `).join(''); - await sendMail( - email, - 'Items waiting in your cart', - `

    Hi${data.firstName ? ' ' + data.firstName : ''},

    -

    You still have items in your cart at Redefined Designs:

    -
      ${itemList}
    -

    View your cart before your reservation expires.

    ` - ); + // Markdown, not HTML. Values are substituted into the template source + // before it is rendered, and the renderer escapes raw HTML — so an
  • + // here would reach the customer as literal angle brackets. + const itemList = data.items + .map(i => `- ${i.name} — reserved until ${i.expiresAt.toLocaleString()}`) + .join('\n'); + + const { subject, html } = renderTemplate('cartReminder', stored, { + greeting: greeting(data.firstName), + itemList, + cartUrl: `${process.env.PUBLIC_URL}/cart` + }); + await sendMail(email, subject, html); const ids = data.items.map(i => i.cartItemId); await pool.query(`UPDATE cart_items SET last_reminder_sent_at = now() WHERE id = ANY($1::int[])`, [ids]); } diff --git a/backend/tests/integration/emailTemplates.integration.test.ts b/backend/tests/integration/emailTemplates.integration.test.ts new file mode 100644 index 0000000..02e6351 --- /dev/null +++ b/backend/tests/integration/emailTemplates.integration.test.ts @@ -0,0 +1,177 @@ +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; +import { renderTemplate } from '../../src/emailTemplates'; +import { loadStoredTemplate } from '../../src/routes/adminEmailTemplates'; + +beforeEach(async () => { + // resetDb clears stored email templates as well, so a template saved by one + // test cannot change the mail another asserts on. + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +const VALID_RESET_BODY = 'New copy. [Choose a new password]({{resetUrl}}) within the hour.'; + +describe('GET /api/admin/email-templates', () => { + it('lists every template with its defaults and placeholder rules', async () => { + const res = await request(app).get('/api/admin/email-templates'); + + expect(res.status).toBe(200); + expect(res.body.map((t: { key: string }) => t.key).sort()).toEqual([ + 'cartReminder', + 'favoriteSold', + 'favoriteWithdrawn', + 'passwordReset', + 'verification' + ]); + + const reset = res.body.find((t: { key: string }) => t.key === 'passwordReset'); + expect(reset.required).toEqual(['resetUrl']); + expect(reset.defaultBody).toContain('{{resetUrl}}'); + }); + + // Null rather than the default text, so the screen can tell "never edited" + // from "edited to something identical to the default". + it('reports an uncustomised template as null rather than as its default', async () => { + const res = await request(app).get('/api/admin/email-templates'); + const reset = res.body.find((t: { key: string }) => t.key === 'passwordReset'); + + expect(reset.subject).toBeNull(); + expect(reset.body).toBeNull(); + }); +}); + +describe('PUT /api/admin/email-templates/:key', () => { + it('stores a replacement subject and body', async () => { + const res = await request(app) + .put('/api/admin/email-templates/passwordReset') + .send({ subject: 'Your reset link', body: VALID_RESET_BODY }); + + expect(res.status).toBe(200); + + const listed = await request(app).get('/api/admin/email-templates'); + const reset = listed.body.find((t: { key: string }) => t.key === 'passwordReset'); + expect(reset.subject).toBe('Your reset link'); + expect(reset.body).toBe(VALID_RESET_BODY); + }); + + // The rule that keeps this from being a way to break password resets from a + // settings screen. + it('refuses a body that has dropped the required placeholder', async () => { + const res = await request(app) + .put('/api/admin/email-templates/passwordReset') + .send({ subject: 'Your reset link', body: 'Just click the thing.' }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('{{resetUrl}}'); + }); + + it('names every missing placeholder, not just the first', async () => { + const res = await request(app) + .put('/api/admin/email-templates/cartReminder') + .send({ subject: 'Your cart', body: 'You left things behind.' }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('{{itemList}}'); + expect(res.body.error).toContain('{{cartUrl}}'); + }); + + it('refuses an empty subject or body', async () => { + const noSubject = await request(app) + .put('/api/admin/email-templates/passwordReset') + .send({ subject: ' ', body: VALID_RESET_BODY }); + expect(noSubject.status).toBe(400); + + const noBody = await request(app) + .put('/api/admin/email-templates/passwordReset') + .send({ subject: 'Something', body: ' ' }); + expect(noBody.status).toBe(400); + }); + + it('refuses a template key it does not recognise', async () => { + const res = await request(app) + .put('/api/admin/email-templates/invoiceOverdue') + .send({ subject: 'x', body: 'y' }); + + expect(res.status).toBe(404); + }); + + // Nothing is stored when a save is refused, or a half-applied edit would sit + // there looking accepted. + it('stores nothing when it refuses', async () => { + await request(app) + .put('/api/admin/email-templates/passwordReset') + .send({ subject: 'Your reset link', body: 'No link here.' }); + + const { rows } = await pool.query(`SELECT key FROM admin_settings WHERE key LIKE 'email%'`); + expect(rows).toHaveLength(0); + }); +}); + +describe('DELETE /api/admin/email-templates/:key', () => { + it('restores the built-in copy by forgetting the stored rows', async () => { + await request(app) + .put('/api/admin/email-templates/passwordReset') + .send({ subject: 'Custom', body: VALID_RESET_BODY }); + + const res = await request(app).delete('/api/admin/email-templates/passwordReset'); + expect(res.status).toBe(200); + + const listed = await request(app).get('/api/admin/email-templates'); + const reset = listed.body.find((t: { key: string }) => t.key === 'passwordReset'); + expect(reset.subject).toBeNull(); + expect(reset.body).toBeNull(); + }); +}); + +// The point of the whole feature: what is stored is what customers receive. +describe('what a stored template does to the email that gets sent', () => { + it('is used in place of the default once saved', async () => { + await request(app) + .put('/api/admin/email-templates/passwordReset') + .send({ subject: 'Reset requested', body: VALID_RESET_BODY }); + + const { subject, html } = renderTemplate( + 'passwordReset', + await loadStoredTemplate('passwordReset'), + { greeting: 'Hi Thom,', resetUrl: 'https://shop.test/r?token=abc' } + ); + + expect(subject).toBe('Reset requested'); + expect(html).toContain('New copy.'); + expect(html).toContain('https://shop.test/r?token=abc'); + }); + + it('falls back to the built-in copy when nothing is stored', async () => { + const { subject, html } = renderTemplate( + 'passwordReset', + await loadStoredTemplate('passwordReset'), + { greeting: 'Hi,', resetUrl: 'https://shop.test/r?token=abc' } + ); + + expect(subject).toBe('Reset your Redefined Designs password'); + expect(html).toContain('Choose a new password'); + }); + + // Editing the copy must not be able to remove the sentence that explains why + // the email is lawful to send. + it('keeps the consent footer on a favorite alert whose body was replaced', async () => { + await request(app) + .put('/api/admin/email-templates/favoriteSold') + .send({ subject: 'Gone', body: 'Sorry, {{itemName}} sold.' }); + + const { html } = renderTemplate('favoriteSold', await loadStoredTemplate('favoriteSold'), { + itemName: 'Oak table', + siteUrl: 'https://shop.test' + }); + + expect(html).toContain('Sorry, Oak table sold.'); + expect(html).toContain('account page'); + }); +}); diff --git a/backend/tests/integration/setup/testDb.ts b/backend/tests/integration/setup/testDb.ts index 9736e76..2f6280c 100755 --- a/backend/tests/integration/setup/testDb.ts +++ b/backend/tests/integration/setup/testDb.ts @@ -33,6 +33,14 @@ export async function resetDb(): Promise { tags, categories RESTART IDENTITY CASCADE `); + + // admin_settings is not truncated — it holds the seeded cart_expiry_hours + // default that other suites read. But the email template rows in it are test + // data like any other, and a stored template outliving the suite that wrote + // it silently changes the mail every later suite asserts on. That is not + // hypothetical: a subject of "Gone" written by the template tests reached the + // favorite-alert tests and made five of them fail somewhere else entirely. + await testPool.query(`DELETE FROM admin_settings WHERE key LIKE 'email\_%'`); } export async function closeDb(): Promise { diff --git a/backend/tests/unit/emailTemplates.test.ts b/backend/tests/unit/emailTemplates.test.ts new file mode 100644 index 0000000..92b231f --- /dev/null +++ b/backend/tests/unit/emailTemplates.test.ts @@ -0,0 +1,148 @@ +import { + TEMPLATES, + TemplateKey, + missingPlaceholders, + renderTemplate +} from '../../src/emailTemplates'; + +const KEYS: TemplateKey[] = [ + 'verification', + 'passwordReset', + 'favoriteSold', + 'favoriteWithdrawn', + 'cartReminder' +]; + +describe('the built-in templates', () => { + it.each(KEYS)('%s has a default subject and body', (key) => { + expect(TEMPLATES[key].defaultSubject.trim()).not.toBe(''); + expect(TEMPLATES[key].defaultBody.trim()).not.toBe(''); + }); + + // A default that would be refused on save is a default nobody can edit and + // put back. + it.each(KEYS)('%s default body satisfies its own required placeholders', (key) => { + expect(missingPlaceholders(key, TEMPLATES[key].defaultBody)).toEqual([]); + }); +}); + +describe('missingPlaceholders', () => { + it('names the placeholder a body has dropped', () => { + expect(missingPlaceholders('passwordReset', 'Hello, no link here.')).toEqual(['resetUrl']); + }); + + it('is satisfied once the placeholder is present', () => { + expect(missingPlaceholders('passwordReset', 'Reset it [here]({{resetUrl}}).')).toEqual([]); + }); + + it('reports every missing placeholder rather than the first', () => { + const missing = missingPlaceholders('cartReminder', 'You have things.'); + expect(missing).toContain('itemList'); + expect(missing).toContain('cartUrl'); + }); + + it('tolerates whitespace inside the braces', () => { + expect(missingPlaceholders('passwordReset', 'Go [here]({{ resetUrl }}).')).toEqual([]); + }); +}); + +describe('renderTemplate', () => { + const resetValues = { resetUrl: 'https://shop.test/reset-password?token=abc' }; + + it('substitutes a placeholder into the rendered body', () => { + const { html } = renderTemplate('passwordReset', {}, resetValues); + expect(html).toContain('https://shop.test/reset-password?token=abc'); + expect(html).not.toContain('{{resetUrl}}'); + }); + + it('renders markdown as HTML', () => { + const { html } = renderTemplate( + 'passwordReset', + { body: 'Use **this** [link]({{resetUrl}}).' }, + resetValues + ); + expect(html).toContain('this'); + expect(html).toContain(' { + const { html } = renderTemplate( + 'passwordReset', + { body: ' [link]({{resetUrl}})' }, + resetValues + ); + expect(html).not.toContain('