pal-e-docs

Review: Schedule data model + migration + seed

review-232-2026-03-28 Doc

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Child of #230 (decomposed parent)
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — story:WS-S13, clear who/what/why
  • [x] Context — thorough, includes 4 key design decisions from parent review
  • [x] File Targets — 3 create/modify targets + 2 do-not-touch exclusions + 3 pattern references with line numbers
  • [x] Acceptance Criteria — 6 items, all testable
  • [x] Test Expectations — 3 test categories + run command (pytest tests/ -v)
  • [x] Constraints — 5 items with specific pattern references
  • [x] Checklist — 4 items
  • [x] Related — parent #230, sibling #233, project, arch doc all listed

Traceability

  • [x] story:WS-S13 — "As an admin, I want schedule data stored in the database so that practices and events are managed programmatically"
  • [x] arch:basketball-api — correct target repo/component
  • [x] Forgejo issue — forgejo_admin/basketball-api#232, open

File Targets

  • [x] src/basketball_api/models.py — verified exists (490 lines). Division enum at line 52 confirmed. Tenant model at line 146 with 4 relationships (lines 156-159) confirmed. Team model at line 328 confirmed. No existing EventType, PracticeSchedule, or Event entities.
  • [x] alembic/versions/xxx_add_schedule_tables.py — to create. alembic/versions/ directory exists with 21 sequential migrations (001 through 021). Pattern clear.
  • [x] scripts/seed_schedule.py — to create. scripts/ directory exists with similar scripts (seed.py, seed_oauth_token.py).
  • [x] Pattern ref lines 1-50 (import/enum style) — verified accurate
  • [x] Pattern ref lines 146-160 (Tenant relationships) — verified accurate
  • [x] Pattern ref lines 327-346 (Team model) — verified accurate (Team class at 328-347)
  • [x] schemas.py "does not exist, not part of this ticket" — confirmed absent from filesystem
  • [x] src/basketball_api/routes/* do-not-touch — directory exists with 18 route files, correctly excluded
Targets are specific enough for an agent to act on without guessing. Line references are accurate.

Repo Placement

OK. Issue filed on basketball-api, all file targets are in basketball-api. Work is single-repo (data layer only). API endpoints are correctly deferred to sibling #233.

Dependencies

  • [x] Parent #230 (decomposed) — status:decomposed on board item #627. Lifecycle correct. No blocker.
  • [x] Sibling #233 (API endpoints) — board item #630 has label blocked-by:232. Correct dependency direction: #233 waits for #232.
  • [x] No upstream blockers — #232 has no blocked-by labels. Can start immediately.
  • [x] Division enum — already exists in models.py (line 52). No migration dependency.

Acceptance Criteria

All 6 AC items are testable and specific:
  • AC1 (EventType enum values) — grep/assert on source. Specific values listed.
  • AC2 (PracticeSchedule columns) — every column, type, and constraint specified. No ambiguity.
  • AC3 (Event columns) — same level of detail as AC2. Self-referencing FK explicitly called out.
  • AC4 (Migration up/down) — standard alembic upgrade/downgrade verification.
  • AC5 (Seed data counts) — exact counts given (8 Kings practices, 3 Kings tournaments, 5 Queens tournaments).
  • AC6 (Existing tests pass) — regression check via pytest tests/ -v.
No ambiguous "works correctly" language. Each criterion has a concrete verification path.

Blast Radius

  • New tables only — no modification to existing table schemas or columns.
  • Adding 2 relationships to Tenant model is additive (existing 4 relationships unchanged).
  • Table name events does not conflict with outbox's event_type column or process_pending_events function — different domain concepts, separate tables.
  • No downstream consumers until #233 (API endpoints) is implemented.
  • Rollback is straightforward: alembic downgrade -1 drops both tables.

Decomposition Assessment

Three-thing limit: 3 file targets (1 modify, 2 create) — within limit.
Five-minute rule: 6 AC items exceeds the >5 threshold on paper. However, the items are tightly coupled — two models + one enum + one migration + one seed script + regression check. This is one cohesive data layer addition, not 6 independent features. All patterns are exhaustively documented with line-number references. Estimated agent time: 3-4 minutes.
Parallelization: No independent subtasks. Models must exist before migration, migration before seed. Sequential by nature.
No decomposition needed.

Recommendation

[BODY] Minor: Constraints section says server_default=text("true") for is_active "matches is_public pattern on Player" — but Player.is_public uses server_default=text("false") (line 239). The actual match is Product.active (line 382). Suggest changing "matches is_public pattern on Player" to "matches active pattern on Product." Not blocking — the constraint value itself is correct.
No other action needed. Ticket is ready for dispatch.