From 9bcdf53d0b98ff98c572989daee36cb9aa91c3f1 Mon Sep 17 00:00:00 2001 From: Ole-Morten Duesund Date: Mon, 25 May 2026 17:35:14 +0200 Subject: [PATCH] refactor(activities): dedupe heart/bookmark POST+DELETE endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four nearly-identical endpoints (heart add/remove, bookmark add/remove) collapse into one toggleMark(c, kind, op) helper. Behaviour is unchanged — idempotent on both sides, 404 on missing activity, 400 if private, same serialized response. Tests pass. Surfaced by /audit simplify. Co-Authored-By: Claude Opus 4.7 --- server/activities.ts | 106 ++++++++++++++++--------------------------- 1 file changed, 39 insertions(+), 67 deletions(-) diff --git a/server/activities.ts b/server/activities.ts index d83ae5f..3bb6e44 100644 --- a/server/activities.ts +++ b/server/activities.ts @@ -1,6 +1,6 @@ import { Hono } from 'hono'; import { getDb } from './db'; -import { requireAuth, currentUserId, type AppVariables } from './session'; +import { requireAuth, currentUserId, type AppVariables, type AppContext } from './session'; import { setActivityTags, clearActivityTags, tagsFor } from './tags'; import { isModerator } from './roles'; import type { @@ -466,80 +466,52 @@ activitiesRoutes.patch('/:id', requireAuth, async (c) => { return c.json(serialize(row, userId)); }); -// --- POST /api/activities/:id/heart ---------------------------------------- -// Idempotent heart: if the viewer already hearted it, this is a no-op rather -// than a 409. The client posts the same shape regardless of state. -activitiesRoutes.post('/:id/heart', requireAuth, (c) => { +/** + * Heart and bookmark POST/DELETE pairs were 95% identical — same shape on + * both sides, only the table name + error code differed. One helper now + * covers all four endpoints. Idempotent on both add and remove sides + * (INSERT OR IGNORE / DELETE) so the client doesn't have to track state. + */ +type Mark = 'heart' | 'bookmark'; +const MARK_TABLES: Record = { + heart: { table: 'activity_hearts', privateError: 'cannot_heart_private' }, + bookmark: { table: 'bookmarks', privateError: 'cannot_bookmark_private' }, +}; + +function toggleMark(c: AppContext, kind: Mark, op: 'add' | 'remove') { const userId = c.get('userId'); const id = c.req.param('id'); + // Route is registered with `/:id`, so Hono always populates the param — + // the narrow is to satisfy TS once we're outside the inline-handler closure + // where Hono's path-shape inference kicks in. + if (!id) return c.json({ error: 'not_found' }, 404); const db = getDb(); + const { table, privateError } = MARK_TABLES[kind]; - const row = db - .prepare('SELECT visibility, owner_id FROM activities WHERE id = ?') - .get(id) as { visibility: Visibility; owner_id: string } | null; - if (!row) return c.json({ error: 'not_found' }, 404); - - // Hearts only make sense on what other people can see. - if (row.visibility === 'private') return c.json({ error: 'cannot_heart_private' }, 400); - - db.prepare( - 'INSERT OR IGNORE INTO activity_hearts (activity_id, user_id, created_at) VALUES (?, ?, ?)', - ).run(id, userId, Date.now()); + if (op === 'add') { + const row = db + .prepare('SELECT visibility FROM activities WHERE id = ?') + .get(id) as { visibility: Visibility } | null; + if (!row) return c.json({ error: 'not_found' }, 404); + // Marks only make sense on what other people can see — see SECURITY.md. + if (row.visibility === 'private') return c.json({ error: privateError }, 400); + db.prepare( + `INSERT OR IGNORE INTO ${table} (user_id, activity_id, created_at) VALUES (?, ?, ?)`, + ).run(userId, id, Date.now()); + } else { + const row = db.prepare('SELECT 1 FROM activities WHERE id = ?').get(id); + if (!row) return c.json({ error: 'not_found' }, 404); + db.prepare(`DELETE FROM ${table} WHERE user_id = ? AND activity_id = ?`).run(userId, id); + } const refreshed = db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow; return c.json(serialize(refreshed, userId)); -}); +} -// --- DELETE /api/activities/:id/heart -------------------------------------- -activitiesRoutes.delete('/:id/heart', requireAuth, (c) => { - const userId = c.get('userId'); - const id = c.req.param('id'); - const db = getDb(); - - const row = db.prepare('SELECT 1 FROM activities WHERE id = ?').get(id); - if (!row) return c.json({ error: 'not_found' }, 404); - - db.prepare('DELETE FROM activity_hearts WHERE activity_id = ? AND user_id = ?').run(id, userId); - - const refreshed = db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow; - return c.json(serialize(refreshed, userId)); -}); - -// --- POST /api/activities/:id/bookmark ------------------------------------- -// Idempotent. Refuses on private rows (the owner already has direct access). -activitiesRoutes.post('/:id/bookmark', requireAuth, (c) => { - const userId = c.get('userId'); - const id = c.req.param('id'); - const db = getDb(); - - const row = db - .prepare('SELECT visibility FROM activities WHERE id = ?') - .get(id) as { visibility: Visibility } | null; - if (!row) return c.json({ error: 'not_found' }, 404); - if (row.visibility === 'private') return c.json({ error: 'cannot_bookmark_private' }, 400); - - db.prepare( - 'INSERT OR IGNORE INTO bookmarks (user_id, activity_id, created_at) VALUES (?, ?, ?)', - ).run(userId, id, Date.now()); - - const refreshed = db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow; - return c.json(serialize(refreshed, userId)); -}); - -// --- DELETE /api/activities/:id/bookmark ----------------------------------- -activitiesRoutes.delete('/:id/bookmark', requireAuth, (c) => { - const userId = c.get('userId'); - const id = c.req.param('id'); - const db = getDb(); - - const row = db.prepare('SELECT 1 FROM activities WHERE id = ?').get(id); - if (!row) return c.json({ error: 'not_found' }, 404); - - db.prepare('DELETE FROM bookmarks WHERE user_id = ? AND activity_id = ?').run(userId, id); - - const refreshed = db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow; - return c.json(serialize(refreshed, userId)); -}); +activitiesRoutes.post('/:id/heart', requireAuth, (c) => toggleMark(c, 'heart', 'add')); +activitiesRoutes.delete('/:id/heart', requireAuth, (c) => toggleMark(c, 'heart', 'remove')); +activitiesRoutes.post('/:id/bookmark', requireAuth, (c) => toggleMark(c, 'bookmark', 'add')); +activitiesRoutes.delete('/:id/bookmark', requireAuth, (c) => toggleMark(c, 'bookmark', 'remove')); // --- DELETE /api/activities/:id --------------------------------------------- // Authz: