Review: Audit and formalize email infrastructure for westside programs

review-661-2026-03-29 Doc

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

Checked against template-issue-feature:
  • [x] Type — Feature
  • [x] Lineage — Standalone, discovered during jersey email session 2026-03-29
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — As Lucas (platform owner), wants organized email infrastructure
  • [x] Context — Detailed gap analysis of 7 issues
  • [x] File Targets — 7 modify/create paths, 2 do-not-touch paths
  • [x] Acceptance Criteria — 9 items
  • [x] Test Expectations — 3 items + run command
  • [x] Constraints — 4 items
  • [x] Checklist — standard 3 items
  • [x] Related — project + sibling issue #243
All required sections present per template-issue-feature. Template is structurally complete.

Traceability

  • [x] story:WS-S7 label — present on board item
  • [x] arch:basketball-api label — present on board item
  • [x] Forgejo issue — forgejo_admin/basketball-api#244, open
  • [ ] arch:email label — missing. This ticket touches westside-emails repo and cross-repo email infrastructure. Should carry arch:email alongside arch:basketball-api.
Traceability triangle is present but the architecture leg is incomplete — does not reflect the cross-repo nature of the work.

File Targets

  • [ ] email-templates/ — ISSUE: Does not exist in basketball-api. MJML source already lives in forgejo_admin/westside-emails repo (src/jersey-reminder.mjml, src/base-layout.mjml, src/partials/). Creating a parallel template directory in basketball-api would duplicate the existing repo.
  • [ ] email-templates/src/ — ISSUE: Same as above. westside-emails already owns MJML sources.
  • [ ] email-templates/Makefile or build.sh — ISSUE: westside-emails already has scripts/build.js and a Woodpecker CI pipeline (.woodpecker.yaml) that runs install, build, and test steps.
  • [x] src/basketball_api/services/email.py — verified exists. Contains load_email_template() (line 1107), send_jersey_reminder_email() (line 1140). Jersey reminder logged as EmailType.announcement (line 1206), confirming the mislogging claim.
  • [x] src/basketball_api/routes/admin.py — verified exists. 4 email endpoints: profile-reminder (L436), roster-export (L485), tryout-announcement (L549), jersey-reminder (L845). None have division/filter params beyond test_email.
  • [x] src/basketball_api/models.py — verified exists. EmailType enum at line 63 with 7 values (registration, admin_registration, reminder, roster_export, announcement, contract_signed, interest_notification). No jersey_reminder type. EmailLog model at line 364.
  • [x] tests/test_jersey_reminder.py — verified exists, 226 lines, covers template loader and jersey reminder endpoint.
3 of 7 file targets point to a directory structure that should not be created — the work belongs in the westside-emails repo.

Repo Placement

MISMATCH. The issue is filed on forgejo_admin/basketball-api and proposes creating email-templates/ with MJML sources inside basketball-api. However, forgejo_admin/westside-emails already exists and contains:
  • src/jersey-reminder.mjml — MJML source for jersey reminder
  • src/base-layout.mjml — shared layout partial
  • src/partials/ — reusable MJML components
  • scripts/build.js — MJML-to-HTML compilation
  • scripts/test.js — template tests
  • .woodpecker.yaml — CI pipeline (install, build, test)
  • preview/ and sample-data/ — preview/test infrastructure
The ticket conflates two repos' responsibilities:
  • westside-emails — owns MJML sources, compilation, template management, preview tooling
  • basketball-api — owns EmailType enum, email_log, admin endpoints, template consumption via load_email_template()
Board item #658 (contract email audit, filed on westside-emails#3) already follows the correct repo placement pattern. This ticket should be similarly split.

Dependencies

  • [x] Board item #660 (basketball-api#243) — jersey deadline email, backlog, story:WS-S7. This is the immediate consumer of the infrastructure this ticket formalizes. Ordering: if #243 is urgent, it should ship first using current patterns, then this audit formalizes afterward. Not a blocker, but sequencing matters.
  • [x] Board item #658 (westside-emails#3) — contract email audit, backlog, story:WS-S7. Sibling audit work targeting the same story. Should be coordinated — both touch email template management.
  • [x] No items currently in_progress block this ticket.
  • [ ] pal-e-deployments ConfigMap automation (AC 8) — no ticket exists for this dependency. The kustomize overlays that generate ConfigMaps from compiled templates are not scoped anywhere.

