feat(04-04): implement item CRUD endpoints + fractional rank (LIST-02)

- Add rank.ts: rankForAppend/rankBetween wrapping fractional-indexing (D-13) - Extend listsRouter: POST /:id/items (fractional rank at active-bottom), GET /:id/items (rank ASC, access-gated) - Add listItemsRouter (mounted /api/list-items): PATCH /:itemId per-field LWW (exactly-one-field zod refine D-08/T-04-07), DELETE /:itemId delete-wins (D-09) - Uncheck recomputes rank to active-bottom in same write (Open Question 2) - All item handlers: access-gate via checkListAccess (T-04-05) - Plan 06 SSE seam comments at each mutation handler - All 48 tests green; typecheck passes
2026-06-09 12:55:32 -04:00
parent b1dc9b8048
commit 5e3151416c
3 changed files with 341 additions and 2 deletions
@@ -7,7 +7,7 @@ import { healthRouter } from './routes/health.js'
 import { meRouter } from './routes/me.js'
 import { eventsRouter } from './routes/events.js'
 import { sseRouter } from './routes/sse.js'
-import { listsRouter } from './routes/lists.js'
+import { listsRouter, listItemsRouter } from './routes/lists.js'
 import { oidcAuthMiddleware, processOAuthCallback } from './auth/middleware.js'
 import { devAuthBypass } from './auth/devBypass.js'
 import { startBrokerPoller } from './broker/poller.js'
@@ -63,6 +63,7 @@ app.route('/api/me', meRouter)
 app.route('/api/events', eventsRouter)
 app.route('/api/sse', sseRouter)
 app.route('/api/lists', listsRouter)
+app.route('/api/list-items', listItemsRouter)

 // WR-04: background worker startup (cron schedules) moved into the isMainModule()
 // guard below. Calling them at top level registered real node-cron schedules whenever
@@ -0,0 +1,42 @@
+/**
+ * Fractional-indexing rank helpers (D-13).
+ *
+ * Wraps the `fractional-indexing` library for use in list item ordering.
+ * String-based ranks (e.g. "a0", "a1", "a0V") are stable — a single reorder
+ * writes only the moved item's rank, playing well with SSE live sync (D-13, D-15).
+ *
+ * API:
+ *   rankForAppend(lastRank)     → rank that sorts after `lastRank` (or "a0" when null)
+ *   rankBetween(prev, next)     → rank that sorts between `prev` and `next`
+ *
+ * Both are pure functions — no DB access, no side effects.
+ */
+
+import { generateKeyBetween } from 'fractional-indexing'
+
+/**
+ * Generate a rank suitable for appending an item AFTER the last active item.
+ *
+ * @param lastRank - the rank of the current last active item, or null if the
+ *                   list is empty (first item).
+ * @returns a rank string that sorts after `lastRank` when ordered ASC.
+ *          An empty list gets "a0" (generateKeyBetween(null, null)).
+ */
+export function rankForAppend(lastRank: string | null): string {
+  return generateKeyBetween(lastRank, null)
+}
+
+/**
+ * Generate a rank suitable for inserting between two existing ranks.
+ * Also serves as the general case: pass (null, null) for empty list,
+ * (null, next) for prepend, (prev, null) for append.
+ *
+ * @param prev - rank of the item immediately before the insertion point,
+ *               or null if inserting at the beginning.
+ * @param next - rank of the item immediately after the insertion point,
+ *               or null if inserting at the end.
+ * @returns a rank string that sorts between `prev` and `next` when ordered ASC.
+ */
+export function rankBetween(prev: string | null, next: string | null): string {
+  return generateKeyBetween(prev, next)
+}
@@ -21,11 +21,12 @@ import { Hono } from 'hono'
 import type { Context } from 'hono'
 import { zValidator } from '@hono/zod-validator'
 import { z } from 'zod'
