pal-e-docs

Review: Email function + blast endpoint: send_first_payment_email + POST /admin/email/first-payment

review-875-2026-04-06 Doc

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Decomposed from #366, ticket C of 3
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — As an admin (Marcus), send branded payment email
  • [x] Context — Two components: email function + blast endpoint
  • [x] File Targets — 2 modify, 2 create, exclusions listed
  • [x] Acceptance Criteria — 9 criteria
  • [x] Test Expectations — 6 unit tests + run command
  • [x] Constraints — 6 constraints with pattern references
  • [x] Checklist — PR, tests, no unrelated changes
  • [x] Related — project, parent, dependency, plan

Traceability

  • [x] story:WS-S7 — "As an admin, I want to send branded email announcements so that parent comms are professional and consistent" — verified in project-westside-basketball user-stories section under Admin (Marcus)
  • [ ] arch:basketball-api — label present on board item, but arch note does NOT exist in pal-e-docs. [SCOPE] Create architecture note arch-basketball-api
  • [ ] arch:email — label present on board item, but arch note does NOT exist in pal-e-docs. [SCOPE] Create architecture note arch-email
  • [x] Forgejo issue — forgejo_admin/basketball-api#369, open

File Targets

  • [x] src/basketball_api/services/email.py — verified: 1424 lines total. MJML template loader section starts at line 1102 (matches ticket's "~line 1102"). _brand_wrapper() exists at line 365 with 7 existing callers.
  • [x] src/basketball_api/routes/admin.py — verified: 1058 lines total. Jersey-reminder blast ends at line 895 (matches ticket's "~line 895"). Pattern at lines 847-895 confirmed.
  • [x] tests/test_first_payment_email.py — to create. Does not exist yet (correct).
  • [x] tests/test_first_payment_blast.py — to create. Does not exist yet (correct).
  • [x] admin_client fixture pattern at tests/test_players.py:84 — verified exists.
  • [x] ContractStatus enum in models.py:104 — verified (none, offered, signed).
  • [x] EmailType enum in models.py:63 — verified. Note: first_payment value does not exist yet; depends on migration ticket #367.
All targets are specific with accurate line references. Agent can act without guessing.

Repo Placement

OK. Issue filed on basketball-api, all file targets are in basketball-api. Single-repo scope. No multi-repo concerns.

Dependencies

  • [x] #367 (migration: add EmailType.first_payment + ProductCategory.monthly) — pending, must land first. Ticket correctly documents: "depends on A (migration)."
  • [x] #368 (checkout endpoint, ticket B) — independent, no ordering required. Ticket correctly states independence.
  • [x] #366 (parent epic) — open, tracking issue.
No unresolved dependency surprises. The migration dependency is the only blocker and is correctly documented.

Acceptance Criteria

9 AC, all machine-verifiable:
  • [x] Email function sends branded HTML — testable via mock + HTML content assertion
  • [x] Email subject format — string assertion
  • [x] CTA link uses base_url + contract_token — string assertion
  • [x] Proration formula ($200->$165, $180->$150, $160->$135) — unit testable with concrete values
  • [x] "What's Next" section with full monthly fee — HTML content assertion
  • [x] EmailLog with EmailType.first_payment — DB assertion (requires migration #367)
  • [x] Blast sends to contract_status=signed only — query mock
  • [x] test_email param restricts blast — filter assertion
  • [x] Returns { sent_count, errors } — response schema assertion
No ambiguous criteria. All are concrete with expected values specified.

Blast Radius

  • Existing blast pattern (jersey-reminder at admin.py:847-895) is well-established — this follows the same pattern, low regression risk.
  • _brand_wrapper() is used by 7 existing email functions — adding another caller is safe, no modification to the wrapper itself.
  • No changes to webhooks, checkout, subscriptions, or existing email functions.
  • The proration formula must match westside-contracts +page.svelte:9 — ticket documents this; agent should cross-verify at implementation time.
  • Rollback is straightforward: revert the single PR. No data migration in this ticket (that's #367).

Decomposition Assessment

Three-thing limit: 2 files to modify + 2 files to create = 4 file touches, but all in one repo following established patterns.
Five-minute rule: 9 AC but all follow the jersey-reminder blast pattern closely. Estimated agent time: 3-4 minutes. Patterns are copy-modify, not novel.
Parallelization: email function and blast endpoint are sequential (blast imports the email function), so no independent subtasks to split.
No decomposition needed.

Recommendation

  • [SCOPE] Create architecture note arch-basketball-api (pre-existing platform gap affecting many tickets, not a blocker for this one)
  • [SCOPE] Create architecture note arch-email (pre-existing platform gap affecting many tickets, not a blocker for this one)
No [BODY] or [LABEL] fixes needed. Ticket is well-scoped and ready for dispatch once #367 (migration) lands.