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
277 lines
9.9 KiB
TypeScript
277 lines
9.9 KiB
TypeScript
import {
|
|
TEMPLATES,
|
|
TemplateKey,
|
|
missingPlaceholders,
|
|
renderTemplate,
|
|
formatDuration,
|
|
greeting,
|
|
SAMPLE_VALUES
|
|
} from '../../src/emailTemplates';
|
|
|
|
const KEYS: TemplateKey[] = [
|
|
'verification',
|
|
'passwordReset',
|
|
'favoriteSold',
|
|
'favoriteWithdrawn',
|
|
'cartReminder',
|
|
'emailChanged'
|
|
];
|
|
|
|
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('<strong>this</strong>');
|
|
expect(html).toContain('<a href="https://shop.test/reset-password?token=abc"');
|
|
});
|
|
|
|
// The reason markdown-it runs with html disabled. An admin editing copy must
|
|
// not be able to put script into a customer's inbox, and sanitising after the
|
|
// fact is a weaker guarantee than never emitting it.
|
|
it('escapes raw HTML in a stored body rather than passing it through', () => {
|
|
const { html } = renderTemplate(
|
|
'passwordReset',
|
|
{ body: '<script>alert(1)</script> [link]({{resetUrl}})' },
|
|
resetValues
|
|
);
|
|
expect(html).not.toContain('<script>');
|
|
expect(html).toContain('<script>');
|
|
});
|
|
|
|
it('falls back to the built-in body when nothing is stored', () => {
|
|
const stored = renderTemplate('passwordReset', {}, resetValues);
|
|
const explicit = renderTemplate(
|
|
'passwordReset',
|
|
{ body: TEMPLATES.passwordReset.defaultBody },
|
|
resetValues
|
|
);
|
|
expect(stored.html).toBe(explicit.html);
|
|
});
|
|
|
|
it('uses a stored subject over the default, and substitutes into it', () => {
|
|
const { subject } = renderTemplate(
|
|
'favoriteSold',
|
|
{ subject: '{{itemName}} is gone' },
|
|
{ itemName: 'Oak table', siteUrl: 'https://shop.test' }
|
|
);
|
|
expect(subject).toBe('Oak table is gone');
|
|
});
|
|
|
|
// Values are substituted into the markdown source, so a value that needs to
|
|
// become a list has to arrive as markdown. Emitting HTML here would be
|
|
// escaped and shown to the customer as literal tags.
|
|
it('renders a markdown list supplied as a placeholder value', () => {
|
|
const { html } = renderTemplate(
|
|
'cartReminder',
|
|
{},
|
|
{
|
|
greeting: 'Hi Thom,',
|
|
itemList: '- Oak table\n- Brass lamp',
|
|
cartUrl: 'https://shop.test/cart'
|
|
}
|
|
);
|
|
expect(html).toContain('<ul>');
|
|
expect(html).toContain('<li>Oak table</li>');
|
|
});
|
|
|
|
// The consent sentence explains why the email is lawful to send. It is
|
|
// appended by the server precisely so that editing the copy cannot remove it.
|
|
it.each(['favoriteSold', 'favoriteWithdrawn'] as TemplateKey[])(
|
|
'appends the unremovable consent footer to %s',
|
|
(key) => {
|
|
const { html } = renderTemplate(
|
|
key,
|
|
{ body: 'Short replacement copy about {{itemName}}.' },
|
|
{ itemName: 'Oak table', siteUrl: 'https://shop.test' }
|
|
);
|
|
expect(html).toContain('favorited');
|
|
expect(html).toContain('account page');
|
|
}
|
|
);
|
|
|
|
it('does not append that footer to templates it does not belong to', () => {
|
|
const { html } = renderTemplate('passwordReset', {}, resetValues);
|
|
expect(html).not.toContain('account page');
|
|
});
|
|
|
|
// An unsubstituted placeholder in the output means a caller forgot a value,
|
|
// and shipping "{{resetUrl}}" to a customer is worse than failing.
|
|
it('leaves no unsubstituted placeholders when every value is supplied', () => {
|
|
const { html, subject } = renderTemplate(
|
|
'cartReminder',
|
|
{},
|
|
{
|
|
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*\}\}/);
|
|
});
|
|
});
|
|
|
|
describe('SAMPLE_VALUES, which the admin preview renders with', () => {
|
|
// A missing sample renders the preview with a literal {{placeholder}} in it,
|
|
// which teaches the admin their copy is broken when it is not. Adding a
|
|
// placeholder to a template must mean adding a sample for it.
|
|
it.each(KEYS)('%s has a sample for every placeholder it accepts', (key) => {
|
|
const missing = TEMPLATES[key].available.filter((name) => !(name in SAMPLE_VALUES));
|
|
expect(missing).toEqual([]);
|
|
});
|
|
|
|
it.each(KEYS)('%s previews with no placeholder left unsubstituted', (key) => {
|
|
const { html, subject } = renderTemplate(key, {}, SAMPLE_VALUES);
|
|
expect(html).not.toMatch(/\{\{\s*\w+\s*\}\}/);
|
|
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'])
|
|
);
|
|
});
|
|
});
|