refactor(activities): dedupe heart/bookmark POST+DELETE endpoints

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 <noreply@anthropic.com>
This commit is contained in:
Ole-Morten Duesund 2026-05-25 17:35:14 +02:00
commit 9bcdf53d0b

View file

@ -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<Mark, { table: string; privateError: string }> = {
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: