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
This commit is contained in:
@@ -7,7 +7,7 @@ import { healthRouter } from './routes/health.js'
|
|||||||
import { meRouter } from './routes/me.js'
|
import { meRouter } from './routes/me.js'
|
||||||
import { eventsRouter } from './routes/events.js'
|
import { eventsRouter } from './routes/events.js'
|
||||||
import { sseRouter } from './routes/sse.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 { oidcAuthMiddleware, processOAuthCallback } from './auth/middleware.js'
|
||||||
import { devAuthBypass } from './auth/devBypass.js'
|
import { devAuthBypass } from './auth/devBypass.js'
|
||||||
import { startBrokerPoller } from './broker/poller.js'
|
import { startBrokerPoller } from './broker/poller.js'
|
||||||
@@ -63,6 +63,7 @@ app.route('/api/me', meRouter)
|
|||||||
app.route('/api/events', eventsRouter)
|
app.route('/api/events', eventsRouter)
|
||||||
app.route('/api/sse', sseRouter)
|
app.route('/api/sse', sseRouter)
|
||||||
app.route('/api/lists', listsRouter)
|
app.route('/api/lists', listsRouter)
|
||||||
|
app.route('/api/list-items', listItemsRouter)
|
||||||
|
|
||||||
// WR-04: background worker startup (cron schedules) moved into the isMainModule()
|
// WR-04: background worker startup (cron schedules) moved into the isMainModule()
|
||||||
// guard below. Calling them at top level registered real node-cron schedules whenever
|
// 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 type { Context } from 'hono'
|
||||||
import { zValidator } from '@hono/zod-validator'
|
import { zValidator } from '@hono/zod-validator'
|
||||||
import { z } from 'zod'
|
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 { db } from '../db/client.js'
|
||||||
import { lists, listShares, listItems, users } from '../db/schema.js'
|
import { lists, listShares, listItems, users } from '../db/schema.js'
|
||||||
import { getAuth } from '../auth/middleware.js'
|
import { getAuth } from '../auth/middleware.js'
|
||||||
import { upsertUser, deriveDisplayName } from '../auth/user.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')
|
// Side-effect import: brings in the ContextVariableMap augmentation for c.get('user')
|
||||||
import '../auth/devBypass.js'
|
import '../auth/devBypass.js'
|
||||||
|
|
||||||
@@ -34,6 +35,18 @@ import '../auth/devBypass.js'
|
|||||||
|
|
||||||
export const listsRouter = new Hono()
|
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.
|
// Auth helper — copied verbatim from events.ts per project convention.
|
||||||
// Duplicated per router (not extracted to shared module).
|
// 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)',
|
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)
|
// Helper: check list access (owner OR sharee)
|
||||||
// Returns: { allowed: true, isOwner: boolean } | { allowed: false }
|
// Returns: { allowed: true, isOwner: boolean } | { allowed: false }
|
||||||
@@ -395,3 +431,263 @@ listsRouter.delete('/:id', async (c) => {
|
|||||||
return c.json({ error: 'Service unavailable' }, 503)
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user