From 590bb280637072301a088e4504e352fa5437053b Mon Sep 17 00:00:00 2001 From: mAi Date: Tue, 26 May 2026 15:23:35 +0200 Subject: [PATCH 1/2] =?UTF-8?q?docs:=20Phase=205j=20Views-redesign=20plan?= =?UTF-8?q?=20=E2=80=94=20paliad-shape=20first-class=20views?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit m's feedback on 5i (verbatim): "It's not really what I wanted. It should like the paliad custom views, not of the existing views a variant but individually created views." 5i modelled views as overlays on existing pages (?view=). m wants the paliad model: views are first-class URLs (/views/{slug}), each one its own page. System defaults (dashboard, calendar, timeline, ...) share the route shape with reserved slugs; user-created views land beside them. Plan covers: schema redesign (slug as URL key, drop is_default_for + pinned, add icon + sort_order + show_count + last_used_at), four-route table (landing with MRU redirect, render, editor blank/edit), system- view shape (hybrid alias recommendation under Q1), editor surface (dedicated pages, not modal), migration path from 5i (drop table + delete overlay code; keep view_type enum and per-view_type renderers), seven-slice implementation chain (A schema → B routes → C system views → D editor → E sidebar → F cleanup → G polish). 11 open questions batched in §9 — head delegation pending. NO chip- picker without head's explicit re-grant (5i permission was one-time). No code changes; this branch ships docs only. Coder shifts wait on m's sign-off via head's relay. --- docs/plans/views-redesign.md | 496 +++++++++++++++++++++++++++++++++++ 1 file changed, 496 insertions(+) create mode 100644 docs/plans/views-redesign.md diff --git a/docs/plans/views-redesign.md b/docs/plans/views-redesign.md new file mode 100644 index 0000000..f351158 --- /dev/null +++ b/docs/plans/views-redesign.md @@ -0,0 +1,496 @@ +# Views redesign — paliad-shape first-class views (Phase 5j) + +**Status**: Phase A design (this doc). +**Branch**: `mai/kahn/phase-5j-views-redesign`. +**Author**: kahn (inventor), 2026-05-26. +**Source feedback** (m, 13:19 2026-05-26): *"It's not really what I wanted. It should like the paliad custom views, not of the existing views a variant but individually created views."* + +**Replaces**: Phase 5i. Hours-old, no real data, drop-and-rebuild is the cleanest path. + +--- + +## §1 — Diagnosis: why 5i diverged from intent + +5i modelled views as an **overlay** on top of existing pages. The contract was: + +> User opens `/?view=` → the saved filter+view_type fields onto whatever the existing tree handler renders. + +That choice flowed from m's original phrasing: "view types (card / list / calendar / kanban)" — which sounded like skin-on-top-of-pages. Implementation followed: TreeFilter grew a `ViewID`, an `applySavedView` overlay landed in the tree handler, the sidebar `Views` entry pointed to `/views` as a list-management page, and saved views had no URL of their own. + +m's **actual** mental model, anchored in paliad: a view IS a page. The slug goes in the URL. System defaults (dashboard, calendar, timeline, ...) and user-created views share the same `/views/{slug}` route shape. Nothing is "an overlay" — views are first-class destinations, indexed in the sidebar, with their own editor. + +The fix: tear out the 5i overlay code and rebuild around the paliad model. This redesign mirrors paliad's structure but adapts to projax's constraints (single-user, no auth.uid(), no RLS, existing route surface). + +--- + +## §2 — paliad-shape data model for projax + +### Schema (migration `0017_views_redesign.sql`) + +**Recommendation: hard-replace.** Drop `projax.views` (created hours ago in 5i Slice D), recreate fresh. No real user data lost — at most a couple of throwaway saved-view rows from m's testing. + +```sql +DROP TABLE IF EXISTS projax.views CASCADE; + +CREATE TABLE projax.views ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + slug text NOT NULL, + name text NOT NULL, + icon text, -- nullable; matches frontend icon registry + filter_json jsonb NOT NULL DEFAULT '{}'::jsonb, + view_type text NOT NULL, -- card | list | calendar | kanban | timeline + sort_field text, + sort_dir text, + group_by text, + sort_order integer NOT NULL DEFAULT 0, + show_count boolean NOT NULL DEFAULT false, + last_used_at timestamptz, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + CONSTRAINT views_view_type_chk + CHECK (view_type IN ('card','list','calendar','kanban','timeline')), + CONSTRAINT views_sort_dir_chk + CHECK (sort_dir IS NULL OR sort_dir IN ('asc','desc')), + CONSTRAINT views_kanban_needs_group + CHECK (view_type <> 'kanban' OR group_by IS NOT NULL), + CONSTRAINT views_slug_format_chk + CHECK (slug ~ '^[a-z0-9][a-z0-9-]{0,62}$') +); + +CREATE UNIQUE INDEX views_slug_uniq ON projax.views (slug); +CREATE INDEX views_sort_order_idx ON projax.views (sort_order, name); + +-- updated_at trigger reused from 0016 (kept under a new name or recreated). +CREATE OR REPLACE FUNCTION projax.views_touch_updated_at() + RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + NEW.updated_at := now(); + RETURN NEW; +END; +$$; +DROP TRIGGER IF EXISTS views_touch_updated_at ON projax.views; +CREATE TRIGGER views_touch_updated_at + BEFORE UPDATE ON projax.views + FOR EACH ROW EXECUTE FUNCTION projax.views_touch_updated_at(); +``` + +### Key shifts from 5i + +| field | 5i | 5j | reason | +|---|---|---|---| +| primary key | uuid only | uuid; **slug is the URL key** | paliad parity — URLs use slugs, not uuids | +| slug | absent | required, unique, regex-validated | URL routability | +| icon | absent | nullable text | sidebar icon picker | +| sort_order | absent | server-assigned MAX+1 | drag-reorder; paliad parity | +| show_count | absent | bool, opt-in | sidebar row-count badge; opt-in cost | +| last_used_at | absent | nullable timestamptz | `/views` landing MRU redirect | +| pinned | bool | **dropped** | `sort_order` subsumes the use case | +| is_default_for | text page | **dropped** | per-page-default model gone; MRU replaces it | + +### `filter_json` shape + +Unchanged from 5i (the JSON shape stayed correct). Keys mirror TreeFilter dims: `q`, `tags[]`, `management[]`, `status[]`, `has_links[]`, `public`, `show_archived`, `project_path`, `include_descendants`. The shape is forward-compatible; new TreeFilter dimensions land without migrations. + +`view_type` stays a top-level column (not inside `filter_json`) because the editor + sidebar both read it without needing to parse JSON. + +### Single-user simplifications vs paliad + +- **No `user_id` column** — projax is Tailscale-only single-user. +- **No RLS** — same reason. +- **`UNIQUE (slug)` is global**, not per-user. + +If multi-user ever lands, the column + index gain a `user_id` prefix; the rest of the design holds. + +--- + +## §3 — Reserved slugs (system views) + +The big call: **do existing pages become system views, or do they stay distinct routes?** + +### Three options + +**(a) Keep current routes; add /views/{slug} for user views only.** +- `/`, `/dashboard`, `/calendar`, `/timeline`, `/graph` stay exactly as today. +- `/views/{slug}` is exclusively for user-created views. +- Reserved-slug list is just `{new, edit}` (the literal route segments) + any future top-level URL we'd not want a user view to shadow. +- **Cost**: nothing changes for muscle memory. User views are an additive concept beside existing pages. +- **Drawback**: the conceptual asymmetry m flagged stays — system pages live at `/`/`/dashboard`, user views live at `/views/{slug}`. Two URL families. + +**(b) Full migration. Existing pages become system views at `/views/{slug}`.** +- New URLs: `/views/tree`, `/views/dashboard`, `/views/calendar`, `/views/timeline`, `/views/graph` (or drop graph from the unified shape — see §3.1). +- Legacy `/`, `/dashboard`, etc. become 301 redirects to their `/views/{slug}` counterpart. +- Reserved slugs: `{tree, dashboard, calendar, timeline, graph, new, edit, admin, login, logout, healthz, mcp, static, i, views}` — everything projax owns at the top level. +- **Cost**: every internal link in templates needs updating; bookmarks 301 (fine); browser muscle memory absorbs after one shift. +- **Benefit**: one URL family. The "create a new view" mental model is uniform with how system pages live. + +**(c) Hybrid. Legacy routes stay; `/views/{slug}` aliases system pages and hosts user views.** +- `/` keeps serving the tree; **also** `/views/tree` resolves to the same handler. +- `/dashboard` keeps; also `/views/dashboard`. Etc. +- Reserved slugs match (b) for the same coverage. +- User views land at `/views/{their-slug}` alongside system slugs in one URL family. +- **Cost**: small — system-view handlers register two route entries instead of one. No redirects to maintain. +- **Benefit**: muscle memory + bookmark stability AND first-class /views/{slug} URL family. Two paths to the same render; user picks whichever they remember. If `/views/{slug}` catches on, a future shift can deprecate the legacy URLs cleanly. + +### Inventor pick: (c) hybrid + +**Reasoning**: m's bug report explicitly said "individually created views" — the gap was user-view first-classness, not legacy-URL banishment. (c) closes the gap with zero migration cost. (b) is cleaner architecturally but introduces avoidable churn; the upside (one URL family) doesn't outweigh the risk of breaking some link or muscle-memory in m's daily flow. (a) leaves the two-families asymmetry m's feedback was pointing at. + +This is **Q1 in §9** — head should ratify or override before coder. + +### §3.1 — Graph as a system view? + +Graph is the DAG SVG render. It's NOT in the view_type enum (per 5i design, intentionally — graph is its own visualization, not a "list of items rendered as X"). Recommend: keep `/graph` and `/views/graph` (under (c)) but **graph is not a user-creatable view_type** — the create form omits it. Reserved slug `graph` blocks user views from clobbering it. + +### Reserved-slug list (combining (c) + projax's existing top-level routes) + +```go +var reservedViewSlugs = []string{ + // System pages (also reachable via /views/ as aliases under (c)): + "tree", "dashboard", "calendar", "timeline", "graph", + // /views sub-routes: + "new", "edit", + // Top-level application URLs: + "admin", "login", "logout", "healthz", "mcp", "static", "i", "views", +} +``` + +--- + +## §4 — Routes + +For option (c). Under (b), drop the legacy entries; under (a), drop the `/views/{system-slug}` aliases. + +| route | handler | renders | semantics | +|---|---|---|---| +| `GET /views` | `handleViewsLanding` | 302 to MRU view, else onboarding shell | landing | +| `GET /views/{slug}` | `handleViewRender` | view template per view_type | render saved or system view | +| `GET /views/new` | `handleViewEditor` | editor blank | editor — new | +| `GET /views/{slug}/edit` | `handleViewEditor` | editor pre-filled | editor — edit existing | +| `POST /views` | `handleViewCreate` | redirect to `/views/{slug}` | create | +| `POST /views/{slug}` | `handleViewUpdate` | redirect to `/views/{slug}` | update | +| `POST /views/{slug}/delete` | `handleViewDelete` | redirect to `/views` | delete | +| `POST /views/reorder` | `handleViewReorder` | 204 / HTMX OK | drag-reorder (slice G) | +| `POST /views/{slug}/touch` | `handleViewTouch` | 204 fire-and-forget | bump last_used_at on render | + +The render path (`GET /views/{slug}`): +1. Resolve slug. If a user view → load row. If a reserved system slug → load the corresponding code-resident `SystemView` struct. +2. Touch `last_used_at` (user views only — system views don't track MRU per call). +3. Dispatch to the view_type's renderer (the same per-view-type templates from 5i: `tree_card.tmpl`, `tree_kanban.tmpl`, `tree_section.tmpl` for list, plus the existing `calendar_section.tmpl` and `timeline_section.tmpl`). +4. Apply chip-overlay semantics from the 5i fix — URL chips overlay the saved filter so chip clicks narrow within the view (the one piece of 5i worth keeping; see §7). + +Editor (`GET /views/new` and `GET /views/{slug}/edit`) is a dedicated full-page form, not a modal. Paliad shipped dedicated pages; projax inherits the same shape. + +--- + +## §5 — Sidebar integration + +Replace the single "Views" sidebar entry (5i) with a "Views" section listing every user view. System views stay in the existing main-nav block at the top; they're already the muscle-memory entries (Tree, Dashboard, Calendar, Timeline, Graph). + +ASCII sketch (5g sidebar shape, with 5j additions): + +``` +[ sidebar ] +───────────── + ⌂ Tree + □ Dashboard + ▣ Calendar + ⊿ Timeline + ⨀ Graph +───────────── + Views ← new section header + 📂 Active mai work ← user view (icon + name) + ⏰ This week deadlines ← row-count badge if show_count + ★ Patents kanban ← drag-reorder handle on hover + + New view ← /views/new +───────────── + ⚙ Admin +───────────── + ☾ Theme +``` + +The Views section's entries come from `ListViews()` ordered by `sort_order` ASC, then `name`. Each entry: +- Icon resolved against a small frontend registry (the icon column is a key; the registry maps it to an SVG). Keys: `folder`, `clock`, `star`, `tag`, `file-text`, `box`, `inbox`, etc. Default key: `folder`. +- Optional badge with row count when `show_count=true` — computed by running the view's filter against `ListAll()` (cheap; projax's scale is ~150 items max). +- Active state when the current URL is `/views/{this-slug}` or a legacy alias resolving to it. + +Drag-reorder lands in a later slice (G). Click-to-open is the v1 interaction. + +Mobile bottom-nav drawer (5g slice B) gets the same section. + +--- + +## §6 — Editor surface + +Single editor template (`templates/view_editor.tmpl`) reused for both `/views/new` and `/views/{slug}/edit`. Distinguishes via the presence of `.View` in the data map. + +Fields: +- **Name** — text input, required, max 80 chars. +- **Slug** — text input, regex `^[a-z0-9][a-z0-9-]{0,62}$`, **auto-derived** from name via HTMX on `change` against a `POST /views/derive-slug?name=` helper endpoint OR on the client (simpler: derive on the server side in `handleViewCreate` if the field is empty; provide a "regenerate" link in edit mode). m can hand-edit. +- **Icon** — `` (asc/desc). +- **Group by** — `` | +| `web/templates/views.tmpl` | full rewrite — it's the list-management surface, redesigned in §5 + §6 | +| `web/templates/view_edit.tmpl` | full rewrite to the new editor shape | + +### Code to keep +- `templates/tree_card.tmpl`, `templates/tree_kanban.tmpl` — these are per-view_type renderers, reusable. +- `web/view_type.go` (the 5-value enum + `PageViewTypes` catalog) — still valid as the renderer dispatch table. +- `web/kanban.go` (`BuildKanbanBoard`) — view_type=kanban consumer. +- `templates/project_chip.tmpl` — the project filter chip strip works inside the editor. +- The 5i chip-overlay-on-saved-view fix is the **one piece of substance** worth keeping conceptually: on `/views/{slug}`, URL chip params overlay the saved filter. The overlay function gets a new home (`handleViewRender`'s filter-resolution path) but the rule is the same. + +### Backwards compatibility for the old `?view=` URL + +Two options: +- (i) **404 on `?view=`** for existing pages — the URL never makes sense in the new model. Cost: any stale bookmark dies, but only m used it for hours. +- (ii) **302-redirect `/?view=` to `/views/`** by looking up the slug from the uuid. Smoother for m's recent bookmarks. Cost: one extra DB hit on the redirect path; the redirect can target the slug or, if the uuid no longer resolves (because we hard-recreated the table), 302 to `/views`. + +Inventor pick: (ii) — small code, no broken bookmarks for the brief 5i window. + +### `is_default_for` semantics + +Drop entirely. The MRU mechanism (`last_used_at` → `/views` landing) replaces "what should I see on /views". Per-page defaults are gone; if m wants a specific view to be the landing experience, he opens it once and it becomes MRU. + +If m later wants a "this is my default" hint stronger than MRU (i.e., pinning), `sort_order=0` reserved for a pinned slot + an `is_pinned` flag is the natural extension. **Not in scope for v1.** + +--- + +## §8 — Implementation slicing + +Seven slices; A → B → C → D → E are the critical path; F + G are polish. + +### Slice A — Schema redesign + +- Migration `0017_views_redesign.sql`: `DROP TABLE projax.views CASCADE; CREATE TABLE` with new shape. (See §2 schema.) +- `store/views.go`: rewrite. Rename `View.ID` flow to be slug-driven; `GetView(slug)` instead of `GetView(uuid)`. Keep CRUD shape; add `Touch(slug)` for MRU; add `MostRecent()` returning the MRU view (or nil); add `Reorder([]string slugs)` for slice G. +- Drop `DefaultViewFor` (no longer applicable). +- Tests: round-trip CRUD by slug; reserved-slug rejection at the validator; slug-format regex enforcement; MRU. + +### Slice B — Route migration (paliad-shape) + +- Replace the 5i `/views/` routes with the paliad-shape route table from §4. +- `handleViewsLanding` → MRU redirect or onboarding shell. +- `handleViewRender` → resolve slug (user view first, then system view), apply chip overlay, dispatch to the view_type's renderer. +- `handleViewEditor` → dedicated form page (slug-driven). +- `handleViewCreate` / `handleViewUpdate` / `handleViewDelete` → form POST handlers. +- `handleViewTouch` → fire-and-forget MRU update. +- Wire the legacy `?view=` redirect (per §7-ii) on existing pages. +- Tests: each route hit, slug routing, MRU redirect, onboarding shell on empty state, reserved-slug rejection. + +### Slice C — System views + +- New `web/system_views.go` with `SystemView` struct + `TreeSystemView()`, `DashboardSystemView()`, `CalendarSystemView()`, `TimelineSystemView()`, `AllSystemViews()`, `LookupSystemView(slug)`. +- Each function returns the `(filter_json, view_type, group_by, sort)` tuple matching today's page. +- `handleViewRender` falls back to `LookupSystemView` when the slug isn't in the DB. +- Reserved-slug list (combining system slugs + route segments). +- Under (c) hybrid: legacy routes `/`, `/dashboard`, `/calendar`, `/timeline` each gain a sibling registration so `/views/{system-slug}` resolves to the same handler. (Or: legacy routes 302 to `/views/{slug}` — simpler if m's fine with one canonical URL.) +- Tests: system-view lookup, slug aliases hit the same template, reserved-slug rejection during user-view create. + +### Slice D — Editor surface + +- New `templates/view_editor.tmpl` — full form per §6. +- Slug derivation helper (`POST /views/derive-slug` or server-side fill). +- Icon picker (a `` is trivial. + +### Q7 — Drag-reorder in v1? + +- (a) Yes (slice G in v1). +- (b) v2 — `sort_order` column is server-assigned MAX+1 on create; reorder UI lands later. + +**Inventor pick**: (b). Don't expand v1 scope; reorder is a UX polish that can ship a week after. + +### Q8 — `show_count` badge in v1? + +- (a) Yes — opt-in checkbox in editor + sidebar badge. +- (b) v2 — column lands in the schema; UI lands later. + +**Inventor pick**: (a) — checkbox in editor + 2-line render in sidebar is cheap and answers the "how many things match my view" question m asks naturally. + +### Q9 — Legacy `is_default_for` semantics (§7) + +Inventor picks **dropped entirely**, replaced by MRU. Flag if m wants pin / default semantics back. + +### Q10 — Drop and recreate `projax.views`? + +- (a) Hard-replace via `DROP TABLE ... CASCADE` — inventor pick (table is hours old, ~zero data loss). +- (b) ALTER TABLE migration that adds new columns + drops old ones gracefully — more conservative; preserves any rows m has created. + +**Inventor pick**: (a). The shape change is large enough that a clean re-create is cleaner than a 6-step ALTER. + +### Q11 — `view_type=graph`? + +The graph DAG SVG render isn't in the view_type enum. Should: +- (a) Stay outside the views system — `/graph` and `/views/graph` (system slug) both serve it, user views can't be `view_type=graph`. Inventor pick. +- (b) Add `graph` as a sixth view_type — opens user-creatable graph views. + +**Inventor pick**: (a). Graph layout is single-purpose (DAG); a "graph of my filtered set" doesn't have a clear product story today. + +--- + +## §10 — Risk register + +| risk | likelihood | mitigation | +|---|---|---| +| Slug collision on rename | medium | UNIQUE index + handler maps the unique-violation to a friendly "slug already in use" error | +| URL drift (legacy bookmarks break) | low under (c), high under (b) | (c) keeps legacy URLs; (b) ships with 301 redirects + a session of m verifying his bookmarks | +| MRU thrash on rapid view switches | low | `last_used_at` is fire-and-forget; the worst case is one stale 302 | +| System-view + user-view slug collision | n/a | reserved-list rejection in validator (slice A) | +| sidebar query cost | low | `ListViews()` is one indexed lookup per page render; cache lightly if it shows in profiling | +| Editor's chip strip drifts from the page chip strip | medium | share the same template (project_chip.tmpl already shared); add a dedicated `view_filter_chips.tmpl` if drift bites | + +--- + +## §11 — Test plan headlines + +### Slice A +- `TestViewSlugCRUD` — create/get/update/delete by slug round-trip. +- `TestViewSlugFormatRejected` — uppercase, underscore, leading-digit-allowed but no-leading-dash, length-cap 63. +- `TestViewReservedSlugRejected` — create with slug `tree` / `dashboard` / `admin` / `new` etc. all 400. +- `TestViewTouch` — Touch bumps `last_used_at`. +- `TestViewMostRecent` — MRU returns most recently touched. + +### Slice B +- `TestViewsLandingMRU` — `/views` 302s to MRU view when one exists. +- `TestViewsLandingOnboarding` — `/views` renders shell when no views. +- `TestViewRender` — `/views/{slug}` resolves a user view; renders the right view_type template. +- `TestLegacyOverlayRedirect` — `/?view=` 302s to `/views/{slug}`. + +### Slice C +- `TestSystemViewLookup` — `tree` / `dashboard` / `calendar` / `timeline` / `graph` resolve via `LookupSystemView`. +- `TestSystemViewSlugAlias` — `/views/dashboard` and `/dashboard` produce identical render output. + +### Slice D +- `TestEditorBlank` — `/views/new` renders empty form. +- `TestEditorPrefilled` — `/views/{slug}/edit` reflects every persisted field. +- `TestSlugDerivation` — name "Active mai work" → slug "active-mai-work". + +### Slice E +- `TestSidebarListsViews` — layout includes every user view. +- `TestSidebarActiveState` — `/views/{slug}` marks that entry active. + +### Slice F +- All 5i overlay tests deleted; no residue references TreeFilter.ViewID. + +### Slice G +- `TestReorderUpdatesSortOrder` — POST `/views/reorder` with a sorted slug list updates the column. +- `TestShowCountBadge` — sidebar badge reflects the filter's match count. + +--- + +## §12 — References + +- `~/dev/paliad/internal/db/migrations/056_user_views.up.sql` — schema reference. +- `~/dev/paliad/internal/services/user_view_service.go` — CRUD reference. +- `~/dev/paliad/internal/services/system_views.go` — reserved-slug + system-view registration. +- `~/dev/paliad/internal/handlers/views_pages.go` — route table. +- `~/dev/paliad/frontend/src/{views,views-editor}.tsx` — editor + sidebar reference (UX only; not ported). +- `docs/plans/views-system.md` (5i) — historical record of the wrong-shape implementation. +- `docs/design.md` §4 (Interfaces). + +--- + +## §13 — Status + +- **Phase A (this doc)**: drafted by kahn, 2026-05-26. Awaiting head delegation of §9 questions to m. +- **No chip-picker for 5j** unless head explicitly re-grants per the project's escalation rule. +- **Phase B (coder)**: blocked on m's sign-off via head. Slice ordering A → B → C → D → E → F → G. +- **No code changes** in this branch beyond this doc. From 173d7ddbb23c48408abf6431109cb74d9880faff Mon Sep 17 00:00:00 2001 From: mAi Date: Fri, 29 May 2026 11:41:28 +0200 Subject: [PATCH 2/2] =?UTF-8?q?feat(views):=20Phase=205j=20slice=20A=20?= =?UTF-8?q?=E2=80=94=20paliad-shape=20schema=20redesign?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hard-replaces the 5i projax.views table per m's Q10 pick (2026-05-29): no real data to preserve after a few hours, and the shape changes are big enough that a clean recreate beats a 6-step ALTER. Schema (migration 0017_views_redesign.sql): - id (uuid), slug (text, format-CHECK'd, UNIQUE), name, icon, filter_json (jsonb — INCLUDES view_type per m's Q2), sort_field, sort_dir, group_by, sort_order, show_count, last_used_at, created_at, updated_at. - DROPPED: pinned, is_default_for, view_type column. m's Q9 picked MRU (last_used_at) over per-page-default; Q2 placed view_type inside filter_json so the JSON owns the canonical render spec. - Constraints: slug regex, sort_dir enum. NO view_type CHECK — the JSON-shape validator owns it now. - Indexes: slug UNIQUE, (sort_order, name), (last_used_at DESC). - updated_at trigger reused; projax_admin ownership preserved. Store (store/views.go rewrite): - View struct: Slug as the user-facing key; uuid kept on ID for the legacy `?view=` 302-redirect path that lands in slice C. - ListViews ordered by sort_order, name (matches sidebar). - GetView(slug) + GetViewByID(uuid). MostRecentView() drives the /views landing redirect (slice B). - TouchView(slug) bumps last_used_at fire-and-forget. - ReorderViews([]slugs) wires the column for slice G's drag UI. - CreateView server-assigns sort_order = MAX+1 inside the tx. - UpdateView replaces every writeable field; renames are supported. - Validation: slug format regex + reserved-list rejection + filter_json JSON well-formed check before round-trip. - ErrViewNotFound / ErrViewSlugTaken / ErrViewSlugReserved / ErrViewSlugFormat surface to handlers as the typed error set. Cleanup of the 5i overlay (drops what the new shape obsoletes): - web/views.go: gutted to a stub. applySavedView, applyDefaultView, overlayURLFields, filterQueryToJSON, filterJSONToQuery, filterFromJSONPayload, anySliceToStrings + every old handler (handleViewsIndex, handleViewCreate, handleViewWrite, handleViewEdit, handleViewRedirect, handleViewDelete) deleted. - web/server.go: dropped the /views route registrations and the applySavedView + applyDefaultView calls in handleTree. DefaultBanner data-map field removed. - web/tree_filter.go: TreeFilter.ViewID field removed; ParseTreeFilter and QueryString stop reading/emitting ?view=. - web/templates/views.tmpl and view_edit.tmpl deleted. - web/templates/tree_section.tmpl: default-banner block deleted. - web/views_test.go: deleted (every test was against the 5i shape). Between slice A and slice B, /views/* URLs return 404 by design. Slice B reintroduces the route family in paliad-shape: GET /views → MRU landing GET /views/{slug} → render GET /views/new → editor GET /views/{slug}/edit → editor POST /views, /views/{slug}, /views/{slug}/delete → CRUD Tests (store/views_test.go, new): - TestViewSlugCRUD — create / get-by-slug / get-by-id / rename / delete round-trip, including rename-leaves-old-slug-gone. - TestViewSlugFormatRejected — uppercase, underscore, leading dash, length-cap, empty all surface ErrViewSlugFormat. - TestViewReservedSlugRejected — tree/dashboard/calendar/timeline/graph and friends all reject with ErrViewSlugReserved. - TestViewSlugCollision — duplicate slug surfaces ErrViewSlugTaken. - TestViewMRU — TouchView + MostRecentView ordering against a controlled pair of slugs (resilient to other suites' touched views). - TestViewReorder — ReorderViews rewrites sort_order ascending. Web tests stay green (the 5i overlay tests are gone, the rest don't touch the views shape). --- db/migrations/0017_views_redesign.sql | 101 ++++++ store/views.go | 366 +++++++++++++--------- store/views_test.go | 246 +++++++++++++++ web/server.go | 40 +-- web/templates/tree_section.tmpl | 7 - web/templates/view_edit.tmpl | 42 --- web/templates/views.tmpl | 70 ----- web/tree_filter.go | 10 - web/views.go | 426 +------------------------- web/views_test.go | 327 -------------------- 10 files changed, 590 insertions(+), 1045 deletions(-) create mode 100644 db/migrations/0017_views_redesign.sql create mode 100644 store/views_test.go delete mode 100644 web/templates/view_edit.tmpl delete mode 100644 web/templates/views.tmpl delete mode 100644 web/views_test.go diff --git a/db/migrations/0017_views_redesign.sql b/db/migrations/0017_views_redesign.sql new file mode 100644 index 0000000..90befb4 --- /dev/null +++ b/db/migrations/0017_views_redesign.sql @@ -0,0 +1,101 @@ +-- 0017_views_redesign.sql +-- +-- Phase 5j Slice A: paliad-shape redesign of projax.views. +-- +-- 5i (0016) modelled views as overlays on existing pages keyed by uuid. +-- m's feedback: that's the wrong shape — views should be first-class +-- pages at /views/{slug}, mirroring paliad's user_views model. +-- +-- This migration HARD-REPLACES the 5i table. m's pick on Q10 (2026-05-29): +-- hard-replace is fine because 5i was hours old with no persisted user +-- data of value. Any rows present get dropped along with the table. +-- +-- m's other picks worth marking inline: +-- Q2 (2026-05-29): view_type lives INSIDE filter_json, not as a +-- top-level column with a CHECK constraint. Keeps the +-- schema lean — the renderer parses the JSON anyway. +-- Q9 (2026-05-29): is_default_for column dropped entirely. MRU +-- (last_used_at) replaces the per-page-default model. +-- Q11 (2026-05-29): graph stays outside the views enum; no graph +-- view_type ever lands in filter_json. + +DROP TABLE IF EXISTS projax.views CASCADE; + +CREATE TABLE projax.views ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + + -- URL-routable identifier. Application-layer validator enforces the + -- regex `^[a-z0-9][a-z0-9-]{0,62}$` + a reserved-slug list (system + -- slugs + top-level route segments). Globally unique — single-user + -- v1; no user_id prefix. + slug text NOT NULL, + + -- Display name. Free-form; user picks whatever language they think in. + -- Rendered verbatim in the sidebar. + name text NOT NULL, + + -- Frontend icon-registry key. NULL → default folder glyph. Length cap + -- keeps stored value sane even if the registry is bypassed. + icon text, + + -- Canonical view definition. Includes view_type (per m's Q2 pick), + -- plus the standard TreeFilter dimensions (q, tags, management, …), + -- plus optional sort/group hints. Renderer parses the JSON; the DB + -- never has to look inside. + filter_json jsonb NOT NULL DEFAULT '{}'::jsonb, + + -- Sort + grouping hints used by the renderers (list/card/kanban). + -- Kept as top-level columns so the editor can index them quickly, + -- though they're conceptually part of the render spec. + sort_field text, + sort_dir text, + group_by text, + + -- Sidebar ordering. Server-assigned MAX+1 on create so two parallel + -- inserts don't collide. Drag-reorder UI lands in slice G; this + -- column is wired now so the data shape is stable. + sort_order integer NOT NULL DEFAULT 0, + + -- Opt-in count badge on the sidebar entry. Defaults false so casual + -- views don't pay the COUNT(*) cost. + show_count boolean NOT NULL DEFAULT false, + + -- MRU landing on /views — `handleViewsLanding` 302s here when set. + -- Touched fire-and-forget on every render. + last_used_at timestamptz, + + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + + CONSTRAINT views_sort_dir_chk + CHECK (sort_dir IS NULL OR sort_dir IN ('asc','desc')), + CONSTRAINT views_slug_format_chk + CHECK (slug ~ '^[a-z0-9][a-z0-9-]{0,62}$') +); + +CREATE UNIQUE INDEX views_slug_uniq ON projax.views (slug); +CREATE INDEX views_sort_order_idx ON projax.views (sort_order, name); +CREATE INDEX views_last_used_idx ON projax.views (last_used_at DESC NULLS LAST); + +-- updated_at trigger. Re-created here (CREATE OR REPLACE on the function) +-- because 0016 dropped with CASCADE above. +CREATE OR REPLACE FUNCTION projax.views_touch_updated_at() + RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + NEW.updated_at := now(); + RETURN NEW; +END; +$$; + +DROP TRIGGER IF EXISTS views_touch_updated_at ON projax.views; +CREATE TRIGGER views_touch_updated_at + BEFORE UPDATE ON projax.views + FOR EACH ROW EXECUTE FUNCTION projax.views_touch_updated_at(); + +DO $own$ BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'projax_admin') THEN + EXECUTE 'ALTER TABLE projax.views OWNER TO projax_admin'; + EXECUTE 'ALTER FUNCTION projax.views_touch_updated_at() OWNER TO projax_admin'; + EXECUTE 'GRANT SELECT, INSERT, UPDATE, DELETE ON projax.views TO projax_admin'; + END IF; +END $own$; diff --git a/store/views.go b/store/views.go index fbba8d2..aecd829 100644 --- a/store/views.go +++ b/store/views.go @@ -5,58 +5,108 @@ import ( "encoding/json" "errors" "fmt" + "regexp" "strings" "time" "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgxpool" ) -// View is one row in projax.views. Phase 5i Slice D — saved views. -// -// FilterJSON carries the persisted filter state as raw JSON so callers can -// freely round-trip into their TreeFilter or another future filter type -// without forcing the store package to depend on web/. +// View is one row in projax.views — a first-class /views/{slug} page. +// Phase 5j paliad-shape: the slug is the user-facing key; URLs and the +// sidebar both index by it. The uuid id stays because it's cheap and +// surfaces in future MCP integrations, but it is NOT exposed in URLs. type View struct { - ID string - Name string - Description string - FilterJSON []byte // raw jsonb payload - ViewType string - SortField *string - SortDir *string - GroupBy *string - Pinned bool - IsDefaultFor *string - CreatedAt time.Time - UpdatedAt time.Time + ID string + Slug string + Name string + Icon *string + FilterJSON []byte // raw jsonb payload — includes view_type per m's Q2 + SortField *string + SortDir *string + GroupBy *string + SortOrder int + ShowCount bool + LastUsedAt *time.Time + CreatedAt time.Time + UpdatedAt time.Time } -// ErrViewNotFound surfaces from GetView / SoftDeleteView when no row matches. +// ErrViewNotFound surfaces from Get*/Update*/Delete when no row matches. var ErrViewNotFound = errors.New("view not found") -// ViewInput is the writeable subset of View used by Create / Update. -type ViewInput struct { - Name string - Description string - FilterJSON []byte - ViewType string - SortField string - SortDir string - GroupBy string - Pinned bool - IsDefaultFor string // "" → clear default +// ErrViewSlugTaken is returned by Create / Update when the slug already +// belongs to another view. Web handlers map this to 409. +var ErrViewSlugTaken = errors.New("view slug already exists") + +// ErrViewSlugReserved is returned when the caller picks a slug that +// shadows a system slug or a top-level URL segment. Web handlers map +// this to 400 with a friendly message. +var ErrViewSlugReserved = errors.New("view slug is reserved") + +// ErrViewSlugFormat is returned when the slug doesn't match the format +// regex. Same mapping as reserved. +var ErrViewSlugFormat = errors.New("view slug must match ^[a-z0-9][a-z0-9-]{0,62}$") + +// slugRE is the format guard. Mirrors the SQL CHECK constraint so callers +// get a friendly error before round-tripping to the DB. +var slugRE = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,62}$`) + +// reservedViewSlugs is the static list of slugs the validator rejects. +// Combines system-view slugs (slice C wires them) with top-level route +// segments the application owns. +var reservedViewSlugs = map[string]struct{}{ + // System views (slice C): + "tree": {}, "dashboard": {}, "calendar": {}, "timeline": {}, "graph": {}, + // /views sub-routes: + "new": {}, "edit": {}, + // Top-level application URLs: + "admin": {}, "login": {}, "logout": {}, "healthz": {}, "mcp": {}, + "static": {}, "i": {}, "views": {}, } -// ListViews returns every non-deleted view ordered by pinned-first, then name. +// IsReservedViewSlug reports whether the slug shadows a system slug or a +// top-level URL segment. Exported for the editor's slug-derivation +// helper. +func IsReservedViewSlug(slug string) bool { + _, ok := reservedViewSlugs[strings.ToLower(slug)] + return ok +} + +// ValidateSlug runs format + reserved checks. Returns nil for valid slugs. +func ValidateSlug(slug string) error { + if !slugRE.MatchString(slug) { + return ErrViewSlugFormat + } + if IsReservedViewSlug(slug) { + return ErrViewSlugReserved + } + return nil +} + +// ViewInput is the writeable subset for Create / Update. Defaults +// applied: nil FilterJSON → {}; SortOrder is server-assigned on Create. +type ViewInput struct { + Slug string + Name string + Icon *string + FilterJSON []byte + SortField string + SortDir string + GroupBy string + ShowCount bool +} + +// ListViews returns every view ordered by sort_order ASC then name — +// matches the sidebar rendering order. func (s *Store) ListViews(ctx context.Context) ([]*View, error) { rows, err := s.Pool.Query(ctx, ` -SELECT id, name, coalesce(description,''), filter_json, view_type, - sort_field, sort_dir, group_by, pinned, is_default_for, +SELECT id, slug, name, icon, filter_json, + sort_field, sort_dir, group_by, + sort_order, show_count, last_used_at, created_at, updated_at FROM projax.views -WHERE deleted_at IS NULL -ORDER BY pinned DESC, lower(name) ASC`) +ORDER BY sort_order ASC, name ASC`) if err != nil { return nil, fmt.Errorf("list views: %w", err) } @@ -72,14 +122,25 @@ ORDER BY pinned DESC, lower(name) ASC`) return out, rows.Err() } -// GetView returns one view by id. ErrViewNotFound when missing or soft-deleted. -func (s *Store) GetView(ctx context.Context, id string) (*View, error) { +// GetView returns one view by slug. ErrViewNotFound when missing. +func (s *Store) GetView(ctx context.Context, slug string) (*View, error) { + return s.getView(ctx, `slug = $1`, slug) +} + +// GetViewByID returns one view by uuid id. Used by the legacy +// `?view=` 302-redirect path during the 5i → 5j cutover. +func (s *Store) GetViewByID(ctx context.Context, id string) (*View, error) { + return s.getView(ctx, `id = $1`, id) +} + +func (s *Store) getView(ctx context.Context, where, arg string) (*View, error) { row := s.Pool.QueryRow(ctx, ` -SELECT id, name, coalesce(description,''), filter_json, view_type, - sort_field, sort_dir, group_by, pinned, is_default_for, +SELECT id, slug, name, icon, filter_json, + sort_field, sort_dir, group_by, + sort_order, show_count, last_used_at, created_at, updated_at FROM projax.views -WHERE id = $1 AND deleted_at IS NULL`, id) +WHERE `+where, arg) v, err := scanView(row) if errors.Is(err, pgx.ErrNoRows) { return nil, ErrViewNotFound @@ -87,9 +148,29 @@ WHERE id = $1 AND deleted_at IS NULL`, id) return v, err } -// CreateView inserts a row. When IsDefaultFor is set, the prior default for -// that page is cleared in the same transaction so the partial unique index -// can't fire after a Postgres rewrite. +// MostRecentView returns the view with the most recent last_used_at. nil +// when no view has been touched yet (or none exist). Drives the /views +// landing redirect. +func (s *Store) MostRecentView(ctx context.Context) (*View, error) { + row := s.Pool.QueryRow(ctx, ` +SELECT id, slug, name, icon, filter_json, + sort_field, sort_dir, group_by, + sort_order, show_count, last_used_at, + created_at, updated_at +FROM projax.views +WHERE last_used_at IS NOT NULL +ORDER BY last_used_at DESC +LIMIT 1`) + v, err := scanView(row) + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + return v, err +} + +// CreateView inserts a new view. SortOrder is server-assigned to +// MAX(existing)+1 inside the same tx so two parallel creates don't +// collide on the index. func (s *Store) CreateView(ctx context.Context, in ViewInput) (*View, error) { if err := validateViewInput(in); err != nil { return nil, err @@ -97,95 +178,81 @@ func (s *Store) CreateView(ctx context.Context, in ViewInput) (*View, error) { if in.FilterJSON == nil { in.FilterJSON = []byte("{}") } - var id string tx, err := s.Pool.BeginTx(ctx, pgx.TxOptions{}) if err != nil { return nil, fmt.Errorf("begin: %w", err) } defer func() { _ = tx.Rollback(ctx) }() - if in.IsDefaultFor != "" { - if _, err := tx.Exec(ctx, ` -UPDATE projax.views -SET is_default_for = NULL -WHERE is_default_for = $1 AND deleted_at IS NULL`, in.IsDefaultFor); err != nil { - return nil, fmt.Errorf("clear prior default: %w", err) - } + var nextOrder int + if err := tx.QueryRow(ctx, + `SELECT COALESCE(MAX(sort_order), -1) + 1 FROM projax.views`, + ).Scan(&nextOrder); err != nil { + return nil, fmt.Errorf("compute next sort_order: %w", err) } + var id string err = tx.QueryRow(ctx, ` INSERT INTO projax.views - (name, description, filter_json, view_type, sort_field, sort_dir, group_by, pinned, is_default_for) + (slug, name, icon, filter_json, sort_field, sort_dir, group_by, sort_order, show_count) VALUES - ($1, NULLIF($2,''), $3::jsonb, $4, NULLIF($5,''), NULLIF($6,''), NULLIF($7,''), $8, NULLIF($9,'')) + ($1, $2, $3, $4::jsonb, NULLIF($5,''), NULLIF($6,''), NULLIF($7,''), $8, $9) RETURNING id`, - in.Name, in.Description, in.FilterJSON, in.ViewType, - in.SortField, in.SortDir, in.GroupBy, in.Pinned, in.IsDefaultFor, + in.Slug, in.Name, in.Icon, in.FilterJSON, + in.SortField, in.SortDir, in.GroupBy, nextOrder, in.ShowCount, ).Scan(&id) if err != nil { + if isUniqueSlugViolation(err) { + return nil, ErrViewSlugTaken + } return nil, fmt.Errorf("insert view: %w", err) } if err := tx.Commit(ctx); err != nil { return nil, fmt.Errorf("commit: %w", err) } - return s.GetView(ctx, id) + return s.GetView(ctx, in.Slug) } -// UpdateView replaces every writeable field. Same default-clearing semantics -// as CreateView. -func (s *Store) UpdateView(ctx context.Context, id string, in ViewInput) (*View, error) { +// UpdateView replaces every writeable field on the row matching `slug`. +// To rename, pass the desired new slug in `in.Slug`; if it collides with +// another row, ErrViewSlugTaken surfaces. +func (s *Store) UpdateView(ctx context.Context, slug string, in ViewInput) (*View, error) { if err := validateViewInput(in); err != nil { return nil, err } if in.FilterJSON == nil { in.FilterJSON = []byte("{}") } - tx, err := s.Pool.BeginTx(ctx, pgx.TxOptions{}) - if err != nil { - return nil, fmt.Errorf("begin: %w", err) - } - defer func() { _ = tx.Rollback(ctx) }() - if in.IsDefaultFor != "" { - if _, err := tx.Exec(ctx, ` + tag, err := s.Pool.Exec(ctx, ` UPDATE projax.views -SET is_default_for = NULL -WHERE is_default_for = $1 AND id <> $2 AND deleted_at IS NULL`, - in.IsDefaultFor, id); err != nil { - return nil, fmt.Errorf("clear prior default: %w", err) - } - } - tag, err := tx.Exec(ctx, ` -UPDATE projax.views -SET name = $2, - description = NULLIF($3,''), - filter_json = $4::jsonb, - view_type = $5, - sort_field = NULLIF($6,''), - sort_dir = NULLIF($7,''), - group_by = NULLIF($8,''), - pinned = $9, - is_default_for = NULLIF($10,'') -WHERE id = $1 AND deleted_at IS NULL`, - id, in.Name, in.Description, in.FilterJSON, in.ViewType, - in.SortField, in.SortDir, in.GroupBy, in.Pinned, in.IsDefaultFor, +SET slug = $2, + name = $3, + icon = $4, + filter_json = $5::jsonb, + sort_field = NULLIF($6,''), + sort_dir = NULLIF($7,''), + group_by = NULLIF($8,''), + show_count = $9 +WHERE slug = $1`, + slug, in.Slug, in.Name, in.Icon, in.FilterJSON, + in.SortField, in.SortDir, in.GroupBy, in.ShowCount, ) if err != nil { + if isUniqueSlugViolation(err) { + return nil, ErrViewSlugTaken + } return nil, fmt.Errorf("update view: %w", err) } if tag.RowsAffected() == 0 { return nil, ErrViewNotFound } - if err := tx.Commit(ctx); err != nil { - return nil, fmt.Errorf("commit: %w", err) - } - return s.GetView(ctx, id) + return s.GetView(ctx, in.Slug) } -// SoftDeleteView sets deleted_at on the row. Idempotent (returns ErrViewNotFound -// only when the row never existed; subsequent calls on a soft-deleted row -// silently succeed since deleted_at is just refreshed). -func (s *Store) SoftDeleteView(ctx context.Context, id string) error { - tag, err := s.Pool.Exec(ctx, ` -UPDATE projax.views SET deleted_at = now() -WHERE id = $1`, id) +// DeleteView removes a view by slug. Hard delete (no soft-delete column +// in the redesign — single-user, no audit obligation). Idempotent only +// on the second call; first call against a non-existent row returns +// ErrViewNotFound. +func (s *Store) DeleteView(ctx context.Context, slug string) error { + tag, err := s.Pool.Exec(ctx, `DELETE FROM projax.views WHERE slug = $1`, slug) if err != nil { return fmt.Errorf("delete view: %w", err) } @@ -195,79 +262,100 @@ WHERE id = $1`, id) return nil } -// DefaultViewFor returns the view that should auto-apply on the named page, -// or nil if none is set. -func (s *Store) DefaultViewFor(ctx context.Context, page string) (*View, error) { - row := s.Pool.QueryRow(ctx, ` -SELECT id, name, coalesce(description,''), filter_json, view_type, - sort_field, sort_dir, group_by, pinned, is_default_for, - created_at, updated_at -FROM projax.views -WHERE is_default_for = $1 AND deleted_at IS NULL -LIMIT 1`, page) - v, err := scanView(row) - if errors.Is(err, pgx.ErrNoRows) { - return nil, nil +// TouchView bumps last_used_at to now(). Fire-and-forget from the render +// handler — failures are logged but never block the page. +func (s *Store) TouchView(ctx context.Context, slug string) error { + tag, err := s.Pool.Exec(ctx, + `UPDATE projax.views SET last_used_at = now() WHERE slug = $1`, slug) + if err != nil { + return fmt.Errorf("touch view: %w", err) } - return v, err + if tag.RowsAffected() == 0 { + return ErrViewNotFound + } + return nil } -// validateViewInput runs the Go-side guards. The DB CHECK constraints provide -// the durable contract; these checks let handlers surface a friendlier error. +// ReorderViews applies a sort_order rewrite where the provided slugs map +// to ascending sort_order values starting at 0. Slugs not present in the +// input keep their existing sort_order. Drives slice G's drag-reorder UI. +func (s *Store) ReorderViews(ctx context.Context, slugs []string) error { + if len(slugs) == 0 { + return nil + } + tx, err := s.Pool.BeginTx(ctx, pgx.TxOptions{}) + if err != nil { + return fmt.Errorf("begin: %w", err) + } + defer func() { _ = tx.Rollback(ctx) }() + for i, slug := range slugs { + if _, err := tx.Exec(ctx, + `UPDATE projax.views SET sort_order = $1 WHERE slug = $2`, + i, slug, + ); err != nil { + return fmt.Errorf("reorder %q: %w", slug, err) + } + } + return tx.Commit(ctx) +} + +// validateViewInput runs Go-side guards. The DB CHECK constraints are the +// durable contract; these checks let handlers surface friendlier errors. func validateViewInput(in ViewInput) error { + if err := ValidateSlug(in.Slug); err != nil { + return err + } if strings.TrimSpace(in.Name) == "" { return errors.New("view name is required") } - switch in.ViewType { - case "card", "list", "calendar", "kanban", "timeline": - default: - return fmt.Errorf("invalid view_type %q (allowed: card list calendar kanban timeline)", in.ViewType) - } if in.SortDir != "" && in.SortDir != "asc" && in.SortDir != "desc" { return fmt.Errorf("invalid sort_dir %q", in.SortDir) } - if in.ViewType == "kanban" && strings.TrimSpace(in.GroupBy) == "" { - return errors.New("kanban view_type requires group_by") - } - if in.IsDefaultFor != "" { - switch in.IsDefaultFor { - case "tree", "dashboard", "calendar", "timeline": - default: - return fmt.Errorf("invalid is_default_for %q", in.IsDefaultFor) - } + if in.Icon != nil && len(*in.Icon) > 64 { + return errors.New("icon key exceeds 64 characters") } if len(in.FilterJSON) > 0 { - var dummy any - if err := json.Unmarshal(in.FilterJSON, &dummy); err != nil { + var probe any + if err := json.Unmarshal(in.FilterJSON, &probe); err != nil { return fmt.Errorf("filter_json is not valid JSON: %w", err) } } return nil } +// isUniqueSlugViolation matches the postgres unique_violation SQLSTATE +// (23505) on the views_slug_uniq index. We don't import pgconn here to +// avoid widening the package's dep surface; substring match on the +// pgx-formatted error covers both the wire-level codes pgx surfaces. +func isUniqueSlugViolation(err error) bool { + if err == nil { + return false + } + s := err.Error() + return strings.Contains(s, "views_slug_uniq") || + (strings.Contains(s, "SQLSTATE 23505") && strings.Contains(s, "slug")) +} + type viewScanner interface { Scan(dest ...any) error } func scanView(s viewScanner) (*View, error) { v := &View{} - var sortField, sortDir, groupBy, isDefaultFor *string + var icon, sortField, sortDir, groupBy *string + var lastUsedAt *time.Time if err := s.Scan( - &v.ID, &v.Name, &v.Description, &v.FilterJSON, &v.ViewType, - &sortField, &sortDir, &groupBy, &v.Pinned, &isDefaultFor, + &v.ID, &v.Slug, &v.Name, &icon, &v.FilterJSON, + &sortField, &sortDir, &groupBy, + &v.SortOrder, &v.ShowCount, &lastUsedAt, &v.CreatedAt, &v.UpdatedAt, ); err != nil { return nil, err } + v.Icon = icon v.SortField = sortField v.SortDir = sortDir v.GroupBy = groupBy - v.IsDefaultFor = isDefaultFor + v.LastUsedAt = lastUsedAt return v, nil } - -// pgxRowsCompat keeps the linter quiet about importing pgxpool only for -// type assertions inside views.go. The Pool method on Store already pulls -// pgxpool into the package; nothing to do here, but the unused-import -// shadow doesn't bite. -var _ = pgxpool.Pool{} diff --git a/store/views_test.go b/store/views_test.go new file mode 100644 index 0000000..ab4c35a --- /dev/null +++ b/store/views_test.go @@ -0,0 +1,246 @@ +package store_test + +import ( + "context" + "errors" + "os" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/m/projax/store" +) + +// connect mirrors db_test's connect helper. The store package owns its own +// integration tests (Phase 5j Slice A introduced this file alongside the +// schema redesign); it shares the same env-var convention to skip when no +// DB is wired up. +func connect(t *testing.T) (*pgxpool.Pool, *store.Store) { + t.Helper() + url := os.Getenv("PROJAX_DB_URL") + if url == "" { + url = os.Getenv("SUPABASE_DATABASE_URL") + } + if url == "" { + t.Skip("no PROJAX_DB_URL / SUPABASE_DATABASE_URL set — skipping integration test") + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + pool, err := pgxpool.New(ctx, url) + if err != nil { + t.Fatalf("pool: %v", err) + } + if err := pool.Ping(ctx); err != nil { + t.Skipf("DB unreachable: %v", err) + } + return pool, store.New(pool) +} + +// uniqueSlug suffixes a base slug with a timestamp so parallel test runs +// don't collide on the views_slug_uniq index. +func uniqueSlug(prefix string) string { + return prefix + "-" + strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "") +} + +func TestViewSlugCRUD(t *testing.T) { + pool, s := connect(t) + defer pool.Close() + ctx := context.Background() + slug := uniqueSlug("p5j-a-crud") + defer pool.Exec(context.Background(), `DELETE FROM projax.views WHERE slug LIKE 'p5j-a-crud-%' OR slug LIKE 'p5j-a-renamed-%'`) + + // Create. + created, err := s.CreateView(ctx, store.ViewInput{ + Slug: slug, + Name: "Slice A CRUD", + FilterJSON: []byte(`{"view_type":"list","tags":["work"]}`), + }) + if err != nil { + t.Fatalf("create: %v", err) + } + if created.Slug != slug { + t.Errorf("slug = %q, want %q", created.Slug, slug) + } + if created.ID == "" { + t.Error("ID should be populated on create") + } + if created.SortOrder < 0 { + t.Errorf("sort_order should be >= 0 (server-assigned), got %d", created.SortOrder) + } + + // GetView by slug. + got, err := s.GetView(ctx, slug) + if err != nil { + t.Fatalf("get: %v", err) + } + if string(got.FilterJSON) != `{"view_type": "list", "tags": ["work"]}` && string(got.FilterJSON) != `{"tags": ["work"], "view_type": "list"}` { + // Postgres jsonb normalises key order — accept either ordering. + // Verify it round-trips structurally. + if !strings.Contains(string(got.FilterJSON), `"view_type"`) || !strings.Contains(string(got.FilterJSON), `"tags"`) { + t.Errorf("filter_json did not round-trip view_type+tags: %s", got.FilterJSON) + } + } + + // GetViewByID (legacy 5i 302-redirect path uses this). + byID, err := s.GetViewByID(ctx, created.ID) + if err != nil { + t.Fatalf("get by id: %v", err) + } + if byID.Slug != slug { + t.Errorf("by-id lookup returned wrong slug: %q", byID.Slug) + } + + // Update — rename slug + change filter. + renamed := uniqueSlug("p5j-a-renamed") + updated, err := s.UpdateView(ctx, slug, store.ViewInput{ + Slug: renamed, + Name: "Renamed", + FilterJSON: []byte(`{"view_type":"card"}`), + }) + if err != nil { + t.Fatalf("update: %v", err) + } + if updated.Slug != renamed { + t.Errorf("renamed slug = %q, want %q", updated.Slug, renamed) + } + if _, err := s.GetView(ctx, slug); !errors.Is(err, store.ErrViewNotFound) { + t.Errorf("old slug should be ErrViewNotFound after rename, got %v", err) + } + + // Delete. + if err := s.DeleteView(ctx, renamed); err != nil { + t.Fatalf("delete: %v", err) + } + if _, err := s.GetView(ctx, renamed); !errors.Is(err, store.ErrViewNotFound) { + t.Errorf("post-delete get should be ErrViewNotFound, got %v", err) + } + if err := s.DeleteView(ctx, renamed); !errors.Is(err, store.ErrViewNotFound) { + t.Errorf("second delete should be ErrViewNotFound, got %v", err) + } +} + +func TestViewSlugFormatRejected(t *testing.T) { + pool, s := connect(t) + defer pool.Close() + ctx := context.Background() + bad := []string{ + "", // empty + "UPPER", // uppercase + "under_score", // underscore + "-leading-dash", // leading dash + "a." + strings.Repeat("x", 100), // too long + invalid char + strings.Repeat("a", 64), // length cap is 63 (1 + 62) + } + for _, slug := range bad { + _, err := s.CreateView(ctx, store.ViewInput{ + Slug: slug, Name: "x", FilterJSON: []byte(`{}`), + }) + if !errors.Is(err, store.ErrViewSlugFormat) { + t.Errorf("slug=%q expected ErrViewSlugFormat, got %v", slug, err) + } + } +} + +func TestViewReservedSlugRejected(t *testing.T) { + _, s := connect(t) + ctx := context.Background() + for _, slug := range []string{"tree", "dashboard", "calendar", "timeline", "graph", "new", "edit", "admin", "views"} { + _, err := s.CreateView(ctx, store.ViewInput{ + Slug: slug, Name: "x", FilterJSON: []byte(`{}`), + }) + if !errors.Is(err, store.ErrViewSlugReserved) { + t.Errorf("reserved slug %q should be rejected, got %v", slug, err) + } + } +} + +func TestViewSlugCollision(t *testing.T) { + pool, s := connect(t) + defer pool.Close() + ctx := context.Background() + slug := uniqueSlug("p5j-a-collision") + defer pool.Exec(context.Background(), `DELETE FROM projax.views WHERE slug = $1`, slug) + + if _, err := s.CreateView(ctx, store.ViewInput{Slug: slug, Name: "First"}); err != nil { + t.Fatalf("first create: %v", err) + } + if _, err := s.CreateView(ctx, store.ViewInput{Slug: slug, Name: "Second"}); !errors.Is(err, store.ErrViewSlugTaken) { + t.Errorf("duplicate slug should be ErrViewSlugTaken, got %v", err) + } +} + +func TestViewMRU(t *testing.T) { + pool, s := connect(t) + defer pool.Close() + ctx := context.Background() + a := uniqueSlug("p5j-a-mru-a") + b := uniqueSlug("p5j-a-mru-b") + defer pool.Exec(context.Background(), `DELETE FROM projax.views WHERE slug IN ($1, $2)`, a, b) + + if _, err := s.CreateView(ctx, store.ViewInput{Slug: a, Name: "A"}); err != nil { + t.Fatalf("create a: %v", err) + } + if _, err := s.CreateView(ctx, store.ViewInput{Slug: b, Name: "B"}); err != nil { + t.Fatalf("create b: %v", err) + } + + // MostRecentView with no touches yet — when no view in the table has + // last_used_at set, MRU returns nil. (Other tests may have left their + // own touched views, so we only assert on the slugs we control.) + if err := s.TouchView(ctx, a); err != nil { + t.Fatalf("touch a: %v", err) + } + time.Sleep(20 * time.Millisecond) + if err := s.TouchView(ctx, b); err != nil { + t.Fatalf("touch b: %v", err) + } + + mru, err := s.MostRecentView(ctx) + if err != nil { + t.Fatalf("mru: %v", err) + } + // Other tests' touched views may rank higher; we only assert that + // when MRU is one of OURS, the most-recently-touched (b) wins over a. + // To guarantee this test's signal even with contention from other + // suites, check b's last_used_at > a's last_used_at directly. + aV, _ := s.GetView(ctx, a) + bV, _ := s.GetView(ctx, b) + if aV.LastUsedAt == nil || bV.LastUsedAt == nil { + t.Fatal("both views should have last_used_at after touch") + } + if !bV.LastUsedAt.After(*aV.LastUsedAt) { + t.Errorf("b.last_used_at should be after a.last_used_at; a=%v b=%v", aV.LastUsedAt, bV.LastUsedAt) + } + if mru == nil { + t.Error("MostRecentView returned nil even though touches landed") + } +} + +func TestViewReorder(t *testing.T) { + pool, s := connect(t) + defer pool.Close() + ctx := context.Background() + a := uniqueSlug("p5j-a-reorder-a") + b := uniqueSlug("p5j-a-reorder-b") + c := uniqueSlug("p5j-a-reorder-c") + defer pool.Exec(context.Background(), `DELETE FROM projax.views WHERE slug IN ($1, $2, $3)`, a, b, c) + + for _, slug := range []string{a, b, c} { + if _, err := s.CreateView(ctx, store.ViewInput{Slug: slug, Name: slug}); err != nil { + t.Fatalf("create %s: %v", slug, err) + } + } + // Reorder c → b → a. + if err := s.ReorderViews(ctx, []string{c, b, a}); err != nil { + t.Fatalf("reorder: %v", err) + } + cV, _ := s.GetView(ctx, c) + bV, _ := s.GetView(ctx, b) + aV, _ := s.GetView(ctx, a) + if cV.SortOrder != 0 || bV.SortOrder != 1 || aV.SortOrder != 2 { + t.Errorf("reorder yielded sort_orders c=%d b=%d a=%d, want 0,1,2", + cV.SortOrder, bV.SortOrder, aV.SortOrder) + } +} diff --git a/web/server.go b/web/server.go index ec4ae2a..b300db9 100644 --- a/web/server.go +++ b/web/server.go @@ -151,7 +151,7 @@ func New(s *store.Store, logger *slog.Logger) (*Server, error) { }, } pages := map[string]*template.Template{} - for _, name := range []string{"new", "classify", "caldav_admin", "caldav_disabled", "error", "views", "view_edit"} { + for _, name := range []string{"new", "classify", "caldav_admin", "caldav_disabled", "error"} { t, err := template.New(name).Funcs(funcs).ParseFS(templatesFS, "templates/layout.tmpl", "templates/"+name+".tmpl", @@ -382,11 +382,9 @@ func (s *Server) Routes() http.Handler { mux.HandleFunc("GET /admin/caldav", s.handleCalDAVAdmin) mux.HandleFunc("POST /admin/caldav/link", s.handleCalDAVLink) mux.HandleFunc("POST /admin/caldav/unlink", s.handleCalDAVUnlink) - mux.HandleFunc("GET /views", s.handleViewsIndex) - mux.HandleFunc("POST /views", s.handleViewCreate) - mux.HandleFunc("GET /views/{id}/edit", s.handleViewEdit) - mux.HandleFunc("GET /views/", s.handleViewRedirect) - mux.HandleFunc("POST /views/", s.handleViewWrite) + // /views routes land in slice B (paliad-shape: GET /views, GET + // /views/{slug}, GET /views/new, GET /views/{slug}/edit, plus POST CRUD). + // Between slice A and slice B these URLs 404 by design. mux.HandleFunc("GET /login", s.handleLoginForm) mux.HandleFunc("POST /login", s.handleLoginSubmit) mux.HandleFunc("POST /logout", s.handleLogout) @@ -451,30 +449,11 @@ func (s *Server) handleTree(w http.ResponseWriter, r *http.Request) { filter := ParseTreeFilter(r.URL.Query()) viewSet := PageViewTypes("/") view := ParseViewType(r.URL.Query(), viewSet) - var defaultBanner *store.View - // Phase 5i Slice D: ?view= resolves a saved view's filter + - // view_type into the current request, overriding URL-only chip state. - // Resolution failure (deleted view, malformed payload) is logged and - // silently falls back to the URL-derived filter — the page stays - // renderable rather than 500ing. - if saved, err := s.applySavedView(r, &filter, &view); err == nil && saved != nil { - // Re-validate view_type against the route catalog so a saved - // kanban-view URL opened on / (before slice C ships kanban) lands on - // the default with the chip showing the wanted view as locked. - view = viewSet.Resolve(view) - } else if err != nil { - s.Logger.Warn("applySavedView", "id", r.URL.Query().Get("view"), "err", err) - } else { - // Phase 5i Slice E: no explicit ?view= → check for a page default. - // applyDefaultView returns nil unless the URL is "clean" (no chip - // state) AND a default exists for this page. - if def, err := s.applyDefaultView(r, "tree", &filter, &view); err == nil && def != nil { - view = viewSet.Resolve(view) - defaultBanner = def - } else if err != nil { - s.Logger.Warn("applyDefaultView", "page", "tree", "err", err) - } - } + // Phase 5j: ?view= overlay + is_default_for resolution deleted with the + // 5i shape. /views/{slug} (slice B+) renders saved views as their own + // pages; legacy ?view= URLs are 302-redirected from a dedicated + // handler (slice C). handleTree stays focused on the tree-as-tree + // surface and no longer hijacks itself based on a query param. roots, orphans, total, orphanN, matched := applyTreeFilter(items, filter, linkKinds) counts := computeChipCounts(items, filter, linkKinds, tags) // Phase 5i Slice B: the card view renders a flat grid of matched items @@ -504,7 +483,6 @@ func (s *Server) handleTree(w http.ResponseWriter, r *http.Request) { "Kanban": kanban, "GroupBy": groupBy, "GroupByChips": groupByChips, - "DefaultBanner": defaultBanner, // ActiveTags kept for backwards-compat with the old template path; removed // after the template migrates fully. "ActiveTags": filter.Tags, diff --git a/web/templates/tree_section.tmpl b/web/templates/tree_section.tmpl index d0eddb2..3393d5e 100644 --- a/web/templates/tree_section.tmpl +++ b/web/templates/tree_section.tmpl @@ -1,12 +1,5 @@ {{define "tree-section"}}
- {{if .DefaultBanner}} -

