Review: Populate team_name in PracticeScheduleResponse from Team join

review-827-2026-04-04 Doc

review ready

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.py lines 39-50 — verified: PracticeScheduleResponse exists, has team_id but no team_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 without team_name, team_id at line 148
  • [x] src/basketball_api/routes/public.py line 200-207 — verified: query lacks joinedload. joinedload import exists at line 14
  • [x] src/basketball_api/routes/schedule.py admin query line 213-217 — verified: also lacks joinedload. Checklist item 4 correctly calls this out
  • [x] src/basketball_api/models.py line 526 — verified: PracticeSchedule.team relationship 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 note arch-basketball-api for component basketball-api (non-blocking, pre-existing gap across all basketball-api tickets)
No blocking issues. Ticket is ready for dispatch.