Review: Generic send_templated_email() + EmailType migration (re-review)

Verdict: READY

Template Completeness

• [x] Type — Feature
• [x] Lineage — references MJML email system ticket dependency
• [x] Repo — forgejo_admin/basketball-api
• [x] User Story — clear developer-facing enabler story
• [x] Context — thorough explanation of current pattern and rationale
• [x] File Targets — 3 specific paths with "Files NOT to touch" section
• [x] Acceptance Criteria — 6 testable conditions
• [x] Test Expectations — 4 unit tests with run command
• [x] Constraints — function signature, OAuth, template rendering, plaintext fallback
• [x] Checklist — 4 items
• [x] Related — links to project page and MJML dependency

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 (Admin list)
• [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 list)
• [x] arch:email — references email architecture component
• [x] arch note verified — arch-email note exists in pal-e-docs (slug: arch-email, project: westside-basketball, tags: architecture,active). Documents: gmail-sdk, services/email.py, email_queries.py, outbox.py, brand.py, MJML templates, load_email_template(), EmailLog, layouts (notification/action/announcement).
• [x] Forgejo issue — forgejo_admin/basketball-api#294, state: open

File Targets

• [x] src/basketball_api/services/email.py — verified exists. Contains 7 existing send functions (lines 65-1342). load_email_template() at line 1107 and get_gmail_client() at line 29 both confirmed present.
• [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 contract_offer or contract_reminder yet — ticket is accurate.
• [x] alembic/versions/ — verified exists. 30 migrations present. Pattern for adding enum values established in 027_add_email_type_enum_values.py. Next migration would be 031.

Repo Placement

Dependencies

• [x] #750 / Issue #293 (MJML email system) — backlog. Soft dependency: templates must exist for integration testing, but unit tests can mock the template loader. Not blocking code merge.
• [x] #752 / Issue #295 (Admin blast endpoint) — backlog, downstream consumer. Correctly identified as separate ticket. Has blocked-by:294 label.
• [x] #724 (Contract reminder email endpoint) — needs_approval column. Future consumer of send_templated_email().
• [x] #658 (Audit contract offer email into MJML pipeline) — backlog. Related email work, not blocking.
• [x] #735 (Complete contract reminder email) — backlog. Related, not blocking.

Acceptance Criteria

• [x] AC1: Function signature concrete with all parameters. Testable via unit test.
• [x] AC2: EmailLog fields well-defined. Testable.
• [x] AC3: Plain text fallback specified. Testable.
• [x] AC4: Enum values are specific strings. Testable.
• [x] AC5: Alembic up+down testable. Downgrade for enum values is no-op per PostgreSQL limitation — consistent with migration 027 pattern.
• [x] AC6: "Existing functions unchanged" — testable by running full test suite.

Blast Radius

• Additive only — no existing functions modified. Low blast radius.
• Alembic enum migration uses ADD VALUE IF NOT EXISTS — safe, idempotent, proven pattern.
• No downstream consumers yet — blast endpoint (#295) and contract emails are separate tickets.

Decomposition

• 3 discrete changes: (1) new function in email.py, (2) enum values in models.py, (3) alembic migration. At the 3-thing limit.
• 6 AC + 4 test expectations. Single repo, cohesive feature.
• Estimated agent time: ~5 minutes. At boundary but acceptable — work is cohesive and interdependent.

Recommendation