pal-e-docs

Review: westside-admin#28 (board 1145) — Input components — 2026-05-04 v2 (APPROVED)

review-1145-2026-05-04-v2 Doc

review approved
Project

Scope Review v2: westside-admin#28 (board item 1145) — Players edit form: input components

Verdict

APPROVED. v2 body addresses all three blockers from review-1145-2026-05-04 (v1, NEEDS_REFINEMENT). Greenlit by Lucas for backlog→todo on this review.

What changed in v2 (verified)

  • Test approach pivoted to pure helpers. Body now extracts parseJsonOnBlur, isoToDateInput, dateInputToIso into src/lib/components/inputs/helpers.ts and tests them in helpers.test.ts. Verified achievable: vitest.config.ts at ~/westside-admin/vitest.config.ts sets environment: 'node'; package.json has neither @testing-library/svelte nor jsdom/happy-dom. The existing src/lib/server/keycloak.test.ts is precedent for pure-logic vitest tests in node env. Component-render visual sanity is correctly scoped to a temp scratch page in PR description (not committed).
  • #1 (Drizzle) downgraded hard-dep → soft-dep. Leaf input components don't import Drizzle types; the metadata-to-component dispatcher lives in #29's +page.svelte. #28 can ship before or independently of #1. Confirmed by reading File Targets (no schema.ts or db imports).
  • Contradictory /dev/inputs checklist item dropped. Body and Checklist now consistent: no committed dev route; visual sanity = temp scratch page screenshot in PR description.

Template Completeness (template-issue-feature)

  • [x] Type — Feature
  • [x] Lineage — split from #5; soft-dep #1; blocks #29; depends-on #6 (closed)
  • [x] Repo — forgejo_admin/westside-admin
  • [x] User Story — story-westside-admin-admin-row-crud
  • [x] Context — Svelte 5 runes, pure CSS, vitest node-env, helper-based test approach explained
  • [x] File Targets — 3 components + 1 helpers.ts + 1 helpers.test.ts; explicit Do-NOT list
  • [x] Acceptance Criteria — 10 testable criteria
  • [x] Test Expectations — 9+ vitest cases on helpers; no DOM tests
  • [x] Constraints — Svelte 5 runes, pure CSS, no Tailwind, no DOM env additions
  • [x] Checklist — PR opened, vitest passes, manual visual sanity in PR desc
  • [x] Edit Log — v1 and v2 entries present

Traceability

  • [x] story:admin-row-crud — verified on project-westside-admin user-stories table (block 35117)
  • [x] story note — story-westside-admin-admin-row-crud entry present
  • [ ] arch:svelte-components — label present, but arch-svelte-components note still MISSING in pal-e-docs (shared gap with #29; carried over as soft note from v1, not a blocker since user explicitly greenlit the backlog→todo move)
  • [x] Forgejo issue — open at forgejo_admin/westside-admin#28

File Targets (against ~/westside-admin)

  • [x] src/lib/components/inputs/EnumSelect.svelte — directory does not yet exist; Create-only path is correct (verified via ls ~/westside-admin/src/lib/components/)
  • [x] src/lib/components/inputs/JsonbEditor.svelte — same, clean greenfield
  • [x] src/lib/components/inputs/DatePicker.svelte — same, clean greenfield
  • [x] src/lib/components/inputs/helpers.ts — Create
  • [x] src/lib/components/inputs/helpers.test.ts — Create. Pattern matches src/lib/server/keycloak.test.ts (vitest, node env)
  • [x] Convention checks: +layout.svelte uses let { children } = $props(); — Svelte 5 runes confirmed as repo convention

Repo Placement

OK. Single-repo change in forgejo_admin/westside-admin. No cross-repo coordination required.

Dependencies

  • Hard depends on #6 (closed, scaffolding done) — satisfied
  • Soft on #1 (Drizzle, todo column) — leaf components don't use Drizzle types; not blocking
  • Blocks #29 (Players edit route) — sibling backlog item 1146
  • No in-progress board items conflict with this scope

Acceptance Criteria

10 ACs, all testable. Helper-coverage criterion ("9+ vitest cases in helpers.test.ts: 3 per helper including edge cases") is concretely verifiable. Component-render ACs (EnumSelect renders all options, etc.) are visually verifiable from the temp scratch page screenshot in PR description. CustomEvent dispatch ACs on JsonbEditor are testable indirectly via helper unit tests on parseJsonOnBlur (the helper returns {valid, error?}; the component just relays).

Blast Radius

Greenfield directory src/lib/components/inputs/. No existing imports to break. Downstream consumer is #29 only.

Decomposition Assessment

5-min envelope passes. 5 files in one repo, all under src/lib/components/inputs/, ~9-12 unit tests. Three small Svelte components + one helper module + one test file. No cross-cutting concerns. No decomposition needed.

Recommendation

No action needed. APPROVED for backlog→todo (per Lucas's standing greenlight on this review).
Soft note carried forward (not blocking): [SCOPE] Create arch-svelte-components architecture note in pal-e-docs as a separate ticket — shared gap with #29.
  • Prior verdict: review-1145-2026-05-04 (v1, NEEDS_REFINEMENT) — Forgejo comment 16227
  • Forgejo issue: forgejo_admin/westside-admin#28
  • Board item: 1145 on board-westside-admin
  • Sibling: forgejo_admin/westside-admin#29 (board 1146) — consumes these components