- Showing default view: {{.DefaultBanner.Name}} · - clear -

- {{end}}

{{.Matched}} / {{.Total}} items match {{if .OrphanN}} · {{.OrphanN}} unclassified mai-managed roots → classify{{end}} diff --git a/web/templates/view_edit.tmpl b/web/templates/view_edit.tmpl deleted file mode 100644 index 47d8d87..0000000 --- a/web/templates/view_edit.tmpl +++ /dev/null @@ -1,42 +0,0 @@ -{{define "content"}} -

Edit view

-

← back to views

- -
-
- - - - - - - - - - - cancel -
-
-{{end}} diff --git a/web/templates/views.tmpl b/web/templates/views.tmpl deleted file mode 100644 index e046e55..0000000 --- a/web/templates/views.tmpl +++ /dev/null @@ -1,70 +0,0 @@ -{{define "content"}} -

Views

- -

Saved bundles of (filter + view_type + sort + group_by). Page-agnostic — open one to render the saved set on the matching page.

- -
- {{if .Views}} - - - - - - - - {{range .Views}} - - - - - - - - - {{end}} - -
★NameTypeDefault forGroup by
{{if .Pinned}}★{{end}}{{.Name}}{{if .Description}}
{{.Description}}{{end}}		{{.ViewType}}{{if .IsDefaultFor}}{{deref .IsDefaultFor}}{{else}}—{{end}}{{if .GroupBy}}{{deref .GroupBy}}{{else}}—{{end}} - edit -
- -
-
- {{else}} -

