From 5719fc3459597a6937345a642cb7ea7688a6a59a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Apr 2026 09:47:09 +0000 Subject: [PATCH] fix: make admin user deletion atomic and surface errors in UI deleteUser() ran each child-table cleanup as a separate statement, so a foreign-key violation on any step (e.g. sync_log rows from other users referencing the deleted user's notes) would abort mid-way and leave the database in a partial state. The API swallowed the error and the admin page silently ignored the non-ok response, so clicking Delete and confirming appeared to do nothing. - Wrap the full cascade in a single sqlite transaction so the deletion is atomic. - Also remove rows in sync_log, note_user_state, note_collaborators, note_versions, and attachments that belong to other users but reference the deleted user's notes, so the final DELETE FROM notes cannot trip a foreign-key check. - Surface delete failures in the users settings UI instead of silently swallowing them. - Add data-testids on the user row and delete button to support tests. - Add e2e coverage for the admin user deletion flow (happy path, cancelled confirmation, and a user with notes / tags / attachments / preferences / api keys). https://claude.ai/code/session_015T2HE6VzB64ZmSSyy1Wz4R --- src/lib/server/auth.ts | 70 ++++++---- src/routes/(app)/settings/users/+page.svelte | 15 ++- tests/e2e/admin-users.spec.ts | 129 +++++++++++++++++++ 3 files changed, 189 insertions(+), 25 deletions(-) create mode 100644 tests/e2e/admin-users.spec.ts diff --git a/src/lib/server/auth.ts b/src/lib/server/auth.ts index 8465613..097db2d 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth.ts @@ -227,29 +227,53 @@ export function getUser(userId: number): User | null { } export async function deleteUser(userId: number): Promise { - // Cascade: delete user's data in dependency order - sqlite.prepare('DELETE FROM user_preferences WHERE user_id = ?').run(userId); - sqlite.prepare('DELETE FROM api_keys WHERE user_id = ?').run(userId); - sqlite.prepare('DELETE FROM note_user_state WHERE user_id = ?').run(userId); - sqlite - .prepare('DELETE FROM note_collaborators WHERE user_id = ? OR added_by = ?') - .run(userId, userId); - sqlite.prepare('DELETE FROM sync_log WHERE user_id = ?').run(userId); - sqlite.prepare('DELETE FROM attachments WHERE user_id = ?').run(userId); - sqlite - .prepare( - 'DELETE FROM shared_notes WHERE note_id IN (SELECT id FROM notes WHERE user_id = ?)' - ) - .run(userId); - sqlite - .prepare( - 'DELETE FROM note_tags WHERE note_id IN (SELECT id FROM notes WHERE user_id = ?)' - ) - .run(userId); - sqlite.prepare('DELETE FROM notes WHERE user_id = ?').run(userId); - sqlite.prepare('DELETE FROM tags WHERE user_id = ?').run(userId); - sqlite.prepare('DELETE FROM sessions WHERE user_id = ?').run(userId); - db.delete(users).where(eq(users.id, userId)).run(); + // Atomic cascade: all-or-nothing. Also clear rows from other users that + // reference the deleted user's notes (e.g. sync_log, note_user_state, + // note_collaborators) so foreign-key checks don't block the final delete. + const tx = sqlite.transaction((uid: number) => { + sqlite.prepare('DELETE FROM user_preferences WHERE user_id = ?').run(uid); + sqlite.prepare('DELETE FROM api_keys WHERE user_id = ?').run(uid); + sqlite + .prepare( + 'DELETE FROM note_user_state WHERE user_id = ? OR note_id IN (SELECT id FROM notes WHERE user_id = ?)' + ) + .run(uid, uid); + sqlite + .prepare( + 'DELETE FROM note_collaborators WHERE user_id = ? OR added_by = ? OR note_id IN (SELECT id FROM notes WHERE user_id = ?)' + ) + .run(uid, uid, uid); + sqlite + .prepare( + 'DELETE FROM sync_log WHERE user_id = ? OR note_id IN (SELECT id FROM notes WHERE user_id = ?)' + ) + .run(uid, uid); + sqlite + .prepare( + 'DELETE FROM attachments WHERE user_id = ? OR note_id IN (SELECT id FROM notes WHERE user_id = ?)' + ) + .run(uid, uid); + sqlite + .prepare( + 'DELETE FROM shared_notes WHERE note_id IN (SELECT id FROM notes WHERE user_id = ?)' + ) + .run(uid); + sqlite + .prepare( + 'DELETE FROM note_tags WHERE note_id IN (SELECT id FROM notes WHERE user_id = ?)' + ) + .run(uid); + sqlite + .prepare( + 'DELETE FROM note_versions WHERE note_id IN (SELECT id FROM notes WHERE user_id = ?)' + ) + .run(uid); + sqlite.prepare('DELETE FROM notes WHERE user_id = ?').run(uid); + sqlite.prepare('DELETE FROM tags WHERE user_id = ?').run(uid); + sqlite.prepare('DELETE FROM sessions WHERE user_id = ?').run(uid); + sqlite.prepare('DELETE FROM users WHERE id = ?').run(uid); + }); + tx(userId); } export async function changePassword( diff --git a/src/routes/(app)/settings/users/+page.svelte b/src/routes/(app)/settings/users/+page.svelte index 7ab2750..323be83 100644 --- a/src/routes/(app)/settings/users/+page.svelte +++ b/src/routes/(app)/settings/users/+page.svelte @@ -70,9 +70,16 @@ const res = await fetch(`/api/admin/users/${userId}`, { method: 'DELETE' }); if (res.ok) { users = users.filter((u) => u.id !== userId); + createMsg = 'User deleted'; + createError = false; + } else { + const d = await res.json().catch(() => ({})); + createMsg = d.message || 'Failed to delete user'; + createError = true; } } catch { - // ignore + createMsg = 'Connection error'; + createError = true; } } @@ -167,7 +174,10 @@
{#each users as user (user.id)} -
+

{user.displayName || user.email} @@ -195,6 +205,7 @@