pal-e-docs

Review: Queens email branding: add brand_color to templates

review-837-2026-04-03 Review

review ready
Project

Verdict: READY

Re-review after refinement. Previous verdict was NEEDS_REFINEMENT (5 issues). All 5 resolved.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Discovered during Kiana/Kelsie contract email send (2026-04-05)
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — As a Queens parent receiving a contract email, I want the email branding to match the Queens pink color scheme
  • [x] Context — Clear motivation, explains template lifecycle (host dir, volume mount, load_email_template)
  • [x] File Targets — 4 files: 3 compiled templates on host + email.py in repo. Paths explicit and correct.
  • [x] Acceptance Criteria — 7 criteria (see assessment below)
  • [x] Test Expectations — pytest + 2 manual send verifications
  • [x] Constraints — Backwards compat, no git for templates, jersey-reminder excluded
  • [x] Checklist — Present
  • [x] Related — Present, references arch note and parent work

Traceability

  • [x] story:WS-S23 label — "As an admin, I want to configure custom contract terms per player so that custom deals render correctly without code changes" (found in project-westside-basketball user-stories, Admin section)
  • [x] story note verified — found in project-westside-basketball user-stories section
  • [x] arch:contracts label — contracts architecture component
  • [x] arch note verified — arch-contracts-westside-basketball note exists in pal-e-docs (id: 1208, status: active)
  • [x] Forgejo issue — forgejo_admin/basketball-api#344, state: open

File Targets

  • [x] ~/westside-email-templates/compiled/action.html — verified: exists on host, 10 occurrences of #d42026, already has {{headline}}/{{body}}/{{cta_url}}/{{cta_text}}/{{footer_note}} placeholders. Adding {{brand_color}} follows existing pattern.
  • [x] ~/westside-email-templates/compiled/announcement.html — verified: exists on host, 13 occurrences of #d42026, already has {{headline}}/{{section_1}}/{{section_2}}/{{section_3}}/{{cta_url}}/{{cta_text}}/{{footer_note}} placeholders.
  • [x] ~/westside-email-templates/compiled/notification.html — verified: exists on host, 9 occurrences of #d42026, already has {{headline}}/{{body}}/{{footer_note}} placeholders.
  • [x] src/basketball_api/services/email.py — verified: load_email_template() at line 1107 does {{key}} replacement via simple string replace. Currently only called for "jersey-reminder". Adding brand_color default here means all future callers (action, announcement, notification) inherit the default automatically.
Note for agent: jersey-reminder.html has 0 occurrences of #d42026 — correctly excluded per Constraints.

Repo Placement

OK. Issue explicitly explains the split: email.py changes go through basketball-api PR. Template files are edited directly on host at ~/westside-email-templates/compiled/ (not version-controlled). Checklist correctly separates "Templates updated on host" from "PR opened (for email.py change only)."

Dependencies

  • [x] Board item #779 ("T7: send Kiana + Kelsie contract emails") — done. This is the work that discovered the need. No blocker.
  • [x] Board item #556 ("Phase 14: Billing Tiers & Contracts") — in_progress but not conflicting. This ticket is a leaf change, not a schema/route change.
  • [x] No in_progress items touch email.py or the template files.

Acceptance Criteria

7 criteria, all verifiable:
  • [x] AC 1-3: Replace #d42026 with {{brand_color}} in all 3 templates — mechanical find-replace, verifiable by grep.
  • [x] AC 4: Default brand_color injection in load_email_template() — verifiable by code inspection + unit test.
  • [x] AC 5: Queens pink render — manual send verification.
  • [x] AC 6: Kings red default — manual send verification.
  • [x] AC 7: No runtime hack — the hack was ad-hoc kubectl, never committed. AC verifies the new system makes the hack unnecessary. Verifiable by absence of html.replace in codebase.

Blast Radius

Contained. All 3 affected templates (action, announcement, notification) are in scope. jersey-reminder.html has no #d42026 — correctly excluded. The inline-built emails in email.py (registration confirmation, profile reminder, etc.) use _BRAND_RED variable — these are separate from the MJML templates and out of scope. Rollback: revert templates to hardcoded #d42026, remove default injection — straightforward.

Decomposition Assessment

7 AC items exceeds the 5-AC threshold, but analysis shows this is inflated by homogeneous work:
  • AC 1-3 are identical sed operations on 3 files (~30 seconds each)
  • AC 4 is a single default injection (~2 minutes)
  • AC 5-7 are manual verification steps, not code changes
Actual code work: 3 identical find-replaces + 1 function edit = 4 file touches, 1 repo + host filesystem. Well within the 5-minute rule. No decomposition needed.

Previous Issues Resolved

  • File target path wrong — RESOLVED. Paths now explicit with ~/westside-email-templates/compiled/ and explanation that files are on host, not in git.
  • Missing caller identification — RESOLVED. Issue explains load_email_template() is the injection point for the default. Agent doesn't need to identify specific callers — the default applies universally.
  • Hack location unclear — RESOLVED. Issue states "kubectl ad-hoc send — never committed."
  • Missing AC for default — RESOLVED. AC #4 is explicit: "load_email_template() injects brand_color: '#d42026' into data dict if not provided."
  • Blast radius — RESOLVED. All 3 templates in scope. jersey-reminder excluded in Constraints. Explicit decision documented.

Recommendation

No action needed.