From e83e7ef413b83114f0ee7e30c74ae405eb2f7865 Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 11 May 2026 09:34:15 -0400 Subject: [PATCH] feat(api): add /items/index skinny-projection endpoint (TASK-1344) (#486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(api): add /items/index skinny-projection endpoint (TASK-1344) Foundation for PLAN-1343 (local-first read model). Adds a new GET /api/v1/workspaces/{ws}/items/index endpoint that returns every item in a workspace minus the rich-text `content` body, so the client can hydrate an in-memory + IndexedDB index from a single request and render every collection page from local state without re-fetching. Response: {items, total, cursor}. The cursor placeholder is the max(updated_at) across the result set — Phase 2 replaces it with a monotonic `seq` cursor. Sort is updated_at DESC, id ASC for deterministic, cursor-friendly ordering. Auth uses the same collection-visibility + item-grant filter as handleListItems. Optional ?collection= filter for use by collection pages. ?include_archived=true mirrors the existing list behavior. Parent: PLAN-1343. * fix(api): move skinny-projection endpoint to /items-index per Codex review (round 1) Codex round 1 [P2] flagged that the original `/items/index` path shadowed the detail URL of any item whose slug is `index` — slugify emits `index` for a title of "Index", and chi's static-over-wildcard preference would route `GET /items/index` to the new index handler instead of the existing `/items/{itemSlug}` detail handler. Move the endpoint up to the workspace level as `/items-index`, sibling to the existing `/plans-progress` route. Slugs cannot contain hyphens adjacent to identifiers in a way that would collide with a static workspace-level path, so this URL space is permanently safe. New test `TestListItemsIndex_DoesNotShadowItemSlug` locks in the contract: a real item titled "Index" still resolves through `/items/{itemSlug}`, while `/items-index` returns the index wrapper. --- internal/server/handlers_items.go | 80 +++++++ internal/server/handlers_items_test.go | 305 +++++++++++++++++++++++++ internal/server/server.go | 7 + internal/store/items.go | 128 +++++++++++ 4 files changed, 520 insertions(+) diff --git a/internal/server/handlers_items.go b/internal/server/handlers_items.go index 5149ac62..f200c89e 100644 --- a/internal/server/handlers_items.go +++ b/internal/server/handlers_items.go @@ -16,6 +16,7 @@ import ( "github.com/PerpetualSoftware/pad/internal/events" "github.com/PerpetualSoftware/pad/internal/items" "github.com/PerpetualSoftware/pad/internal/models" + "github.com/PerpetualSoftware/pad/internal/store" ) // errStaleCollabSnapshot signals an UnderItemLock-wrapped @@ -72,6 +73,85 @@ func (s *Server) handleListItems(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, result) } +// itemsIndexResponse wraps the skinny-projection items list with bookkeeping +// the local-first read model (PLAN-1343) needs: +// - total: the row count so the client can size its in-memory array. +// - cursor: a placeholder for the Phase 2 monotonic seq cursor — for now +// we return the maximum updated_at (RFC3339), or "0" when the workspace +// is empty. Clients should treat its value as opaque. +type itemsIndexResponse struct { + Items []models.Item `json:"items"` + Total int `json:"total"` + Cursor string `json:"cursor"` +} + +// handleListItemsIndex returns the skinny-projection of every item in a +// workspace — same row set as handleListItems but without the rich-text +// `content` body. It exists to bootstrap the local-first read model +// (PLAN-1343 Phase 1): the client hydrates its in-memory index + IndexedDB +// cache from one request, then renders every collection page from local +// state without re-fetching. +// +// Filters: optional ?collection=. No pagination — callers want the +// full set. Auth: same as handleListItems (collection visibility + item +// grants). +func (s *Server) handleListItemsIndex(w http.ResponseWriter, r *http.Request) { + workspaceID, ok := s.getWorkspaceID(w, r) + if !ok { + return + } + + params := store.ItemIndexParams{ + CollectionSlug: r.URL.Query().Get("collection"), + } + if r.URL.Query().Get("include_archived") == "true" { + params.IncludeArchived = true + } + + // Collection visibility filter — same shape as handleListItems. + visibleIDs, err := s.visibleCollectionIDs(r, workspaceID) + if err != nil { + writeInternalError(w, err) + return + } + params.CollectionIDs = visibleIDs + + // Item-level grants for guests / restricted members. + fullCollIDs, grantedItemIDs, grantErr := s.guestResourceFilter(r, workspaceID) + if grantErr != nil { + writeInternalError(w, grantErr) + return + } + if len(grantedItemIDs) > 0 { + params.CollectionIDs = fullCollIDs + params.ItemIDs = grantedItemIDs + } + + result, err := s.store.ListItemsIndex(workspaceID, params) + if err != nil { + writeInternalError(w, err) + return + } + if result == nil { + result = []models.Item{} + } + s.enrichItemsWithParent(workspaceID, result, visibleIDs) + + // Cursor placeholder: max(updated_at) across the returned set. ListItemsIndex + // already sorts by updated_at DESC, so the first row holds it. Phase 2 will + // replace this with a monotonic `seq` cursor (see PLAN-1343). + cursor := "0" + if len(result) > 0 { + cursor = result[0].UpdatedAt.UTC().Format(time.RFC3339Nano) + } + + writeJSON(w, http.StatusOK, itemsIndexResponse{ + Items: result, + Total: len(result), + Cursor: cursor, + }) +} + // handleListCollectionItems lists items within a specific collection. func (s *Server) handleListCollectionItems(w http.ResponseWriter, r *http.Request) { workspaceID, ok := s.getWorkspaceID(w, r) diff --git a/internal/server/handlers_items_test.go b/internal/server/handlers_items_test.go index 7415b8f5..57d16a37 100644 --- a/internal/server/handlers_items_test.go +++ b/internal/server/handlers_items_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/PerpetualSoftware/pad/internal/models" ) @@ -1241,3 +1242,307 @@ func TestPatchItem_FlexibleFieldsShape(t *testing.T) { } }) } + +// itemsIndexBody mirrors the server-side response wrapper for /items-index. +// Kept local to the test file so the public handler doesn't need to export it. +type itemsIndexBody struct { + Items []models.Item `json:"items"` + Total int `json:"total"` + Cursor string `json:"cursor"` +} + +// TestListItemsIndex_SkinnyProjectionAndShape covers the foundational +// behavior of the local-first read model bootstrap endpoint (TASK-1344): +// - response shape: {items, total, cursor} +// - content body excluded (skinny projection) +// - cursor placeholder reflects the newest updated_at +// - core projected fields populate (ref, collection_slug, fields, …) +// +// Sort order is exercised separately by TestListItemsIndex_SortByUpdatedAt, +// which forces distinct timestamps — store.now() has RFC3339 second +// resolution, so two creates inside the same second tie on updated_at +// and the ID tiebreaker (not creation order) decides the row order. +func TestListItemsIndex_SkinnyProjectionAndShape(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "First task", + "content": "Body text that MUST NOT be returned by the index endpoint", + "fields": `{"status":"open","priority":"high"}`, + }) + createItem(t, srv, slug, "ideas", map[string]interface{}{ + "title": "Second idea", + "content": "Another body that the skinny projection should skip", + "fields": `{"status":"new"}`, + }) + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp itemsIndexBody + parseJSON(t, rr, &resp) + + if resp.Total != 2 { + t.Fatalf("expected total=2, got %d", resp.Total) + } + if len(resp.Items) != 2 { + t.Fatalf("expected 2 items, got %d", len(resp.Items)) + } + if resp.Cursor == "" || resp.Cursor == "0" { + t.Fatalf("expected cursor to be populated for non-empty workspace, got %q", resp.Cursor) + } + + // Cursor must match the newest updated_at — and because the sort is + // updated_at DESC, that's the row at index 0. + want := resp.Items[0].UpdatedAt.UTC().Format(time.RFC3339Nano) + if resp.Cursor != want { + t.Fatalf("cursor mismatch: got %q, want %q", resp.Cursor, want) + } + + // Skinny projection: content body excluded for every row. + for i, it := range resp.Items { + if it.Content != "" { + t.Fatalf("items[%d] (%s): expected empty content, got %q", i, it.Ref, it.Content) + } + if it.Ref == "" { + t.Errorf("items[%d]: missing computed ref", i) + } + if it.CollectionSlug == "" { + t.Errorf("items[%d]: missing collection_slug", i) + } + if it.Fields == "" { + t.Errorf("items[%d]: missing fields JSON", i) + } + } +} + +// TestListItemsIndex_SortByUpdatedAt forces a >1s gap between the two +// items so they land in distinct RFC3339 buckets, then asserts the +// updated_at DESC ordering. The sleep is the price of admission for +// testing a sort whose tiebreaker (id ASC) would otherwise win — see +// the comment on TestListItemsIndex_SkinnyProjectionAndShape. +func TestListItemsIndex_SortByUpdatedAt(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + older := createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "Older", + "fields": `{"status":"open"}`, + }) + // store.now() uses RFC3339 (second precision). Sleep just over 1s + // so the second create lands in a distinct timestamp bucket. + time.Sleep(1100 * time.Millisecond) + newer := createItem(t, srv, slug, "ideas", map[string]interface{}{ + "title": "Newer", + "fields": `{"status":"new"}`, + }) + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp itemsIndexBody + parseJSON(t, rr, &resp) + + if len(resp.Items) != 2 { + t.Fatalf("expected 2 items, got %d", len(resp.Items)) + } + if resp.Items[0].ID != newer.ID { + t.Fatalf("expected newer item first; got %q (want %q)", resp.Items[0].ID, newer.ID) + } + if resp.Items[1].ID != older.ID { + t.Fatalf("expected older item second; got %q (want %q)", resp.Items[1].ID, older.ID) + } +} + +// TestListItemsIndex_EmptyWorkspace covers the edge case the cursor +// placeholder is documented to handle: no items → cursor "0". +func TestListItemsIndex_EmptyWorkspace(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index empty: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp itemsIndexBody + parseJSON(t, rr, &resp) + + if resp.Total != 0 { + t.Fatalf("expected total=0, got %d", resp.Total) + } + if len(resp.Items) != 0 { + t.Fatalf("expected empty items slice, got %d", len(resp.Items)) + } + // JSON contract: items must marshal as [] (not null) so the client can + // rely on Array semantics. + if !bytes.Contains(rr.Body.Bytes(), []byte(`"items":[]`)) { + t.Fatalf("expected empty items array, body=%s", rr.Body.String()) + } + if resp.Cursor != "0" { + t.Fatalf("expected cursor=\"0\" on empty workspace, got %q", resp.Cursor) + } +} + +// TestListItemsIndex_CollectionFilter covers ?collection=. +func TestListItemsIndex_CollectionFilter(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "A task", + "fields": `{"status":"open"}`, + }) + createItem(t, srv, slug, "ideas", map[string]interface{}{ + "title": "An idea", + "fields": `{"status":"new"}`, + }) + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index?collection=tasks", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index?collection=tasks: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp itemsIndexBody + parseJSON(t, rr, &resp) + + if resp.Total != 1 { + t.Fatalf("expected total=1 task, got %d", resp.Total) + } + if resp.Items[0].CollectionSlug != "tasks" { + t.Fatalf("expected collection_slug=tasks, got %q", resp.Items[0].CollectionSlug) + } +} + +// TestListItemsIndex_ParentEnrichment confirms parent_link_id / parent_ref +// are populated for child items (same enrichment path the existing items +// list uses). +func TestListItemsIndex_ParentEnrichment(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + plan := createItem(t, srv, slug, "plans", map[string]interface{}{ + "title": "Parent plan", + "fields": `{"status":"active"}`, + }) + + createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "Child task", + "fields": `{"status":"open","parent":"` + plan.Ref + `"}`, + }) + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index?collection=tasks", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp itemsIndexBody + parseJSON(t, rr, &resp) + + if len(resp.Items) != 1 { + t.Fatalf("expected 1 child task, got %d", len(resp.Items)) + } + child := resp.Items[0] + if child.ParentLinkID == "" { + t.Fatalf("expected parent_link_id to be populated, got empty") + } + if child.ParentRef != plan.Ref { + t.Fatalf("expected parent_ref=%q, got %q", plan.Ref, child.ParentRef) + } +} + +// TestListItemsIndex_ExcludesArchivedByDefault confirms the IncludeArchived +// gate matches handleListItems' default behavior. +func TestListItemsIndex_ExcludesArchivedByDefault(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + keep := createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "Keep me", + "fields": `{"status":"open"}`, + }) + archive := createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "Archive me", + "fields": `{"status":"open"}`, + }) + + rr := doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/items/"+archive.Slug, nil) + if rr.Code != http.StatusNoContent { + t.Fatalf("archive: expected 204, got %d: %s", rr.Code, rr.Body.String()) + } + + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp itemsIndexBody + parseJSON(t, rr, &resp) + + if resp.Total != 1 || resp.Items[0].ID != keep.ID { + t.Fatalf("expected only the live item to remain; got %d items, first=%q want=%q", + resp.Total, firstID(resp.Items), keep.ID) + } + + // With include_archived=true, both items must come back. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index?include_archived=true", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index include_archived: expected 200, got %d", rr.Code) + } + + parseJSON(t, rr, &resp) + if resp.Total != 2 { + t.Fatalf("expected total=2 with include_archived, got %d", resp.Total) + } +} + +// TestListItemsIndex_DoesNotShadowItemSlug confirms /items-index lives in +// a non-conflicting URL space — an item titled "Index" (slug "index") still +// resolves through /items/{itemSlug}, while /items-index serves the new +// index wrapper. This is the contract that drove the path choice: keeping +// the endpoint outside the /items/{itemSlug} subtree means no item slug +// can ever shadow it (or vice versa). See Codex round 1 [P2] on PR #486. +func TestListItemsIndex_DoesNotShadowItemSlug(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + indexItem := createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "Index", + "fields": `{"status":"open"}`, + }) + if indexItem.Slug != "index" { + t.Fatalf("expected slug 'index' for title 'Index', got %q", indexItem.Slug) + } + + // /items-index → index wrapper. + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items-index", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items-index: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + if !bytes.Contains(rr.Body.Bytes(), []byte(`"items":[`)) { + t.Fatalf("expected wrapped response, got %s", rr.Body.String()) + } + + // /items/index → the item titled "Index", same as before this change. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/items/index", nil) + if rr.Code != http.StatusOK { + t.Fatalf("items/index detail: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + var fetched models.Item + parseJSON(t, rr, &fetched) + if fetched.ID != indexItem.ID { + t.Fatalf("expected item ID %q at /items/index, got %q", indexItem.ID, fetched.ID) + } +} + +func firstID(items []models.Item) string { + if len(items) == 0 { + return "" + } + return items[0].ID +} diff --git a/internal/server/server.go b/internal/server/server.go index f5161cc3..7ec82202 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1028,6 +1028,13 @@ func (s *Server) setupRouter() { // Plans progress r.Get("/plans-progress", s.handlePlansProgress) + // Skinny-projection cross-collection items list for the + // local-first read model bootstrap (PLAN-1343 / TASK-1344). + // Lives at workspace level — sibling to /plans-progress + // and /starred — so the path can't ever collide with an + // item slug under /items/{itemSlug}. + r.Get("/items-index", s.handleListItemsIndex) + // User grants (all grants for a specific user in this workspace) r.Get("/users/{userID}/grants", s.handleListUserGrants) diff --git a/internal/store/items.go b/internal/store/items.go index 8bd503e6..e0caddc3 100644 --- a/internal/store/items.go +++ b/internal/store/items.go @@ -684,6 +684,134 @@ func (s *Store) ListItems(workspaceID string, params models.ItemListParams) ([]m return scanItems(rows) } +// ItemIndexParams is the trimmed parameter set for ListItemsIndex. +// It deliberately omits sort/search/pagination/field-filter knobs that the +// "skinny projection" endpoint doesn't expose — the local-first read model +// fetches the entire workspace once and does its own client-side filtering. +type ItemIndexParams struct { + // CollectionSlug optionally restricts to a single collection by slug. + CollectionSlug string + // CollectionIDs is the permission filter for visible collections. + // nil = unfiltered. A non-nil empty slice means "no visible collections" + // and (combined with empty ItemIDs) returns an empty result immediately, + // matching ListItems semantics. + CollectionIDs []string + // ItemIDs additionally allows specific items through (item-level grants + // for guests / restricted members). + ItemIDs []string + // IncludeArchived returns soft-deleted items when true. + IncludeArchived bool +} + +// ListItemsIndex returns the skinny-projection of items in a workspace — +// every column EXCEPT i.content. Used by the local-first read model +// (PLAN-1343) so the client can hydrate an in-memory + IndexedDB index +// without paying the rich-text body cost. +// +// Deterministic sort: updated_at DESC, id ASC (stable tiebreaker so cursors +// over equal-timestamp items are reproducible). +func (s *Store) ListItemsIndex(workspaceID string, params ItemIndexParams) ([]models.Item, error) { + // Mirror ListItems: a non-nil empty CollectionIDs without item-level grants + // means "no visible collections" — return empty immediately. + if params.CollectionIDs != nil && len(params.CollectionIDs) == 0 && len(params.ItemIDs) == 0 { + return nil, nil + } + + query := ` + SELECT i.id, i.workspace_id, i.collection_id, i.title, i.slug, i.fields, i.tags, + i.pinned, i.sort_order, i.parent_id, i.assigned_user_id, i.agent_role_id, i.role_sort_order, + i.created_by, i.last_modified_by, i.source, + i.item_number, i.created_at, i.updated_at, + c.slug, c.name, c.icon, c.prefix, + COALESCE(au.name, ''), COALESCE(au.email, ''), + COALESCE(ar.name, ''), COALESCE(ar.slug, ''), COALESCE(ar.icon, '') + FROM items i + JOIN collections c ON c.id = i.collection_id + LEFT JOIN users au ON au.id = i.assigned_user_id + LEFT JOIN agent_roles ar ON ar.id = i.agent_role_id + WHERE i.workspace_id = ? + ` + args := []interface{}{workspaceID} + + if !params.IncludeArchived { + query += " AND i.deleted_at IS NULL" + } + + if params.CollectionSlug != "" { + query += " AND c.slug = ?" + args = append(args, params.CollectionSlug) + } + + if len(params.CollectionIDs) > 0 && len(params.ItemIDs) > 0 { + collPlaceholders := make([]string, len(params.CollectionIDs)) + for i, id := range params.CollectionIDs { + collPlaceholders[i] = "?" + args = append(args, id) + } + itemPlaceholders := make([]string, len(params.ItemIDs)) + for i, id := range params.ItemIDs { + itemPlaceholders[i] = "?" + args = append(args, id) + } + query += " AND (i.collection_id IN (" + strings.Join(collPlaceholders, ",") + ") OR i.id IN (" + strings.Join(itemPlaceholders, ",") + "))" + } else if len(params.CollectionIDs) > 0 { + placeholders := make([]string, len(params.CollectionIDs)) + for i, id := range params.CollectionIDs { + placeholders[i] = "?" + args = append(args, id) + } + query += " AND i.collection_id IN (" + strings.Join(placeholders, ",") + ")" + } else if len(params.ItemIDs) > 0 { + placeholders := make([]string, len(params.ItemIDs)) + for i, id := range params.ItemIDs { + placeholders[i] = "?" + args = append(args, id) + } + query += " AND i.id IN (" + strings.Join(placeholders, ",") + ")" + } + + // Deterministic sort: most-recently-updated first, with id as a stable + // secondary key so equal-timestamp rows have a reproducible order. + query += " ORDER BY i.updated_at DESC, i.id ASC" + + rows, err := s.db.Query(s.q(query), args...) + if err != nil { + return nil, fmt.Errorf("list items index: %w", err) + } + defer rows.Close() + + return scanItemsIndex(rows) +} + +// scanItemsIndex scans rows from ListItemsIndex (skinny projection — no +// i.content column). +func scanItemsIndex(rows *sql.Rows) ([]models.Item, error) { + var items []models.Item + for rows.Next() { + var item models.Item + var createdAt, updatedAt string + var pinned bool + if err := rows.Scan( + &item.ID, &item.WorkspaceID, &item.CollectionID, &item.Title, &item.Slug, + &item.Fields, &item.Tags, + &pinned, &item.SortOrder, &item.ParentID, &item.AssignedUserID, &item.AgentRoleID, &item.RoleSortOrder, + &item.CreatedBy, &item.LastModifiedBy, &item.Source, + &item.ItemNumber, &createdAt, &updatedAt, + &item.CollectionSlug, &item.CollectionName, &item.CollectionIcon, &item.CollectionPrefix, + &item.AssignedUserName, &item.AssignedUserEmail, + &item.AgentRoleName, &item.AgentRoleSlug, &item.AgentRoleIcon, + ); err != nil { + return nil, err + } + item.Pinned = pinned + item.CreatedAt = parseTime(createdAt) + item.UpdatedAt = parseTime(updatedAt) + hydrateItemComputedMetadata(&item) + items = append(items, item) + } + return items, rows.Err() +} + func (s *Store) listItemsFTS(workspaceID string, params models.ItemListParams) ([]models.Item, error) { var query string var args []interface{}