Review: Public coaches endpoint — GET /public/coaches

review-430-2026-03-27 Review

review needs-refinement

Project

Template Completeness

[x] Type — Feature
[x] Lineage
[x] Repo
[x] User Story
[x] Context
[x] Security — Field Allowlist
[x] File Targets
[x] Acceptance Criteria (6 items)
[x] Test Expectations (3 items)
[x] Constraints
[x] Checklist
[x] Related

All required sections for a Feature issue are present.

Traceability

[x] story:WS-S26 label — public site enablement
[x] arch:basketball-api label — correct component
[x] Forgejo issue — forgejo_admin/basketball-api#177, open

File Targets

[x] src/basketball_api/routes/public.py — verified: file exists (230 lines), contains public router with /teams and /interest endpoints, dedicated public schemas, registered in main.py at prefix /public
[ ] ISSUE — Missing model columns: The proposed PublicCoachResponse schema references three fields that do NOT exist on the Coach SQLAlchemy model (models.py lines 257-283):

Repo Placement

OK — issue filed on forgejo_admin/basketball-api, file target is in the same repo.

Dependencies

depends:bb-176 (issue #176: Public teams endpoint) — CLOSED. public.py exists with the public router already registered in main.py. Dependency satisfied.
Board item #429 (Public teams endpoint) is in done column.
Board item #431 (SvelteKit public site layout) is in done column — the downstream consumer exists.

Acceptance Criteria

6 acceptance criteria + 3 test expectations = 9 total verifiable items. All are testable by an agent. However, criteria 3 ("slug derived from name") and criteria 4 ("photo_url points to MinIO path") cannot be satisfied without either:

(a) An Alembic migration adding bio, photo_url, and slug columns to the coaches table, OR
(b) Revising the schema so that slug is computed at runtime from name (which the Constraints section already implies), and bio/photo_url are either dropped from the response or added via migration

The ticket's Constraints section says slug = name.lower().replace(" ", "-"), suggesting runtime derivation — but the schema still lists it as a field. This ambiguity needs resolution.

Blast Radius

Existing auth-protected coach endpoints (coaches_api.py: GET /coaches/me, GET /coaches/{id}) use CoachDashboardResponse and CoachProfileResponse — these expose email, phone, and onboarding_status. The ticket correctly specifies a SEPARATE public schema. No blast radius issue if followed.
The coach onboarding routes (coach.py) are HTML-rendered forms, not JSON API — no schema conflict.
The CoachRole enum (head_coach, assistant, director) maps to the role field the ticket wants to expose. The ticket schema uses role: str | None — should clarify whether this is the enum value string or a freeform title.

Decomposition Assessment

1 file target, 1 repo, 6 acceptance criteria — fits in a single agent pass IF the missing-columns issue is resolved first. If a migration is needed, that is a separate ticket (model change + migration + seed data).

Recommendation

Three issues must be resolved before this ticket is READY:

Decide on bio and photo_url — The Coach model has neither column. Options:
Clarify slug strategy — The Constraints section says derive at runtime (name.lower().replace(" ", "-")), but the schema lists it as a field. If runtime-derived, update the schema comment to make this explicit and remove it from the "allowlisted fields" framing (it is computed, not stored). If stored, a migration is needed.
Clarify role type — The CoachRole enum has values head_coach, assistant, director. The schema uses role: str | None. Should the public endpoint expose the raw enum value, or a display-friendly string (e.g., "Head Coach")?