pal-e-docs

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

review-750-2026-04-03 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Well-scoped feature ticket with complete template sections, verified file targets, and correct repo placement. Three fixable issues prevent READY status: missing architecture note, minor context inaccuracy, and a deployment coordination gap that should be explicit.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, supersedes #658 and #735
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — As an admin, I want email templates in MJML...
  • [x] Context — Thorough: line count, function count, brand tokens, email HTML constraints
  • [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

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
  • [ ] arch note MISSING — [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 is 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 is correctly scoped as a separate ticket (#753 / pal-e-deployments#83).

Dependencies

  • [x] #751 (send_templated_email + EmailType migration) — depends on this ticket. Correctly in backlog behind this. Status: pending.
  • [x] #752 (admin blast endpoint) — depends on templates + #751. Correctly sequenced. Status: pending.
  • [x] #753 (remove ConfigMap, pal-e-deployments#83) — downstream deployment cleanup. Coordination risk: once config.py default changes, BASKETBALL_EMAIL_TEMPLATES_DIR env var must NOT be set so new default takes effect. Not documented in ticket. Status: pending.
  • [x] #658 (audit contract email into MJML) — superseded by this ticket, noted in Lineage. Status: satisfied.
  • [x] #735 (contract reminder branding) — partially superseded, noted in Lineage. Status: satisfied.
  • [x] #724 (contract reminder email endpoint, in needs_approval) — will benefit from templates once #751 migrates send functions. No hard dependency. Status: satisfied.

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.
  • AC7: jersey-reminder MJML parity — risk: current compiled jersey-reminder.html lives in an untracked k8s ConfigMap, not in any repo. Agent needs access to reference HTML for parity comparison. Ticket should specify where to find it.

Blast Radius

Low. Changes are additive (new files + build step). The only breaking change is the config.py default path, which affects deployment coordination with pal-e-deployments. No other repos consume email_templates_dir. The 9 inline HTML send functions in email.py are untouched — migration is deferred to #751. 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). Exceeds the 5-AC guideline numerically. However, all work is a single conceptual unit: MJML authoring + build pipeline + Dockerfile integration. The files are tightly coupled — templates reference brand.mjml includes, package.json defines the compile script, Dockerfile runs the script, config.py points to the output. Decomposing into "create templates" and "wire build" would create artificial boundaries requiring a partial Dockerfile change in each pass. No independent subtasks exist that could be parallelized.
Estimated agent time: 3-5 minutes. Borderline but within the 5-minute rule given the mechanical nature of MJML authoring from known brand tokens.
No decomposition needed.

Recommendation

  • [SCOPE] Create architecture note arch-email for the email component in pal-e-docs.
  • [BODY] Fix send function count: issue says "11 send functions" but email.py has 9 (send_confirmation_email, send_profile_reminder_email, send_roster_export_email, send_tryout_announcement_email, send_contract_signed_email, send_password_reset_email, send_jersey_reminder_email, send_admin_registration_notification, send_interest_notification).
  • [BODY] Add deployment coordination note to Context or Constraints: "When deployed, ensure BASKETBALL_EMAIL_TEMPLATES_DIR env var is NOT set in deployment manifests so the new default /app/templates/email/compiled/ takes effect. ConfigMap removal is tracked in #753."
  • [BODY] AC7 (jersey-reminder parity): specify where the agent can find the current compiled jersey-reminder.html for reference. If only in the k8s ConfigMap, add a kubectl command or paste the reference HTML into the issue.