pal-e-docs

Review: QA nit cleanup

review-462-2026-03-27 Review

review ready
Project

Verdict: READY (already completed)

Issue #188 is closed and PR #194 is merged (commit 96f0a76). Board item #462 is still in todo — should be moved to done.

Template Completeness

  • [x] Type — "Cleanup"
  • [x] Lineage — References PR #187, discovered scope
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — Developer-facing validation tightening story
  • [x] Context — Clear motivation from QA review
  • [x] File Targets — 3 files listed with clear modify/don't-touch separation
  • [x] Acceptance Criteria — 5 criteria, all verifiable
  • [x] Test Expectations — 5 test cases listed
  • [x] Constraints — Migration and scope constraints documented
  • [x] Checklist — Standard PR/tests/no-unrelated
  • [x] Related — Links to PR #187, issue #186, project
Note: Type is "Cleanup" which is not one of the standard types (Feature/Bug/Spike/Task). This is a minor template deviation — "Cleanup" is close enough to "Task" and the intent is clear.

Traceability

  • [x] story:WS-S17 label — present on board item
  • [x] arch:basketball-api label — present on board item
  • [x] Forgejo issue — #188, now closed (status:approved, merged)

File Targets

  • [x] src/basketball_api/models.py — verified: InterestLead.tenant_id has index=True at line 406
  • [x] src/basketball_api/routes/public.py — verified: min_length=1 on player_name (L148) and age_grade (L149); max_length matches DB constraints (200, 100, 254, 30, 20); strip_whitespace validator (L154-160); parent_email logged at DEBUG only (L196), not INFO
  • [x] tests/test_interest.py — verified: 18 tests total (up from 14). New tests: empty player_name (L228), empty age_grade (L240), whitespace-only player_name (L252), whitespace-stripped player_name (L264)
  • [x] alembic/versions/026_add_index_on_interest_leads_tenant_id.py — new migration for tenant_id index (not modifying existing 025)

Repo Placement

OK — issue filed on forgejo_admin/basketball-api, all changes are in that repo. No cross-repo impact.

Dependencies

  • Parent issue #186 (POST /public/interest) — closed, PR #187 merged
  • No blocking dependencies on other board items
  • Board item #308 (reconciliation script fix) is in next_up but unrelated
  • Board item #430 (public coaches endpoint) is in todo but unrelated

Acceptance Criteria

All 5 acceptance criteria verified against current codebase:
  • tenant_id index — model has index=True, migration 026 creates the index. PASS.
  • min_length=1 on player_name/age_grade — both fields have Field(min_length=1, ...). Empty strings return 422. PASS.
  • max_length matches DB — all 5 fields match: 200, 100, 254, 30, 20. PASS.
  • parent_email not at INFO — INFO log only contains player_name, age_grade, program. Email at DEBUG. PASS.
  • whitespace stripping — strip_whitespace validator on player_name and age_grade. Whitespace-only rejected. PASS.
All test expectations met: 4 new tests added, all 18 pass.

Blast Radius

Minimal. Changes are isolated to the interest leads endpoint (public route). No other routes or models touched. No downstream consumers affected — this is a public-facing form capture endpoint.

Decomposition

Not needed. 3 file targets in 1 repo, 5 acceptance criteria, single agent pass. Work was completed in one PR.

Recommendation

Move board item #462 from todo to done. Issue is closed, PR is merged, all acceptance criteria verified against the live codebase. No further action needed.