Review: Populate team_name in PracticeScheduleResponse from Team join
Verdict: READY
Re-review after refinement. All three NEEDS_REFINEMENT issues from the initial review are resolved. Ticket is actionable.
Template Completeness
- [x] Type — Bug
- [x] Lineage — story, arch, blocked-by, blocks documented
- [x] Repo — forgejo_admin/basketball-api
- [x] What Broke — specific: PracticeScheduleResponse missing team_name field, frontend can't group
- [x] Repro Steps — concrete curl command with jq
- [x] Expected Behavior — team_name field added and populated from Team.name
- [x] Environment — basketball-api, /public/schedule and /admin/schedule
- [x] Acceptance Criteria — 4 criteria, all testable
- [x] Related — westside-landing#209, PR #216
- [x] File Targets — explicit files with line numbers (enhanced beyond bug template)
- [x] Constraints — schema change, eager loading, admin endpoint coverage
- [x] Checklist — 6 discrete steps
- [x] Test Expectations — existing tests + new test specified
Traceability
- [x] story:WS-S13 label — "As an admin, I want to view and manage the program schedule so that I have a single source of truth for all planned activities"
- [x] story note verified — found in project-westside-basketball user-stories section under Admin (Marcus)
- [x] arch:basketball-api label — basketball-api component
- [ ] arch note MISSING — [SCOPE] Create architecture note arch-basketball-api for component basketball-api (non-blocking, pre-existing gap)
- [x] Forgejo issue — forgejo_admin/basketball-api#341, open
File Targets
- [x]
src/basketball_api/routes/schedule.pylines 39-50 — verified:PracticeScheduleResponseexists, hasteam_idbut noteam_name. Issue correctly states "Add team_name: str | None = None" - [x]
src/basketball_api/routes/schedule.py_practice_to_response()line 135 — verified: builds response withoutteam_name,team_idat line 148 - [x]
src/basketball_api/routes/public.pyline 200-207 — verified: query lacksjoinedload.joinedloadimport exists at line 14 - [x]
src/basketball_api/routes/schedule.pyadmin query line 213-217 — verified: also lacksjoinedload. Checklist item 4 correctly calls this out - [x]
src/basketball_api/models.pyline 526 — verified:PracticeSchedule.teamrelationship exists (team: Mapped["Team | None"] = relationship()) - [x]
tests/test_schedule.py— verified: existing test file exists for regression checks
Note:
schedule.py line 8 imports from sqlalchemy.orm import Session — agent will need to add joinedload to that import. Implied by checklist but not explicitly stated. Minor — agent will figure it out.Refinement Resolution
All three issues from initial review are resolved:
- Schema change needed — FIXED: Issue body now says "Add team_name: str | None = None to PracticeScheduleResponse" and Constraints says "Schema change required"
- File targets vague — FIXED: Explicit line numbers for schema (39-50), helper (_practice_to_response), and query (public.py)
- Checklist missing schema step — FIXED: First checklist item is "Add team_name: str | None = None to PracticeScheduleResponse"
Repo Placement
Correct. Issue filed on forgejo_admin/basketball-api, fix is entirely in basketball-api. Single-repo change.
Dependencies
- Blocked by: Nothing
- Blocks: Board item #804 "Fix local teams schedule — group by team name" (needs_approval, PR #216 merged) — frontend grouping depends on non-null team_name from this API fix
- Dependencies correctly documented in the Lineage section
Acceptance Criteria
4 criteria, all agent-verifiable:
- AC1: PracticeScheduleResponse includes team_name field — verifiable via schema inspection
- AC2: GET /public/schedule returns team_name populated — verifiable via curl + jq
- AC3: Practices without team_id return team_name: null — verifiable with test fixture
- AC4: No N+1 queries — verifiable via SQLAlchemy echo or query count assertion
Test expectations realistic. Existing test file at
tests/test_schedule.py.Blast Radius
Warning:
EventResponse has the same pattern — has team_id (line 86) but no team_name. The _event_to_response() helper (line 153) similarly omits team_name. Separate ticket territory if events need team name display. No other downstream consumers of PracticeScheduleResponse besides public and admin schedule endpoints.Decomposition Assessment
- 2 files in 1 repo (schedule.py + public.py) — under 3-thing limit
- 4 acceptance criteria — under 5 AC threshold
- Estimated agent time: 3-4 minutes — under 5-minute rule
- No independent subtasks to parallelize — all changes tightly coupled
No decomposition needed.
Recommendation
[SCOPE]Create architecture notearch-basketball-apifor component basketball-api (non-blocking, pre-existing gap across all basketball-api tickets)
No blocking issues. Ticket is ready for dispatch.