docs(website): the template editor as built (engagement Phase 5b)
ENGAGEMENT.md gains an "As built - 5b" section and BACKEND_DESIGN.md the eight routes under /admin/engagement. The scope decision is recorded first because the plan contradicted itself: the Phase 5 body names only the editor, while Q4's answer and 6.2 both promise "Triggers, Templates and the send log" in Phase 5. All three shipped - leaving either out would have left the nav group half-built and G15 open with the rows already on disk. Five more decisions, each because the tree said something the plan did not: the preview is rendered server-side and framed; `status` is now enforced by renderByKey; a test send is logged under a synthetic trigger rather than making the column nullable; a template a rule uses refuses deletion with a 409; and duplicate is the only creation path. Also records the correction that changed the most code - 4.6.2 says duplicate is how a protected template is customized, the schema says "Editable, NOT deletable", and the org lead's ruling is the schema's - and the three things only building it found, including the Phase 4a template-key pattern that could not match any key this system uses. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -180,16 +180,19 @@ server/
|
||||
from a manifest URL, enable,
|
||||
disable, uninstall, purge, restart
|
||||
and the source allowlist
|
||||
engagement.router.js (14) /admin/engagement — adminOnly,
|
||||
engagement.router.js (22) /admin/engagement — adminOnly,
|
||||
the declared event catalog (three
|
||||
table-free reads, served from the
|
||||
module registries) plus the rules
|
||||
and audience segments an operator
|
||||
configures over it, and the
|
||||
count-only reach preview. Templates
|
||||
and the send log land under this
|
||||
same prefix in ENGAGEMENT.md
|
||||
Phase 5
|
||||
configures over it, the count-only
|
||||
reach preview, the message
|
||||
templates and their sandboxed
|
||||
preview / test send, and the send
|
||||
log (G15). Two of these are POSTs
|
||||
that write nothing: preview and
|
||||
test-send act on the draft in the
|
||||
request, not the stored row
|
||||
email.router.js (4) /admin/email — outbound mail:
|
||||
transport + credentials + send
|
||||
test — adminOnly. The two
|
||||
@@ -1306,6 +1309,14 @@ file a route sits in — that is the property the route manifest freezes.
|
||||
| PATCH | `/engagement/rules/:id/enabled` | flip that column and no other, **without re-validating the rule**. Turning a rule off is the panic button: a rule whose module has been uninstalled, or whose trigger has since narrowed its ceiling under a saved audience, is the rule an operator most urgently wants stopped and the one a re-validating `PUT` refuses to save. Turning one on is safe unvalidated because the engine re-checks the ceiling at send time |
|
||||
| GET · POST | `/engagement/segments` | list every saved audience segment annotated with dormancy (and which audience ids are missing), or save a new one. The stored `ceiling` is **derived** as the narrowest in the expression and is never taken from the caller; `not` is legal only as a child of `and`; two incomparable ceilings have no meet and the composition is refused rather than guessed. `adminOnly`. See [ENGAGEMENT.md](ENGAGEMENT.md) §5.1a |
|
||||
| PUT · DELETE | `/engagement/segments/:id` | update (re-deriving the ceiling) or delete. **`409` while any rule still points at it**, with the count in the message. No foreign key does this on purpose: `CASCADE` would delete an operator's rules and `SET NULL` would silently fall each rule back to its plain `audience` column, which reaches a *different set of people* |
|
||||
| GET | `/engagement/templates` | every message template, each annotated with three separately-meaningful warnings: `dormant` (pinned to a trigger no installed module declares, so its variables cannot be checked and nothing will send it), `triggerBehind` (the module is installed but its declaration has moved on past the version this template was authored against) and `seedBehind` (a newer shipped default exists and was **not** applied, because a person had edited this row). `adminOnly`. See [ENGAGEMENT.md](ENGAGEMENT.md) §4.6.2 |
|
||||
| GET | `/engagement/templates/:id` | one template plus `variables` — the palette the editor offers, resolved from the trigger declaration or, for a template tied to no trigger, from the shipped seed, merged with the ambient variables every template may use. Served with the row so the editor never guesses what is legal |
|
||||
| PUT | `/engagement/templates/:id` | edit any template, **including a shipped default, in place**: the save sets `customized = 1`, which is what stops the next seed bump from taking the edit back. `key` and `channel` are **immutable and the attempt is refused rather than ignored** — `mailer` renders by key, so a rename would break the message it names with no error anywhere. Two refusals are the point of the route: a token (or an `email.itemList` naming a bare variable) referencing something the trigger does not declare is refused **with the variable named**, and a `published` template whose plain-text part renders empty is refused — checked by *rendering* with the declared examples, because whether a text part exists depends on what each block's `toText` does with these props |
|
||||
| POST | `/engagement/templates/:id/duplicate` | the **only** way a template that is not a shipped seed comes into being, so every template on a deployment descends from one that renders. The copy always starts as a `draft`, is never `protected`, and **inherits the source's `seed_key`** — that is what carries its variable palette, not bookkeeping: a seedless, triggerless copy would resolve to the ambient variables alone and be refused for the tokens it was copied with. `409` on a taken key |
|
||||
| DELETE | `/engagement/templates/:id` | `409` for a `protected` template — the system breaks without a password-reset body, so those are editable and not deletable — and `409` while any rule's `template_keys` points at the key, **naming the rules**. The same answer a segment in use gets, for the same reason: the alternative is a rule that silently stops producing mail |
|
||||
| POST | `/engagement/templates/:id/preview` | renders the body **in the request**, not the stored row, using each variable's declared `example` — which is why `example` is a required part of a trigger declaration rather than documentation. A `POST` that writes nothing: an editor that could only preview what was already saved would make saving the way to find out whether a change was right. Returns both parts as JSON strings; the client renders the HTML inside `<iframe sandbox="" srcdoc>` with **no `allow-scripts`**. Serving it as a document from this origin would run operator-authored HTML under the site's own CSP with access to its cookies |
|
||||
| POST | `/engagement/templates/:id/test-send` | sends what is on screen, saved or not, through the configured transport, and records the attempt in `engagement_sends` **including when it fails** — the outcome an operator most needs a record of. `trigger_id` is `NOT NULL` and a transactional template has no trigger, so the row is logged under the synthetic **`core.admin.test-send`**, which is deliberately not a registered trigger. It does not consult channel preferences or the suppression list: the address is typed by an admin about their own deployment and is not derived from a user. `409` when mail is unconfigured, `502` when the relay refuses |
|
||||
| GET | `/engagement/sends` | the send log, newest first, paged (`limit` 1–200, `offset`) and filterable by `triggerId`, `ruleId`, `userId` and `status`, with a `total` matching the same filters. **G15's answer.** `address_hash` is stored but **never returned**: the log keeps it so a bounce can be correlated back to a recipient (Phase 9) without becoming a second address book, and shipping it to a browser would turn a delivery screen into an offline dictionary attack against every address on the deployment |
|
||||
| GET | `/teams` | every Team incl. hidden ones, plus the module's **sync state verbatim** — last attempt, last success, consecutive failures, the last error and any held empty answer. Verbatim because an operator debugging a stale projection needs what the provider actually said |
|
||||
| GET | `/teams/:id` | one Team with its roster (departed members included), its grant ledger and its pending requests. Each roster row carries the **resolved** leadership and `isLeaderSynced` — what the game actually said — so an override reads as a decision rather than as fact |
|
||||
| POST | `/teams/resync` | run a reconciliation now, **awaited**, so the response carries the outcome including the provider's own refusal reason. The four refusal gates still apply: a manual resync cannot make core act on an answer it does not trust |
|
||||
|
||||
@@ -2070,7 +2070,7 @@ refuses anything else, and it also refuses an id with no dot in it at all.
|
||||
|
||||
---
|
||||
|
||||
### Phase 5 — Templates: the seeded set, then the editor
|
||||
### Phase 5 — Templates: the seeded set, then the editor ✅
|
||||
|
||||
Two slices, landing in this order **on purpose** — the seeded set has to exist before the editor, so the
|
||||
editor is opening something rather than facing a blank page.
|
||||
@@ -2081,9 +2081,12 @@ transactional bodies move out of `mailer.js` into seeded rows and `mailer` rende
|
||||
yet** — this slice is provably done when the same mail goes out from a template that used to come from a
|
||||
string literal.
|
||||
|
||||
**5b — the editor.** The §4.6.2 surface: variable palette from the trigger declaration, live preview
|
||||
from `example` values, side-by-side HTML/text, three preview widths plus dark mode, test send, duplicate.
|
||||
Built on the existing block/prop-panel machinery, not a second one.
|
||||
**5b — the editor.** ✅ The §4.6.2 surface: variable palette from the trigger declaration, live preview
|
||||
from `example` values, side-by-side HTML/text, preview widths plus dark mode, test send, duplicate.
|
||||
Built on the existing block/prop-panel machinery, not a second one — and, because a mail body is rendered
|
||||
by the SERVER, the preview is rendered there too and framed rather than redrawn in React. It also lands
|
||||
the other two screens Q4 promised Phase 5: **Triggers** (read-only, from the registries) and the
|
||||
**Send Log** (G15).
|
||||
|
||||
**Acceptance (5a):** every one of the five current message types renders byte-comparably from its seeded
|
||||
template; re-running the seeder is a no-op; a seeder bump updates a `customized = 0` row and **skips** a
|
||||
@@ -2242,6 +2245,94 @@ accept and two real `.email` hosts it must still catch.
|
||||
undeclared-variable refusal (`variablesFor` is in place and is what it will ask), the sandboxed preview
|
||||
and its CSP test, and the `runicgateway.com` admin docs page §6.0b assigns the pair.
|
||||
|
||||
#### As built — 5b (2026-08-29)
|
||||
|
||||
Built as website#TBD. **Q4 was already settled** (Phase 4a), so the only thing needing a decision before
|
||||
code was the scope: §Phase 5's body names the editor and nothing else, but Q4's answer and §6.2 both
|
||||
promise "Triggers, Templates and the send log" in Phase 5. **All three shipped.** Triggers is a
|
||||
read-only render of two endpoints Phase 2 already serves and cost no server work; the send log is one
|
||||
paged route over a table that has been filling since Phase 4a and whose index was built for it. Leaving
|
||||
either out would have left the nav group half-built and **G15 — "no send log, no delivery status, no
|
||||
audit" — open with the rows already on disk.**
|
||||
|
||||
Five more decisions were settled by the org lead before any code, each because the tree contradicted
|
||||
the plan or the plan contradicted itself.
|
||||
|
||||
| | What the tree said | Decision |
|
||||
| --- | --- | --- |
|
||||
| **Where the preview comes from** | The client block registry mirrors the server's, but its entry shape carries a **React `component`** — page blocks are drawn in the browser. Email blocks are drawn on the SERVER; a mail body is a string this process produces | **A server preview route**, rendered into a sandboxed iframe. A React preview would be a second renderer for one artifact, agreeing with the send path on the day it was written and drifting from the first Outlook fix onward |
|
||||
| **`status` was unenforced** | `draft`/`published` shipped in 5a and **nothing read it** — `getByKey` returns any row, so an operator who saved a template as a draft kept mailing it | **`renderByKey` requires `published`** and otherwise falls back to the shipped seed, the posture 5a already built for a missing or unusable row. A draft now means what the word means |
|
||||
| **Test send vs. `trigger_id NOT NULL`** | §4.6.2 wants a test send "recorded in `engagement_sends` like any other message", but every transactional template has `trigger_id` NULL and there was nothing honest to put in the column | **A synthetic `core.admin.test-send` id.** No schema change, no nullable column, and the log keeps meaning one thing. It is deliberately not a registered trigger, and the screen renders it by name so nobody goes looking for it in the catalog |
|
||||
| **Deleting** | `protected` blocks deletion. Nothing stopped deleting a template a rule's `template_keys` points at | **409 while a rule uses it, naming the rules** — the answer Phase 4b already gives for a segment in use, for the same reason: the alternative is a rule that silently stops producing mail |
|
||||
| **Creating** | §4.6.2 names duplicate and never mentions a blank create | **Duplicate only.** Every template on a deployment descends from a shipped one that renders, which is the whole reason 5a landed first |
|
||||
|
||||
##### The correction that changed the most code
|
||||
|
||||
§4.6.2 introduces duplicate as "how an operator customizes a `protected` template safely: duplicate,
|
||||
edit, point the rule at the copy, leave the original intact". The schema comment written in 5a says the
|
||||
opposite — "Editable, NOT deletable" — and **the org lead's ruling is the schema's: a default template
|
||||
is edited in place.** `customized = 1` is what stops the next seed bump from taking that edit back, and
|
||||
it has been in the UPDATE's own WHERE since 5a. So `protected` now blocks deletion and *nothing else*,
|
||||
and duplicate is how a NEW template comes into being rather than how an existing one is customized.
|
||||
|
||||
##### Three things the plan did not know, found by building it
|
||||
|
||||
**1. The variable check cannot be a token scan, because of one block.** §4.6.2's refusal — "a template
|
||||
referencing an undeclared variable is refused at save with the variable named" — reads as a scan for
|
||||
`{{name}}`. It is not sufficient. `email.itemList.variable` holds a **bare name** (`items`), because the
|
||||
block iterates the value rather than interpolating it; a token scan sees nothing there. A digest pointed
|
||||
at `itmes` would have saved clean and arrived empty, which is the one variable mistake a reader of the
|
||||
template cannot see. Blocks now optionally declare `variables(props)` in the registry — `itemList` is the
|
||||
only one that does — and `emailBlocks/variables.js` walks tokens *and* declarations across the subject,
|
||||
the text override and every block prop. The editor makes that field a `<select>` over the trigger's list
|
||||
variables rather than a text input, so the mistake is unavailable in the first place.
|
||||
|
||||
**2. A duplicate that drops its seed reference cannot be saved.** The obvious shape for `create` was
|
||||
`seed_key = NULL` — a copy is not a seed and must never be adopted by the seeder. But
|
||||
`templates.variablesFor()` resolves the palette from the trigger *or, for the generic templates tied to
|
||||
no trigger, from the seed*, so a seedless, triggerless copy has only the four ambient variables, and
|
||||
**duplicating `notify.event` would have been refused for the `{{title}}` and `{{intro}}` it was copied
|
||||
with** — the one action §4.6.2 offers, refusing itself. The copy inherits `seed_key`, and it is safe to
|
||||
because `customized = 1` is what the seeder actually reads: `seedOne`'s UPDATE carries `AND customized =
|
||||
0`, so it can only ever match the seeded row. `staleCustomized` *does* match a copy, and should — "the
|
||||
default you duplicated has been improved" is worth telling someone.
|
||||
|
||||
**3. `validateEmailBlocks` returns `{ valid, errors }`, not an array** — and the first version of the
|
||||
model tested it with `.length`, which is `undefined` on an object, so **block validation never ran at
|
||||
all**. Unvalidated props would have reached both the renderer and the row. Nothing about the code looked
|
||||
wrong; the test that caught it was the one asserting a preview refuses a block with an unknown prop.
|
||||
`pages.model.js` destructures it, which is why that caller has never had the bug.
|
||||
|
||||
##### The one genuinely new CSP surface, and how it is held
|
||||
|
||||
§4.6.2 called the preview out as "the one genuinely new CSP surface", and it is: operator-authored HTML
|
||||
reaching a rendered surface. Three things hold it, in order of what they cost to remove:
|
||||
|
||||
- The preview renders in `<iframe sandbox="" srcDoc={…}>` — **every restriction on, nothing granted
|
||||
back**, from an opaque origin. No `allow-scripts`, no `allow-same-origin`.
|
||||
- The HTML crosses as a **JSON string**, never as a document served from this origin. That is not a
|
||||
convenience: served as a document it would run under the site's own CSP with access to its cookies.
|
||||
- Blocks are validated then sanitized **on write**, against the `email.*` registry, so the stored row is
|
||||
already through the gate before any of the above matters.
|
||||
|
||||
The sibling test §4.6.2 asked for is `client/test/emailTemplates.test.js`. It reads the **attribute**,
|
||||
not the file — the first version searched the source for `allow-scripts` and failed on the comment above
|
||||
the iframe explaining that there is no `allow-scripts`. A check a correct file fails is worse than no
|
||||
check, because the way to make it pass is to delete the explanation.
|
||||
|
||||
##### What was verified
|
||||
|
||||
- **23 model tests** (`server/test/engagementTemplatesAdmin.test.js`), one per acceptance criterion plus
|
||||
the two the tree made necessary. Server suite 1403, client 324, both green.
|
||||
- **The drift check was verified by breaking it both ways** — a client `version: 2` against the server's
|
||||
`1`, and deleting a registration outright — because a pairing check that spans a process boundary is
|
||||
the kind that silently stops checking.
|
||||
- **`renderByKey`'s draft arm has its own test**, asserting the seed goes out and the draft's words do
|
||||
not; the 5a fixture that carried no `status` was updated rather than the rule being softened.
|
||||
|
||||
**Still Phase 6's:** the email channel's `deliver` on the engine, and the `teamNotify` /
|
||||
`teamDigestWorker` migration onto the four `notify.*` templates this phase can now edit.
|
||||
|
||||
---
|
||||
|
||||
### Phase 6 — The email channel on the engine, and the Teams migration
|
||||
|
||||
Reference in New Issue
Block a user