-import { and, eq, inArray, sql } from 'drizzle-orm'
+import { and, asc, eq, inArray, sql } from 'drizzle-orm'
 import { db } from '../db/client.js'
 import { lists, listShares, listItems, users } from '../db/schema.js'
 import { getAuth } from '../auth/middleware.js'
 import { upsertUser, deriveDisplayName } from '../auth/user.js'
+import { rankForAppend } from '../lib/rank.js'
 // Side-effect import: brings in the ContextVariableMap augmentation for c.get('user')
 import '../auth/devBypass.js'

@@ -34,6 +35,18 @@ import '../auth/devBypass.js'

 export const listsRouter = new Hono()

+/**
+ * listItemsRouter — single-item mutation routes.
+ *
+ * Mounted at /api (not /api/lists) in index.ts so the URLs resolve as:
+ *   PATCH  /api/list-items/:itemId
+ *   DELETE /api/list-items/:itemId
+ *
+ * Separate from listsRouter (mounted at /api/lists) per the RESEARCH.md
+ * architecture diagram.
+ */
+export const listItemsRouter = new Hono()
+
 // ---------------------------------------------------------------------------
 // Auth helper — copied verbatim from events.ts per project convention.
 // Duplicated per router (not extracted to shared module).
@@ -81,6 +94,29 @@ const patchListSchema = z
    message: 'PATCH must update at least one field (name or isShared)',
  })

+/**
+ * Create item — text 1..500 (T-04-06 XSS: plain-text only, no HTML).
+ */
+const createItemSchema = z.object({
+  text: z.string().min(1).max(500),
+})
+
+/**
+ * Per-field PATCH for list items (D-08, T-04-07).
+ * Exactly one field must be present — enforces single-field last-write-wins.
+ * Fields: checked (boolean), text (string), position (fractional rank string).
+ */
+const patchItemSchema = z
+  .object({
+    checked: z.boolean(),
+    text: z.string().min(1).max(500),
+    position: z.string().min(1).max(255),
+  })
+  .partial()
+  .refine((obj) => Object.keys(obj).length === 1, {
+    message: 'PATCH must update exactly one field (checked, text, or position)',
+  })
+
 // ---------------------------------------------------------------------------
 // Helper: check list access (owner OR sharee)
 // Returns: { allowed: true, isOwner: boolean } | { allowed: false }
@@ -395,3 +431,263 @@ listsRouter.delete('/:id', async (c) => {
    return c.json({ error: 'Service unavailable' }, 503)
  }
 })
