Review: Email: Local teams — no Monday, first practice Tuesday

review-764-2026-04-03 Doc

review needs-refinement

Project

Template Completeness

[x] Type — Feature
[x] Lineage — Standalone
[x] Repo — forgejo_admin/basketball-api
[x] User Story — clear "who wants what and why"
[x] Context — sufficient background for fresh-context agent
[x] File Targets — specific files listed with modify/not-touch guidance
[x] Acceptance Criteria — 5 testable criteria
[x] Test Expectations — present with run command
[x] Constraints — dependencies and patterns documented
[x] Checklist — present
[x] Related — present

Traceability

[x] story:WS-S7 — "As an admin, I want to send branded email announcements so that parent comms are professional and consistent" — verified on project-westside-basketball user-stories section
[ ] arch:basketball-api — arch note MISSING — [SCOPE] Create architecture note arch-basketball-api for component basketball-api
[x] Forgejo issue — forgejo_admin/basketball-api#313, open

File Targets

[x] src/basketball_api/services/email.py — verified: exists, contains 9 send_* functions and EmailLog pattern. No welcome/local-team functions exist yet.
[x] src/basketball_api/routes/admin.py — verified: exists, contains email endpoints (profile-reminder, jersey-reminder, tryout-announcement, roster-export). No welcome-practice endpoint exists yet.
[ ] File Targets section references basketball-api#281 as "welcome-practice email endpoint" — ISSUE: #281 is actually "Add missing parent_phone/parent_email to admin CRM response models" (closed). The correct dependency is #312 ("Email: Welcome to first practice — all teams, parent-player meeting Tuesday Apr 7"), which is open but not yet implemented.

Repo Placement

OK — issue filed on basketball-api, file targets are in basketball-api. Single repo. No cross-repo concerns.

Dependencies

[ ] basketball-api#312 (Forgejo issue, board item #763) — pending. The welcome-practice endpoint for all teams. If #312 ships first, #313 can reuse the endpoint with a team filter. If not, #313 must create a standalone function. The ticket body correctly identifies this conditional dependency but incorrectly references #281 instead of #312.
[x] gmail-sdk — satisfied, already in use across 9 email functions
[x] EmailLog table — satisfied, existing pattern
[ ] Team IDs 6 and 7 — unknown. Hardcoded team_id references for 16U/17U Local Kings. No hardcoded team IDs found anywhere in the codebase. Agent must verify these IDs at runtime via DB query.

Acceptance Criteria

5 criteria, all verifiable. The team_id filter (6 and 7) is testable via DB fixture. Email content assertions are verifiable via test_email param + email_log inspection. However, team_id 6 and 7 are hardcoded assumptions — the ticket should note that the agent must query the DB to verify these map to the correct team names before sending. The EmailType enum may need a new value or can reuse announcement.

Blast Radius

Low. New endpoint + new email function added to existing patterns. 9 existing send_* functions are stable and untouched. The EmailType enum may need a new value (e.g., practice_notice) or can reuse announcement. No downstream consumers affected. Rollback is trivial — remove the new endpoint.

Decomposition Assessment

2 file targets, 1 repo, 5 AC. Estimated agent work: 3-5 minutes. No independent subtasks that need parallelization. All changes are tightly coupled (one new function + one new route). No decomposition needed.

Recommendation

[BODY] Fix incorrect issue reference: change basketball-api#281 to basketball-api#312 in File Targets and Related sections. #281 is a closed CRM issue, not the welcome-practice endpoint.
[BODY] Add explicit note that team_id 6 and 7 should be verified against the database before hardcoding — include a constraint to query team names and confirm they match "16U Local Kings" and "17U Local Kings".
[SCOPE] Create architecture note arch-basketball-api for component basketball-api.