feat(admin): make the token lifetimes, cart hold and greeting configurable (#136)
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:
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
if (Number.isNaN(hours) || hours <= 0) {
|
||||
return res.status(400).json({ error: 'cartExpiryHours must be a positive number' });
|
||||
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: `${name} must be a positive number` });
|
||||
}
|
||||
values[name] = hours;
|
||||
}
|
||||
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 });
|
||||
|
||||
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;
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user