pal-e-docs

Review: westside-admin#4 (board 1092) — Players list (v2) — 2026-05-04 APPROVED

review-1092-2026-05-04-v2 Doc

review approved
Project

Verdict: APPROVED

Re-review of forgejo_admin/westside-admin#4 (board item 1092) after v2 body edits. The three v1 schema contradictions are resolved; new 10-column allowlist verified end-to-end against the live public.players table. No further refinements required — clear to advance from todo to next_up.

Headline

v2 reconciliation is complete. All ten allowlist column names exist on the live schema with the data types the ticket claims; monthly_fee rendering is corrected to whole-dollar; the explicit "Columns NOT in v1" callout closes the loop on the v1 mistakes.

Live DB Verification (2026-05-04, post-NP fix)

Queried information_schema.columns via kubectl -n basketball-api exec postgres-9b5b87b5-5nccx -- psql -U basketball -d basketball. Result: public.players has 37 columns; SELECT COUNT(*) = 58 rows. The 10 v2 allowlist columns matched as follows:
# Column Live data_type Live is_nullable Ticket claim Match
1 <code>id</code> integer NO integer, NOT NULL OK
2 <code>name</code> character varying NO varchar, NOT NULL OK
3 <code>division</code> USER-DEFINED YES USER-DEFINED enum, nullable OK
4 <code>position</code> character varying YES varchar, nullable OK
5 <code>jersey_number</code> character varying YES varchar, nullable OK
6 <code>jersey_size</code> USER-DEFINED YES USER-DEFINED enum, nullable OK
7 <code>contract_status</code> USER-DEFINED NO USER-DEFINED enum, NOT NULL OK
8 <code>subscription_status</code> USER-DEFINED NO USER-DEFINED enum, NOT NULL OK
9 <code>monthly_fee</code> integer YES integer DOLLARS, nullable OK
10 <code>created_at</code> timestamp without time zone NO timestamp without time zone, NOT NULL OK
Every name, type, and nullability matches the live schema. The three previously-broken columns (email, team_name, updated_at) are confirmed absent from public.players and correctly relocated to the "Columns NOT in v1" section with the right rationale (parents.email, player_teams M2M, only created_at exists).

Template Completeness (Feature)

  • [x] Type — Feature
  • [x] Lineage — hard deps documented (#6, #1, #2, #3) plus indirect platform deps (#301, #302)
  • [x] Repo — forgejo_admin/westside-admin
  • [x] User Story — story-westside-admin-admin-row-crud
  • [x] Context — first end-to-end seam, justifies players-first scope
  • [x] v1 Column Allowlist — 10 cols, all schema-verified, with explicit NOT-in-v1 callout
  • [x] File Targets — Create + Modify + Do NOT Create lists
  • [x] Acceptance Criteria — 11 items, each verifiable
  • [x] Test Expectations — integration + unit + manual
  • [x] Constraints — scopedDb only, SSR only, pure CSS, allowlist inline, 10 cols, no JOINs
  • [x] Checklist — PR / screenshot / Lighthouse
  • [x] Related — project page, arch notes, dependency / blocks links
  • [x] Edit Log — v1 → v2 transition documented in body

Traceability

  • [x] story:admin-row-crud label — present on board item 1092
  • [x] Story note verified — story-westside-admin-admin-row-crud exists, AC #1 ("Index page lists all visible tables...") and AC #4 ("paginated, type-aware grid... 50/page, filter inputs, enums as badges, dates formatted") are exactly the surface this ticket implements (READ half of the story)
  • [x] Story listed on project-westside-admin user-stories section (table-4000)
  • [x] arch:page-server label — present on board item 1092
  • [~] Arch note — no standalone arch-page-server note exists, but this is consistent with the rest of board-westside-admin (no per-component arch-X notes for scoped-db, hooks-server, keycloak, harbor either). The arch:page-server label references the page-server component within arch-dataflow-westside-admin Flow 1 (cited in the issue body's Related section). Inherited acceptable from v1 review per project convention.
  • [x] Forgejo issue — forgejo_admin/westside-admin#4, state OPEN, valid URL

File Targets

  • [x] Create src/routes/players/+page.server.ts — parent src/routes/ exists; players/ subdir does not yet, will be created by the agent. OK.
  • [x] Create src/routes/players/+page.svelte — same, OK.
  • [x] Create src/lib/components/EnumBadge.svelte — parent src/lib/components/ exists (currently empty). OK.
  • [x] Create src/lib/components/DateCell.svelte — same, OK.
  • [x] Create src/lib/components/JsonbCell.svelte — same, OK. (Ticket explicitly notes JsonbCell is shipped now for parity with #29 even though v1 allowlist doesn't include a jsonb column.)
  • [x] Modify src/routes/+layout.svelte — file exists at expected path. OK.

Repo Placement

Issue is filed on forgejo_admin/westside-admin; all file targets are inside that repo. No cross-repo work. OK.

Dependencies

Hard deps: #6 (qa), #1 (todo), #2 (qa), #3 (todo). Of these, #1 and #3 are still in todo — both belong to arch:scoped-db, which is the helper this ticket consumes (scopedDb.select(allowlistedColumns).from(players)). The dev agent for #4 cannot start until #1 and #3 land. This blocker chain is correctly documented in the body's Lineage section ("Hard depends on #6, #1, #2, #3"); it does not invalidate scope but does mean #4 cannot advance to in_progress until #1 + #3 reach at minimum qa. Indirect platform deps (#301 done, #302 qa) are also tracked.
Blocks: #29 (edit page) — correctly noted.

Acceptance Criteria

11 AC. Each has an objective verification path: AC1 (50 rows, id ASC) is a count + ordering assert; AC2 (10 columns) is a snapshot or column-count test; AC3 (pagination) is a URL test; AC4 (name filter) is a query-param test; AC5 (4 enum badges) is a render assert; AC6 (DateCell) is a render assert; AC7 ($NN) is a string-format test; AC8 (no contract_token in HTML) is a body-grep assert; AC9–10 (auth) inherit from #2's already-tested handlers; AC11 (58 rows live) is a manual visual confirm against the production DB. All testable.

Blast Radius

This is a NEW route (/players) on a NEW app (westside-admin) with no existing UI users beyond Lucas + Marcus. Risk is contained. Sensitive-column exclusion (contract_token) is enforced both at the allowlist level AND by integration test grep (AC8) — defense in depth. The ticket explicitly forbids JOINs to parents or teams in v1, which keeps the auth/scoping seam single-table and matches the scoped-db design (#1, #3). No similar routes exist elsewhere to drift from.

Decomposition Assessment

6 file targets, 11 AC, ~15-25 min of agent work. Slightly over the 5-minute rule, but: (a) already split — #4 list / #28 inputs / #29 edit; (b) further splitting fragments the load→render seam; (c) v1 review already accepted this. No further decomposition needed.

Recommendation

APPROVED — no action needed. v2 body resolves every [BODY] recommendation from the v1 review. Ticket is ready to advance from todo to next_up once its hard-dep chain (#1, #3) clears.
  • Prior review: review-1092-2026-05-04 (NEEDS_REFINEMENT, v1)
  • Forgejo issue: forgejo_admin/westside-admin#4 (state: open)
  • Board item: 1092 on board-westside-admin (column: todo)
  • Story: story-westside-admin-admin-row-crud
  • Project: project-westside-admin