fix: confirm admin role toggle and keep the checkbox controlled
The admin checkbox fired an immediate, irreversible privilege change with no confirmation, and on cancel/failure the optimistic DOM flip stuck while the server state was unchanged. Add a confirm() gate (like Delete) and make the checkbox controlled by u.is_admin: onchange reverts the DOM immediately and it only flips once the server confirms — so a cancelled or failed toggle never misrepresents the real role. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.125.0"
|
||||
version = "0.125.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.125.0"
|
||||
version = "0.125.1"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
132
frontend/e2e/admin-users.spec.ts
Normal file
132
frontend/e2e/admin-users.spec.ts
Normal file
@@ -0,0 +1,132 @@
|
||||
import { test, expect, type Page } from './fixtures';
|
||||
|
||||
// E2E for the admin Users page role toggle: flipping the Admin checkbox is a
|
||||
// one-click privilege change, so it must confirm first, and the checkbox must
|
||||
// never show a state the server didn't actually apply (cancel or failure keeps
|
||||
// it on the true value).
|
||||
|
||||
const DESKTOP = { width: 1280, height: 720 } as const;
|
||||
|
||||
const adminUser = {
|
||||
id: 'u11111111-1111-1111-1111-111111111111',
|
||||
username: 'admin',
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
is_admin: true
|
||||
};
|
||||
|
||||
const bob = {
|
||||
id: 'b22222222-2222-2222-2222-222222222222',
|
||||
username: 'bob',
|
||||
created_at: '2026-02-01T00:00:00Z',
|
||||
is_admin: false
|
||||
};
|
||||
|
||||
type Captured = { patched: boolean };
|
||||
|
||||
async function mockAdmin(page: Page, cap: Captured, opts: { patchStatus: number }) {
|
||||
await page.route('**/api/v1/auth/config', (r) =>
|
||||
r.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({ self_register_enabled: true, private_mode: false })
|
||||
})
|
||||
);
|
||||
await page.route('**/api/v1/auth/me', (r) =>
|
||||
r.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({ user: adminUser })
|
||||
})
|
||||
);
|
||||
await page.route('**/api/v1/auth/me/preferences', (r) =>
|
||||
r.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({ reader_mode: 'single', reader_page_gap: 'small' })
|
||||
})
|
||||
);
|
||||
await page.route('**/api/v1/me/bookmarks*', (r) =>
|
||||
r.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({ items: [], page: { limit: 50, offset: 0, total: 0 } })
|
||||
})
|
||||
);
|
||||
await page.route('**/api/v1/admin/system', (r) =>
|
||||
r.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({
|
||||
disk: null,
|
||||
memory: { total_bytes: 1, used_bytes: 0, percent_used: 0 },
|
||||
cpu: { percent_used: 0 },
|
||||
alerts: []
|
||||
})
|
||||
})
|
||||
);
|
||||
// The PATCH toggle — record whether it was ever sent.
|
||||
await page.route('**/api/v1/admin/users/*', async (r) => {
|
||||
cap.patched = true;
|
||||
await r.fulfill({
|
||||
status: opts.patchStatus,
|
||||
contentType: 'application/json',
|
||||
body:
|
||||
opts.patchStatus < 400
|
||||
? JSON.stringify({ ...bob, is_admin: true })
|
||||
: JSON.stringify({ error: { code: 'internal_error', message: 'boom' } })
|
||||
});
|
||||
});
|
||||
// The user list (registered after the specific PATCH glob; list is GET).
|
||||
await page.route('**/api/v1/admin/users**', async (r) => {
|
||||
if (r.request().method() !== 'GET') return r.fallback();
|
||||
await r.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({
|
||||
items: [adminUser, bob],
|
||||
page: { limit: 100, offset: 0, total: 2 }
|
||||
})
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
test.describe('/admin/users role toggle', () => {
|
||||
test('cancelling the confirm leaves the checkbox unchanged and sends no request', async ({
|
||||
page
|
||||
}) => {
|
||||
const cap: Captured = { patched: false };
|
||||
await mockAdmin(page, cap, { patchStatus: 200 });
|
||||
page.on('dialog', (d) => d.dismiss());
|
||||
await page.setViewportSize(DESKTOP);
|
||||
await page.goto('/admin/users');
|
||||
|
||||
const bobRow = page.locator('tr', { hasText: 'bob' });
|
||||
const checkbox = bobRow.getByLabel('admin');
|
||||
await expect(checkbox).not.toBeChecked();
|
||||
|
||||
await checkbox.click();
|
||||
|
||||
// Cancelled → no PATCH, and the box still reflects the server state.
|
||||
await expect(checkbox).not.toBeChecked();
|
||||
expect(cap.patched).toBe(false);
|
||||
});
|
||||
|
||||
test('a failed toggle reverts the checkbox to the server state', async ({ page }) => {
|
||||
const cap: Captured = { patched: false };
|
||||
await mockAdmin(page, cap, { patchStatus: 500 });
|
||||
page.on('dialog', (d) => d.accept());
|
||||
await page.setViewportSize(DESKTOP);
|
||||
await page.goto('/admin/users');
|
||||
|
||||
const bobRow = page.locator('tr', { hasText: 'bob' });
|
||||
const checkbox = bobRow.getByLabel('admin');
|
||||
await expect(checkbox).not.toBeChecked();
|
||||
|
||||
await checkbox.click();
|
||||
|
||||
// The PATCH was attempted and failed → the checkbox must snap back to
|
||||
// the actual (still non-admin) server state rather than stay flipped.
|
||||
await expect.poll(() => cap.patched).toBe(true);
|
||||
await expect(checkbox).not.toBeChecked();
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.125.0",
|
||||
"version": "0.125.1",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
@@ -51,12 +51,19 @@
|
||||
}
|
||||
|
||||
async function onToggleAdmin(id: string, next: boolean) {
|
||||
// Privilege changes are one-click and consequential — confirm first,
|
||||
// like Delete. The checkbox is kept controlled by `u.is_admin` (see the
|
||||
// onchange handler), so a cancel here leaves it showing the true state.
|
||||
const verb = next ? 'grant admin rights to' : 'revoke admin rights from';
|
||||
if (!confirm(`Are you sure you want to ${verb} this user?`)) return;
|
||||
busyId = id;
|
||||
try {
|
||||
await setUserAdmin(id, next);
|
||||
await load();
|
||||
} catch (e) {
|
||||
error = e instanceof ApiError ? e.message : 'update failed';
|
||||
// The checkbox was reverted in onchange and `u.is_admin` is
|
||||
// unchanged, so it correctly still reflects the server state.
|
||||
} finally {
|
||||
busyId = null;
|
||||
}
|
||||
@@ -187,7 +194,15 @@
|
||||
type="checkbox"
|
||||
checked={u.is_admin}
|
||||
disabled={busyId === u.id || isSelf}
|
||||
onchange={(e) => onToggleAdmin(u.id, e.currentTarget.checked)}
|
||||
onchange={(e) => {
|
||||
const desired = e.currentTarget.checked;
|
||||
// Keep the box controlled by `u.is_admin`: undo the
|
||||
// optimistic flip so it only changes once the server
|
||||
// confirms — and never lies if the change is
|
||||
// cancelled or the request fails.
|
||||
e.currentTarget.checked = u.is_admin;
|
||||
onToggleAdmin(u.id, desired);
|
||||
}}
|
||||
aria-label="admin"
|
||||
/>
|
||||
</td>
|
||||
|
||||
Reference in New Issue
Block a user