pal-e-docs

Review: Jersey email: add division filter + persist template

review-660-2026-03-29 Doc

review ready
Project

Verdict: READY

Re-review. Original review returned NEEDS_REFINEMENT with 3 recommendations: decompose code/ops, clarify directory creation, add model field references. All addressed: code-only scope kept in basketball-api#243, ops split to westside-landing#182, issue body refined.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Decomposed from original #243, code-only scope kept here, ops split to westside-landing#182
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — As Marcus (admin), I want to filter jersey emails by division so I can target boys or girls separately
  • [x] Context — 38/39 boys never visited jersey page; template hotfixed via ConfigMap needs repo persistence
  • [x] File Targets — 4 modify/create targets, 3 do-not-touch boundaries
  • [x] Acceptance Criteria — 7 testable conditions (down from 12 after ops split)
  • [x] Test Expectations — 4 unit tests + run command (pytest tests/test_jersey_reminder.py -v)
  • [x] Constraints — endpoint pattern matching, backwards compat, template path
  • [x] Checklist — PR, tests, no unrelated changes
  • [x] Related — project-westside-basketball, westside-landing#180, westside-landing#182

Traceability

  • [x] story:WS-S7 — jersey email flow
  • [x] story:WS-S11 — division filtering
  • [x] arch:basketball-api — correct target repo
  • [x] Forgejo issue — basketball-api#243, open

File Targets

  • [x] src/basketball_api/routes/admin.py — verified: jersey-reminder endpoint at line 847, current signature has only test_email query param. Existing pattern (profile-reminder, tryout-announcement) uses same query param style.
  • [x] src/basketball_api/services/email.py — verified: send_jersey_reminder_email at line 1140, plain-text fallback logic present. Template loader reads from settings.email_templates_dir (default /data/email-templates).
  • [x] email-templates/jersey-reminder.html — does not exist yet (expected: ticket says commit from ConfigMap). Directory also missing — agent must create both. Runtime path is /data/email-templates via ConfigMap mount; repo copy is for CI/CD persistence.
  • [x] tests/test_jersey_reminder.py — verified: exists (227 lines) with 4 existing tests across 2 test classes (TestLoadEmailTemplate, TestJerseyReminderEndpoint). Agent extends with 4 new tests.
Data model verified: Player.division is Enum(Division) with values boys/girls (models.py line 216). Player.jersey_option is Enum(JerseyOption), nullable (models.py line 240). Both fields confirmed present on the Player model.

Repo Placement

Correct. Issue filed on basketball-api, all 4 file targets are in basketball-api. Ops/validation work properly decomposed to a separate ticket (westside-landing#182). No cross-repo concerns.

Dependencies

  • [x] westside-landing#180 — deadline text fix. Still open. Deployment blocker (must deploy first so email has correct dates), but not a code blocker for this ticket.
  • [x] westside-landing#182 — E2E validation + production send. Depends on this ticket. Filed correctly as a downstream ops ticket.
  • [x] No in_progress items on board-westside-basketball block this work.

Acceptance Criteria

7 ACs, all testable via pytest:
  • division=boys filter — testable: mock Player with Division.boys/girls, assert only matching parents receive email
  • exclude_ordered=true — testable: mock Player with jersey_option set vs null, assert filtering
  • Both params combined — testable: combination assertion
  • No params = send to all — testable: backwards compat (existing test covers this pattern)
  • Plain-text fallback copy — testable: assert string content in fallback path
  • Subject line — testable: assert against email send call kwargs
  • Template file committed — testable: file existence check in repo
All criteria are specific and machine-verifiable. No ambiguous language.

Blast Radius

Low. Division filtering is a new pattern for email endpoints but implemented as optional query params with backwards-compatible defaults. Three sibling email endpoints (profile-reminder, tryout-announcement, roster-export) are unaffected. No shared state mutations. Template commit is additive. Rollback is straightforward — revert the PR.

Decomposition Assessment

4 file targets in 1 repo. 7 ACs (exceeds 5-AC guideline). However:
  • All changes serve a single endpoint — highly cohesive
  • No independent subtasks that benefit from parallelization
  • Estimated agent time: ~5 minutes (add 2 query params + filter logic, update copy, commit template, write 4 tests)
  • Three discrete changes: (1) endpoint params + filtering, (2) template commit, (3) tests — at the limit but not over
  • Ops work already split to westside-landing#182 — addresses the original decomposition recommendation
No further decomposition needed. The AC count is elevated but the work is a single cohesive unit within one repo.

Recommendation

  • [BODY] Fix Related section: westside-landing#181 should be westside-landing#182. Issue #181 is a closed unrelated PR ("feat: admin user management with UUID validation"). The ops/validation ticket is #182 ("Jersey email: E2E validation + production send to boys").
One minor body fix. All other scope elements verified and correct. Ticket is ready for execution after the fix.