diff --git a/internal/server/handlers_items.go b/internal/server/handlers_items.go index f200c89e..e43d917b 100644 --- a/internal/server/handlers_items.go +++ b/internal/server/handlers_items.go @@ -1072,6 +1072,103 @@ func (s *Server) publishItemEventWithName(eventType, workspaceID, itemID, title, }) } +// handleCollectionCheckboxProgress returns per-item markdown-checkbox +// progress for items in a single collection — the bookkeeping that +// powers the list/board/table progress badges for non-plans +// collections. Pairs with /items-index (TASK-1349 / PLAN-1343 Phase 1): +// the index endpoint omits content to keep the wire payload small, +// and this endpoint computes the checkbox counts server-side via +// LENGTH/REPLACE arithmetic so the client doesn't need content at +// all to render the progress UI. +// +// Visibility: enforces the same collection-visibility + item-grant +// rules as handleListItems / handleListItemsIndex. Items the caller +// can't see contribute zero rows to the response. +func (s *Server) handleCollectionCheckboxProgress(w http.ResponseWriter, r *http.Request) { + workspaceID, ok := s.getWorkspaceID(w, r) + if !ok { + return + } + + collSlug := chi.URLParam(r, "collSlug") + coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) + if err != nil { + writeInternalError(w, err) + return + } + if coll == nil { + writeError(w, http.StatusNotFound, "not_found", "Collection not found") + return + } + + // Collection-visibility gate. + visibleIDs, err := s.visibleCollectionIDs(r, workspaceID) + if err != nil { + writeInternalError(w, err) + return + } + if !isCollectionVisible(coll.ID, visibleIDs) { + writeError(w, http.StatusNotFound, "not_found", "Collection not found") + return + } + + includeArchived := r.URL.Query().Get("include_archived") == "true" + progress, err := s.store.CollectionCheckboxProgress(workspaceID, coll.ID, includeArchived) + if err != nil { + writeInternalError(w, err) + return + } + + // Item-grant filtering: if the caller has item-level grants (guest + // / restricted member) and the collection isn't fully granted, drop + // rows for items outside their grant set. This mirrors the same + // filter handleListItems / handleListItemsIndex apply via + // guestResourceFilter — without it, a guest could enumerate the + // existence of items they can't otherwise see by reading their + // checkbox progress. + fullCollIDs, grantedItemIDs, grantErr := s.guestResourceFilter(r, workspaceID) + if grantErr != nil { + writeInternalError(w, grantErr) + return + } + if len(grantedItemIDs) > 0 { + hasFullCollectionGrant := false + for _, id := range fullCollIDs { + if id == coll.ID { + hasFullCollectionGrant = true + break + } + } + if !hasFullCollectionGrant && workspaceRole(r) != "guest" { + memberColls, _ := s.store.GetMemberCollectionAccess(workspaceID, currentUserID(r)) + for _, id := range memberColls { + if id == coll.ID { + hasFullCollectionGrant = true + break + } + } + } + if !hasFullCollectionGrant { + granted := make(map[string]struct{}, len(grantedItemIDs)) + for _, id := range grantedItemIDs { + granted[id] = struct{}{} + } + filtered := progress[:0] + for _, p := range progress { + if _, ok := granted[p.ItemID]; ok { + filtered = append(filtered, p) + } + } + progress = filtered + } + } + + if progress == nil { + progress = []store.ItemCheckboxProgress{} + } + writeJSON(w, http.StatusOK, progress) +} + // handlePlansProgress returns child item completion progress for all non-deleted plans. // This is a backward-compat endpoint; the general form is per-item via /items/{slug}/children. func (s *Server) handlePlansProgress(w http.ResponseWriter, r *http.Request) { diff --git a/internal/server/handlers_items_test.go b/internal/server/handlers_items_test.go index 57d16a37..c773faf9 100644 --- a/internal/server/handlers_items_test.go +++ b/internal/server/handlers_items_test.go @@ -1540,6 +1540,114 @@ func TestListItemsIndex_DoesNotShadowItemSlug(t *testing.T) { } } +// TestCollectionCheckboxProgress covers the markdown-checkbox progress +// endpoint that pairs with /items-index (TASK-1349). Verifies SQL +// LENGTH/REPLACE arithmetic produces the same per-item counts the +// client used to compute from item.content before /items-index made +// content unavailable in list view. +func TestCollectionCheckboxProgress(t *testing.T) { + srv := testServer(t) + slug := createWSWithCollections(t, srv) + + // Item with 2 open + 1 done checkbox. + mixed := createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "Has checklist", + "content": "Do this:\n- [ ] alpha\n- [x] beta\n- [ ] gamma\n", + "fields": `{"status":"open"}`, + }) + // Item with no checkboxes — must be excluded from the response. + createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "No checklist", + "content": "Just prose, nothing to count here.", + "fields": `{"status":"open"}`, + }) + // Item with only done checkboxes — total == done. + allDone := createItem(t, srv, slug, "tasks", map[string]interface{}{ + "title": "All done", + "content": "- [x] one\n- [x] two\n", + "fields": `{"status":"done"}`, + }) + + rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/tasks/checkbox-progress", nil) + if rr.Code != http.StatusOK { + t.Fatalf("checkbox-progress: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + type progressRow struct { + ItemID string `json:"item_id"` + Total int `json:"total"` + Done int `json:"done"` + } + var resp []progressRow + parseJSON(t, rr, &resp) + + if len(resp) != 2 { + t.Fatalf("expected 2 rows (mixed + allDone), got %d: %+v", len(resp), resp) + } + byID := map[string]progressRow{} + for _, r := range resp { + byID[r.ItemID] = r + } + if r, ok := byID[mixed.ID]; !ok { + t.Fatalf("mixed item missing from response") + } else if r.Total != 3 || r.Done != 1 { + t.Fatalf("mixed item: expected total=3, done=1, got total=%d, done=%d", r.Total, r.Done) + } + if r, ok := byID[allDone.ID]; !ok { + t.Fatalf("allDone item missing from response") + } else if r.Total != 2 || r.Done != 2 { + t.Fatalf("allDone item: expected total=2, done=2, got total=%d, done=%d", r.Total, r.Done) + } + + // Unknown collection → 404. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/nonexistent/checkbox-progress", nil) + if rr.Code != http.StatusNotFound { + t.Fatalf("expected 404 for unknown collection, got %d: %s", rr.Code, rr.Body.String()) + } + + // Empty result (collection has no items with checkboxes) → 200 + []. + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/ideas/checkbox-progress", nil) + if rr.Code != http.StatusOK { + t.Fatalf("checkbox-progress empty: expected 200, got %d", rr.Code) + } + if !bytes.Contains(rr.Body.Bytes(), []byte(`[]`)) { + t.Fatalf("expected empty array body, got %s", rr.Body.String()) + } + + // Archive `allDone` and confirm it drops out of the default response + // but reappears with ?include_archived=true. Mirrors the Archived + // toggle on the collection page (Codex round 2 [P2] on PR #491). + rr = doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/items/"+allDone.Slug, nil) + if rr.Code != http.StatusNoContent { + t.Fatalf("archive allDone: expected 204, got %d", rr.Code) + } + + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/tasks/checkbox-progress", nil) + if rr.Code != http.StatusOK { + t.Fatalf("checkbox-progress default: expected 200, got %d", rr.Code) + } + resp = nil + parseJSON(t, rr, &resp) + for _, r := range resp { + if r.ItemID == allDone.ID { + t.Fatalf("archived item should not appear in default checkbox-progress response") + } + } + if len(resp) != 1 { + t.Fatalf("expected 1 row (mixed) after archiving allDone, got %d", len(resp)) + } + + rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/tasks/checkbox-progress?include_archived=true", nil) + if rr.Code != http.StatusOK { + t.Fatalf("checkbox-progress include_archived: expected 200, got %d", rr.Code) + } + resp = nil + parseJSON(t, rr, &resp) + if len(resp) != 2 { + t.Fatalf("expected 2 rows with include_archived, got %d", len(resp)) + } +} + func firstID(items []models.Item) string { if len(items) == 0 { return "" diff --git a/internal/server/server.go b/internal/server/server.go index 7ec82202..b8fe343e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1009,6 +1009,11 @@ func (s *Server) setupRouter() { // Items within collection r.Get("/items", s.handleListCollectionItems) r.Post("/items", s.handleCreateItem) + // Pairs with /items-index — server-side checkbox + // progress so the collection page can render + // list/board/table progress badges without + // fetching item content (TASK-1349). + r.Get("/checkbox-progress", s.handleCollectionCheckboxProgress) // Collection grants r.Get("/grants", s.handleListCollectionGrants) r.Post("/grants", s.handleCreateCollectionGrant) diff --git a/internal/store/items.go b/internal/store/items.go index e0caddc3..0e29bf37 100644 --- a/internal/store/items.go +++ b/internal/store/items.go @@ -783,6 +783,82 @@ func (s *Store) ListItemsIndex(workspaceID string, params ItemIndexParams) ([]mo return scanItemsIndex(rows) } +// ItemCheckboxProgress is the per-item count of markdown checkboxes +// (`- [ ]` / `- [x]`) extracted from item content. Used by the +// collection page to render checklist progress badges without +// shipping the rich-text body over the wire (PLAN-1343 Phase 1 / +// TASK-1349). +type ItemCheckboxProgress struct { + ItemID string `json:"item_id"` + Total int `json:"total"` + Done int `json:"done"` +} + +// checkboxCountSQL is the SQL fragment used to count `- [ ]` and +// `- [x]` markers inside item content. Implemented identically on +// SQLite and PostgreSQL via the LENGTH/REPLACE arithmetic trick — +// both dialects support LENGTH and REPLACE on TEXT, and integer +// division is identical. +// +// The `i.deleted_at` clause is appended dynamically in +// CollectionCheckboxProgress so callers can request progress for +// archived rows (matches /items-index's include_archived semantics). +const checkboxCountSQL = ` + SELECT i.id, + (LENGTH(i.content) - LENGTH(REPLACE(i.content, '- [ ]', ''))) / 5 + + (LENGTH(i.content) - LENGTH(REPLACE(i.content, '- [x]', ''))) / 5 AS total, + (LENGTH(i.content) - LENGTH(REPLACE(i.content, '- [x]', ''))) / 5 AS done + FROM items i + WHERE i.workspace_id = ? + AND i.collection_id = ? + AND i.content LIKE '%- [%]%' +` + +// CollectionCheckboxProgress returns the per-item checkbox totals for +// every item in a collection whose content has at least one +// `- [ ]` / `- [x]` marker. The query computes counts server-side via +// LENGTH/REPLACE arithmetic so the wire payload stays small (three +// ints per non-zero item) — much cheaper than shipping every item's +// rich-text body just so the client can grep for checkboxes. +// +// includeArchived controls whether soft-deleted items contribute +// rows. The default (false) matches the pre-existing client-side +// parse for the un-toggled view. With the page's Archived toggle +// on, the collection page renders archived items too — passing +// true preserves their progress badges (per Codex round 2 [P2] on +// PR #491). +// +// Items with no markers, or with non-positive totals after subtracting +// done from open, are filtered out. Result order is unspecified. +func (s *Store) CollectionCheckboxProgress(workspaceID, collectionID string, includeArchived bool) ([]ItemCheckboxProgress, error) { + query := checkboxCountSQL + if !includeArchived { + query += " AND i.deleted_at IS NULL" + } + rows, err := s.db.Query(s.q(query), workspaceID, collectionID) + if err != nil { + return nil, fmt.Errorf("collection checkbox progress: %w", err) + } + defer rows.Close() + + var result []ItemCheckboxProgress + for rows.Next() { + var p ItemCheckboxProgress + if err := rows.Scan(&p.ItemID, &p.Total, &p.Done); err != nil { + return nil, err + } + // Skip rows with no checkboxes — the LIKE filter is a fast + // preliminary check, but item bodies can contain the substring + // inside a code block or other context that doesn't end up as + // a markdown checkbox; the per-row Total accounts for that. + if p.Total <= 0 { + continue + } + result = append(result, p) + } + return result, rows.Err() +} + // scanItemsIndex scans rows from ListItemsIndex (skinny projection — no // i.content column). func scanItemsIndex(rows *sql.Rows) ([]models.Item, error) { diff --git a/web/src/lib/api/client.ts b/web/src/lib/api/client.ts index 9b9463e2..dbe4877d 100644 --- a/web/src/lib/api/client.ts +++ b/web/src/lib/api/client.ts @@ -472,6 +472,26 @@ export const api = { plansProgress: (ws: string) => request<{item_id: string; total: number; done: number}[]>(`/workspaces/${ws}/plans-progress`), + /** + * Markdown-checkbox progress for items in a single collection. + * The server scans `- [ ]` / `- [x]` markers in each item's + * `content` and returns `{item_id, total, done}` for items with + * at least one checkbox. Pairs with `listIndex` (TASK-1349): + * the index endpoint omits content for bandwidth, this endpoint + * supplies the small derived counts the views need to render + * progress badges. + */ + collectionCheckboxProgress: ( + ws: string, + coll: string, + opts?: { includeArchived?: boolean } + ) => + request<{item_id: string; total: number; done: number}[]>( + `/workspaces/${ws}/collections/${coll}/checkbox-progress${qs({ + include_archived: opts?.includeArchived ? 'true' : undefined, + })}` + ), + /** Star an item for the current user (idempotent) */ star: (ws: string, itemSlug: string) => request(`/workspaces/${ws}/items/${itemSlug}/star`, { diff --git a/web/src/routes/[username]/[workspace]/[collection]/+page.svelte b/web/src/routes/[username]/[workspace]/[collection]/+page.svelte index 15a7a526..d7cac256 100644 --- a/web/src/routes/[username]/[workspace]/[collection]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/[collection]/+page.svelte @@ -290,7 +290,7 @@ case 'item_archived': case 'item_restored': { try { - items = await api.items.listByCollection(ws, coll); + items = await fetchSkinnyItems(ws, coll, false); } catch { // Ignore fetch errors — will retry on next event } @@ -298,7 +298,7 @@ } case 'item_updated': { try { - items = await api.items.listByCollection(ws, coll); + items = await fetchSkinnyItems(ws, coll, false); } catch { // Ignore fetch errors } @@ -357,8 +357,7 @@ // Full refresh fallback try { - const listParams = showArchived ? { include_archived: true } : undefined; - const freshItems = await api.items.listByCollection(wsSlug, collSlug, listParams); + const freshItems = await fetchSkinnyItems(wsSlug, collSlug, showArchived); items = freshItems; await refreshProgress(wsSlug, collSlug, freshItems); syncService.markSynced(); // Advance cursor now that reload succeeded @@ -382,6 +381,27 @@ } }); + /** + * Fetch a collection's items via the skinny /items-index endpoint + * (TASK-1349 / PLAN-1343 Phase 1). The endpoint omits the rich-text + * `content` body, which is the bulk of an item's wire size and is + * only needed when the user opens the detail page. + * + * The result is widened to `Item[]` by setting `content: ''` on + * every row. This satisfies the existing type contract — view + * components and progress code already treat empty content as + * "nothing to compute" — without leaking a custom skinny type + * through the whole call graph. The detail-page fetch still + * returns full items, so opening any item rehydrates `content`. + */ + async function fetchSkinnyItems(ws: string, coll: string, includeArchived: boolean): Promise { + const resp = await api.items.listIndex(ws, { + collection: coll, + includeArchived, + }); + return resp.items.map((row) => ({ ...row, content: '' })); + } + async function refreshProgress(ws: string, coll: string, itemList: typeof items) { if (coll === 'plans') { const progress = await api.items.plansProgress(ws).catch(() => []); @@ -391,13 +411,23 @@ } itemProgress = map; } else { + // Non-plans collections: pull markdown-checkbox progress from + // the new server-side endpoint. Pre-TASK-1349 this loop walked + // `it.content` client-side, but the skinny /items-index + // payload doesn't ship content. The server endpoint computes + // the same counts via LENGTH/REPLACE arithmetic on the + // stored rows — same shape `{item_id, total, done}` as + // /plans-progress. + // + // Pass `includeArchived` so the toggle-on view (which renders + // archived rows alongside live ones) still gets their + // progress badges. Per Codex round 2 [P2] on PR #491. const map: Record = {}; - for (const it of itemList) { - if (!it.content) continue; - const total = (it.content.match(/- \[[ x]\]/g) ?? []).length; - if (total === 0) continue; - const done = (it.content.match(/- \[x\]/g) ?? []).length; - map[it.id] = { total, done }; + const progress = await api.items + .collectionCheckboxProgress(ws, coll, { includeArchived: showArchived }) + .catch(() => []); + for (const p of progress) { + map[p.item_id] = { total: p.total, done: p.done }; } itemProgress = map; } @@ -406,10 +436,9 @@ async function loadCollection(ws: string, coll: string, includeArchived = false) { loading = true; try { - const listParams = includeArchived ? { include_archived: true } : undefined; const [collData, itemsData, viewsData, membersData] = await Promise.all([ api.collections.get(ws, coll), - api.items.listByCollection(ws, coll, listParams), + fetchSkinnyItems(ws, coll, includeArchived), api.views.list(ws, coll).catch(() => [] as View[]), api.members.list(ws).catch(() => ({ members: [], invitations: [] })) ]); @@ -433,23 +462,32 @@ itemProgress = {}; } } else { - // Compute checklist progress from item content (markdown checkboxes) - const map: Record = {}; - for (const it of itemsData) { - if (!it.content) continue; - const total = (it.content.match(/- \[[ x]\]/g) ?? []).length; - if (total === 0) continue; - const done = (it.content.match(/- \[x\]/g) ?? []).length; - map[it.id] = { total, done }; + // Non-plans collections: pull progress from the new + // /collections/{coll}/checkbox-progress endpoint instead + // of parsing `item.content` client-side. /items-index + // doesn't ship content, so the old client-side parse + // would be a no-op; the server-side endpoint computes + // the same counts via LENGTH/REPLACE arithmetic on the + // stored rows. `includeArchived` is plumbed through so + // the toggle-on view keeps progress badges on archived + // items (per Codex round 2 [P2] on PR #491). + try { + const progress = await api.items.collectionCheckboxProgress(ws, coll, { includeArchived }); + const map: Record = {}; + for (const p of progress) { + map[p.item_id] = { total: p.total, done: p.done }; + } + itemProgress = map; + } catch { + itemProgress = {}; } - itemProgress = map; progressLabel = 'done'; } // Fetch plan names for relation display on task cards if (coll === 'tasks') { try { - const plans = await api.items.listByCollection(ws, 'plans'); + const plans = await fetchSkinnyItems(ws, 'plans', false); const labels: Record = {}; for (const p of plans) { labels[p.id] = p.title;