diff --git a/.planning/phases/35-mail-templates-and-layouts-editor/35-RESEARCH.md b/.planning/phases/35-mail-templates-and-layouts-editor/35-RESEARCH.md new file mode 100644 index 00000000..44ab4ec8 --- /dev/null +++ b/.planning/phases/35-mail-templates-and-layouts-editor/35-RESEARCH.md @@ -0,0 +1,705 @@ +# Phase 35: Mail templates and layouts editor - Research + +**Researched:** 2026-11-06 +**Domain:** postcard mail rendering with a database override layer, cabana YAML admin screens, a CodeMirror 6 field in the Vue admin SPA +**Confidence:** HIGH for the codebase facts and integration points, MEDIUM for the recommended designs (they rest on verified facts but are proposals) + + +## User Constraints (from CONTEXT.md) + +### Locked Decisions + +#### Placement and permissions +- **D-01:** The screens live **inside cabana**, as an internal pseudo-plugin. This is the same pattern as Administrators/Roles in 19.6.1 (`modules/cabana/backend_screens.go`, `modules/cabana/backendscreens/`). Every app gets them with no setup. postcard stays free of admin and cabana code; cabana (or a thin glue layer) supplies postcard with the DB-backed layer through an interface postcard defines. +- **D-02:** **Navigation: a "Mail" section in Settings**, with entries for Mail templates, Mail layouts, Mail partials, Mail branding and Mail images. Links use `pact.HasSettings`, as in 19.6.1 D-02. There is no top-level nav item. +- **D-03:** **Two permission codes**, registered by the cabana pseudo-plugin: + - `system.manage_mail_templates` covers the templates list/form, template preview and test send. + - `system.manage_mail_layouts` covers layouts, partials, Branding and the image library, because these change every mail. + + Neither code is granted to any role by default (as in 19.6.1 D-04). Only superusers reach the screens until someone grants a code. The image library and its insert picker must also be usable from the template form by holders of `system.manage_mail_templates`, either read/pick only or with upload too (planner's call, stated in the plan). — **Reversibility:** costly — the codes end up stored in `backend_user_roles.permissions` / `backend_users.permissions` JSON on every install that grants them. + +#### Editing model: templates +- **D-04:** **A template override stores subject + Markdown body + layout.** + - The body stays Markdown with Go `html/template` variables (`{{ .name }}`), the same format as the file. + - The plain-text part is still derived from the body; there is no hand-written text part. + - The layout is a dropdown over all layouts (file and custom). + - Description stays file-owned and read-only. +- **D-05:** **Locale variants stay separate templates.** `activate.htm` and `activate-en.htm` are their own rows. The list has a locale column, parsed from the suffix convention postcard already uses, plus a locale filter. There is no grouping and no new locale model. +- **D-06:** **Reset to the file version** deletes the override row, on templates, file layouts and file partials alike. +- **D-07:** **Drift notice.** When an override is saved, it stores a hash of the file content it was based on. If the plugin file later changes (for example after a plugin upgrade), the override keeps winning, but the list and the form flag "file changed since override". The form offers a way to view the current file version (a diff if cheap) and the reset button. — **Reversibility:** costly — the hash column is part of the override table schema (migration). + +#### Editing model: layouts, Branding, partials +- **D-08:** **Layouts use the same override model as templates.** + - Every file layout is listed: postcard's built-in default plus plugin layouts such as sm-user-plugin `layouts/user.htm` and paymentgateway `layouts/paymentgateway.htm`. + - Each file layout can get a DB override with reset. + - Admins can also **create custom layouts** that exist only in the DB. Custom layouts can be deleted (not reset); deleting one still assigned to template overrides is refused, with a count of those templates. + - "Seeded predefined layouts" means a couple of framework-shipped file layouts, for example the existing default plus one or two more such as system/minimal (the planner picks the set). They are not rows copied into the DB at boot. +- **D-09:** **Layouts are edited in a new `codeeditor` form field type**, Winter-style, with syntax highlighting for HTML/CSS/text. It is added to cabana's field registry and to the admin SPA. The layout form has tabs for the HTML wrapper, the text wrapper and the CSS/header. The field type is reusable by later phases. — **Reversibility:** costly — a published field type name in `fields.yaml` that plugins may start using. +- **D-10:** **Branding page**, as in Winter's mail brand settings. + - It is a settings page (cabana settings model) with Winter's brand colour set: body/content background, text, link, the button primary/positive/negative colours, and header/footer colours. + - It also has a **logo** upload, exposed to layouts as `{{ .logoUrl }}`. + - The colours compile into the brand CSS slot that postcard already inlines. +- **D-11:** **Precedence: Branding replaces config brandCss.** While no Branding record has been saved, `mail.brandCss` from config is used, so existing apps are unchanged. Once Branding is saved, its compiled CSS replaces `mail.brandCss`. `mail.css` is unaffected and still applied. +- **D-12:** **Mail partials are in scope, with file + DB override like templates.** + - Files live at `views/mail/partials/.htm` and are named `::mail.partials.`. + - Plugins declare them through a **new additive optional pact capability**. `pact.HasMailTemplates` is not changed: core plugins must not break. + - Partials are used from template bodies and layouts through a Go template call (the planner picks the exact syntax or helper). + - A few built-in partials ship with postcard (button, panel, table). + - Admins override and reset partials; editing them needs `system.manage_mail_layouts`. + + — **Reversibility:** costly — a published pact capability and naming convention that plugins implement. + +#### Validation and send-time safety +- **D-13:** **Saves are validated.** Saving a template, layout or partial override parses it, test-renders it with the sample vars (D-15) and runs postcard's existing safety checks (script/iframe/object/embed, inline handlers, `javascript:`/`vbscript:`/`data:` URLs, CR/LF in the subject). Failures are field validation errors. +- **D-14:** **Fallback at send.** If an override still fails to render at send time (for example a variable missing at runtime), postcard logs the failure and sends the file version, so mail delivery doesn't stop. A template with no file (impossible for templates; custom layouts only) falls back to the default layout. + +#### Preview and test send +- **D-15:** **Sample variables**: plugins may add optional sample values to a template file. One option is a sample section in the INI header; another is an additive capability (planner's call). Variables without a sample render as a visible placeholder such as `[name]`. Existing template files work unchanged. +- **D-16:** **Live preview of the unsaved draft.** The template, layout and partial forms render the current draft through postcard with sample vars. The HTML part is shown in a **sandboxed iframe**, and a tab shows the plain-text part. Rendering happens on the server through postcard, so preview matches real sends. +- **D-17:** **Test send to my own address only.** A button sends the current draft (with sample vars) to the signed-in admin's email through the configured driver. It needs the same permission as editing that record and is rate-limited per admin. There is no free-text recipient field. + +#### Images +- **D-18:** **A shared mail image library.** A "Mail images" list stores images through `lagoon/attach` on the public disk. Any template, layout or partial can use them, and the Branding logo also uses this storage. +- **D-19:** **Delivery via absolute public URLs.** `` uses the attachment's absolute public URL, built from `storage.uploads.public_url` / `app.url`. No CID inline parts; postcard's `RenderedMessage` and drivers are unchanged. +- **D-20:** **An insert button in the editors.** A picker next to the markdown editor (template bodies) and the codeeditor (layouts/partials) inserts `![alt](url)` or `` for a chosen library image. + +#### Quality gates +- **D-21:** Unit tests are the last plan (project rule). They cover: + - override precedence and reset; + - drift detection; + - fallback at send; + - save validation; + - Branding precedence over config; + - the partial capability (boot errors, namespace rule); + - the permission split; + - the test-send recipient restriction and rate limit. + + The postcard and cabana READMEs and the affected `docs/` pages are updated in the same commits as API/config/permission changes. The `admin/` changes regenerate `admin/openapi/admin.json` + `schema.d.ts` (`scripts/check-admin-openapi.sh`) and rebuild the committed `modules/boardwalk/dist` (`scripts/check-admin-dist.sh`). + +### Claude's Discretion +- Table names and schema (one overrides table per kind, or a shared one), as long as it is Postgres/GORM with gormigrate migrations owned by the cabana pseudo-plugin, or by postcard if the planner justifies it. +- Exact partial call syntax and the built-in partial set beyond button/panel/table. +- Which extra predefined file layouts ship (D-08). +- Whether the image picker on the template form needs `manage_mail_layouts` for uploads (D-03 note). +- How postcard learns about overrides: a lookup interface checked per send, or a cache invalidated on save. It must be correct with several app instances, so per-send lookup or explicit invalidation, never a stale boot-time snapshot. +- How the SPA codeeditor is built (the library choice needs a STACK.md/decision note per the dependency rule). +- The plan split (lean planning: present the count before writing PLAN.md files). + +### Deferred Ideas (OUT OF SCOPE) +- Hand-written plain-text template parts (Winter's `content_text`), not needed now. +- A WYSIWYG/rich-editor mail body. +- CID-embedded inline images (only absolute URLs in v1). +- Test send to an arbitrary address. +- Mail driver/SMTP configuration screen in the admin. + +#### Reviewed Todos (not folded) +- **Port MailBlocker and the block-mail checkbox** (`.planning/todos/pending/mailblocker-with-mailing.md`): kept separate. It is a send-path feature owned by sm-user-plugin (MailBlocker table, check before send, `block_mail` checkbox) and stays a todo for later mail work. + + + +## Phase Requirements + +No REQUIREMENTS.md IDs are mapped to this phase (ROADMAP: "Requirements: TBD"). The related shipped requirement is **I18N-03** ("Plugins register mail templates and layouts by dotted name with the per-locale suffix convention, rendered with html/template and sent through a driver interface (SMTP via go-mail)"), which this phase must keep satisfied. The table maps the locked decisions to the research findings that enable them. + +| ID | Decision | Research Support | +|----|----------|------------------| +| D-01 | Screens inside cabana; postcard defines the interface | Pattern 1 (store split), Pattern 2 (index rows); cabana pseudo-plugin facts | +| D-02 | Settings → Mail entries | `pact.SettingsItem.Controller` link entries; UI-SPEC C14 | +| D-03 | Two permission codes, no default grants | `pact.Permission` with empty `Roles`; `cabana.Allows` any-of semantics for the picker | +| D-04 | Override = subject + body + layout | Schema; FormBeforeUpdate/FormAfterUpdate ordering (Pitfall 6) | +| D-05 | Locale column/filter | Locale parsing rule (Pattern 2) | +| D-06 | Reset | Record action `reset` that clears the override columns (Open Question 1) | +| D-07 | Drift hash | `file_hash` vs `base_hash`; stdlib LCS diff (Don't Hand-Roll note) | +| D-08 | File layouts + custom layouts + predefined layouts | Index rows; built-in alias fallback; FK RESTRICT for in-use refusal | +| D-09 | `codeeditor` field type | Built-in type list changes; CodeMirror 6 packages, keymap pitfall | +| D-10/D-11 | Branding page and precedence | Settings model facts; brand CSS compiled by postcard; per-send brand lookup | +| D-12 | Partials capability | `pact.HasMailPartials` design; placeholder substitution (verified goldmark behaviour) | +| D-13 | Save validation | postcard draft-validation API; parameterized messages (Pitfall 9) | +| D-14 | Fallback at send | Per-send lookup + render retry without overrides | +| D-15 | Sample vars | INI `sample.*` keys (backward compatible) + verified placeholder walker | +| D-16 | Live preview | Preview route; srcdoc sandbox is allowed by the admin CSP | +| D-17 | Test send to self, rate-limited | Principal has no email; limiter reachability (Pattern 7) | +| D-18/D-19/D-20 | Image library, absolute URLs, insert button | attach URL facts; worker-process URL pitfall (Pitfall 4) | +| D-21 | Tests and gates | Validation Architecture | + + +## Project Constraints (from CLAUDE.md) + +- Standard library first. A new dependency needs a research doc or a phase/decision note naming it (`.planning/research/STACK.md`). The CodeMirror packages need a STACK.md "Admin SPA (npm) additions" row and a decision note written **before** the plan task that installs them. +- YAML is parsed with `goccy/go-yaml`, never `gopkg.in/yaml.v3`. +- Postgres only; GORM plus per-plugin gormigrate migrations. AutoMigrate is not the schema source. +- No runtime plugin loading. +- A change to a module's exported API, config keys, CLI commands or dependencies updates that module's `README.md` and the affected `docs/` pages **in the same commit**. Every identifier in a README or docs page must exist in the package (docs checker). Config keys are reviewed by hand. +- Framework code never names a consuming application (READMEs, docs and examples use neutral names such as `blog` or `acme`). +- A new module needs a beach name, a README in the standard structure and a root README row. (This phase should not need a new module; see Pattern 1.) +- Before tagging a release that changes `admin/`: rebuild, commit `modules/boardwalk/dist`, pass `scripts/check-admin-dist.sh`. +- `TestMain` functions that start containers call `flag.Parse()` and check `testing.Short()`. +- Keep `go vet ./...` and `go test ./...` green at every commit. +- Lean planning: few large plans; present the plan count with a one-line scope each and wait for confirmation. **Unit tests are the last plan.** +- Update STATE.md only through `msd_run query state.*` handlers. g15office sync on phase advance. +- No co-author tags. One logical change per commit; planning docs and code in separate commits. Stage files by path; several sessions work in this repo (Phase 33 is executing and recently touched cabana: `4cd35c9 feat(cabana): per-request options for plugin field types`). +- No push, tag or deploy without the user's go-ahead. +- Core plugins (user, blog, pages, payment) must not get breaking changes: `pact.HasMailTemplates`, `postcard.Catalog.Register` and `postcard.Mailer` keep their exact signatures. + +## Summary + +postcard today is an in-memory catalog: `Catalog.Register` reads each plugin's `views/mail/*.htm` at boot, and `Mailer.Send` renders a template (Markdown body through `html/template`, then goldmark) inside a layout, puts `mail.brandCss` + `mail.css` into the layout's ``), it does not inline it into attributes [VERIFIED: modules/postcard/assets/default.htm:9]. The sm-user-plugin and paymentgateway layouts only carry `.layout-user` / `.layout-paymentgateway` wrappers [VERIFIED: their `views/mail/layouts/*.htm`]. So the compiled CSS must style generic elements (`body`, `a`, `h1`–`h3`, `p`, `table`) plus the built-in partial classes, otherwise Branding seems to do nothing in plugin layouts. +- Layout data today is exactly `"Content"`, `"Subject"`, `"css"`, `"brandCss"` [VERIFIED: modules/postcard/templates.go:358-372]. Add `"logoUrl"` (string; html/template sanitizes it in `src` attributes). + +### Pattern 7: preview, test send, permissions, rate limit (D-16, D-17, D-03) + +- Mount the routes like `/markdown/preview` (`g.Post("/markdown/preview", requireAjax(s.markdownPreview))`) [VERIFIED: modules/cabana/http.go:342], with swag annotations. Check permission per `mode`: template → `system.manage_mail_templates`; layout/partial/branding → `system.manage_mail_layouts`. Use `cabana.Allows`, which is any-of with a superuser bypass (`if principal.IsSuperuser || len(required) == 0 { return true }`) [VERIFIED: modules/cabana/contracts.go:327-343]. The image-library list calls `Allows(p, []string{templates, layouts})`; upload calls `Allows(p, []string{layouts})` (UI-SPEC recommendation: `canUpload` only for the layouts code). +- **Test-send recipient:** `bouncer.Principal` has `ID`, `IsSuperuser`, `PermissionGrants`, `Scope` but no email [VERIFIED: modules/bouncer/context.go:26-35]. Load `BackendUser.Email` by `principal.ID` [VERIFIED: modules/cabana/contracts.go:60-65] and never accept a recipient in the body. Restrict to interactive sessions (an `interactiveOnly` wrapper exists for account routes [VERIFIED: modules/cabana/http.go:333]). +- **Rate limit:** cabana cannot import surf, because surf imports cabana [VERIFIED: `go list` imports of modules/surf]. surf publishes its Redis-backed limiter only as `app.Publish[Store](…)` [VERIFIED: modules/surf/serve.go:237]. `cooler.Attempter` has "exactly the method set of surf.Store" [VERIFIED: modules/cooler/cooler.go:68-73]. Recommended: surf also publishes the resolved store as `cooler.Attempter` (one line where it publishes `Store`), and cabana looks it up with a small in-process fixed-window fallback (tests, memory store). Key `mail_test:`; answer 429 with `Retry-After` as surf does. +- **Preview iframe:** the admin CSP is `"frame-ancestors 'none'; base-uri 'none'; object-src 'none'; script-src 'self'; form-action 'self'; frame-src 'none'"`, and its comment states that "a sandboxed srcdoc preview is not fetched, so it still renders" [VERIFIED: modules/boardwalk/boardwalk.go:61-66]. No CSP change is needed. + +### Pattern 8: image library and absolute URLs (D-18-D-20) + +- `attach.PublicURL` returns `strings.TrimRight(base, "/") + "/" + key` when `storage.uploads.public_url` is set, otherwise a root-relative path from `PublicPathPrefix()` [VERIFIED: modules/lagoon/attach/thumb.go:80-96]. Mail needs absolute URLs, so prefix `app.url` when the result starts with `/`. When neither is configured, refuse the upload/listing with a clear message. +- `PublicURLBase()` is a process global set by `OpenBucket` [VERIFIED: modules/lagoon/attach/bucket.go:42-55, 154-157]. `OpenBucket` is called only from `surf/serve.go:213` and `lagoon/commands.go:479` [VERIFIED: grep], not from `queue:work`. Any URL computed at send time in a separate worker would come out relative. Recommendation: the picker inserts the absolute URL into the content at edit time (D-20, which is text), and the logo URL is resolved by a config-based helper (add a pure `attach` function that takes `*compass.Config` instead of the globals) or stored absolute on save. +- Library host: an `AdminSingleton` controller (`SingletonRecord(ctx, db) (any, error)`, first-or-create with a unique-index race retry) [VERIFIED: modules/pact/capabilities.go:1041-1043; modules/cabana/singleton.go:41-80] on a one-row host table; `fileupload` attachMany `images`, public, `useCaption: true`. The picker's immediate upload goes through `attach.Store(ctx, db, bucket, attach.Upload{FileName, Body, Public: true}, attach.Limits{…})` [VERIFIED: modules/lagoon/attach/store.go:72-76,123] attached to the host row. That bypasses deferred binding on purpose (UI-SPEC C4). + +### Pattern 9: admin wiring facts the plans touch + +- **Built-in field types:** add `codeeditor`, `colorpicker`, `mailimage`, `mailpreview` to `formFieldTypes` (currently `"text" … "taglist"`) [VERIFIED: modules/cabana/form_schema.go:24-31]. Add `language` and `mailImages` to `formFieldKeys` (`mode` and `size` already exist) [VERIFIED: form_schema.go:35-51]. In the SPA, register the renderers in `registry.ts`, add `mailpreview` to `valueless` (`new Set([RELATION_MANAGER, 'widget', 'partial', 'fileupload'])`) and the group-labelled types to `groupLabelledTypes` [VERIFIED: admin/src/components/form/registry.ts]. +- **Row states:** pact's set is fixed: `RowStateDeleted RowState = "deleted"`, `RowStateNegative RowState = "negative"`, `RowStateDisabled RowState = "disabled"` [VERIFIED: modules/pact/capabilities.go:1055-1062]. cabana's order is `var rowStateOrder = []pact.RowState{pact.RowStateDeleted, pact.RowStateNegative, pact.RowStateDisabled}` [VERIFIED: modules/cabana/query.go:182]; messages are in `cabana/messages.go:42,61,163`; the SPA has `const ORDER = ['deleted', 'negative', 'disabled']` (RowStateBadges.vue:19) and `const ROW_STATES = ['deleted', 'negative', 'disabled']` (DataTable.vue:160); `TestRowStateValues` pins `len(set) != 3` [VERIFIED: modules/pact/capabilities_test.go:414-415]. All of these change for `customized`/`outdated`, and the OpenAPI message schema gains two keys. +- **Save hooks ordering:** the update path runs Fill → rules → `formBeforeUpdate` → relation checks → `assignBelongsTo` → `tx.Unscoped().Save(target)` → `formAfterUpdate`, with no dirty-check skip [VERIFIED: modules/cabana/crud.go:876-952]. Stamp `is_custom`, `base_hash` and `customized_at` in `FormBeforeUpdate`; run postcard validation in `FormAfterUpdate`, where a belongsTo `layout_id` is already assigned. A `*ValidationError` or `*ForbiddenError` passes through `lifecycleFailure` [VERIFIED: crud.go:1090-1106] and rolls the transaction back. +- **Partials (server-rendered `_info.htm`, `_file_version.htm`):** the allowlist has `div, span, p, strong, … code, pre` and global attributes `class, title, lang, dir, role` [VERIFIED: modules/cabana/partial_render.go:105-118]. That is enough for the drift callout and `.summer-diff`. Data comes from `AdminPartialData.PartialData` as a curated view model, never a GORM model [VERIFIED: modules/pact/capabilities.go:655-663]. +- **Consumer layouts to keep rendering byte-identically:** `golem15.user::mail.layouts.user` (text wrapper `-- {{ .Content }} --`) and `golem15.paymentgateway::mail.layouts.paymentgateway` [VERIFIED: ../sm-user-plugin/plugin.go:309-313, views/mail/layouts/user.htm; ../sm-paymentgateway-plugin/mail.go:83-86]. + +### Recommended schema (design; table names follow the framework's Winter-name convention: `system_files`, `backend_users`) + +```sql +CREATE TABLE system_mail_layouts ( + id SERIAL PRIMARY KEY, + code TEXT NOT NULL UNIQUE, -- file: full name (contains '::'); database-only: ^[a-z0-9-]+$ + source TEXT, -- owning plugin ID; NULL = database-only (deletable, never reset) + name TEXT NOT NULL, + content_html TEXT NOT NULL DEFAULT '', content_text TEXT NOT NULL DEFAULT '', content_css TEXT NOT NULL DEFAULT '', + is_custom BOOLEAN NOT NULL DEFAULT FALSE, -- file row: override active; database-only row: TRUE + file_hash TEXT, base_hash TEXT, -- D-07: current file hash (sync) / hash the override was based on (save) + customized_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE TABLE system_mail_templates ( + id SERIAL PRIMARY KEY, code TEXT NOT NULL UNIQUE, source TEXT NOT NULL, locale TEXT, description TEXT, + subject TEXT NOT NULL DEFAULT '', body TEXT NOT NULL DEFAULT '', + layout_id INTEGER REFERENCES system_mail_layouts(id) ON DELETE RESTRICT, -- D-08 in-use refusal backstop + is_custom BOOLEAN NOT NULL DEFAULT FALSE, file_hash TEXT, base_hash TEXT, customized_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE TABLE system_mail_partials ( /* code, source, name, content_html, content_text, is_custom, file_hash, base_hash, customized_at, timestamps */ ); +CREATE TABLE system_mail_brand_settings ( id INTEGER PRIMARY KEY, logo_file_id INTEGER, /* 14 colour TEXT columns */ created_at …, updated_at … ); +CREATE TABLE system_mail_image_library ( id SERIAL PRIMARY KEY, created_at …, updated_at … ); -- attachMany host only +``` +"Outdated" = `is_custom AND source IS NOT NULL AND base_hash IS DISTINCT FROM file_hash`, which the list filter can express in SQL. The hash is sha256 over the raw file bytes with CRLF normalized (postcard already normalizes in `splitEQ`, `strings.ReplaceAll(src, "\r\n", "\n")`) [VERIFIED: modules/postcard/templates.go:273]. + +### Anti-Patterns to Avoid +- **Boot-time snapshot of overrides in the mailer.** It is stale across instances (CONTEXT forbids it). +- **Adding methods to `postcard.Mailer` or changing `Catalog.Register`/`HasMailTemplates`.** Outside code implements or calls them directly (sm-user-plugin `mail_test.go:81`, paymentgateway `mail_smoke_test.go:49`). +- **`html.WithUnsafe` on the body renderer to "make partials work".** It changes every existing mail. +- **Deleting index rows on sync.** IDs churn during rolling deploys; filter by catalog membership instead. +- **Building image URLs in the SPA.** The server returns absolute URLs (UI-SPEC C4). +- **Sharing the Branding row as the image-library host.** It flips D-11 precedence. + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Code editor | textarea + highlight overlay | CodeMirror 6 modular packages | Caret/scroll sync, a11y, bracket matching | +| Upload, storage, thumbnails, image guard | custom file handling | `attach.Store` + existing `fileupload` field | MIME/image guard, partitioned keys, public URL logic | +| Rate limiting across instances | per-process map | `cooler.Attempter` (Redis when configured) | Shared counters; same semantics as `throttle` | +| Template parsing / variable discovery | regex over `{{ }}` | `html/template` `Tree` + `text/template/parse` walk | Handles `if`/`range`/`with`/pipelines correctly | +| Markdown | own converter | goldmark (existing pin) | Same output as real sends | +| Settings singleton, record actions, confirm/danger, 409/403 handling | custom endpoints | cabana SettingsService, `pact.AdminRecordAction`, `ForbiddenError` | 19.6.1 already proved these paths | + +**Key insight:** the one place a small hand-written piece is justified is the D-07 line diff. Inputs are a few hundred lines, the stdlib-first rule applies, and the UI-SPEC allows omitting the diff. A bounded LCS (cap of about 2,000 lines, beyond which only the file content is shown) is less risk than a new dependency. + +## Common Pitfalls + +### Pitfall 1: overrides ignored in a separate job worker +**What goes wrong:** mail sent from River jobs uses file versions while serve uses overrides. +**Why:** cabana only activates in serve (Pattern 1). +**How to avoid:** postcard's own DB source, wired in `Activate`, reading `app.Lookup[*gorm.DB]()` per send. +**Warning signs:** a test that sends through a mailer from `party.Activate` with a published DB and no `cabana.Activate` ignores an override. + +### Pitfall 2: partial HTML silently vanishes from bodies +**What goes wrong:** `` appears where the button should be (verified behaviour). +**How to avoid:** placeholder substitution after goldmark (Pattern 4), a per-render nonce, and `[A-Z0-9]` tokens. + +### Pitfall 3: save validation rejects working templates +**What goes wrong:** with an empty sample map, `{{ .order.code }}` fails with "nil pointer evaluating interface {}.code", so every paymentgateway template override would be refused under D-13. +**How to avoid:** validate and preview with the walker-built placeholder map (Pattern 5). Test against all 55 paymentgateway files and the 8 user files. + +### Pitfall 4: relative image/logo URLs in mail +**What goes wrong:** `/storage/app/uploads/public/…` in a mail client, or a different URL from a worker process. +**How to avoid:** absolute URLs inserted at edit time, a config-based logo URL helper, and refusal when neither `storage.uploads.public_url` nor `app.url` is set. Note that a URL with a space breaks Markdown image syntax (`![Logo](https://cdn.example.com/a b.png)` stays literal text) [VERIFIED: scratch run]. Percent-encode in `insertImage.ts`; attach disk names are generated, so this mostly matters for `public_url` paths. + +### Pitfall 5: CodeMirror swallows the form's Mod+Enter +**What goes wrong:** `defaultKeymap` contains `{ key: "Mod-Enter", run: insertBlankLine }` (and `Escape: simplifySelection`) [VERIFIED: @codemirror/commands 6.11.1 dist/index.js:1796-1816], so Mod+Enter inserts a line instead of saving (UI-SPEC C1). +**How to avoid:** `keymap.of([...historyKeymap, ...defaultKeymap.filter(b => b.key !== 'Mod-Enter')])`. Do not add `indentWithTab`; CodeMirror leaves Tab unbound by default to avoid a keyboard trap [CITED: codemirror.net/examples/tab]. Cover it in the browser test. + +### Pitfall 6: validating a template against the old layout +**What goes wrong:** `FormBeforeUpdate` runs before `assignBelongsTo`, so a changed `layout_id` is not on the model yet [VERIFIED: crud.go:895-928]. +**How to avoid:** validate in `FormAfterUpdate` (same transaction; a `ValidationError` rolls back). + +### Pitfall 7: tables missing after a binary upgrade +**What goes wrong:** the new binary starts before `migrate` runs. Sync fails and per-send lookups hit `undefined_table`. +**How to avoid:** sends treat any lookup error as "no overrides" with a rate-limited warning; sync retries on the first list request. `serve` already reports pending migrations at startup [VERIFIED: modules/surf/serve.go:111]. Update `lagoon/pending_test.go:349` (`wantSets := []string{"summercms.attach", "summercms.cabana", DeferredHistoryID, QueueHistoryID, "demo.alpha", "demo.beta"}`) [VERIFIED]. + +### Pitfall 8: safety-regex false positives +**What goes wrong:** the existing checks `(?i)(?:javascript|vbscript|data):` and `(?i)\son[a-z]+\s*=` [VERIFIED: modules/postcard/templates.go:25-26] match ordinary text such as "metadata:" or " online = ", and now show up as save errors. +**How to avoid:** keep the behaviour (it matches sends) but pass the matched snippet in `:message` so admins can see why. + +### Pitfall 9: parameterized validation lines come out raw +**What goes wrong:** `localizeDetails` localizes plain phrase keys only [VERIFIED: modules/cabana/crud.go:104-150], so `mail.invalid_render` with `:message`, or `mail.layout.delete_in_use` with `:name`/`:count`, shows unsubstituted. +**How to avoid:** build the final text in the hook with the translator, as `backendMessageParams(ctx, tr, key, fallback, params)` does [VERIFIED: modules/cabana/account_tokens.go:206,227]. Use a `ForbiddenError` (not a `ValidationError`) for the delete refusal (UI-SPEC C15 note). + +### Pitfall 10: the templates list can be empty +**What goes wrong:** the UI-SPEC says the unfiltered list is "never empty, because postcard registers built-in mail". postcard registers **no** templates today, only the default layout [VERIFIED: modules/postcard/templates.go:62-74]. An app without mail-sending plugins shows an empty list. +**How to avoid:** accept it (the empty copy exists) or ship one built-in template. Decide in the plan. + +## Code Examples + +### Placeholder walker (prototype, renders the real paymentgateway shapes) +```go +// Source: scratch prototype run 2026-11-06 against Go 1.27.2 html/template. +// Output for the paymentgateway "paid" body: "Cześć [user.first_name], … **ZAM-1** … +// - [order.items.description] × [order.items.count]: … [banking_details.bankDetailsLines]" +func fill(root map[string]any, n parse.Node, prefix []string, samples map[string]string) { + switch x := n.(type) { + case *parse.ListNode: + if x == nil { return } + for _, c := range x.Nodes { fill(root, c, prefix, samples) } + case *parse.ActionNode: + fill(root, x.Pipe, prefix, samples) + case *parse.PipeNode: + if x == nil { return } + for _, c := range x.Cmds { fill(root, c, prefix, samples) } + case *parse.CommandNode: + for _, a := range x.Args { fill(root, a, prefix, samples) } + case *parse.FieldNode: + set(root, x.Ident, prefix, samples) // nested maps, "[a.b]" or the sample + case *parse.IfNode: + fill(root, x.Pipe, prefix, samples); fill(root, x.List, prefix, samples); fill(root, x.ElseList, prefix, samples) + case *parse.WithNode: + fill(root, x.Pipe, prefix, samples); fill(root, x.List, prefix, samples); fill(root, x.ElseList, prefix, samples) + case *parse.RangeNode: + // one-element slice: a map of the fields used in the body, or a string when the body prints {{ . }} + } +} +// t := template.Must(template.New("t").Option("missingkey=zero").Parse(src)); fill(vars, t.Tree.Root, nil, samples) +``` +The production version must also handle `ChainNode`/`VariableNode` and `with` re-rooting (a `with` body's fields are relative to the pipeline value), and must apply the `partial` func map before parsing. + +### CodeMirror setup (verified export names; `editor.ts` lazy target) +```ts +// Sources: codemirror.net/examples/tab, /examples/decoration; exports verified in the pinned dist .d.ts files. +import { EditorState } from '@codemirror/state' +import { EditorView, keymap, lineNumbers, Decoration, MatchDecorator, ViewPlugin } from '@codemirror/view' +import { defaultKeymap, history, historyKeymap } from '@codemirror/commands' +import { bracketMatching, HighlightStyle, syntaxHighlighting } from '@codemirror/language' +import { html } from '@codemirror/lang-html' +import { css } from '@codemirror/lang-css' +import { tags } from '@lezer/highlight' + +const actionMatcher = new MatchDecorator({ + regexp: /\{\{[\s\S]*?\}\}/g, + decoration: () => Decoration.mark({ class: 'cm-summer-action' }), +}) +const goActions = ViewPlugin.fromClass(class { + decorations + constructor(view: EditorView) { this.decorations = actionMatcher.createDeco(view) } + update(u: import('@codemirror/view').ViewUpdate) { this.decorations = actionMatcher.updateDeco(u, this.decorations) } +}, { decorations: (v) => v.decorations }) + +const keys = keymap.of([...historyKeymap, ...defaultKeymap.filter((b) => b.key !== 'Mod-Enter')]) // Pitfall 5 +// No indentWithTab: Tab moves focus (WCAG 2.1.2). Line wrapping only for language: text (EditorView.lineWrapping). +``` + +### Lagoon framework set (where the migration goes) +```go +// modules/lagoon/migrations.go, today [VERIFIED: lines 224-230]: +sets := []migrationSet{ + {"summercms.attach", "system_files", attach.Migrations}, + {"summercms.cabana", "backend admin", BackendAdminMigrations}, + {DeferredHistoryID, "deferred bindings", DeferredBindingMigrations}, + {QueueHistoryID, "queue", QueueMigrations(sqlDB)}, +} +// recommended addition (after attach, because brand logo_file_id may reference system_files): +// {"summercms.postcard", "mail", postcard.Migrations}, +``` + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| Winter: Twig mail + DB rows synced on list open, CSS inlined by CssToInlineStyles | postcard: Go templates, ``) | Tampering | strict `#rrggbb` validation before compiling; `validateHTML` backstop | +| Header injection via subject override | Tampering | existing CR/LF refusal, also at save | +| Spam/abuse through test send | Abuse | recipient fixed to the admin's own address; per-admin rate limit; permission per record kind | +| Placeholder forgery (user value contains the partial token) | Tampering | per-render random nonce in the token | +| Over-privileged picker upload | Elevation | upload requires `system.manage_mail_layouts`; list allowed for either code | +| Privilege via permission codes | Elevation | codes declared with no `Roles`; superuser-only until granted (19.6.1 pattern) | +| Information disclosure in preview errors | Info disclosure | `:message` limited to parser/safety text, never DB errors or paths beyond the file name | + +## Sources + +### Primary (HIGH confidence) +- Codebase, read this session: `modules/postcard/{templates.go,mailer.go,drivers.go,README.md,assets/default.htm}`, `modules/pact/capabilities.go`, `modules/party/registry.go`, `modules/backpack/services.go`, `modules/cabana/{backend_screens.go,http.go,settings.go,singleton.go,crud.go,query.go,form_schema.go,field_markdown.go,contracts.go,partial_render.go,messages.go}`, `modules/lagoon/{migrations.go,backend_admin_migrations.go,ondatabase.go,connection.go}`, `modules/lagoon/attach/{bucket.go,thumb.go,store.go,migrations.go}`, `modules/conga/commands.go`, `modules/surf/{serve.go,limiter.go,router.go}`, `modules/cooler/{cooler.go,README.md}`, `modules/boardwalk/boardwalk.go`, `admin/package.json`, `admin/src/components/form/registry.ts` +- Consumers: `../sm-user-plugin/{plugin.go,views/mail/**}`, `../sm-paymentgateway-plugin/{mail.go,views/mail/**,events/mailers.go,mail_smoke_test.go}` +- Winter reference: `examples/golem15-wintercms-starter/modules/system/{controllers/MailTemplates.php,models/MailTemplate.php,models/MailLayout.php,views/mail/*.php,models/mailbrandsetting/*}` +- npm registry (`npm view`, `npm pack`) for all seven SPA packages; MSD package-legitimacy seam +- Scratch programs (goldmark v1.8.6 from the module cache; html/template walker), 2026-11-06 + +### Secondary (MEDIUM confidence) +- codemirror.net/examples/tab (Tab handling, `indentWithTab`, `toggleTabFocusMode`) +- codemirror.net/examples/decoration (`MatchDecorator`, `ViewPlugin`, `createDeco`/`updateDeco`) + +### Tertiary (LOW confidence) +- none + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH. Go dependencies are already in the graph; npm versions, maintainers and exports were verified against the registry and the pinned tarballs. +- Architecture: MEDIUM-HIGH. Every integration fact was read in the code. The store split, index rows and placeholder design are recommendations, flagged in the Assumptions Log where they reinterpret CONTEXT wording. +- Pitfalls: HIGH. The goldmark, missing-key, keymap, hook-order, worker-URL and limiter-reachability pitfalls were each confirmed by execution or by reading the source. + +**Research date:** 2026-11-06 +**Valid until:** 2026-12-06 (codebase facts may move with concurrent Phase 33 work in cabana; re-check `cabana/http.go`, `form_schema.go` and `crud.go` line references before planning if Phase 33 lands first) diff --git a/.planning/phases/35-mail-templates-and-layouts-editor/35-VALIDATION.md b/.planning/phases/35-mail-templates-and-layouts-editor/35-VALIDATION.md new file mode 100644 index 00000000..43b4c75d --- /dev/null +++ b/.planning/phases/35-mail-templates-and-layouts-editor/35-VALIDATION.md @@ -0,0 +1,91 @@ +--- +phase: "35" +slug: "mail-templates-and-layouts-editor" +# status lifecycle: draft (seeded by plan-phase) → validated (set by validate-phase §6) +status: draft +nyquist_compliant: false +wave_0_complete: false +created: "2026-11-06" +--- + +# Phase 35 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. Derived from `35-RESEARCH.md` § Validation Architecture. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | Go `testing` (+ testcontainers-go for Postgres/Mailpit); Vitest (happy-dom) and Vitest browser mode with Playwright for the admin SPA | +| **Config file** | `admin/vitest.config.*`, `admin/vitest.browser.config.ts`; Go: none | +| **Quick run command** | `go test -short ./modules/postcard/... ./modules/pact/... ./modules/cabana/... ./modules/lagoon/...` | +| **Full suite command** | `go vet ./... && go test ./... && npm --prefix admin run test && npm --prefix admin run test:browser && npm --prefix admin run typecheck && scripts/check-admin-openapi.sh --check && scripts/check-admin-dist.sh && summer docs:build --check` | +| **Estimated runtime** | ~60 seconds quick, ~10 minutes full | + +--- + +## Sampling Rate + +- **After every task commit:** quick run command plus `go vet ./...` (and `npm --prefix admin run test` for SPA tasks) +- **After every plan wave:** full suite command +- **Before `/msd-verify-work`:** full suite plus `scripts/check-phase35.sh` (also runs the sm-user-plugin and sm-paymentgateway-plugin mail tests) must be green +- **Max feedback latency:** 120 seconds + +--- + +## Per-Decision Verification Map + +| Decision | Behavior | Test Type | Automated Command | File Exists | Status | +|----------|----------|-----------|-------------------|-------------|--------| +| D-11/D-14 | Override wins; file wins without one; render failure and DB error fall back to file; caller subject still wins | unit | `go test -short ./modules/postcard -run 'TestOverride\|TestFallback'` | ❌ W0 | ⬜ pending | +| store | DB source reads real tables; works with no cabana activation | Postgres | `go test ./modules/postcard -run TestStore` | ❌ W0 | ⬜ pending | +| D-12 | `HasMailPartials` boot errors; body partial survives Markdown; nonce forgery fails; layout partial; text part | unit | `go test -short ./modules/postcard -run TestPartial` | ❌ W0 | ⬜ pending | +| D-15 | `sample.*` keys parsed; old keys ignored; placeholder walker fills nested/range/dot | unit | `go test -short ./modules/postcard -run TestSample` | ❌ W0 | ⬜ pending | +| D-13 | Draft validation errors mapped to fields | unit + Postgres | `go test -short ./modules/postcard -run TestValidateDraft`; `go test ./modules/cabana -run TestMailSave` | ❌ W0 | ⬜ pending | +| D-07 | Drift flagged and filterable; re-save clears | Postgres | `go test ./modules/cabana -run TestMailDrift` | ❌ W0 | ⬜ pending | +| D-06/D-08 | Reset clears override; in-use custom layout delete refused with count; file layout never deletable | Postgres | `go test ./modules/cabana -run 'TestMailReset\|TestMailLayoutDelete'` | ❌ W0 | ⬜ pending | +| D-10/D-11 | No brand row → `mail.brandCss`; saved → compiled CSS replaces it; `mail.css` still applied; bad colour 422 | unit + Postgres | `go test -short ./modules/postcard -run TestBrand`; `go test ./modules/cabana -run TestMailBrand` | ❌ W0 | ⬜ pending | +| D-03 | Permission split per controller, preview, picker list vs upload; no default role grants | Postgres | `go test ./modules/cabana -run TestMailPermissions` | ❌ W0 | ⬜ pending | +| D-17 | Recipient always the admin's own email; 429 + Retry-After after the limit | Postgres | `go test ./modules/cabana -run TestMailTestSend` | ❌ W0 | ⬜ pending | +| D-05 | Locale parsed for enabled locales; locale filter | unit | `go test -short ./modules/cabana -run TestMailLocale` | ❌ W0 | ⬜ pending | +| sync | Index sync idempotent, concurrency-safe, custom rows untouched | Postgres | `go test ./modules/postcard -run TestSync` | ❌ W0 | ⬜ pending | +| consumers | user + paymentgateway mail renders unchanged | unit (sibling repos) | `(cd ../sm-user-plugin && go test -run 'TestMail\|TestPolishMailSubjects' ./...)`; `(cd ../sm-paymentgateway-plugin && go test -run 'TestMailCatalog\|TestMailTracer\|TestMailersMatrixSmoke' ./...)` | ✅ | ⬜ pending | +| byte-identity | Existing postcard tests pass; default layout output unchanged | unit | `go test -short ./modules/postcard/...` | ✅ | ⬜ pending | +| UI | codeeditor keys, insertImage escaping, footer record actions, preview sandbox | SPA unit + browser | `npm --prefix admin run test -- mail codeeditor`; `npm --prefix admin run test:browser -- codeeditor` | ❌ W0 | ⬜ pending | +| gates | OpenAPI/schema in sync; dist matches; docs identifiers exist | scripts | `scripts/check-admin-openapi.sh --check`; `scripts/check-admin-dist.sh`; `go test ./cmd/summer -run TestDocsTree` | ✅ | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* + +--- + +## Wave 0 Requirements + +- [ ] `modules/postcard/override_test.go`, `partials_test.go`, `samples_test.go`, `brand_test.go`, `store_postgres_test.go` (TestMain with `flag.Parse()` + `testing.Short()`) +- [ ] `modules/cabana/mail_*_postgres_test.go` reusing the 19.6.1 screens harness +- [ ] `admin/tests/form/codeeditor*.test.ts`, `admin/tests/browser/codeeditor.test.ts`, `admin/tests/form/mailpreview.test.ts`, `admin/tests/form/insertImage.test.ts` +- [ ] Update `modules/pact/capabilities_test.go` `TestRowStateValues` and `modules/lagoon/pending_test.go` set list +- [ ] `scripts/check-phase35.sh` + +--- + +## Manual-Only Verifications + +| Behavior | Decision | Why Manual | Test Instructions | +|----------|----------|------------|-------------------| +| Test-send mail looks right in a real inbox | D-17 | Visual rendering in mail clients | Send a draft from the template form to Mailpit and check HTML and text parts | +| Code editor feel (highlighting, focus, theme) | D-09 | Visual | Open a layout form in the dev SPA, edit each tab | + +--- + +## Validation Sign-Off + +- [ ] All tasks have `` verify or Wave 0 dependencies +- [ ] Sampling continuity: no 3 consecutive tasks without automated verify +- [ ] Wave 0 covers all MISSING references +- [ ] No watch-mode flags +- [ ] Feedback latency < 120s +- [ ] `nyquist_compliant: true` set in frontmatter + +**Approval:** pending