Acceptance Criteria

9 acceptance criteria assessed:
  • AC 1 (MJML source in repo) — belongs in westside-emails, not basketball-api. Agent dispatched to basketball-api cannot satisfy this.
  • AC 2 (compiled HTML alongside source) — belongs in westside-emails. Same issue.
  • AC 3 (build step documented) — belongs in westside-emails. Already partially satisfied by existing scripts/build.js.
  • AC 4 (EmailType enum covers all campaigns) — testable in basketball-api. Agent can grep enum and add missing values. Specific and verifiable.
  • AC 5 (email_log distinguishes types) — testable in basketball-api. Can grep for email_type=EmailType.announcement calls that should use specific types. Confirmed: jersey reminder at line 1206 uses wrong type.
  • AC 6 (division/filter params on all admin email endpoints) — testable but substantial: requires modifying 4+ endpoints. Each needs new query params and query builder changes.
  • AC 7 (plain-text fallbacks match HTML) — testable but cross-repo: requires reading templates from westside-emails to compare against fallback strings in basketball-api.
  • AC 8 (ConfigMap from repo via CI) — cross-repo, touches pal-e-deployments kustomize overlays. Not scoped in any existing ticket.
  • AC 9 (README/docs on email campaigns) — ambiguous: which repo's README? basketball-api or westside-emails?
Only AC 4-6 are cleanly scoped to basketball-api and testable by an agent. AC 1-3 belong in westside-emails. AC 7-9 are cross-repo or ambiguous.

Blast Radius

  • pal-e-deployments — ConfigMap generation changes (AC 8) require kustomize overlay updates. Not mentioned in the issue.
  • westside-emails — Primary target for template formalization but not mentioned in File Targets.
  • westside-contracts — uses contract_signed email type. Changes to EmailType enum or email infrastructure affect this consumer.
  • All 4 email endpoints — adding division/filter params (AC 6) changes API surface. If any external consumers call these endpoints, this is a breaking change. Backwards-compatible approach needed.
  • Rollback: API changes are reversible via revert. Template changes in westside-emails are independent. ConfigMap changes require deploy rollback.

Decomposition Assessment

NEEDS DECOMPOSITION.
  • 9 acceptance criteria — exceeds 5-criterion threshold
  • 7+ file targets across 3+ repos (basketball-api, westside-emails, pal-e-deployments) — exceeds 3-file/2-repo threshold
  • Estimated agent work: 30+ minutes minimum — exceeds 5-minute rule
  • Mixed concerns: template management (Node.js), API changes (Python), CI pipeline (YAML), deployment config (kustomize)
  • Independent subtasks exist: westside-emails template work and basketball-api enum/endpoint work can be parallelized
Recommend decomposition via template-board into at least 3 sub-tickets:
  • westside-emails: Add missing templates (profile-reminder, tryout-announcement, registration) to MJML pipeline. Update build/test. (~AC 1-3)
  • basketball-api: Add jersey_reminder to EmailType enum, fix email_log accuracy, add division/filter params to admin endpoints, sync plain-text fallbacks. (~AC 4-7)
  • pal-e-deployments: ConfigMap generation from westside-emails repo output, CI-driven not hand-edited. (~AC 8)

Recommendation

  • [BODY] Remove email-templates/, email-templates/src/, and email-templates/Makefile from File Targets. MJML source management belongs in westside-emails repo, which already exists with build tooling and CI.
  • [BODY] Update Context to acknowledge westside-emails repo exists and owns template sources.
  • [BODY] Remove or reassign AC 1-3 and AC 8 to westside-emails and pal-e-deployments issues respectively.
  • [BODY] Clarify AC 9 — specify which repo's README gets the email campaign documentation.
  • [BODY] Scope basketball-api issue to only: EmailType enum fix, email_log accuracy, division/filter params, plain-text sync, and test coverage.
  • [LABEL] Add arch:email label to board item to reflect cross-repo concern.
  • [DECOMPOSE] 9 AC across 3 repos, estimated 30+ min. Split into 3 tickets via template-board: (1) westside-emails template formalization, (2) basketball-api enum/endpoint fixes, (3) pal-e-deployments ConfigMap automation.