Review: MJML email system — brand base + three layouts + docker build (R3)

review-750-2026-04-03-r3 Doc

review ready

Verdict: APPROVED

Re-review round 3. The sole remaining blocker from R2 — arch-email note missing — is now resolved. The note exists (ID 1102, slug arch-email, created 2026-04-03). All other items were fixed in R2. This ticket is fully scoped and ready for dispatch.

Previous Review Fix Verification

  • [x] arch-email note created — FIXED. get_note(slug="arch-email") returns note ID 1102 ("Architecture: Email System"), tagged architecture+active, project westside-basketball. Substantive content: Overview, Components, Layouts, Email Flow, Preview & Approval Workflow, Decisions. Note: search_notes("arch-email") returns empty due to FTS index lag, but direct slug lookup confirms existence.
  • [x] Send function count corrected to 9 — fixed in R2.
  • [x] Deployment coordination section added — fixed in R2.
  • [x] AC7 updated with kubectl command — fixed in R2.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, supersedes #658 and partially #735
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — As an admin, I want email templates authored in MJML...
  • [x] Context — 1,424 lines, 9 send functions, brand tokens, email HTML constraints, architecture reference
  • [x] Deployment Coordination — bonus section, explains env var behavior
  • [x] File Targets — 7 files to create, 2 to modify, 2 explicitly excluded
  • [x] Acceptance Criteria — 7 criteria
  • [x] Test Expectations — 3 test items + run command
  • [x] Constraints — MJML v5, no Jinja2, mj-attributes, 600px, mobile-first, gitignore compiled/
  • [x] Checklist — PR, tests, no unrelated changes, docker build
  • [x] Related — project + superseded items + arch reference + downstream ticket

Traceability

  • [x] story:WS-S7 label — "As an admin, I want to send branded email announcements so that parent comms are professional and consistent"
  • [x] story note verified — found in project-westside-basketball user-stories section (stories-admin-list block)
  • [x] arch:email label — present on board item #750
  • [x] arch note verified — get_note(slug="arch-email") returns note ID 1102. Created 2026-04-03 with full architecture content (Overview, Components, Layouts, Email Flow, Preview & Approval, Decisions).
  • [x] Forgejo issue — forgejo_admin/basketball-api#293, open

File Targets

  • [x] templates/email/brand.mjml — new file, templates/ dir does not exist yet (expected for new files)
  • [x] templates/email/notification.mjml — new file
  • [x] templates/email/action.mjml — new file
  • [x] templates/email/announcement.mjml — new file
  • [x] templates/email/compiled/ — new dir, to be gitignored
  • [x] package.json — new file (no existing package.json in repo)
  • [x] templates/email/jersey-reminder.mjml — new file (migration proof from compiled HTML)
  • [x] Dockerfile — verified at repo root, no npm steps yet, modification target valid
  • [x] src/basketball_api/config.py — verified: email_templates_dir at line 38, default "/data/email-templates", change to "/app/templates/email/compiled/" is valid
  • [x] src/basketball_api/services/email.py — correctly listed as NOT to touch
  • [x] src/basketball_api/brand.py — verified: COLOR_RED=#d42026, COLOR_BLACK=#0a0a0a, COLOR_DARK=#141414, COLOR_GRAY_800=#262626, FONT_FAMILY present. Correctly listed as NOT to touch

Repo Placement

OK. Issue filed on forgejo_admin/basketball-api, all file targets are in basketball-api. Downstream ConfigMap removal correctly scoped as separate ticket (#753 / pal-e-deployments#83).

Dependencies

  • [x] #751 (send_templated_email + EmailType migration) — depends on this ticket. In backlog. Correctly sequenced.
  • [x] #752 (admin blast endpoint) — depends on templates + #751. In backlog. Has blocked-by:293 and blocked-by:294 labels.
  • [x] #753 (remove ConfigMap, pal-e-deployments#83) — downstream cleanup. In backlog. ConfigMap mount at /data/email-templates/ confirmed in deployment-patch.yaml lines 77-78, 95-97. New config.py default points to /app/templates/email/compiled/ so ConfigMap mount becomes inert.
  • [x] #658 (audit contract email into MJML) — superseded. In backlog.
  • [x] #735 (contract reminder branding) — partially superseded. In backlog.
  • [x] #724 (contract reminder email endpoint) — in needs_approval. Will benefit from templates once #751 migrates. No hard dependency.

Acceptance Criteria

7 criteria, all verifiable by an agent:
  • AC1-2: File existence + npm build — straightforward shell checks.
  • AC3-4: Brand rendering + mobile CTA — verifiable by grep on compiled HTML output.
  • AC5: Dockerfile build — docker build command.
  • AC6: load_email_template integration — existing test infrastructure covers this (tests/test_jersey_reminder.py already tests load_email_template with mock settings).
  • AC7: jersey-reminder MJML parity — kubectl command provided for retrieving reference HTML. Verifiable.

Blast Radius

Low. Changes are additive (new files + build step). The only breaking change is the config.py default path. BASKETBALL_EMAIL_TEMPLATES_DIR is NOT set as an env var in pal-e-deployments (confirmed via grep). The ConfigMap mount at /data/email-templates/ remains inert once the default changes. No other repos consume email_templates_dir. The 9 inline HTML send functions in email.py are untouched. Rollback is straightforward: revert config.py default, remove Dockerfile npm steps.

Decomposition Assessment

7 ACs across 1 repo, 9 file targets (7 create + 2 modify). All work is a single conceptual unit: MJML authoring + build pipeline + Dockerfile integration. Files are tightly coupled — templates reference brand.mjml includes, package.json defines compile script, Dockerfile runs script, config.py points to output. No independent subtasks exist. Estimated agent time: 3-5 minutes. No decomposition needed.

Recommendation

No action needed. Ticket is fully scoped and ready for dispatch.