pal-e-docs

Review: MJML email system — brand base + three layouts + docker build (re-review)

review-750-2026-04-03-r2 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Re-review after refinement. Three of four items from the previous review (review-750-2026-04-03) are fixed. One remains: the arch-email architecture note was not created in pal-e-docs. The issue body itself is now fully scoped and ready — this is a backing-note gap, not a spec gap.

Previous Review Fix Verification

  • [x] arch-email note created — NOT FIXED. search_notes("arch-email") returns empty. Note does not exist.
  • [x] Send function count corrected to 9 — FIXED. Issue body says "9 send functions", verified 9 def send_ in email.py.
  • [x] Deployment coordination section added — FIXED. New ### Deployment Coordination section explains env var behavior and references pal-e-deployments#83.
  • [x] AC7 updated with kubectl command — FIXED. AC7 now includes kubectl get configmap email-templates -n basketball-api -o jsonpath command.

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
  • [ ] arch note MISSING — search_notes("arch-email") returned empty. [SCOPE] Create architecture note arch-email for the email component.
  • [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 (27 lines), 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. Correctly sequenced. Has blocked-by:293 and blocked-by:294 labels.
  • [x] #753 (remove ConfigMap, pal-e-deployments#83) — downstream cleanup. In backlog. ConfigMap currently mounts 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 — no conflict.
  • [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 now 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

  • [SCOPE] Create architecture note arch-email in pal-e-docs for the email component. This was item #1 in the previous review and remains unresolved. The issue body references it, the board item has the arch:email label, but the backing note does not exist.
Once arch-email note is created, this ticket is READY.