Review: Jersey reminder template needs division-aware image placeholders (re-review)

review-893-2026-04-04-r2 Doc

review ready

Verdict: READY

Re-review of board item #893 after refinement. Previous review review-893-2026-04-04 returned NEEDS_REFINEMENT with 7 recommendations. All 7 have been addressed.

Refinement Resolution

  • [LABEL] arch label fixed — Changed from arch:westside-app to arch:basketball-api. Verified on board item.
  • [SCOPE] Option B chosen — Issue body now specifies Option B (placeholders in single template). Decision documented in "Fix Required" section.
  • [BODY] AC6 removed — Resend operation removed from this ticket. Will be a separate blast-gated ticket.
  • [BODY] Cross-repo documented — Issue body explicitly lists PR 1 (westside-emails) and PR 2 (basketball-api).
  • [DECOMPOSE] MinIO upload split — Separated to basketball-api#393 (confirmed open on Forgejo: "Upload Kings jersey images to MinIO and rename Queens images").
  • [BODY] /tmp paths noted — Issue acknowledges images are ephemeral on archbox. MinIO upload is the prerequisite ops ticket.
  • Scope reduced — 4 ACs across 2 repos (template change + send function). Fits single agent pass.

Template Completeness

Checked against template-issue-bug:
  • [x] Type — Bug
  • [x] Lineage — discovered after jersey reminder blast 2026-04-08
  • [x] Repo — forgejo_admin/westside-emails + forgejo_admin/basketball-api
  • [x] What Broke — 32 Kings families received Queens jersey images due to hardcoded URLs
  • [x] Repro Steps — clear 3-step reproduction
  • [x] Expected Behavior — division-aware image placeholders, Kings see Kings, Queens see Queens
  • [x] Environment — template path, send function line number, image naming convention
  • [x] Acceptance Criteria — 4 criteria, all verifiable
  • [x] Related — project, story, prerequisite ops ticket, follow-up blast ticket

Traceability

  • [x] story:WS-S22 — "As a parent, I want to receive clear email communications with action links so that I never miss a deadline" — verified in project-westside-basketball user-stories section (Parent stories list)
  • [x] arch:basketball-api — label matches primary code change target. The fix spans two repos but basketball-api owns the send logic which is the behavioral fix.
  • [ ] arch note MISSING — no arch-basketball-api note found in pal-e-docs. Acceptable: basketball-api is a well-known repo, not a novel architecture component. Not blocking.
  • [x] Forgejo issue — forgejo_admin/westside-emails#10, open

File Targets

  • [x] westside-emails/src/jersey-reminder.mjml — verified via Forgejo API. Hardcoded Queens images confirmed at lines 47 (IMG_4164.jpeg) and 79 (IMG_4165.jpeg), both pointing to minio-api.tail5b443a.ts.net/assets/westside/jerseys/.
  • [x] basketball-api/src/basketball_api/services/email.py:1140 — verified: send_jersey_reminder_email exists at line 1140. Currently passes only name and jersey_url to load_email_template (line 1160-1165). No division awareness.
  • [x] Player.division field — verified at models.py:216. Mapped[Division | None]. Already used in other email functions (e.g., registration notification at line 1240).
  • [x] load_email_template — verified at line 1107. Generic dict-based {{key}} replacer. Adding new placeholders requires zero changes to the function itself.

Repo Placement

OK. Issue filed on westside-emails (template owner). Fix requires 2 PRs:
  • PR 1: westside-emails — replace hardcoded image URLs with {{jersey_image_1}} and {{jersey_image_2}} placeholders, recompile MJML to HTML
  • PR 2: basketball-api — update send_jersey_reminder_email to pass division-specific image URLs
Cross-repo dependency is documented in the issue. PR 1 must merge first (template consumed by basketball-api at runtime).

Dependencies

  • [x] basketball-api#393 (open) — MinIO image upload prerequisite. Must complete before test emails can show Kings images. Documented in issue as "Prerequisite."
  • [x] PR ordering — westside-emails PR merges first, then basketball-api PR. Documented in issue.
  • [x] Board item #733 (in_progress) — "Fix 9 failing jersey/checkout tests" — no direct conflict. Different code paths (checkout tests vs email send function).
  • [x] Board item #720 (backlog) — "Legacy jersey webhook handler missing payment_status guard" — no dependency. Different scope (webhook vs email template).

Acceptance Criteria

  • [x] AC1: "Template uses {{jersey_image_1}} and {{jersey_image_2}} placeholders" — verifiable via grep on compiled HTML template
  • [x] AC2: "send_jersey_reminder_email passes correct image URLs based on player division" — verifiable via code inspection of the dict passed to load_email_template
  • [x] AC3: "Test email to Kings parent shows Kings jerseys" — verifiable via manual email inspection (requires basketball-api#393 complete first)
  • [x] AC4: "Test email to Queens parent shows Queens jerseys" — verifiable via manual email inspection
All 4 ACs are concrete and testable. AC3/AC4 require MinIO prerequisite but that is correctly scoped as a separate ticket.

Blast Radius

Low. The load_email_template function is a generic placeholder replacer — adding new keys to the dict does not affect existing placeholders. The template change only affects jersey-reminder.mjml. No other templates reference these image URLs. Queens emails will continue to work because the Queens image URLs are preserved as the default/Queens path in the send function logic.

Decomposition

No decomposition needed. 4 ACs across 2 repos, 2 file targets (one per repo). Each PR is a small, focused change:
  • PR 1: ~4 line changes in MJML (swap 2 hardcoded URLs for placeholders) + recompile
  • PR 2: ~10 line changes in email.py (add division check + 2 new dict keys)
Estimated agent time: well under 5 minutes per PR. Two agents can run in parallel.

Recommendation

No action needed. Ticket is ready for dispatch.