fix(activities): preserve viewer's sort_position on single-row fetches

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.
This commit is contained in:
Ole-Morten Duesund 2026-05-25 19:46:24 +02:00
commit fb193b4914
2 changed files with 69 additions and 5 deletions

View file

@ -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));
}

View file

@ -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<Activity>(ctx, 'POST', `/api/activities/${a1.id}/heart`, {
cookie: user.cookie,
});
expect(hearted.sort_position).toBe(customPos);
// Same for done.
const doneRes = await reqJson<Activity>(ctx, 'POST', `/api/activities/${a1.id}/done`, {
cookie: user.cookie,
});
expect(doneRes.sort_position).toBe(customPos);
// And for GET /:id.
const single = await reqJson<Activity>(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);