Review: Single-player contract email endpoint

review-997-2026-04-12 Review

review ready

Verdict: READY

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, discovered during contract schedule fix session 2026-04-12
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — As an admin, I want to send a branded contract email to a single player's parent
  • [x] Context — Explains the gap (blast-only endpoint, no single-player send)
  • [x] File Targets — 2 files to modify, 1 exclusion
  • [x] Acceptance Criteria — 5 criteria
  • [x] Test Expectations — 3 unit tests + run command
  • [x] Constraints — 3 constraints listed
  • [x] Checklist — standard 3-item
  • [x] Related — project, story, existing endpoint referenced

Traceability

  • [x] story:WS-S20 — "As a parent, I want to sign contracts digitally so that paperwork is handled online"
  • [x] story note verified — found in project-westside-basketball user-stories section (Parent stories list)
  • [x] arch:email label present
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-email for the email component. This is a pre-existing gap across all email tickets, not specific to this one. Non-blocking for this ticket.
  • [x] Forgejo issue — forgejo_admin/basketball-api#462, open

File Targets

  • [x] src/basketball_api/routes/admin.py — verified: file exists, contains 10 existing @router.post("/email/...") endpoints including the blast endpoint at line 1044. New endpoint /email/contract/{player_id} fits the established pattern.
  • [x] src/basketball_api/services/email.py — verified: file exists, send_contract_reminder_email defined at line 1669. Takes (tenant, parent, unsigned_players, db). Function already handles a list of players — a single-player call just passes a 1-element list. _brand_wrapper is used throughout (line 202+). Reuse is straightforward.
  • [x] src/basketball_api/services/contract_offers.py — correctly excluded (contract minting, unrelated)

Repo Placement

OK. Issue filed on basketball-api, file targets are in basketball-api. Single repo, no cross-repo concerns.

Dependencies

None identified. The existing send_contract_reminder_email function and _brand_wrapper are already in place. No migrations needed — this is a pure route addition reusing existing service logic. No other board items block this.

Acceptance Criteria

All 5 criteria are testable by an agent:
  • POST endpoint with player_id path param — standard FastAPI route test
  • _brand_wrapper usage — verifiable via grep/code inspection
  • test_email query param — mirrors existing pattern on contract-reminder blast (line 1046)
  • Return confirmation payload — standard response model
  • 404 for missing player/token — standard error case
Test expectations are well-scoped: 3 unit tests covering happy path, missing token, and test_email redirect. Run command provided.

Blast Radius

Low. This adds a new endpoint — it does not modify any existing endpoint. The send_contract_reminder_email function is already called by the blast endpoint and accepts a list of players, so passing a single-element list is safe. No downstream consumers affected. No similar single-player gap exists for other email types (jersey, profile, etc.) — those could be future tickets but are out of scope here.

Decomposition Assessment

No decomposition needed. 2 file targets in 1 repo, 5 acceptance criteria, estimated agent time well under 5 minutes. The work is: add one route handler (reusing existing service function) + 3 unit tests.

Recommendation

  • [SCOPE] Create architecture note arch-email for the email component (pre-existing gap, non-blocking)
No blocking issues. Ticket is ready for dispatch.