pal-e-docs

Review: T4 — basketball-api Gmail notification on submission

review-949-2026-04-10 Review

review approved
Project

Verdict: APPROVED

Scope is tight, well-constrained, and implementable in a single agent pass once T3 (#430) merges.

Template Completeness

  • [x] Type (Feature)
  • [x] Lineage (depends on #430, arch-jersey-intake)
  • [x] Repo
  • [x] User Story (Marcus notification)
  • [x] Context (references feedback_gmail_oauth_not_smtp.md, feedback_email_architecture.md)
  • [x] File Targets (modify + new helper)
  • [x] Email content (subject + plain-text body template)
  • [x] Acceptance Criteria (5 items, all testable)
  • [x] Test Expectations (2 unit tests + run command)
  • [x] Constraints (reuse gmail-sdk, no submitter email, no receipt, async, no real emails in CI)
  • [x] Checklist
  • [x] Related (story, arch, dep)

Traceability

  • [x] story:WS-S31 label — "long-standing public jersey order link"
  • [x] story note verified — found in project-westside-basketball user-stories section (stories-admin-list)
  • [x] arch:jersey-intake label — Jersey Intake System B (public)
  • [x] arch note verified — arch-jersey-intake exists in pal-e-docs (note_type=doc, status=active)
  • [x] Forgejo issue — forgejo_admin/basketball-api#431, open
  • [x] wave:3 label — sequencing correct (T3 wave 2 blocks this)

File Targets

  • [~] src/basketball_api/routes/jersey_public.py — file does NOT exist yet, which is correct: T3 (#430) creates it. T4 modifies it after merge. Dependency ordering is sound.
  • [~] src/basketball_api/services/jersey_public_email.py — new file (helper), correct target directory (services/ exists with sibling email.py).
  • [x] Existing gmail-sdk integration verified — src/basketball_api/services/email.py imports from gmail_sdk import GmailClient and exposes get_gmail_client(tenant, db) which loads OAuth tokens from Postgres via token_store. Reuse path is concrete.
  • [x] BackgroundTasks pattern verified — already in use at src/basketball_api/routes/players.py:7 and line 260. Agent has a working reference.

Repo Placement

OK. basketball-api is correct — the POST endpoint lives there, and gmail-sdk is already wired. No cross-repo scope.

Dependencies

Hard dependency on #430 (T3, POST endpoint) — must merge first. Ticket explicitly documents this in Lineage, Checklist, and Related sections. Board column (backlog) and wave:3 label reflect correct sequencing. Board hook should enforce: do not move to in_progress until #430 is merged.

Acceptance Criteria

All 5 AC are testable by an agent:
  • "Email enqueued via gmail-sdk" — assertable by mocking the sender
  • "Uses BackgroundTasks" — verifiable via signature inspection + non-blocking behavior
  • "Send failure logged, POST still 201" — covered by explicit test ("mocked sender raises → POST still 201")
  • "Subject/body match template exactly" — string comparison test
  • "No real emails in CI" — satisfied by mocking the sender module-level
Test command is concrete: pytest tests/ -k jersey_public_email.

Blast Radius

Low. New helper file is isolated. Only modification to T3's route is adding a BackgroundTasks param and a post-insert enqueue call. No schema changes, no migrations, no admin-UI surface. Gmail failure path is explicitly non-fatal, so worst case is a silent miss logged to the outbox — admin can still check the admin UI (mentioned in email body link). No downstream consumers affected. The submitter is explicitly NOT emailed, so no risk of contract-email-style approval-gate violation (feedback_no_email_without_five_approvals.md does not apply — this is an internal Marcus-only notification, same pattern as existing admin-registration-notification tests).

Decomposition Assessment

No decomposition needed. File count: 2 (1 modify + 1 new helper). AC count: 5. Estimated agent work: 3-5 minutes (small helper, 2 unit tests, minor route edit). Fits the 5-minute rule — single agent pass.

Recommendation

No action needed. Ready to move to todo once #430 merges. Notes for dev agent:
  • Reuse services/email.py::get_gmail_client pattern rather than instantiating GmailClient directly — keeps token-store fallback consistent.
  • Mock at the services.jersey_public_email module boundary (e.g., patch the send function), not the gmail_sdk library, so tests stay decoupled from SDK internals.
  • Log failure via logger.exception() inside the BackgroundTasks callable so the traceback reaches the outbox without propagating to the request handler.