feat(admin): make the token lifetimes, cart hold and greeting configurable (#136)
Linting / lint (pull_request) Failing after 35s
SonarQube Analysis / sonarqube (pull_request) Successful in 20m8s

Giving the customer emails a tab of their own (#135) made a family of related holes visible: an admin could edit the wording of every customer email, but not the facts that wording asserted, and two templates could not address the customer at all.

Both token lifetimes were hardcoded twice. VERIFY_TOKEN_TTL_MS sat in routes/customers.ts while the verification body separately said "This link expires in 24 hours", and RESET_TOKEN_TTL_MS sat beside a body separately saying "one hour". The prose was not derived from the constant, it was a second hand-written copy of the same fact — so making the constants configurable without addressing that would have made things worse, not better: the setting moves to two hours and the email keeps confidently promising one. Both are now settings, and both templates state their lifetime through an {{expiresIn}} placeholder rendered from the setting.

The cart reminder gains {{holdDuration}} for the same reason. Per-item deadlines were already inside {{itemList}}, but there was no way to write a sentence about the hold itself without hardcoding a number the cart expiry setting could change underneath the author.

All three durations render through one formatDuration(), so the reset email and the cart reminder say "one hour" the same way rather than in two authors' phrasing. A fractional hour drops to minutes, because "0.5 hours" reads badly and "1.5 hours" reads worse in a sentence a customer is meant to act on.

Every template now offers greeting, firstName and lastName. favoriteSold and favoriteWithdrawn previously offered only itemName and siteUrl and could not address anyone — the query behind them never selected a name, so it does now.

The greeting itself is two settings, a format and a fallback, rather than the wording baked into greeting(). The fallback is separate rather than the format with the name edited out: that editing is guesswork that has to be right every time, and getting it wrong ships "Hi ," to everyone who registered while first names were still optional (#106). Those customers exist, which is why greeting() guarded the case in the first place.

The admin preview renders the durations and the greeting from the live settings rather than from a static sample. The preview exists so an admin sees the email that will be sent, and a sample reading "one hour" while the setting says two is the precise failure these placeholders were added to remove.

Settings are read through a new adminSettings accessor. cart_expiry_hours was previously read by an inline query in two places, each with its own `|| '24'`; with five settings and read sites across three routes and the cron job, a default written twice is a default that will eventually disagree with itself. Values are stored as text, so each definition declares how to read it back — numbers were the only kind until the greeting format arrived.

The new placeholders are available but never required, so every template an admin has already saved keeps rendering and keeps sending.

Also fixes updateAdminSettings announcing success for a save the server refused: it returned the 400 body as though it were the saved settings, so the form reported "Settings saved" either way.

Closes #136
This commit is contained in:
2026-08-23 08:52:48 -05:00
parent 7df897c0fd
commit 49279a8f72
13 changed files with 646 additions and 81 deletions
+81
View File
@@ -0,0 +1,81 @@
import { pool } from './db';
/**
* Every admin-configurable setting, in one table.
*
* `cart_expiry_hours` used to be read by an inline query in two places, each
* with its own `|| '24'`. With several settings and read sites scattered across
* routes and the cron job that stops being tenable: a default written twice is
* a default that will eventually disagree with itself. Adding a setting means
* adding a row here and nothing else.
*
* Values are stored as text, so each row declares how to read it back. Numbers
* were the only kind until the greeting format arrived; typing it per setting
* rather than assuming means the next string one costs nothing.
*/
const DEFINITIONS = [
{ key: 'cart_expiry_hours', name: 'cartExpiryHours', type: 'hours', fallback: 24 },
{ key: 'verify_token_hours', name: 'verifyTokenHours', type: 'hours', fallback: 24 },
{ key: 'password_reset_hours', name: 'passwordResetHours', type: 'hours', fallback: 1 },
{ key: 'greeting_format', name: 'greetingFormat', type: 'text', fallback: 'Hi {{firstName}},' },
{ key: 'greeting_fallback', name: 'greetingFallback', type: 'text', fallback: 'Hi,' }
] as const;
type Definition = (typeof DEFINITIONS)[number];
export type SettingName = Definition['name'];
export type HoursSettingName = Extract<Definition, { type: 'hours' }>['name'];
export type TextSettingName = Extract<Definition, { type: 'text' }>['name'];
export type AdminSettings = Record<HoursSettingName, number> & Record<TextSettingName, string>;
export const HOURS_SETTINGS: readonly HoursSettingName[] = DEFINITIONS.filter(
(d): d is Extract<Definition, { type: 'hours' }> => d.type === 'hours'
).map(d => d.name);
export const TEXT_SETTINGS: readonly TextSettingName[] = DEFINITIONS.filter(
(d): d is Extract<Definition, { type: 'text' }> => d.type === 'text'
).map(d => d.name);
export async function getSettings(): Promise<AdminSettings> {
const { rows } = await pool.query(`SELECT key, value FROM admin_settings`);
const stored = new Map<string, string>(rows.map(r => [r.key, r.value]));
const settings = {} as Record<SettingName, number | string>;
for (const { key, name, type, fallback } of DEFINITIONS) {
const raw = stored.get(key);
if (type === 'text') {
// An empty format would render every greeting as nothing at all, which
// reads as a bug in the email rather than a setting someone cleared.
settings[name] = raw !== undefined && raw.trim() !== '' ? raw : fallback;
continue;
}
const parsed = parseFloat(raw ?? '');
// A row that is present but unparseable falls back rather than yielding
// NaN, which would otherwise reach Date arithmetic and mint a token with an
// Invalid Date expiry that no query could ever match.
settings[name] = Number.isFinite(parsed) && parsed > 0 ? parsed : fallback;
}
return settings as AdminSettings;
}
/**
* Writes the supplied settings, ignoring names that were not sent.
*
* Partial rather than whole-object so a caller updating one field does not have
* to know the current value of the others to avoid clobbering them.
*/
export async function updateSettings(
values: Partial<Record<SettingName, number | string>>
): Promise<void> {
for (const { key, name } of DEFINITIONS) {
const value = values[name];
if (value === undefined) continue;
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()`,
[key, String(value)]
);
}
}
+51 -14
View File
@@ -49,30 +49,30 @@ export const TEMPLATES: Record<TemplateKey, TemplateDefinition> = {
verification: {
label: 'Email verification',
required: ['verifyUrl'],
available: ['greeting', 'verifyUrl'],
available: ['greeting', 'firstName', 'lastName', 'verifyUrl', 'expiresIn'],
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.'
'This link expires in {{expiresIn}}.'
},
passwordReset: {
label: 'Password reset',
required: ['resetUrl'],
available: ['greeting', 'resetUrl'],
available: ['greeting', 'firstName', 'lastName', 'resetUrl', 'expiresIn'],
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' +
'[Choose a new password]({{resetUrl}}). This link expires in {{expiresIn}}.\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'],
available: ['greeting', 'firstName', 'lastName', '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' +
@@ -85,7 +85,7 @@ export const TEMPLATES: Record<TemplateKey, TemplateDefinition> = {
favoriteWithdrawn: {
label: 'Favorited item withdrawn',
required: ['itemName'],
available: ['itemName', 'siteUrl'],
available: ['greeting', 'firstName', 'lastName', 'itemName', 'siteUrl'],
defaultSubject: '"{{itemName}}" is no longer available',
defaultBody:
'An item you favorited has been withdrawn and is no longer available.\n\n' +
@@ -101,7 +101,7 @@ export const TEMPLATES: Record<TemplateKey, TemplateDefinition> = {
// whether it was them. This is the mail that catches an account
// takeover, so it goes to the address being replaced.
required: ['newEmail'],
available: ['greeting', 'newEmail'],
available: ['greeting', 'firstName', 'lastName', 'newEmail'],
defaultSubject: 'Your Redefined Designs email address was changed',
defaultBody:
'{{greeting}}\n\n' +
@@ -114,27 +114,57 @@ export const TEMPLATES: Record<TemplateKey, TemplateDefinition> = {
cartReminder: {
label: 'Cart reminder',
required: ['itemList', 'cartUrl'],
available: ['greeting', 'itemList', 'cartUrl'],
available: ['greeting', 'firstName', 'lastName', 'itemList', 'cartUrl', 'holdDuration'],
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' +
'Items are held for {{holdDuration}} from when they were added.\n\n' +
'[View your cart]({{cartUrl}}) before your reservation expires.'
}
};
/**
* The `{{greeting}}` value: "Hi Thom," when a first name is known, "Hi,"
* otherwise.
* Renders a configured lifetime, in hours, as the words an email should use.
*
* All three duration placeholders go through this, so the reset email and the
* cart reminder say "one hour" the same way rather than in two authors'
* phrasing. A fractional hour drops to minutes: "0.5 hours" reads badly, and
* "1.5 hours" reads worse in a sentence a customer is meant to act on.
*/
export function formatDuration(hours: number): string {
if (!Number.isInteger(hours)) {
return `${Math.round(hours * 60)} minutes`;
}
return hours === 1 ? 'one hour' : `${hours} hours`;
}
/**
* Builds the `{{greeting}}` value from the admin-configured format.
*
* 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).
* `{{firstName}}` and `{{lastName}}` are still offered for a template that
* genuinely wants the name inline, but the greeting is the safe default.
*
* The fallback is a separate setting rather than the format with the name
* removed. Editing a name out of a sentence is the kind of thing that has to be
* right every time and cannot be, so an admin writes both and neither is
* guessed.
*/
export function greeting(firstName: string | null | undefined): string {
const name = (firstName ?? '').trim();
return name ? `Hi ${name},` : 'Hi,';
export function greeting(
firstName: string | null | undefined,
format: string,
fallback: string,
lastName?: string | null
): string {
const first = (firstName ?? '').trim();
if (!first) return fallback;
return format
.replace(/\{\{\s*firstName\s*\}\}/g, first)
.replace(/\{\{\s*lastName\s*\}\}/g, (lastName ?? '').trim());
}
/** Matches `{{name}}`, tolerating whitespace inside the braces. */
@@ -175,13 +205,20 @@ function substitute(text: string, values: Record<string, string>): string {
*/
export const SAMPLE_VALUES: Record<string, string> = {
greeting: 'Hi Ada,',
firstName: 'Ada',
lastName: 'Lovelace',
verifyUrl: 'https://example.com/verify-email?token=sample-token',
resetUrl: 'https://example.com/reset-password?token=sample-token',
itemName: 'Walnut sideboard',
siteUrl: 'https://example.com',
newEmail: 'new.address@example.com',
itemList: '- Walnut sideboard\n- Brass table lamp',
cartUrl: 'https://example.com/cart'
cartUrl: 'https://example.com/cart',
// Fallbacks only. The admin preview overrides both from the live settings,
// so the pane shows the duration that would actually be sent rather than a
// plausible-looking number that disagrees with it.
expiresIn: 'one hour',
holdDuration: '24 hours'
};
export interface StoredTemplate {
+11 -2
View File
@@ -1,6 +1,7 @@
import { pool } from './db';
import { sendMail } from './mailer';
import { renderTemplate, TemplateKey } from './emailTemplates';
import { renderTemplate, greeting, TemplateKey } from './emailTemplates';
import { getSettings } from './adminSettings';
import { loadStoredTemplate } from './routes/adminEmailTemplates';
// Shown to the customer when they opt in, and stored verbatim against their
@@ -11,6 +12,10 @@ export const FAVORITE_ALERTS_CONSENT_TEXT =
export interface FavoriteRecipient {
email: string;
// Nullable because customers who registered while names were optional have
// none — the same reason the greeting needs a fallback at all.
first_name: string | null;
last_name: string | null;
item_name: string;
}
@@ -26,7 +31,7 @@ export async function collectFavoriteRecipients(
if (!itemIds.length) return [];
const { rows } = await pool.query<FavoriteRecipient>(
`SELECT c.email, i.name AS item_name
`SELECT c.email, c.first_name, c.last_name, i.name AS item_name
FROM favorites f
JOIN customers c ON c.id = f.customer_id
JOIN items i ON i.id = f.item_id
@@ -51,9 +56,13 @@ async function send(recipients: FavoriteRecipient[], key: TemplateKey): Promise<
// for everyone, only the item name differs.
const stored = await loadStoredTemplate(key);
const siteUrl = process.env.PUBLIC_URL ?? '';
const { greetingFormat, greetingFallback } = await getSettings();
for (const recipient of recipients) {
const { subject, html } = renderTemplate(key, stored, {
greeting: greeting(recipient.first_name, greetingFormat, greetingFallback, recipient.last_name),
firstName: recipient.first_name ?? '',
lastName: recipient.last_name ?? '',
itemName: recipient.item_name,
siteUrl
});
+33 -1
View File
@@ -7,8 +7,11 @@ import {
StoredTemplate,
missingPlaceholders,
renderTemplate,
formatDuration,
greeting,
SAMPLE_VALUES
} from '../emailTemplates';
import { getSettings } from '../adminSettings';
const router = Router();
@@ -73,6 +76,35 @@ router.get('/', asyncRoute(async (_req: Request, res: Response) => {
//
// Deliberately does not enforce required placeholders. Saving refuses a body
// that dropped one; previewing it is how an admin sees what they have done.
/**
* The sample values, with the three duration placeholders replaced by what the
* settings actually hold.
*
* The preview exists so an admin sees the email that will be sent. A duration
* drawn from a static sample would show "one hour" while the setting said two,
* which is the precise failure this placeholder was added to remove.
*/
async function previewValues(key: TemplateKey): Promise<Record<string, string>> {
const {
cartExpiryHours,
verifyTokenHours,
passwordResetHours,
greetingFormat,
greetingFallback
} = await getSettings();
// `expiresIn` names one placeholder but two different lifetimes, so the value
// depends on which template is being previewed. The route knows the key.
const expiresIn = key === 'passwordReset' ? passwordResetHours : verifyTokenHours;
return {
...SAMPLE_VALUES,
holdDuration: formatDuration(cartExpiryHours),
expiresIn: formatDuration(expiresIn),
// Built from the configured format for the same reason as the durations:
// the preview is meant to show the email that will be sent.
greeting: greeting(SAMPLE_VALUES.firstName, greetingFormat, greetingFallback, SAMPLE_VALUES.lastName)
};
}
router.post('/:key/preview', asyncRoute(async (req: Request, res: Response) => {
const key = req.params.key;
if (!isTemplateKey(key)) {
@@ -86,7 +118,7 @@ router.post('/:key/preview', asyncRoute(async (req: Request, res: Response) => {
subject: typeof subject === 'string' ? subject : null,
body: typeof body === 'string' ? body : null
},
SAMPLE_VALUES
await previewValues(key)
);
res.json(rendered);
+31 -16
View File
@@ -1,30 +1,45 @@
import { Router, Request, Response } from 'express';
import { pool } from '../db';
import { asyncRoute } from '../asyncRoute';
import {
getSettings,
updateSettings,
HOURS_SETTINGS,
TEXT_SETTINGS,
SettingName
} from '../adminSettings';
const router = Router();
router.get('/', asyncRoute(async (_req: Request, res: Response) => {
const { rows } = await pool.query(`SELECT key, value FROM admin_settings`);
const map: Record<string, string> = {};
for (const r of rows) map[r.key] = r.value;
res.json({
cartExpiryHours: parseFloat(map.cart_expiry_hours || '24')
});
res.json(await getSettings());
}));
router.put('/', asyncRoute(async (req: Request, res: Response) => {
const { cartExpiryHours } = req.body;
const hours = parseFloat(cartExpiryHours);
const values: Partial<Record<SettingName, number | string>> = {};
// Only what was sent is validated and written, so a caller updating one field
// does not have to echo the others back to avoid clobbering them.
for (const name of HOURS_SETTINGS) {
const raw = req.body[name];
if (raw === undefined) continue;
const hours = parseFloat(raw);
if (Number.isNaN(hours) || hours <= 0) {
return res.status(400).json({ error: 'cartExpiryHours must be a positive number' });
return res.status(400).json({ error: `${name} must be a positive number` });
}
await pool.query(
`INSERT INTO admin_settings (key, value, updated_at) VALUES ('cart_expiry_hours', $1, now())
ON CONFLICT (key) DO UPDATE SET value = $1, updated_at = now()`,
[String(hours)]
);
res.json({ cartExpiryHours: hours });
values[name] = hours;
}
for (const name of TEXT_SETTINGS) {
const raw = req.body[name];
if (raw === undefined) continue;
if (typeof raw !== 'string' || raw.trim() === '') {
return res.status(400).json({ error: `${name} cannot be empty` });
}
values[name] = raw;
}
await updateSettings(values);
res.json(await getSettings());
}));
export default router;
+3 -7
View File
@@ -2,14 +2,10 @@ import { Router, Request, Response } from 'express';
import { pool } from '../db';
import { asyncRoute } from '../asyncRoute';
import { requireCustomer } from '../middleware/customerAuth';
import { getSettings } from '../adminSettings';
const router = Router();
async function getCartExpiryHours(): Promise<number> {
const { rows } = await pool.query(`SELECT value FROM admin_settings WHERE key = 'cart_expiry_hours'`);
return rows.length ? parseFloat(rows[0].value) : 24;
}
const CART_ITEM_SELECT = `
SELECT
ci.item_id, ci.added_at, ci.expires_at,
@@ -60,8 +56,8 @@ router.post('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, r
cartId = newCart[0].id;
}
const hours = await getCartExpiryHours();
const expiresAt = new Date(Date.now() + hours * 60 * 60 * 1000);
const { cartExpiryHours } = await getSettings();
const expiresAt = new Date(Date.now() + cartExpiryHours * 60 * 60 * 1000);
await client.query(
`INSERT INTO cart_items (cart_id, item_id, expires_at) VALUES ($1, $2, $3)`,
[cartId, itemId, expiresAt]
+25 -15
View File
@@ -4,7 +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 { renderTemplate, greeting, formatDuration } from '../emailTemplates';
import { getSettings } from '../adminSettings';
import { loadStoredTemplate } from './adminEmailTemplates';
import { MARKETING_CONSENT_TEXT, isValidEmail } from '../utils';
import { FAVORITE_ALERTS_CONSENT_TEXT } from '../favoriteAlerts';
@@ -15,8 +16,6 @@ const router = Router();
const SESSION_DAYS = 30;
const VERIFY_TOKEN_TTL_MS = 24 * 60 * 60 * 1000;
// Registration, changing an address, and resending all need the same three
// steps: supersede any outstanding link, mint a new one, send it. Written out
// three times they would drift, and the step most likely to be forgotten is the
@@ -29,21 +28,26 @@ const VERIFY_TOKEN_TTL_MS = 24 * 60 * 60 * 1000;
async function issueVerificationEmail(
customerId: number,
email: string,
firstName: string | null
firstName: string | null,
lastName: string | null = null
): Promise<void> {
await pool.query(
`DELETE FROM customer_tokens WHERE customer_id = $1 AND kind = 'verify_email'`,
[customerId]
);
const { verifyTokenHours, greetingFormat, greetingFallback } = await getSettings();
const token = crypto.randomBytes(24).toString('hex');
await pool.query(
`INSERT INTO customer_tokens (token, customer_id, kind, expires_at) VALUES ($1, $2, 'verify_email', $3)`,
[token, customerId, new Date(Date.now() + VERIFY_TOKEN_TTL_MS)]
[token, customerId, new Date(Date.now() + verifyTokenHours * 60 * 60 * 1000)]
);
const verifyUrl = `${process.env.PUBLIC_URL}/verify-email?token=${token}`;
const template = renderTemplate('verification', await loadStoredTemplate('verification'), {
greeting: greeting(firstName),
verifyUrl
greeting: greeting(firstName, greetingFormat, greetingFallback, lastName),
firstName: firstName ?? '',
lastName: lastName ?? '',
verifyUrl,
expiresIn: formatDuration(verifyTokenHours)
});
sendMail(email, template.subject, template.html)
.catch(err => console.error('verify email send failed', err));
@@ -133,7 +137,7 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => {
);
const customer = rows[0];
await issueVerificationEmail(customer.id, customer.email, customer.first_name);
await issueVerificationEmail(customer.id, customer.email, customer.first_name, customer.last_name);
const sessionToken = await createSession(customer.id);
setSessionCookie(res, sessionToken);
@@ -169,12 +173,11 @@ router.post(
return res.status(400).json({ error: 'your email address is already verified' });
}
await issueVerificationEmail(customer.id, customer.email, customer.first_name);
await issueVerificationEmail(customer.id, customer.email, customer.first_name, customer.last_name);
res.status(204).end();
})
);
const RESET_TOKEN_TTL_MS = 60 * 60 * 1000;
// Always answers 200, whether or not the address has an account. A response
// that differed would let anyone test addresses for membership.
@@ -195,16 +198,20 @@ router.post('/request-password-reset', passwordResetRequestLimiter, asyncRoute(a
// from an older message in the customer's inbox.
await pool.query(`DELETE FROM customer_tokens WHERE customer_id = $1 AND kind = 'password_reset'`, [customer.id]);
const { passwordResetHours, greetingFormat, greetingFallback } = await getSettings();
const token = crypto.randomBytes(32).toString('hex');
await pool.query(
`INSERT INTO customer_tokens (token, customer_id, kind, expires_at) VALUES ($1, $2, 'password_reset', $3)`,
[token, customer.id, new Date(Date.now() + RESET_TOKEN_TTL_MS)]
[token, customer.id, new Date(Date.now() + passwordResetHours * 60 * 60 * 1000)]
);
const resetUrl = `${process.env.PUBLIC_URL}/reset-password?token=${token}`;
const resetTemplate = renderTemplate('passwordReset', await loadStoredTemplate('passwordReset'), {
greeting: greeting(customer.first_name),
resetUrl
greeting: greeting(customer.first_name, greetingFormat, greetingFallback, customer.last_name),
firstName: customer.first_name ?? '',
lastName: customer.last_name ?? '',
resetUrl,
expiresIn: formatDuration(passwordResetHours)
});
sendMail(customer.email, resetTemplate.subject, resetTemplate.html)
.catch(err => console.error('password reset email send failed', err));
@@ -437,10 +444,13 @@ router.put('/me/email', requireCustomer, asyncRoute(async (req: Request, res: Re
// Both sends happen after the row is written, never before — the same rule
// favoriteAlerts follows, so a change that failed cannot produce mail saying
// it succeeded.
await issueVerificationEmail(req.customerId as number, normalized, customer.first_name);
await issueVerificationEmail(req.customerId as number, normalized, customer.first_name, customer.last_name);
const { greetingFormat, greetingFallback } = await getSettings();
const notice = renderTemplate('emailChanged', await loadStoredTemplate('emailChanged'), {
greeting: greeting(customer.first_name),
greeting: greeting(customer.first_name, greetingFormat, greetingFallback, customer.last_name),
firstName: customer.first_name ?? '',
lastName: customer.last_name ?? '',
newEmail: normalized
});
sendMail(previousEmail, notice.subject, notice.html)
+12 -6
View File
@@ -2,7 +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 { renderTemplate, greeting, formatDuration } from './emailTemplates';
import { getSettings } from './adminSettings';
import { loadStoredTemplate } from './routes/adminEmailTemplates';
import { validateEnv } from './envValidation';
@@ -25,7 +26,7 @@ async function sweepExpiredCarts(): Promise<void> {
async function sendCartReminders(): Promise<void> {
try {
const { rows } = await pool.query(`
SELECT c.email, c.first_name, i.name AS item_name, ci.expires_at, ci.id AS cart_item_id
SELECT c.email, c.first_name, c.last_name, i.name AS item_name, ci.expires_at, ci.id AS cart_item_id
FROM cart_items ci
JOIN carts ca ON ca.id = ci.cart_id
JOIN customers c ON c.id = ca.customer_id
@@ -35,15 +36,17 @@ async function sendCartReminders(): Promise<void> {
AND ci.expires_at > now()
`);
const byEmail = new Map<string, { firstName: string | null; items: { name: string; expiresAt: Date; cartItemId: number }[] }>();
const byEmail = new Map<string, { firstName: string | null; lastName: string | null; items: { name: string; expiresAt: Date; cartItemId: number }[] }>();
for (const row of rows) {
if (!byEmail.has(row.email)) byEmail.set(row.email, { firstName: row.first_name, items: [] });
if (!byEmail.has(row.email)) byEmail.set(row.email, { firstName: row.first_name, lastName: row.last_name, items: [] });
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');
const { cartExpiryHours, greetingFormat, greetingFallback } = await getSettings();
const holdDuration = formatDuration(cartExpiryHours);
for (const [email, data] of byEmail) {
// Markdown, not HTML. Values are substituted into the template source
@@ -54,9 +57,12 @@ async function sendCartReminders(): Promise<void> {
.join('\n');
const { subject, html } = renderTemplate('cartReminder', stored, {
greeting: greeting(data.firstName),
greeting: greeting(data.firstName, greetingFormat, greetingFallback, data.lastName),
firstName: data.firstName ?? '',
lastName: data.lastName ?? '',
itemList,
cartUrl: `${process.env.PUBLIC_URL}/cart`
cartUrl: `${process.env.PUBLIC_URL}/cart`,
holdDuration
});
await sendMail(email, subject, html);
const ids = data.items.map(i => i.cartItemId);
@@ -0,0 +1,103 @@
import request from 'supertest';
import app from '../../src/app';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
import { getSettings } from '../../src/adminSettings';
// resetDb only clears the stored email templates out of admin_settings — it
// matches `email\_%`. The settings themselves survive it, so without this a
// value written by one test is the baseline the next one reads. Cleared after
// the file too, so nothing leaks into the suites that run behind it.
beforeEach(async () => {
await resetDb();
await pool.query(`DELETE FROM admin_settings`);
});
afterAll(async () => {
await pool.query(`DELETE FROM admin_settings`);
await pool.end();
await closeDb();
});
describe('GET /api/admin/settings', () => {
it('answers with every setting, falling back for the ones never written', async () => {
const res = await request(app).get('/api/admin/settings');
expect(res.status).toBe(200);
expect(res.body).toEqual({
cartExpiryHours: 24,
verifyTokenHours: 24,
passwordResetHours: 1,
greetingFormat: 'Hi {{firstName}},',
greetingFallback: 'Hi,'
});
});
});
describe('PUT /api/admin/settings', () => {
it('stores the lifetimes and reads them back', async () => {
const res = await request(app)
.put('/api/admin/settings')
.send({ verifyTokenHours: 2, passwordResetHours: 0.5 });
expect(res.status).toBe(200);
expect(res.body.verifyTokenHours).toBe(2);
expect(res.body.passwordResetHours).toBe(0.5);
// Read back through the accessor the senders use, not just the response.
expect((await getSettings()).verifyTokenHours).toBe(2);
});
// Partial rather than whole-object: a caller updating one field should not
// have to echo the others back to avoid clobbering them.
it('leaves settings it was not sent alone', async () => {
await request(app).put('/api/admin/settings').send({ cartExpiryHours: 6 });
await request(app).put('/api/admin/settings').send({ verifyTokenHours: 3 });
const { cartExpiryHours, verifyTokenHours } = await getSettings();
expect(cartExpiryHours).toBe(6);
expect(verifyTokenHours).toBe(3);
});
it('stores the greeting format and its fallback', async () => {
const res = await request(app)
.put('/api/admin/settings')
.send({ greetingFormat: 'Dear {{firstName}} {{lastName}}:', greetingFallback: 'Hello there,' });
expect(res.status).toBe(200);
expect(res.body.greetingFormat).toBe('Dear {{firstName}} {{lastName}}:');
expect(res.body.greetingFallback).toBe('Hello there,');
});
it.each([
['cartExpiryHours', 0],
['verifyTokenHours', -1],
['passwordResetHours', 'soon']
])('refuses %s of %p, naming the field', async (name, value) => {
const res = await request(app).put('/api/admin/settings').send({ [name]: value });
expect(res.status).toBe(400);
expect(res.body.error).toContain(name);
});
// An empty format would render every greeting as nothing at all, which reads
// as a broken email rather than a setting someone cleared.
it.each(['greetingFormat', 'greetingFallback'])('refuses an empty %s', async (name) => {
const res = await request(app).put('/api/admin/settings').send({ [name]: ' ' });
expect(res.status).toBe(400);
expect(res.body.error).toContain(name);
});
// Every field is validated before any is written, so a request that is part
// nonsense does not half-apply.
it('does not write anything when one field in the request is invalid', async () => {
await request(app).put('/api/admin/settings').send({ cartExpiryHours: 6 });
const res = await request(app)
.put('/api/admin/settings')
.send({ cartExpiryHours: 8, verifyTokenHours: -3 });
expect(res.status).toBe(400);
expect((await getSettings()).cartExpiryHours).toBe(6);
});
});
+111 -1
View File
@@ -3,6 +3,8 @@ import {
TemplateKey,
missingPlaceholders,
renderTemplate,
formatDuration,
greeting,
SAMPLE_VALUES
} from '../../src/emailTemplates';
@@ -142,7 +144,12 @@ describe('renderTemplate', () => {
const { html, subject } = renderTemplate(
'cartReminder',
{},
{ greeting: 'Hi Thom,', itemList: '- One thing', cartUrl: 'https://shop.test/cart' }
{
greeting: 'Hi Thom,',
itemList: '- One thing',
cartUrl: 'https://shop.test/cart',
holdDuration: '24 hours'
}
);
expect(html).not.toMatch(/\{\{\s*\w+\s*\}\}/);
expect(subject).not.toMatch(/\{\{\s*\w+\s*\}\}/);
@@ -164,3 +171,106 @@ describe('SAMPLE_VALUES, which the admin preview renders with', () => {
expect(subject).not.toMatch(/\{\{\s*\w+\s*\}\}/);
});
});
describe('formatDuration, which puts a configured lifetime into email copy', () => {
// The three templates that mention a duration all render it through this, so
// "one hour" in the reset email and "one hour" in the cart reminder are the
// same string produced the same way rather than two authors' phrasing.
it('spells a single hour rather than printing a numeral', () => {
expect(formatDuration(1)).toBe('one hour');
});
it('counts whole hours', () => {
expect(formatDuration(2)).toBe('2 hours');
expect(formatDuration(24)).toBe('24 hours');
});
// A fractional hour reads badly as "0.5 hours", and worse as "1.5 hours" in a
// sentence a customer is meant to act on.
it('drops to minutes for anything that is not a whole number of hours', () => {
expect(formatDuration(0.5)).toBe('30 minutes');
expect(formatDuration(0.25)).toBe('15 minutes');
expect(formatDuration(1.5)).toBe('90 minutes');
});
});
describe('the duration placeholders', () => {
it('offers expiresIn on the two templates that carry a link with a lifetime', () => {
expect(TEMPLATES.verification.available).toContain('expiresIn');
expect(TEMPLATES.passwordReset.available).toContain('expiresIn');
});
it('offers holdDuration on the cart reminder', () => {
expect(TEMPLATES.cartReminder.available).toContain('holdDuration');
});
// Required would reject every template an admin saved before this existed,
// and the whole point is that their copy keeps sending.
it.each([
['verification', 'expiresIn'],
['passwordReset', 'expiresIn'],
['cartReminder', 'holdDuration']
] as const)('does not make %s require %s', (key, name) => {
expect(TEMPLATES[key].required).not.toContain(name);
});
it('renders a body saved before the placeholder existed, unchanged', () => {
const { html } = renderTemplate(
'passwordReset',
{ subject: 'Reset it', body: 'Go [here]({{resetUrl}}). This link expires in one hour.' },
{ resetUrl: 'https://shop.test/r', expiresIn: 'two hours' }
);
expect(html).toContain('one hour');
expect(html).not.toMatch(/\{\{\s*\w+\s*\}\}/);
});
it('substitutes the duration when a body does use the placeholder', () => {
const { html } = renderTemplate(
'passwordReset',
{ subject: 'Reset it', body: 'Go [here]({{resetUrl}}). Expires in {{expiresIn}}.' },
{ resetUrl: 'https://shop.test/r', expiresIn: 'two hours' }
);
expect(html).toContain('two hours');
});
});
describe('greeting, built from the configured format', () => {
const FORMAT = 'Hi {{firstName}},';
const FALLBACK = 'Hi,';
it('substitutes the first name into the format', () => {
expect(greeting('Ada', FORMAT, FALLBACK)).toBe('Hi Ada,');
});
it('honours a format an admin has rewritten', () => {
expect(greeting('Ada', 'Dear {{firstName}} {{lastName}}:', FALLBACK, 'Lovelace'))
.toBe('Dear Ada Lovelace:');
});
it('tolerates whitespace inside the braces, as the renderer does', () => {
expect(greeting('Ada', 'Hi {{ firstName }},', FALLBACK)).toBe('Hi Ada,');
});
// The case #106 was about. Customers who registered while first names were
// optional genuinely have none, and substituting an empty string into the
// format would send them "Hi ,".
it.each([null, undefined, '', ' '])(
'uses the fallback whole rather than a format with a hole in it (%p)',
(name) => {
expect(greeting(name, FORMAT, FALLBACK)).toBe('Hi,');
}
);
it('does not leave a lastName placeholder behind when there is no last name', () => {
expect(greeting('Ada', 'Hi {{firstName}} {{lastName}},', FALLBACK, null))
.not.toMatch(/\{\{/);
});
});
describe('every template can address the customer', () => {
it.each(KEYS)('%s offers greeting, firstName and lastName', (key) => {
expect(TEMPLATES[key].available).toEqual(
expect.arrayContaining(['greeting', 'firstName', 'lastName'])
);
});
});
+85 -9
View File
@@ -5,6 +5,8 @@ import Button from 'antd/es/button';
import Typography from 'antd/es/typography';
import message from 'antd/es/message';
import Card from 'antd/es/card';
import Space from 'antd/es/space';
import Input from 'antd/es/input';
import { fetchAdminSettings, updateAdminSettings } from './adminSettingsApi';
const { Title, Text } = Typography;
@@ -12,12 +14,11 @@ const { Title, Text } = Typography;
export default function Settings() {
const [form] = Form.useForm();
const [loading, setLoading] = useState(true);
const [saving, setSaving] = useState(false);
useEffect(() => {
fetchAdminSettings()
.then(s => {
form.setFieldsValue({ cartExpiryHours: s.cartExpiryHours });
})
.then(s => form.setFieldsValue(s))
// A rejection here used to leave the form spinning indefinitely.
.catch(() => message.error('Could not load settings'))
.finally(() => setLoading(false));
@@ -25,22 +26,97 @@ export default function Settings() {
async function handleSave() {
const values = await form.validateFields();
setSaving(true);
try {
await updateAdminSettings(values);
message.success('Settings saved');
} catch (err) {
// Reported rather than swallowed: the previous version announced success
// whatever the server said.
message.error((err as Error).message);
} finally {
setSaving(false);
}
}
// One form across both cards, so Save commits every field rather than each
// card needing a button of its own.
return (
<Card style={{ maxWidth: 480 }}>
<Title level={4}>Cart Settings</Title>
<Form form={form} layout="vertical" disabled={loading}>
<Space direction="vertical" size={16} style={{ display: 'flex', maxWidth: 480 }}>
<Card>
<Title level={4}>Cart</Title>
<Text type="secondary">
How long an item stays reserved in a customer's cart before it's automatically released back to available inventory.
</Text>
<Form form={form} layout="vertical" style={{ marginTop: 16 }} disabled={loading}>
<Form.Item name="cartExpiryHours" label="Cart expiry (hours)" rules={[{ required: true }]}>
<Form.Item
name="cartExpiryHours"
label="Cart expiry (hours)"
rules={[{ required: true }]}
style={{ marginTop: 16, marginBottom: 0 }}
>
<InputNumber min={0.5} step={0.5} style={{ width: '100%' }} />
</Form.Item>
<Button type="primary" onClick={handleSave} loading={loading}>Save</Button>
</Form>
</Card>
<Card>
<Title level={4}>Link lifetimes</Title>
{/* The emails state these durations from a placeholder, so changing a
value here changes what the customer is told. That is the point:
the wording used to be a second, hand-written copy of the number. */}
<Text type="secondary">
How long the links in the verification and password reset emails stay valid. The emails state these
durations, so they follow whatever is set here.
</Text>
<Form.Item
name="verifyTokenHours"
label="Email verification link (hours)"
rules={[{ required: true }]}
style={{ marginTop: 16 }}
>
<InputNumber min={0.25} step={0.5} style={{ width: '100%' }} />
</Form.Item>
<Form.Item
name="passwordResetHours"
label="Password reset link (hours)"
rules={[{ required: true }]}
style={{ marginBottom: 0 }}
>
<InputNumber min={0.25} step={0.5} style={{ width: '100%' }} />
</Form.Item>
</Card>
<Card>
<Title level={4}>Greeting</Title>
{/* Two fields rather than one. Editing the name out of a format for
customers who have none is guesswork that has to be right every
time, and getting it wrong ships "Hi ," — the exact failure #106
was about. An admin writes both and neither is guessed. */}
<Text type="secondary">
What <code>{'{{greeting}}'}</code> becomes in every email. Use <code>{'{{firstName}}'}</code> and{' '}
<code>{'{{lastName}}'}</code> in the format. The fallback is used whole for customers who registered
without a first name.
</Text>
<Form.Item
name="greetingFormat"
label="Greeting format"
rules={[{ required: true, message: 'A greeting format is required' }]}
style={{ marginTop: 16 }}
>
<Input placeholder="Hi {{firstName}}," />
</Form.Item>
<Form.Item
name="greetingFallback"
label="Fallback, when there is no first name"
rules={[{ required: true, message: 'A fallback greeting is required' }]}
style={{ marginBottom: 0 }}
>
<Input placeholder="Hi," />
</Form.Item>
</Card>
<Button type="primary" onClick={handleSave} loading={loading || saving}>Save</Button>
</Space>
</Form>
);
}
+11 -1
View File
@@ -1,9 +1,14 @@
export interface AdminSettings {
cartExpiryHours: number;
verifyTokenHours: number;
passwordResetHours: number;
greetingFormat: string;
greetingFallback: string;
}
export async function fetchAdminSettings(): Promise<AdminSettings> {
const res = await fetch('/api/admin/settings');
if (!res.ok) throw new Error('Could not load settings');
return res.json();
}
@@ -13,5 +18,10 @@ export async function updateAdminSettings(settings: AdminSettings): Promise<Admi
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(settings)
});
return res.json();
const body = await res.json();
// The refusal names the field that was rejected, and it is the only useful
// thing to say. Without this check a 400 was returned as though it were the
// saved settings, and the form reported success for a save the server refused.
if (!res.ok) throw new Error(body?.error || 'Could not save settings');
return body;
}
@@ -0,0 +1,80 @@
import { test, expect } from './fixtures';
// These edit global settings that other specs depend on — cart expiry above
// all — so they run serially and put everything back afterwards.
test.describe.configure({ mode: 'serial' });
const DEFAULTS = {
cartExpiryHours: 24,
verifyTokenHours: 24,
passwordResetHours: 1,
greetingFormat: 'Hi {{firstName}},',
greetingFallback: 'Hi,'
};
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: 'Link lifetimes' })).toBeVisible();
}
test.describe('The email settings', () => {
test.afterEach(async ({ page }) => {
await page.request.put('/api/admin/settings', { data: DEFAULTS });
});
test('offers the link lifetimes and the greeting alongside cart expiry', async ({ page }) => {
await openSettings(page);
// antd renders a stepped InputNumber to the step's precision, so "24.0".
await expect(page.getByLabel('Cart expiry (hours)')).toHaveValue('24.0');
await expect(page.getByLabel('Email verification link (hours)')).toHaveValue('24.0');
await expect(page.getByLabel('Password reset link (hours)')).toHaveValue('1.0');
await expect(page.getByLabel('Greeting format')).toHaveValue('Hi {{firstName}},');
await expect(page.getByLabel('Fallback, when there is no first name')).toHaveValue('Hi,');
});
test('saves a new lifetime, and the server keeps it', async ({ page }) => {
await openSettings(page);
await page.getByLabel('Password reset link (hours)').fill('3');
await page.getByRole('button', { name: 'Save' }).click();
await expect(page.getByText('Settings saved')).toBeVisible();
const stored = await (await page.request.get('/api/admin/settings')).json();
expect(stored.passwordResetHours).toBe(3);
});
// The whole reason the placeholder exists: the sentence in the email is
// rendered from the setting rather than written out beside it.
test('the password reset preview states the configured lifetime', async ({ page }) => {
await page.request.put('/api/admin/settings', { data: { passwordResetHours: 2 } });
await page.goto('/admin');
await page.getByRole('tab', { name: 'Emails' }).click();
await page.getByRole('tab', { name: /Password reset/ }).click();
const preview = page.frameLocator('iframe[title="Password reset preview"]');
await expect(preview.getByText('2 hours')).toBeVisible();
});
test('the preview greets through the configured format', async ({ page }) => {
await page.request.put('/api/admin/settings', { data: { greetingFormat: 'Salutations {{firstName}}!' } });
await page.goto('/admin');
await page.getByRole('tab', { name: 'Emails' }).click();
await page.getByRole('tab', { name: /Email verification/ }).click();
const preview = page.frameLocator('iframe[title="Email verification preview"]');
await expect(preview.getByText('Salutations Ada!')).toBeVisible();
});
test('refuses a lifetime of zero rather than reporting a save', async ({ page }) => {
await openSettings(page);
await page.getByLabel('Password reset link (hours)').fill('0');
await page.getByRole('button', { name: 'Save' }).click();
await expect(page.getByText('Settings saved')).toHaveCount(0);
});
});