From fb193b491431f4e34111d74963a9d046890b1a0b Mon Sep 17 00:00:00 2001 From: Ole-Morten Duesund Date: Mon, 25 May 2026 19:46:24 +0200 Subject: [PATCH] fix(activities): preserve viewer's sort_position on single-row fetches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Toggling "gjort" (and heart, and bookmark, and edit, and GET /:id) silently reset the row's effective sort_position to -created_at, wiping any custom drag-sort the viewer had applied to that row. The list endpoint joins user_activity_sort to get the per-viewer position; single-row endpoints were doing plain `SELECT * FROM activities WHERE id = ?` and serialize() was falling back to -created_at when sort_position was missing from the row. User-visible effect on the private list (which often has custom ordering since it's the user's todo list): toggling a checkbox made that row jump back to its created_at slot. Fix: fetchRowForViewer(id, viewerId) helper that does the same LEFT JOIN as the list query. Routed through every single-row return path — GET /:id, POST /, PATCH /:id, POST/DELETE /:id/heart, POST/DELETE /:id/bookmark, POST/DELETE /:id/done. Regression test covers heart + done + GET-by-id all preserving a custom sort_position written via PATCH /:id/sort. --- server/activities.ts | 38 +++++++++++++++++++++++++++++++++----- tests/activities.test.ts | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/server/activities.ts b/server/activities.ts index b9d7aa5..2d8238b 100644 --- a/server/activities.ts +++ b/server/activities.ts @@ -125,6 +125,31 @@ function b64(b: Uint8Array | null): string | null { return b === null ? null : Buffer.from(b).toString('base64'); } +/** + * Single-row fetch that includes the viewer's custom sort_position via the + * same LEFT JOIN as the list endpoint. Single-row endpoints (POST, PATCH, + * GET /:id, heart/bookmark/done toggles) used to call plain + * `SELECT * FROM activities WHERE id = ?` and let serialize() fall back to + * -created_at, which silently overwrote any custom drag-sort the viewer + * had on that row. Use this helper instead so toggles preserve the user's + * ordering. + */ +function fetchRowForViewer(id: string, viewerId: string | null): ActivityRow | null { + const db = getDb(); + if (viewerId) { + return db + .prepare(` + SELECT activities.*, s.position AS sort_position + FROM activities + LEFT JOIN user_activity_sort s + ON s.activity_id = activities.id AND s.user_id = ? + WHERE activities.id = ? + `) + .get(viewerId, id) as ActivityRow | null; + } + return db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow | null; +} + function b64ToBuf(s: string): Buffer { return Buffer.from(s, 'base64'); } @@ -466,7 +491,7 @@ activitiesRoutes.patch('/:id/sort', requireAuth, async (c) => { // --- GET /api/activities/:id ------------------------------------------------ activitiesRoutes.get('/:id', (c) => { const viewerId = currentUserId(c); - const row = getDb().prepare('SELECT * FROM activities WHERE id = ?').get(c.req.param('id')) as ActivityRow | null; + const row = fetchRowForViewer(c.req.param('id'), viewerId); if (!row) return c.json({ error: 'not_found' }, 404); // Apply the same visibility rules as the list endpoint. We return 404 @@ -533,7 +558,10 @@ activitiesRoutes.post('/', requireAuth, async (c) => { setActivityTags(id, body.tags ?? []); } - const row = db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow; + // No custom sort_position can exist for a row this user just created, so + // the LEFT JOIN is a strict no-op here — but using the helper keeps the + // single return path uniform. + const row = fetchRowForViewer(id, userId) as ActivityRow; return c.json(serialize(row, userId), 201); }); @@ -601,7 +629,7 @@ activitiesRoutes.patch('/:id', requireAuth, async (c) => { setActivityTags(id, body.tags ?? []); } - const row = db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow; + const row = fetchRowForViewer(id, userId) as ActivityRow; return c.json(serialize(row, userId)); }); @@ -643,7 +671,7 @@ function toggleMark(c: AppContext, kind: Mark, op: 'add' | 'remove') { 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; + const refreshed = fetchRowForViewer(id, userId) as ActivityRow; return c.json(serialize(refreshed, userId)); } @@ -700,7 +728,7 @@ function toggleDone(c: AppContext, op: 'add' | 'remove') { db.prepare('DELETE FROM activity_done WHERE activity_id = ? AND user_id = ?').run(id, userId); } - const refreshed = db.prepare('SELECT * FROM activities WHERE id = ?').get(id) as ActivityRow; + const refreshed = fetchRowForViewer(id, userId) as ActivityRow; return c.json(serialize(refreshed, userId)); } diff --git a/tests/activities.test.ts b/tests/activities.test.ts index 402727e..a27dcad 100644 --- a/tests/activities.test.ts +++ b/tests/activities.test.ts @@ -333,6 +333,42 @@ describe('per-user sort order', () => { expect(finalIds[0]).toBe(fresh.id); }); + ttest('single-row endpoints preserve the viewer\'s custom sort_position', async () => { + // Regression: heart / bookmark / done toggles (and PATCH/GET-by-id) used + // to do plain `SELECT * FROM activities` without the user_activity_sort + // LEFT JOIN, so serialize() silently fell back to -created_at and + // overwrote any custom position. Now they go through fetchRowForViewer. + const user = await signupAndGetCookie(ctx, 'sort-toggle@test.invalid'); + const a1 = await createActivity(ctx, user.cookie, { visibility: 'public', title: 'one', tags: [] }); + await new Promise((r) => setTimeout(r, 5)); + const a2 = await createActivity(ctx, user.cookie, { visibility: 'public', title: 'two', tags: [] }); + + // Drag a1 below a2 (more positive sort_position → later in the ASC sort). + const customPos = -a2.created_at + 100; + await reqJson(ctx, 'PATCH', `/api/activities/${a1.id}/sort`, { + cookie: user.cookie, body: { position: customPos }, + }); + + // Heart toggle should return a1 with the SAME custom sort_position, + // not the default -created_at. + const hearted = await reqJson(ctx, 'POST', `/api/activities/${a1.id}/heart`, { + cookie: user.cookie, + }); + expect(hearted.sort_position).toBe(customPos); + + // Same for done. + const doneRes = await reqJson(ctx, 'POST', `/api/activities/${a1.id}/done`, { + cookie: user.cookie, + }); + expect(doneRes.sort_position).toBe(customPos); + + // And for GET /:id. + const single = await reqJson(ctx, 'GET', `/api/activities/${a1.id}`, { + cookie: user.cookie, + }); + expect(single.sort_position).toBe(customPos); + }); + test('PATCH /sort requires auth', async () => { const res = await req(ctx, 'PATCH', '/api/activities/whatever/sort', { body: { position: 1 } }); expect(res.status).toBe(401);