fix(admin): address PR review feedback from all personas
Blockers resolved:
- localStorage key collision: UsersListPanel/GroupsListPanel/TagsListPanel
now each use their own key (admin_*_list_collapsed)
- $effect autocollapse replaced with $derived(autocollapse || manualCollapse)
across all three list panels (Felix — Svelte 5 rule violation)
- groups/new: add READ_ALL and ANNOTATE_ALL to available standard permissions
- Mobile back-to-list links added to all five detail panel headers (md:hidden)
so users landing directly on a detail URL on mobile can navigate back
- onDestroy(() => stopPolling()) added to system/+page.svelte (Tobias)
High priority resolved:
- Permission labels in groups/[id] and groups/new now use Paraglide i18n keys
(admin_perm_read_all, admin_perm_annotate_all, etc.) across de/en/es
- $derived used for permission arrays (reactive i18n) — Felix Svelte 5 rule
- UserGroup type in +layout.server.ts now uses generated API type (Markus/Felix)
- discardTarget annotation changed to variable-level type annotation
Accessibility (Leonie):
- EntityNav tablet icon strip buttons: min-h-[44px] for WCAG 2.5.8 compliance
- Flyout focus management: openFlyout() focuses first link, closeFlyout()
returns focus to the trigger button that opened it
- Flyout animation replaced: broken inline style -> transition:fly={{ x: -160 }}
Tests (Sara/Felix):
- localStorage key assertion tests added per panel
- localStorage.removeItem calls updated to use the panel-specific keys
- page.server.spec.ts added for groups/[id] and tags/[id] delete actions
- Polling lifecycle tests added to system/page.svelte.spec.ts
Note: Paraglide types for new admin_perm_* keys regenerate automatically on
next npm run dev (Vite plugin). No manual compilation step needed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -16,17 +16,15 @@ let {
|
||||
autocollapse?: boolean;
|
||||
} = $props();
|
||||
|
||||
let isCollapsed = $state(
|
||||
typeof localStorage !== 'undefined' && localStorage.getItem('admin_list_collapsed') === 'true'
|
||||
let manualCollapse = $state(
|
||||
typeof localStorage !== 'undefined' &&
|
||||
localStorage.getItem('admin_groups_list_collapsed') === 'true'
|
||||
);
|
||||
|
||||
$effect(() => {
|
||||
if (autocollapse) isCollapsed = true;
|
||||
});
|
||||
const isCollapsed = $derived(autocollapse || manualCollapse);
|
||||
|
||||
$effect(() => {
|
||||
if (typeof localStorage !== 'undefined') {
|
||||
localStorage.setItem('admin_list_collapsed', String(isCollapsed));
|
||||
localStorage.setItem('admin_groups_list_collapsed', String(manualCollapse));
|
||||
}
|
||||
});
|
||||
</script>
|
||||
@@ -34,7 +32,7 @@ $effect(() => {
|
||||
{#if isCollapsed}
|
||||
<!-- Collapsed handle: 32px -->
|
||||
<button
|
||||
onclick={() => (isCollapsed = false)}
|
||||
onclick={() => (manualCollapse = false)}
|
||||
aria-label={m.admin_btn_expand_list()}
|
||||
class="flex w-8 flex-shrink-0 flex-col items-center gap-2 border-r border-line bg-surface pt-2 hover:bg-muted"
|
||||
>
|
||||
@@ -74,7 +72,7 @@ $effect(() => {
|
||||
</svg>
|
||||
</a>
|
||||
<button
|
||||
onclick={() => (isCollapsed = true)}
|
||||
onclick={() => (manualCollapse = true)}
|
||||
aria-label={m.admin_btn_collapse_list()}
|
||||
class="flex h-6 w-6 items-center justify-center rounded-sm text-xs font-bold text-ink-2 transition-colors hover:bg-muted"
|
||||
>
|
||||
|
||||
@@ -7,7 +7,7 @@ let { data, form } = $props();
|
||||
|
||||
let isDirty = $state(false);
|
||||
let showUnsavedWarning = $state(false);
|
||||
let discardTarget = $state<string | null>(null);
|
||||
let discardTarget: string | null = $state(null);
|
||||
|
||||
beforeNavigate(({ cancel, to }) => {
|
||||
if (isDirty) {
|
||||
@@ -24,23 +24,38 @@ $effect(() => {
|
||||
}
|
||||
});
|
||||
|
||||
const STANDARD_PERMISSIONS: { value: string; label: string }[] = [
|
||||
{ value: 'READ_ALL', label: 'Nur lesen' },
|
||||
{ value: 'ANNOTATE_ALL', label: 'Lesen & Annotieren' },
|
||||
{ value: 'WRITE_ALL', label: 'Lesen & Schreiben' }
|
||||
];
|
||||
const STANDARD_PERMISSIONS = $derived([
|
||||
{ value: 'READ_ALL', label: m.admin_perm_read_all() },
|
||||
{ value: 'ANNOTATE_ALL', label: m.admin_perm_annotate_all() },
|
||||
{ value: 'WRITE_ALL', label: m.admin_perm_write_all() }
|
||||
]);
|
||||
|
||||
const ADMIN_PERMISSIONS: { value: string; label: string }[] = [
|
||||
{ value: 'ADMIN', label: 'Vollzugriff (Admin)' },
|
||||
{ value: 'ADMIN_USER', label: 'Benutzer verwalten' },
|
||||
{ value: 'ADMIN_TAG', label: 'Schlagworte verwalten' },
|
||||
{ value: 'ADMIN_PERMISSION', label: 'Berechtigungen verwalten' }
|
||||
];
|
||||
const ADMIN_PERMISSIONS = $derived([
|
||||
{ value: 'ADMIN', label: m.admin_perm_admin() },
|
||||
{ value: 'ADMIN_USER', label: m.admin_perm_admin_user() },
|
||||
{ value: 'ADMIN_TAG', label: m.admin_perm_admin_tag() },
|
||||
{ value: 'ADMIN_PERMISSION', label: m.admin_perm_admin_permission() }
|
||||
]);
|
||||
</script>
|
||||
|
||||
<div class="flex flex-1 flex-col overflow-hidden">
|
||||
<!-- Header -->
|
||||
<div class="flex items-center border-b border-line px-5 py-3">
|
||||
<a
|
||||
href="/admin/groups"
|
||||
class="mr-3 inline-flex items-center gap-1 text-xs text-ink-3 hover:text-ink md:hidden"
|
||||
>
|
||||
<svg
|
||||
class="h-4 w-4"
|
||||
fill="none"
|
||||
viewBox="0 0 24 24"
|
||||
stroke="currentColor"
|
||||
stroke-width="2"
|
||||
aria-hidden="true"
|
||||
>
|
||||
<path stroke-linecap="round" stroke-linejoin="round" d="M15 19l-7-7 7-7" />
|
||||
</svg>
|
||||
</a>
|
||||
<h2 class="flex-1 font-sans text-sm font-bold text-ink">
|
||||
{m.admin_group_edit_heading({ name: data.group.name })}
|
||||
</h2>
|
||||
|
||||
87
frontend/src/routes/admin/groups/[id]/page.server.spec.ts
Normal file
87
frontend/src/routes/admin/groups/[id]/page.server.spec.ts
Normal file
@@ -0,0 +1,87 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { actions } from './+page.server';
|
||||
|
||||
const mockApi = {
|
||||
PATCH: vi.fn(),
|
||||
DELETE: vi.fn()
|
||||
};
|
||||
|
||||
vi.mock('$lib/api.server', () => ({
|
||||
createApiClient: () => mockApi
|
||||
}));
|
||||
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
// ─── update action ─────────────────────────────────────────────────────────────
|
||||
|
||||
describe('groups/[id] — update action', () => {
|
||||
it('returns success: true when API responds ok', async () => {
|
||||
mockApi.PATCH.mockResolvedValue({ response: { ok: true }, data: {} });
|
||||
|
||||
const formData = new FormData();
|
||||
formData.set('name', 'Editors');
|
||||
formData.append('permissions', 'WRITE_ALL');
|
||||
|
||||
const result = await actions.update({
|
||||
params: { id: 'g1' },
|
||||
request: { formData: async () => formData },
|
||||
fetch
|
||||
} as never);
|
||||
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('returns fail with error message when API responds not ok', async () => {
|
||||
mockApi.PATCH.mockResolvedValue({
|
||||
response: { ok: false, status: 409 },
|
||||
error: { code: 'GROUP_NOT_FOUND' }
|
||||
});
|
||||
|
||||
const formData = new FormData();
|
||||
formData.set('name', 'Editors');
|
||||
|
||||
const result = await actions.update({
|
||||
params: { id: 'g1' },
|
||||
request: { formData: async () => formData },
|
||||
fetch
|
||||
} as never);
|
||||
|
||||
expect((result as { status: number }).status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── delete action ─────────────────────────────────────────────────────────────
|
||||
|
||||
describe('groups/[id] — delete action', () => {
|
||||
it('redirects to /admin/groups on successful delete', async () => {
|
||||
mockApi.DELETE.mockResolvedValue({ response: { ok: true } });
|
||||
|
||||
let redirectUrl: string | null = null;
|
||||
try {
|
||||
await actions.delete({
|
||||
params: { id: 'g1' },
|
||||
fetch
|
||||
} as never);
|
||||
} catch (e: unknown) {
|
||||
// SvelteKit redirect throws a Response-like object
|
||||
const r = e as { location?: string; status?: number };
|
||||
redirectUrl = r.location ?? null;
|
||||
}
|
||||
|
||||
expect(redirectUrl).toBe('/admin/groups');
|
||||
});
|
||||
|
||||
it('returns fail with error message when delete API responds not ok', async () => {
|
||||
mockApi.DELETE.mockResolvedValue({
|
||||
response: { ok: false, status: 403 },
|
||||
error: { code: 'FORBIDDEN' }
|
||||
});
|
||||
|
||||
const result = await actions.delete({
|
||||
params: { id: 'g1' },
|
||||
fetch
|
||||
} as never);
|
||||
|
||||
expect((result as { status: number }).status).toBe(403);
|
||||
});
|
||||
});
|
||||
@@ -81,7 +81,7 @@ describe('GroupsListPanel — empty state', () => {
|
||||
// ─── Collapse toggle ──────────────────────────────────────────────────────────
|
||||
|
||||
describe('GroupsListPanel — collapse toggle', () => {
|
||||
beforeEach(() => localStorage.removeItem('admin_list_collapsed'));
|
||||
beforeEach(() => localStorage.removeItem('admin_groups_list_collapsed'));
|
||||
|
||||
it('renders a collapse button with aria-label', async () => {
|
||||
render(GroupsListPanel, { groups });
|
||||
@@ -107,4 +107,12 @@ describe('GroupsListPanel — collapse toggle', () => {
|
||||
.element(page.getByRole('button', { name: /Liste ausklappen/i }))
|
||||
.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('persists collapse state using the groups-specific localStorage key', async () => {
|
||||
const setSpy = vi.spyOn(Storage.prototype, 'setItem');
|
||||
render(GroupsListPanel, { groups });
|
||||
document.querySelector<HTMLButtonElement>('[aria-label="Liste einklappen"]')!.click();
|
||||
expect(setSpy).toHaveBeenCalledWith('admin_groups_list_collapsed', 'true');
|
||||
setSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,19 +3,23 @@ import { enhance } from '$app/forms';
|
||||
import { beforeNavigate, goto } from '$app/navigation';
|
||||
import { m } from '$lib/paraglide/messages.js';
|
||||
|
||||
const availableStandard = [{ value: 'WRITE_ALL', label: 'Lesen & Schreiben' }];
|
||||
const availableAdmin = [
|
||||
{ value: 'ADMIN', label: 'Vollzugriff (Admin)' },
|
||||
{ value: 'ADMIN_USER', label: 'Benutzer verwalten' },
|
||||
{ value: 'ADMIN_TAG', label: 'Schlagworte verwalten' },
|
||||
{ value: 'ADMIN_PERMISSION', label: 'Berechtigungen verwalten' }
|
||||
];
|
||||
const availableStandard = $derived([
|
||||
{ value: 'READ_ALL', label: m.admin_perm_read_all() },
|
||||
{ value: 'ANNOTATE_ALL', label: m.admin_perm_annotate_all() },
|
||||
{ value: 'WRITE_ALL', label: m.admin_perm_write_all() }
|
||||
]);
|
||||
const availableAdmin = $derived([
|
||||
{ value: 'ADMIN', label: m.admin_perm_admin() },
|
||||
{ value: 'ADMIN_USER', label: m.admin_perm_admin_user() },
|
||||
{ value: 'ADMIN_TAG', label: m.admin_perm_admin_tag() },
|
||||
{ value: 'ADMIN_PERMISSION', label: m.admin_perm_admin_permission() }
|
||||
]);
|
||||
|
||||
let { form } = $props();
|
||||
|
||||
let isDirty = $state(false);
|
||||
let showUnsavedWarning = $state(false);
|
||||
let discardTarget = $state<string | null>(null);
|
||||
let discardTarget: string | null = $state(null);
|
||||
|
||||
beforeNavigate(({ cancel, to }) => {
|
||||
if (isDirty) {
|
||||
@@ -31,6 +35,21 @@ beforeNavigate(({ cancel, to }) => {
|
||||
<div
|
||||
class="flex items-center border-b border-green-200 bg-green-50 px-5 py-3 dark:border-green-900 dark:bg-green-950/30"
|
||||
>
|
||||
<a
|
||||
href="/admin/groups"
|
||||
class="mr-3 inline-flex items-center gap-1 text-xs text-green-700 hover:text-green-900 md:hidden"
|
||||
>
|
||||
<svg
|
||||
class="h-4 w-4"
|
||||
fill="none"
|
||||
viewBox="0 0 24 24"
|
||||
stroke="currentColor"
|
||||
stroke-width="2"
|
||||
aria-hidden="true"
|
||||
>
|
||||
<path stroke-linecap="round" stroke-linejoin="round" d="M15 19l-7-7 7-7" />
|
||||
</svg>
|
||||
</a>
|
||||
<h2 class="flex-1 font-sans text-sm font-bold text-green-800 dark:text-green-300">
|
||||
{m.admin_group_new_heading()}
|
||||
</h2>
|
||||
|
||||
Reference in New Issue
Block a user