+
+// ===========================================================================
+// Item routes (LIST-02)
+//
+// Route paths (per RESEARCH.md architecture diagram):
+//   Items-by-list:   /:id/items   (nested under lists)
+//   Single-item ops: /list-items/:itemId (at listsRouter root, mounts under /api)
+//
+// Security: T-04-05 — all item handlers verify list membership (owner OR sharee).
+//           T-04-06 — item text rendered plain-text in UI; no HTML/dangerouslySetInnerHTML.
+//           T-04-07 — patchItemSchema enforces exactly one field per D-08.
+//           T-04-09 — DELETE is final; PATCH on deleted item 404s (no upsert path).
+// ===========================================================================
+
+// ---------------------------------------------------------------------------
+// POST /api/lists/:id/items
+//
+// Add an item to the list, assigning a fractional rank at the bottom of the
+// active section. Rank is generateKeyBetween(lastActiveRank, null) per D-13.
+//
+// Security: list access check before insert.
+// ---------------------------------------------------------------------------
+listsRouter.post('/:id/items', zValidator('json', createItemSchema), async (c) => {
+  const currentUserId = await resolveUserId(c)
+  if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401)
+
+  const listId = Number(c.req.param('id'))
+  const { text } = c.req.valid('json')
+
+  try {
+    const access = await checkListAccess(listId, currentUserId)
+    if (!access.allowed) {
+      if (access.notFound) return c.json({ error: 'Not found' }, 404)
+      return c.json({ error: 'Access denied' }, 403)
+    }
+
+    // Find the last active item's rank (unchecked, sorted DESC by rank, limit 1)
+    const [lastActive] = await db
+      .select({ rank: listItems.rank })
+      .from(listItems)
+      .where(and(eq(listItems.listId, listId), eq(listItems.checked, false)))
+      .orderBy(sql`${listItems.rank} DESC`)
+      .limit(1)
+
+    const newRank = rankForAppend(lastActive?.rank ?? null)
+
+    const [inserted] = await db
+      .insert(listItems)
+      .values({ listId, text, rank: newRank })
+      .$returningId()
+
+    const [newItem] = await db
+      .select()
+      .from(listItems)
+      .where(eq(listItems.id, inserted.id))
+      .limit(1)
+
+    // Plan 06 SSE seam:
+    // publishListEvent(listId, { type: 'item:added', listId, payload: newItem })
+
+    return c.json(
+      {
+        id: newItem.id,
+        listId: newItem.listId,
+        text: newItem.text,
+        checked: newItem.checked,
+        rank: newItem.rank,
+        createdAt: newItem.createdAt,
+        updatedAt: newItem.updatedAt,
+      },
+      201,
+    )
+  } catch (err) {
+    console.error('[lists/POST /:id/items] DB operation failed:', err)
+    return c.json({ error: 'Service unavailable' }, 503)
+  }
+})
+
+// ---------------------------------------------------------------------------
+// GET /api/lists/:id/items
+//
+// Return all items for the list, ordered by rank ASC.
+// Access-gated: owner OR sharee only (T-04-05).
+// ---------------------------------------------------------------------------
+listsRouter.get('/:id/items', async (c) => {
+  const currentUserId = await resolveUserId(c)
+  if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401)
+
+  const listId = Number(c.req.param('id'))
+
+  try {
+    const access = await checkListAccess(listId, currentUserId)
+    if (!access.allowed) {
+      if (access.notFound) return c.json({ error: 'Not found' }, 404)
+      return c.json({ error: 'Access denied' }, 403)
+    }
+
+    const items = await db
+      .select()
+      .from(listItems)
+      .where(eq(listItems.listId, listId))
+      .orderBy(asc(listItems.rank))
+
+    const result = items.map((item) => ({
+      id: item.id,
+      listId: item.listId,
+      text: item.text,
+      checked: item.checked,
+      rank: item.rank,
+      createdAt: item.createdAt,
+      updatedAt: item.updatedAt,
+    }))
+
+    return c.json({ items: result })
+  } catch (err) {
+    console.error('[lists/GET /:id/items] DB query failed:', err)
+    return c.json({ error: 'Service unavailable' }, 503)
+  }
+})
+
+// ---------------------------------------------------------------------------
+// PATCH /api/list-items/:itemId
+//
+// Per-field last-write-wins update (D-08). Exactly one of: checked, text, position.
+//
+// Special case: { checked: false } (uncheck) recomputes rank to append-to-active-bottom
+// in the same write — per Open Question 2 resolution.
+//
+// Security: T-04-05 — item must belong to a list the caller can access.
+//           T-04-07 — patchItemSchema enforces exactly-one-field.
+//           T-04-09 — if item row is missing (deleted), 404 (no upsert).
+// ---------------------------------------------------------------------------
+listItemsRouter.patch('/:itemId', zValidator('json', patchItemSchema), async (c) => {
+  const currentUserId = await resolveUserId(c)
+  if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401)
+
+  const itemId = Number(c.req.param('itemId'))
+  const patch = c.req.valid('json')
+
+  try {
+    // Fetch the item to get its listId for access verification
+    const [item] = await db
+      .select()
+      .from(listItems)
+      .where(eq(listItems.id, itemId))
+      .limit(1)
+
+    // T-04-09: if deleted, 404 (no resurrection)
+    if (!item) return c.json({ error: 'Not found' }, 404)
+
+    const access = await checkListAccess(item.listId, currentUserId)
+    if (!access.allowed) {
+      if (access.notFound) return c.json({ error: 'Not found' }, 404)
+      return c.json({ error: 'Access denied' }, 403)
+    }
+
+    // Build the update payload — single-field write with updatedAt=NOW()
+    let updateValues: {
+      checked?: boolean
+      text?: string
+      rank?: string
+      updatedAt?: Date
+    } = {}
+
+    if (patch.checked !== undefined) {
+      updateValues.checked = patch.checked
+
+      // Open Question 2: uncheck → recompute rank to active-bottom
+      if (patch.checked === false) {
+        const [lastActive] = await db
+          .select({ rank: listItems.rank })
+          .from(listItems)
+          .where(and(
+            eq(listItems.listId, item.listId),
+            eq(listItems.checked, false),
+            sql`${listItems.id} != ${itemId}`,
+          ))
+          .orderBy(sql`${listItems.rank} DESC`)
+          .limit(1)
+
+        updateValues.rank = rankForAppend(lastActive?.rank ?? null)
+      }
+    } else if (patch.text !== undefined) {
+      updateValues.text = patch.text
+    } else if (patch.position !== undefined) {
+      updateValues.rank = patch.position
+    }
+
+    await db
+      .update(listItems)
+      .set(updateValues)
+      .where(eq(listItems.id, itemId))
+
+    // Re-fetch to return the updated row
+    const [updated] = await db
+      .select()
+      .from(listItems)
+      .where(eq(listItems.id, itemId))
+      .limit(1)
+
+    // Plan 06 SSE seam:
+    // publishListEvent(item.listId, { type: 'item:updated', listId: item.listId, payload: updated })
+
+    return c.json({
+      id: updated.id,
+      listId: updated.listId,
+      text: updated.text,
+      checked: updated.checked,
+      rank: updated.rank,
+      createdAt: updated.createdAt,
+      updatedAt: updated.updatedAt,
+    })
+  } catch (err) {
+    console.error('[lists/PATCH /list-items/:itemId] DB operation failed:', err)
+    return c.json({ error: 'Service unavailable' }, 503)
+  }
+})
+
+// ---------------------------------------------------------------------------
+// DELETE /api/list-items/:itemId
+//
+// Delete an individual item instantly — no confirmation (D-06).
+// Delete-wins semantics (D-09): no rollback path, no resurrection via in-flight edits.
+//
+// Security: T-04-05 — list access check (owner OR sharee) before delete.
+// ---------------------------------------------------------------------------
+listItemsRouter.delete('/:itemId', async (c) => {
+  const currentUserId = await resolveUserId(c)
+  if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401)
+
+  const itemId = Number(c.req.param('itemId'))
+
+  try {
+    // Fetch item to get listId for access check
+    const [item] = await db
+      .select({ id: listItems.id, listId: listItems.listId })
+      .from(listItems)
+      .where(eq(listItems.id, itemId))
+      .limit(1)
+
+    if (!item) return c.json({ error: 'Not found' }, 404)
+
+    const access = await checkListAccess(item.listId, currentUserId)
+    if (!access.allowed) {
+      if (access.notFound) return c.json({ error: 'Not found' }, 404)
+      return c.json({ error: 'Access denied' }, 403)
+    }
+
+    // Delete-wins (D-09): delete is final; no rollback path.
+    await db.delete(listItems).where(eq(listItems.id, itemId))
+
+    // Plan 06 SSE seam:
+    // publishListEvent(item.listId, { type: 'item:deleted', listId: item.listId, payload: { id: itemId } })
+
+    return c.json({ id: itemId })
+  } catch (err) {
+    console.error('[lists/DELETE /list-items/:itemId] DB operation failed:', err)
+    return c.json({ error: 'Service unavailable' }, 503)
+  }
+})