feat(engagement): the template editor, the trigger catalog and the send log (engagement Phase 5b)
All checks were successful
PR Checks / bot-tests (pull_request) Successful in 27s
PR Checks / client-build (pull_request) Successful in 31s
PR Checks / server-tests (pull_request) Successful in 2m38s

Phase 5a gave templates a table, a renderer and nine seeded rows; nothing could
change one. This is the screen that lets an operator change one without being able
to break the mail the system depends on — plus the two screens Q4 promised Phase 5:
Triggers (read-only, from the registries) and the Send Log, which closes G15.

The shape follows from one fact: a mail body is rendered by the SERVER, so the
preview is too, and framed rather than redrawn in React. A client-side renderer
would be a second implementation of the one artifact that matters, agreeing with
the send path on the day it was written and drifting from the first Outlook fix on.

Settled with the org lead before any code: a shipped default is edited IN PLACE
(`protected` blocks deletion and nothing else, `customized = 1` keeps the edit);
duplicate is the only way to a new template; `renderByKey` now requires
`published`; a test send is logged under a synthetic `core.admin.test-send`; and a
template a rule points at refuses deletion with a 409 naming the rules.

Three things the plan did not know, found by building it:

  - The undeclared-variable check cannot be a token scan. `email.itemList.variable`
    holds a BARE name, so a digest pointed at `itmes` would have saved clean and
    arrived empty. Blocks now declare `variables(props)`; the editor makes that
    field a select over the trigger's list variables so the typo is unavailable.
  - A duplicate that drops `seed_key` loses its variable palette, so duplicating
    `notify.event` would have been refused for the tokens it was copied with — the
    one action §4.6.2 offers, refusing itself. The copy inherits it; `customized`
    is what the seeder actually reads.
  - `validateEmailBlocks` returns `{ valid, errors }`, not an array, and the first
    version tested it with `.length` — so block validation never ran at all.

Also fixes a Phase 4a defect the live walk found, with the org lead's approval: a
rule's template key was checked against a pattern with no dot in it, so no rule
could name any template that exists — §4.6.2's whole duplicate-and-point-a-rule-at-it
workflow was unreachable. Both models now read one pattern.

Verified against the running stack: real multipart mail into a mailpit catcher
including an unsaved draft, the draft/published arms both ways through the real
mailer path, every refusal, and the end-to-end duplicate → rule → 409 walk.
Server 1428 tests green, client 324.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-29 18:13:57 -05:00
parent 42b40fdec2
commit 3f90070566
27 changed files with 3754 additions and 20 deletions

View File