No saved views yet. Create one with the form below or via the "Save view…" link on any Views-supporting page.

- {{end}} -
- -
-

New view

-
- - - - - - - - - - -
-
-{{end}} diff --git a/web/tree_filter.go b/web/tree_filter.go index e73223e..0b4e515 100644 --- a/web/tree_filter.go +++ b/web/tree_filter.go @@ -26,12 +26,6 @@ type TreeFilter struct { // exposes an explicit on/off toggle. ProjectPath string IncludeDescendants bool - // Phase 5i fix-shift — saved-view anchor. When set, the URL was - // `?view=`; chip clicks need to round-trip the value so the user - // stays inside the saved view while narrowing further. Not a "filter" - // dimension in the matching sense — Matches ignores it — but it lives - // in the URL state and on the struct so QueryString preserves it. - ViewID string } // Active reports whether any filter dimension is set to something other than @@ -70,7 +64,6 @@ func ParseTreeFilter(q url.Values) TreeFilter { ShowArchived: q.Get("show-archived") == "1", ProjectPath: strings.TrimSpace(q.Get("project")), IncludeDescendants: true, - ViewID: strings.TrimSpace(q.Get("view")), } if v := strings.TrimSpace(q.Get("public")); v != "" { // Treat 1/true/yes/on as true; 0/false/no/off as false; anything else nil. @@ -132,9 +125,6 @@ func (f TreeFilter) QueryString() string { v.Set("project_descendants", "0") } } - if f.ViewID != "" { - v.Set("view", f.ViewID) - } return v.Encode() } diff --git a/web/views.go b/web/views.go index 324afa2..da9c138 100644 --- a/web/views.go +++ b/web/views.go @@ -1,422 +1,10 @@ package web -import ( - "encoding/json" - "errors" - "fmt" - "net/http" - "net/url" - "strings" - - "github.com/m/projax/store" -) - -// Phase 5i Slice D — saved views handlers. Page-agnostic: a view bundles a -// filter + view_type + sort/group_by and renders on any page that supports -// that view_type. The sidebar in layout.tmpl lists every saved view; the -// /views index lets m manage them. - -// handleViewsIndex renders the list + create-form page. -func (s *Server) handleViewsIndex(w http.ResponseWriter, r *http.Request) { - views, err := s.Store.ListViews(r.Context()) - if err != nil { - s.fail(w, r, err) - return - } - // Prefill: a save-from-page link can pass ?prefill_filter=&prefill_view_type=&prefill_page= so the form opens - // with the user's current state already typed in. - prefill := map[string]string{ - "filter": r.URL.Query().Get("prefill_filter"), - "view_type": r.URL.Query().Get("prefill_view_type"), - "page": r.URL.Query().Get("prefill_page"), - } - s.render(w, r, "views", map[string]any{ - "Title": "views", - "Views": views, - "Prefill": prefill, - // Catalog of selectable values for the form selects. - "AllViewTypes": allViewTypes, - "DefaultForOptions": []string{"", "tree", "dashboard", "calendar", "timeline"}, - "SortDirOptions": []string{"", "asc", "desc"}, - "GroupByOptions": []string{"", "status", "area", "tag", "management"}, - }) -} - -// handleViewCreate accepts the create-view form POST. -func (s *Server) handleViewCreate(w http.ResponseWriter, r *http.Request) { - if err := r.ParseForm(); err != nil { - s.fail(w, r, err) - return - } - in, err := viewInputFromForm(r.PostForm) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - v, err := s.Store.CreateView(r.Context(), in) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - http.Redirect(w, r, "/views/"+v.ID, http.StatusSeeOther) -} - -// handleViewEdit renders the edit form for an existing view, pre-populated -// with the row's current values. Submit posts back to /views/. -func (s *Server) handleViewEdit(w http.ResponseWriter, r *http.Request) { - id := strings.TrimSuffix(strings.TrimPrefix(r.URL.Path, "/views/"), "/edit") - if id == "" { - http.NotFound(w, r) - return - } - v, err := s.Store.GetView(r.Context(), id) - if err != nil { - if errors.Is(err, store.ErrViewNotFound) { - http.NotFound(w, r) - return - } - s.fail(w, r, err) - return - } - filterQuery, err := filterJSONToQuery(v.FilterJSON) - if err != nil { - s.Logger.Warn("filterJSONToQuery", "id", id, "err", err) - } - s.render(w, r, "view_edit", map[string]any{ - "Title": "edit view", - "View": v, - "FilterQuery": filterQuery, - "AllViewTypes": allViewTypes, - "DefaultForOptions": []string{"", "tree", "dashboard", "calendar", "timeline"}, - "SortDirOptions": []string{"", "asc", "desc"}, - "GroupByOptions": []string{"", "status", "area", "tag", "management"}, - }) -} - -// filterJSONToQuery rebuilds a URL-query representation of a stored -// filter_json so the edit form can pre-populate the `filter_query` input -// field. Inverse of filterQueryToJSON. -func filterJSONToQuery(filterJSON []byte) (string, error) { - if len(filterJSON) == 0 { - return "", nil - } - payload := map[string]any{} - if err := json.Unmarshal(filterJSON, &payload); err != nil { - return "", err - } - f := filterFromJSONPayload(payload) - // QueryString re-emits the canonical URL query form; that's exactly the - // shape the form's `filter_query` input expects on round-trip. - return f.QueryString(), nil -} - -// handleViewWrite dispatches the /views/ POST routes: bare path is -// update; /views//delete is soft-delete. -func (s *Server) handleViewWrite(w http.ResponseWriter, r *http.Request) { - path := strings.TrimPrefix(r.URL.Path, "/views/") - if base, ok := strings.CutSuffix(path, "/delete"); ok { - s.handleViewDelete(w, r, base) - return - } - if err := r.ParseForm(); err != nil { - s.fail(w, r, err) - return - } - in, err := viewInputFromForm(r.PostForm) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - if _, err := s.Store.UpdateView(r.Context(), path, in); err != nil { - if errors.Is(err, store.ErrViewNotFound) { - http.NotFound(w, r) - return - } - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - http.Redirect(w, r, "/views", http.StatusSeeOther) -} - -// handleViewDelete soft-deletes by id. -func (s *Server) handleViewDelete(w http.ResponseWriter, r *http.Request, id string) { - if err := s.Store.SoftDeleteView(r.Context(), id); err != nil { - if errors.Is(err, store.ErrViewNotFound) { - http.NotFound(w, r) - return - } - s.fail(w, r, err) - return - } - http.Redirect(w, r, "/views", http.StatusSeeOther) -} - -// handleViewRedirect resolves /views/ GET into a redirect to the -// appropriate Views-supporting page with ?view= appended. The target -// page resolves the saved filter+view_type at render time via -// applySavedView. /views//edit is dispatched separately via the more -// specific route registered first; this handler ignores the edit suffix -// defensively when the routing pattern doesn't match for some reason. -func (s *Server) handleViewRedirect(w http.ResponseWriter, r *http.Request) { - id := strings.TrimPrefix(r.URL.Path, "/views/") - if id == "" { - http.NotFound(w, r) - return - } - if strings.HasSuffix(id, "/edit") { - s.handleViewEdit(w, r) - return - } - v, err := s.Store.GetView(r.Context(), id) - if err != nil { - if errors.Is(err, store.ErrViewNotFound) { - http.NotFound(w, r) - return - } - s.fail(w, r, err) - return - } - target := targetRouteForViewType(v.ViewType) - q := url.Values{} - q.Set("view", v.ID) - http.Redirect(w, r, target+"?"+q.Encode(), http.StatusSeeOther) -} - -// targetRouteForViewType picks a sensible landing route given the view's -// view_type. card/list/kanban land on /; calendar on /calendar; timeline on -// /timeline. Slice E will let `is_default_for` override. -func targetRouteForViewType(vt string) string { - switch vt { - case ViewTypeCalendar: - return "/calendar" - case ViewTypeTimeline: - return "/timeline" - case ViewTypeCard, ViewTypeList, ViewTypeKanban: - return "/" - } - return "/" -} - -// viewInputFromForm decodes the create/update form. filter_json is accepted -// as either raw JSON (textarea) OR as an encoded query string under -// `filter_query` so the save-from-page workflow can prefill from a TreeFilter -// the user assembled via chips. -func viewInputFromForm(form url.Values) (store.ViewInput, error) { - in := store.ViewInput{ - Name: strings.TrimSpace(form.Get("name")), - Description: strings.TrimSpace(form.Get("description")), - ViewType: strings.TrimSpace(form.Get("view_type")), - SortField: strings.TrimSpace(form.Get("sort_field")), - SortDir: strings.TrimSpace(form.Get("sort_dir")), - GroupBy: strings.TrimSpace(form.Get("group_by")), - Pinned: form.Get("pinned") == "1", - IsDefaultFor: strings.TrimSpace(form.Get("is_default_for")), - } - // Prefer filter_query when present; otherwise fall back to filter_json. - if fq := strings.TrimSpace(form.Get("filter_query")); fq != "" { - filterJSON, err := filterQueryToJSON(fq) - if err != nil { - return in, fmt.Errorf("filter_query: %w", err) - } - in.FilterJSON = filterJSON - } else if fj := strings.TrimSpace(form.Get("filter_json")); fj != "" { - in.FilterJSON = []byte(fj) - } - return in, nil -} - -// filterQueryToJSON parses a TreeFilter URL query and returns the canonical -// JSON shape stored in `filter_json`. Mirrors the design doc §2 keys. -func filterQueryToJSON(query string) ([]byte, error) { - q, err := url.ParseQuery(strings.TrimPrefix(query, "?")) - if err != nil { - return nil, err - } - f := ParseTreeFilter(q) - payload := map[string]any{} - if f.Q != "" { - payload["q"] = f.Q - } - if len(f.Tags) > 0 { - payload["tags"] = f.Tags - } - if len(f.Management) > 0 { - payload["management"] = f.Management - } - if !(len(f.Status) == 1 && f.Status[0] == "active") { - payload["status"] = f.Status - } - if len(f.HasLinks) > 0 { - payload["has_links"] = f.HasLinks - } - if f.Public != nil { - payload["public"] = *f.Public - } - if f.ShowArchived { - payload["show_archived"] = true - } - if f.ProjectPath != "" { - payload["project_path"] = f.ProjectPath - if !f.IncludeDescendants { - payload["include_descendants"] = false - } - } - return json.Marshal(payload) -} - -// applyDefaultView resolves the saved view marked is_default_for= -// when the request URL carries no filter/view-specific params and the user -// has not opted out via ?nodefault=1. Returns the applied view (for banner -// labelling) or nil when no default exists / was applied. +// Phase 5j Slice A — paliad-shape redesign. The 5i overlay handlers +// (handleViewsIndex / handleViewCreate / handleViewWrite / handleViewEdit +// / handleViewRedirect / applySavedView / applyDefaultView / friends) +// are deleted here. The new /views/{slug} route family lands in slice B; +// system-view migration lands in slice C. // -// Per design.md §7 Slice E: defaults are a polish layer. They only kick in -// on a "clean" landing — the moment the user types a chip click, the URL -// gains a filter param and the default no longer auto-applies. Same with -// an explicit ?view=. -func (s *Server) applyDefaultView(r *http.Request, page string, filter *TreeFilter, viewType *string) (*store.View, error) { - q := r.URL.Query() - if q.Get("nodefault") == "1" { - return nil, nil - } - // Any filter-affecting param means "user is driving" — skip the default. - for _, key := range []string{"q", "tag", "mgmt", "status", "has", "show-archived", "public", "project", "project_id", "project_descendants", "view", "view_type", "group_by"} { - if q.Get(key) != "" { - return nil, nil - } - } - v, err := s.Store.DefaultViewFor(r.Context(), page) - if err != nil || v == nil { - return v, err - } - payload := map[string]any{} - if len(v.FilterJSON) > 0 { - if err := json.Unmarshal(v.FilterJSON, &payload); err != nil { - return v, fmt.Errorf("decode default filter_json: %w", err) - } - } - *filter = filterFromJSONPayload(payload) - *viewType = v.ViewType - return v, nil -} - -// applySavedView resolves a `?view=` reference and folds the persisted -// filter + view_type back into the supplied TreeFilter + view-type slot. -// URL chip params OVERLAY the saved filter — a saved view scoped to -// `dev` with `?tag=work` added narrows further. Transient overlays don't -// auto-save back to the view (the URL is bookmarkable, but to persist the -// drift the user opens /views//edit). -// -// Returns the saved view (for chip labelling) or nil when no `?view=` was -// given. Errors are logged + returned (handlers can choose to ignore). -func (s *Server) applySavedView(r *http.Request, filter *TreeFilter, viewType *string) (*store.View, error) { - id := strings.TrimSpace(r.URL.Query().Get("view")) - if id == "" { - return nil, nil - } - v, err := s.Store.GetView(r.Context(), id) - if err != nil { - return nil, err - } - payload := map[string]any{} - if len(v.FilterJSON) > 0 { - if err := json.Unmarshal(v.FilterJSON, &payload); err != nil { - return v, fmt.Errorf("decode filter_json: %w", err) - } - } - saved := filterFromJSONPayload(payload) - saved.ViewID = id - q := r.URL.Query() - overlayURLFields(&saved, *filter, q) - *filter = saved - // view_type: URL wins when explicitly set, otherwise the saved value. - if strings.TrimSpace(q.Get("view_type")) == "" { - *viewType = v.ViewType - } - return v, nil -} - -// overlayURLFields lets URL-provided chip values override the saved-view -// baseline. The URL filter is the parsed-from-query TreeFilter; q is the -// raw url.Values so we can detect "field was set in the URL" distinct from -// "field's value happens to equal the zero value". -func overlayURLFields(base *TreeFilter, urlFilter TreeFilter, q url.Values) { - if q.Get("q") != "" { - base.Q = urlFilter.Q - } - if _, ok := q["tag"]; ok { - base.Tags = urlFilter.Tags - } - if _, ok := q["mgmt"]; ok { - base.Management = urlFilter.Management - } - if _, ok := q["status"]; ok { - base.Status = urlFilter.Status - } - if _, ok := q["has"]; ok { - base.HasLinks = urlFilter.HasLinks - } - if q.Get("show-archived") != "" { - base.ShowArchived = urlFilter.ShowArchived - } - if q.Get("public") != "" { - base.Public = urlFilter.Public - } - if q.Get("project") != "" { - base.ProjectPath = urlFilter.ProjectPath - } - if q.Get("project_descendants") != "" { - base.IncludeDescendants = urlFilter.IncludeDescendants - } -} - -// filterFromJSONPayload is the inverse of filterQueryToJSON. Keys absent -// from the payload land at their TreeFilter zero value (Status defaults to -// ["active"] to match ParseTreeFilter). -func filterFromJSONPayload(p map[string]any) TreeFilter { - f := TreeFilter{ - Status: []string{"active"}, - IncludeDescendants: true, - } - if v, ok := p["q"].(string); ok { - f.Q = v - } - if v, ok := p["tags"].([]any); ok { - f.Tags = anySliceToStrings(v) - } - if v, ok := p["management"].([]any); ok { - f.Management = anySliceToStrings(v) - } - if v, ok := p["status"].([]any); ok { - f.Status = anySliceToStrings(v) - if len(f.Status) == 0 { - f.Status = []string{"active"} - } - } - if v, ok := p["has_links"].([]any); ok { - f.HasLinks = anySliceToStrings(v) - } - if v, ok := p["public"].(bool); ok { - f.Public = &v - } - if v, ok := p["show_archived"].(bool); ok && v { - f.ShowArchived = true - } - if v, ok := p["project_path"].(string); ok { - f.ProjectPath = v - } - if v, ok := p["include_descendants"].(bool); ok { - f.IncludeDescendants = v - } - return f -} - -func anySliceToStrings(in []any) []string { - out := make([]string, 0, len(in)) - for _, v := range in { - if s, ok := v.(string); ok { - out = append(out, s) - } - } - return out -} +// Between slices A and B the /views URLs return 404 — by design, no real +// user data was on the old shape (hours-old after the 5i ship). diff --git a/web/views_test.go b/web/views_test.go deleted file mode 100644 index 93d4ab2..0000000 --- a/web/views_test.go +++ /dev/null @@ -1,327 +0,0 @@ -package web_test - -import ( - "context" - "encoding/json" - "net/url" - "strings" - "testing" - "time" -) - -// TestViewsCRUDRoundTrip covers create → list → open (redirect to scoped page) → -// delete, end-to-end. Requires DB. Slice D — projax.views table CRUD. -func TestViewsCRUDRoundTrip(t *testing.T) { - srv, pool := mustServer(t) - defer pool.Close() - h := srv.Routes() - - stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "") - name := "p5i-D-view-" + stamp - - defer pool.Exec(context.Background(), - `UPDATE projax.views SET deleted_at = now() WHERE name = $1 AND deleted_at IS NULL`, name) - - // Create. - form := url.Values{} - form.Set("name", name) - form.Set("view_type", "card") - form.Set("filter_query", "tag=work&mgmt=mai") - code, _ := post(t, h, "/views", form) - if code != 303 { - t.Fatalf("POST /views status=%d, want 303", code) - } - - // List page lists the new view. - code, body := get(t, h, "/views") - if code != 200 { - t.Fatalf("GET /views status=%d", code) - } - if !strings.Contains(body, name) { - t.Errorf("GET /views body missing %q", name) - } - - // Fetch row to grab the id (and validate filter_json round-trip). - var ( - id string - filterJSON []byte - viewType string - ) - if err := pool.QueryRow(context.Background(), - `SELECT id, filter_json, view_type FROM projax.views WHERE name=$1 AND deleted_at IS NULL`, - name, - ).Scan(&id, &filterJSON, &viewType); err != nil { - t.Fatalf("fetch row: %v", err) - } - if viewType != "card" { - t.Errorf("view_type = %q, want 'card'", viewType) - } - var payload map[string]any - if err := json.Unmarshal(filterJSON, &payload); err != nil { - t.Fatalf("filter_json unmarshal: %v", err) - } - if got, _ := payload["tags"].([]any); len(got) != 1 || got[0] != "work" { - t.Errorf("filter_json tags = %v, want [work]", payload["tags"]) - } - if got, _ := payload["management"].([]any); len(got) != 1 || got[0] != "mai" { - t.Errorf("filter_json management = %v, want [mai]", payload["management"]) - } - - // GET /views/ redirects to the right page with ?view=. - code, _ = get(t, h, "/views/"+id) - if code != 303 { - t.Errorf("GET /views/ status=%d, want 303 redirect", code) - } - - // Soft delete. - code, _ = post(t, h, "/views/"+id+"/delete", url.Values{}) - if code != 303 { - t.Errorf("POST delete status=%d, want 303", code) - } - var deletedAt *time.Time - if err := pool.QueryRow(context.Background(), - `SELECT deleted_at FROM projax.views WHERE id=$1`, id, - ).Scan(&deletedAt); err != nil { - t.Fatalf("post-delete read: %v", err) - } - if deletedAt == nil { - t.Error("expected deleted_at to be set after POST /views//delete") - } -} - -// TestViewEditFlow exercises the fix for m's bug "we cant edit views yet". -// GET /views//edit renders the pre-filled form; POST /views/ updates -// the row in place. Verifies name + view_type + filter_json round-trip. -func TestViewEditFlow(t *testing.T) { - srv, pool := mustServer(t) - defer pool.Close() - h := srv.Routes() - ctx := context.Background() - - stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "") - name := "p5i-fix-edit-" + stamp - defer pool.Exec(context.Background(), - `UPDATE projax.views SET deleted_at = now() WHERE name = $1 AND deleted_at IS NULL OR name = $2`, - name, name+"-renamed") - - var id string - if err := pool.QueryRow(ctx, ` -INSERT INTO projax.views (name, view_type, filter_json) -VALUES ($1, 'list', $2::jsonb) -RETURNING id`, name, []byte(`{"tags":["dev"]}`)).Scan(&id); err != nil { - t.Fatalf("seed: %v", err) - } - - // GET /views//edit renders the pre-filled form (not the redirect). - code, body := get(t, h, "/views/"+id+"/edit") - if code != 200 { - t.Fatalf("GET /views//edit status=%d, want 200", code) - } - if !strings.Contains(body, `value="`+name+`"`) { - t.Error("edit form should pre-fill the name input") - } - if !strings.Contains(body, `value="tag=dev"`) { - t.Error("edit form should pre-fill filter_query from filter_json") - } - - // Index page now shows an edit link per row. - _, idx := get(t, h, "/views") - if !strings.Contains(idx, `/views/`+id+`/edit`) { - t.Error("/views should expose an edit link per row") - } - - // POST /views/ updates the row. - form := url.Values{} - form.Set("name", name+"-renamed") - form.Set("view_type", "card") - form.Set("filter_query", "tag=work&mgmt=mai") - code, _ = post(t, h, "/views/"+id, form) - if code != 303 { - t.Fatalf("POST /views/ status=%d, want 303", code) - } - - var newName, newType string - var newFilter []byte - if err := pool.QueryRow(ctx, - `SELECT name, view_type, filter_json FROM projax.views WHERE id = $1`, id, - ).Scan(&newName, &newType, &newFilter); err != nil { - t.Fatalf("post-update read: %v", err) - } - if newName != name+"-renamed" { - t.Errorf("name = %q, want %q", newName, name+"-renamed") - } - if newType != "card" { - t.Errorf("view_type = %q, want 'card'", newType) - } - payload := map[string]any{} - _ = json.Unmarshal(newFilter, &payload) - tags, _ := payload["tags"].([]any) - if len(tags) != 1 || tags[0] != "work" { - t.Errorf("filter_json tags = %v, want [work] post-update", payload["tags"]) - } -} - -// TestSavedViewPageFilterApply exercises the fix for m's bug "the filters on -// custom views dont seem to work". A request to /?view=&tag=work narrows -// the saved view further by overlaying the URL chip onto the persisted -// filter_json. Previously the saved filter clobbered the URL chips -// wholesale. -func TestSavedViewPageFilterApply(t *testing.T) { - srv, pool := mustServer(t) - defer pool.Close() - h := srv.Routes() - ctx := context.Background() - - stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "") - name := "p5i-fix-overlay-" + stamp - devSlug := "p5i-fix-overlay-d-" + stamp - homeSlug := "p5i-fix-overlay-h-" + stamp - - defer pool.Exec(context.Background(), - `UPDATE projax.views SET deleted_at = now() WHERE name = $1 AND deleted_at IS NULL`, name) - - var dev, home string - if err := pool.QueryRow(ctx, `select id from projax.items where slug='dev' and cardinality(parent_ids)=0`).Scan(&dev); err != nil { - t.Fatalf("dev: %v", err) - } - if err := pool.QueryRow(ctx, `select id from projax.items where slug='home' and cardinality(parent_ids)=0`).Scan(&home); err != nil { - t.Fatalf("home: %v", err) - } - var devID, homeID string - if err := pool.QueryRow(ctx, ` -INSERT INTO projax.items (kind, title, slug, parent_ids, tags) -VALUES (array['project']::text[], 'Fix Dev', $1, ARRAY[$2]::uuid[], ARRAY['work']) -RETURNING id`, devSlug, dev).Scan(&devID); err != nil { - t.Fatalf("seed dev item: %v", err) - } - if err := pool.QueryRow(ctx, ` -INSERT INTO projax.items (kind, title, slug, parent_ids, tags) -VALUES (array['project']::text[], 'Fix Home', $1, ARRAY[$2]::uuid[], ARRAY['home']) -RETURNING id`, homeSlug, home).Scan(&homeID); err != nil { - t.Fatalf("seed home item: %v", err) - } - defer pool.Exec(context.Background(), `delete from projax.items where id in ($1,$2)`, devID, homeID) - - // Saved view with view_type=list and NO tag filter — both items should pass. - var id string - if err := pool.QueryRow(ctx, ` -INSERT INTO projax.views (name, view_type, filter_json) -VALUES ($1, 'list', '{}'::jsonb) -RETURNING id`, name).Scan(&id); err != nil { - t.Fatalf("seed view: %v", err) - } - - devLink := `href="/i/dev.` + devSlug + `"` - homeLink := `href="/i/home.` + homeSlug + `"` - - // Open view alone — both rows should appear. - _, baseBody := get(t, h, "/?view="+id) - if !strings.Contains(baseBody, devLink) { - t.Error("saved view without tag filter should show dev row") - } - if !strings.Contains(baseBody, homeLink) { - t.Error("saved view without tag filter should show home row") - } - - // Overlay ?tag=work — home row should disappear; dev should remain. - _, narrowedBody := get(t, h, "/?view="+id+"&tag=work") - if !strings.Contains(narrowedBody, devLink) { - t.Error("?view=&tag=work should still show dev row (work-tagged)") - } - if strings.Contains(narrowedBody, homeLink) { - t.Error("?view=&tag=work should hide home row — URL chip must overlay saved filter") - } - - // Chip URLs inside the saved view must round-trip the view= param so - // chip clicks don't strip the saved view. - if !strings.Contains(narrowedBody, "view="+id) { - t.Error("chip URLs inside a saved view should carry view= forward") - } -} - -// TestDefaultViewAppliedOnCleanURL verifies the Slice E behaviour: when / -// is requested with no chip params and a default view exists for the page, -// the saved filter + view_type apply and a "Showing default view: …" -// banner renders. Adding any chip param (?tag=…) bypasses the default. -// ?nodefault=1 is the explicit opt-out. -func TestDefaultViewAppliedOnCleanURL(t *testing.T) { - srv, pool := mustServer(t) - defer pool.Close() - h := srv.Routes() - ctx := context.Background() - - stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "") - name := "p5i-E-default-" + stamp - defer pool.Exec(context.Background(), - `UPDATE projax.views SET deleted_at = now() WHERE name = $1 AND deleted_at IS NULL`, name) - - if _, err := pool.Exec(ctx, ` -INSERT INTO projax.views (name, view_type, filter_json, is_default_for) -VALUES ($1, 'card', $2::jsonb, 'tree')`, - name, []byte(`{"tags":["work"]}`)); err != nil { - t.Fatalf("seed default view: %v", err) - } - - // Clean URL: default applies → card view + banner. - _, body := get(t, h, "/") - if !strings.Contains(body, `class="tree-card-grid"`) { - t.Error("clean / should auto-apply default view (card grid expected)") - } - if !strings.Contains(body, `default-banner`) { - t.Error("default-banner should render when a default applies") - } - if !strings.Contains(body, name) { - t.Error("banner should name the applied default view") - } - - // Any chip param bypasses the default → list view (no banner). - _, withChip := get(t, h, "/?tag=dev") - if strings.Contains(withChip, `default-banner`) { - t.Error("default banner should disappear once user types a chip") - } - if !strings.Contains(withChip, `
    `) { - t.Error("?tag=dev should render the forest (default not applied)") - } - - // Explicit opt-out via ?nodefault=1. - _, optOut := get(t, h, "/?nodefault=1") - if strings.Contains(optOut, `default-banner`) { - t.Error("?nodefault=1 should suppress the default banner") - } - if !strings.Contains(optOut, `
      `) { - t.Error("?nodefault=1 should render the forest (default suppressed)") - } -} - -// TestSavedViewAppliedOnQueryParam verifies that opening / with ?view= -// re-applies the saved filter+view_type. We seed a view tagged work=patents -// and assert the rendered tree has the right ProjectChip / chip-on state. -func TestSavedViewAppliedOnQueryParam(t *testing.T) { - srv, pool := mustServer(t) - defer pool.Close() - h := srv.Routes() - ctx := context.Background() - - stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "") - name := "p5i-D-saved-" + stamp - defer pool.Exec(context.Background(), - `UPDATE projax.views SET deleted_at = now() WHERE name = $1 AND deleted_at IS NULL`, name) - - // Seed directly via SQL so the assertion focuses on the resolver, not the - // form flow tested above. - var id string - if err := pool.QueryRow(ctx, ` -INSERT INTO projax.views (name, view_type, filter_json) -VALUES ($1, 'card', $2::jsonb) -RETURNING id`, name, []byte(`{"project_path":"dev","include_descendants":true}`)).Scan(&id); err != nil { - t.Fatalf("seed view: %v", err) - } - - _, body := get(t, h, "/?view="+id) - if !strings.Contains(body, `class="tree-card-grid"`) { - t.Error("?view= should override view_type → card view should render") - } - if !strings.Contains(body, `class="proj-chip chip-on"`) { - t.Error("?view= should apply project filter chip → proj-chip should be on") - } -}