@@ -0,0 +1,154 @@
import { test, beforeEach } from 'node:test'
import assert from 'node:assert/strict'
import fs from 'node:fs'
import path from 'node:path'
import { fileURLToPath } from 'node:url'
import {
RESERVED_KEYS,
registerEmailBlock,
getEmailBlock,
listEmailBlocks,
newEmailBlock,
} from '../src/emailBlocks/registry.js'
// Engagement Phase 5b — the client half of the template editor.
//
// Two kinds of test, and the second kind is the one worth explaining.
//
// `registry.js` is plain `.js` and imports nothing, so it is exercised directly.
// `types.jsx` and `EngagementTemplates.jsx` cannot be: this runner has no JSX
// transform and no DOM, the same limit `moduleRegistry.test.js` documents. So the
// properties that live in those files are asserted **against their source text**.
//
// That is a weaker test than executing them, and it is used for exactly two things
// where a weak test still beats none:
//
// • **The preview sandbox.** `sandbox=""` with no `allow-scripts` is the reason
// operator-authored HTML cannot run under this site's origin. It is one
// attribute, on one element, and it is precisely the sort of thing someone
// removes to debug a rendering problem and does not put back. A source
// assertion catches that in review; nothing else here would.
// • **Registry drift.** Every `email.*` type this client offers must exist in
// the server registry with the same version, because the server validates
// against its own and a drifted client produces a refused save with no
// explanation on screen. Reading both trees is the only way to check a
// pairing that spans a process boundary.
const here = path.dirname(fileURLToPath(import.meta.url))
const read = (rel) => fs.readFileSync(path.join(here, '..', rel), 'utf8')
// The registry is module state; each test starts from a known entry.
beforeEach(() => {
if (!getEmailBlock('email.test')) {
registerEmailBlock({
type: 'email.test',
version: 2,
label: 'Test block',
defaults: () => ({ text: 'hi' }),
editor: () => null,
})
}
})
// ── The registry ───────────────────────────────────────────────────────────
test('a definition must be namespaced "email."', () => {
assert.throws(() => registerEmailBlock({ type: 'heading' }), /namespaced/)
assert.throws(() => registerEmailBlock({}), /namespaced/)
})
test('a duplicate type is a programmer error, caught at import', () => {
assert.throws(() => registerEmailBlock({ type: 'email.test' }), /already registered/)
})
test('a new block carries the envelope the server expects, and a unique id', () => {
const a = newEmailBlock('email.test')
const b = newEmailBlock('email.test')
assert.deepEqual(Object.keys(a).sort(), [...RESERVED_KEYS].sort())
assert.equal(a.type, 'email.test')
assert.equal(a.version, 2)
assert.deepEqual(a.props, { text: 'hi' })
// Ids are unique across a whole document. A counter would re-issue an id after
// a delete and the save would be refused for a reason nothing on screen explains.
assert.notEqual(a.id, b.id)
})
test('an unknown type yields nothing rather than a half-built block', () => {
assert.equal(newEmailBlock('email.nope'), null)
assert.equal(getEmailBlock('email.nope'), null)
})
// ── The sandbox: §4.6.2's security posture, as an attribute ────────────────
test('the preview frame is sandboxed with no allow-scripts', () => {
const source = read('src/routes/admin/views/EngagementTemplates.jsx')
// It renders in an iframe at all — not into the page.
assert.match(source, /<iframe/)
// Read the ATTRIBUTE, not the file. The first version of this test searched the
// whole source for "allow-scripts" and failed on the comment above the iframe
// explaining that there is no allow-scripts — a check that a correct file fails
// is worse than no check, because the fix is to delete the explanation.
const sandboxes = [...source.matchAll(/sandbox=(?:"([^"]*)"|\{([^}]*)\})/g)].map((m) => m[1] ?? m[2])
assert.equal(sandboxes.length, 1, 'expected exactly one sandboxed frame')
// Empty: every restriction on, nothing granted back.
assert.equal(sandboxes[0], '')
// The two grants that would undo it, whatever else were listed.
assert.doesNotMatch(sandboxes[0], /allow-scripts/)
assert.doesNotMatch(sandboxes[0], /allow-same-origin/)
// And no iframe without one at all.
assert.equal((source.match(/<iframe/g) || []).length, sandboxes.length)
// From srcDoc — an opaque origin — rather than a src pointing at this site.
assert.match(source, /srcDoc=/)
})
test('the preview HTML is never injected into this document', () => {
const source = read('src/routes/admin/views/EngagementTemplates.jsx')
// The one API that would undo all of the above in a single line.
assert.doesNotMatch(source, /dangerouslySetInnerHTML/)
})
// ── Drift between the two registries ───────────────────────────────────────
test('every client email block pairs with a server definition at the same version', () => {
const clientSource = read('src/emailBlocks/types.jsx')
const clientTypes = [...clientSource.matchAll(/type:\s*'(email\.[A-Za-z]+)',\s*\n\s*version:\s*(\d+)/g)].map(
(m) => [m[1], Number(m[2])],
)
assert.ok(clientTypes.length >= 6, 'expected the six block definitions to be found')
const serverDir = path.join(here, '..', '..', 'server', 'src', 'emailBlocks', 'types')
const serverTypes = new Map()
for (const file of fs.readdirSync(serverDir)) {
const src = fs.readFileSync(path.join(serverDir, file), 'utf8')
const type = src.match(/type:\s*'(email\.[A-Za-z]+)'/)
const version = src.match(/\n\s*version:\s*(\d+)/)
if (type) serverTypes.set(type[1], version ? Number(version[1]) : 1)
}
for (const [type, version] of clientTypes) {
assert.ok(serverTypes.has(type), `${type} has no server definition`)
assert.equal(serverTypes.get(type), version, `${type} version differs between client and server`)
}
// And the other direction: a server block with no authoring form is a block an
// operator can be sent a template containing and cannot edit.
for (const type of serverTypes.keys()) {
assert.ok(
clientTypes.some(([t]) => t === type),
`${type} exists on the server but has no editor in this client`,
)
}
})
test('no client email block declares a React renderer', () => {
// The structural claim in registry.js's header. A `component` here would be a
// second renderer for a body the server produces, and the two would agree only
// until the first Outlook fix.
const clientSource = read('src/emailBlocks/types.jsx')
assert.doesNotMatch(clientSource, /\n\s*component:/)
assert.ok(listEmailBlocks().every((d) => !('component' in d)))
})