feat(admin): expand /admin overview into an actionable workbench

The admin overview was just two stat cards. Promotes it to a proper workbench by surfacing the top-5 of each actionable queue with inline handlers, while keeping the dedicated /admin/requests and /admin/quarantine pages as the deep-action surfaces. Changes: - Top stats grow from 2 cards to 3 (requests / quarantine / lidarr). - New "Pending requests" preview list — top-5 rows with inline Approve and Reject buttons that call the same approveRequest/rejectRequest endpoints used on the dedicated page (default profiles, no override modal — that flow stays on /admin/requests for power users). - New "Quarantine" preview list — top-5 rows with three inline actions matching the dedicated page: Resolve (single click), Delete file (two-click confirm with inline "Confirm?" state), Delete via Lidarr (two-click confirm). The two-click confirm avoids modal duplication on the overview while still gating the irreversible operations. - "View all <N> →" link surfaces beneath each preview only when the full queue exceeds the preview limit, pointing to the dedicated page. - Shared toast surface with errorCopy mapping for the same lidarr_* codes the dedicated requests page handles, so failed actions surface meaningful text instead of "unknown". Test fixture migrated from per-test dynamic imports to top-level imports — the previous pattern was producing stale-mock symptoms where mockReturnValue was set after the page bound against the original mocks. 12 tests cover stat counts, empty states, links, each handler binding, the two-click confirm pattern, and the "View all" surfacing rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-02 12:51:57 -04:00
parent 1c39001482
commit 7b91718fc9
2 changed files with 481 additions and 43 deletions
@@ -1,5 +1,5 @@
 import { afterEach, describe, expect, test, vi } from 'vitest';
-import { render, screen } from '@testing-library/svelte';
+import { render, screen, fireEvent, cleanup } from '@testing-library/svelte';
 import { mockQuery } from '../../test-utils/query';

 // Mutable mock-user handle so individual tests can flip is_admin / null
@@ -18,10 +18,25 @@ vi.mock('$app/state', () => ({

 vi.mock('$lib/api/admin', () => ({
  createAdminRequestsQuery: vi.fn(),
-  createLidarrConfigQuery: vi.fn()
+  createLidarrConfigQuery: vi.fn(),
+  createAdminQuarantineQuery: vi.fn(),
+  approveRequest: vi.fn().mockResolvedValue({}),
+  rejectRequest: vi.fn().mockResolvedValue({}),
+  resolveQuarantine: vi.fn().mockResolvedValue({}),
+  deleteQuarantineFile: vi.fn().mockResolvedValue({}),
+  deleteQuarantineViaLidarr: vi.fn().mockResolvedValue({})
 }));

+vi.mock('@tanstack/svelte-query', async (orig) => {
+  const actual = (await orig()) as Record<string, unknown>;
+  return {
+    ...actual,
+    useQueryClient: () => ({ invalidateQueries: vi.fn() })
+  };
+});
+
 afterEach(() => {
+  cleanup();
  vi.clearAllMocks();
  userState.current = null;
 });
@@ -31,8 +46,6 @@ describe('admin/+layout.ts load gate', () => {
    userState.current = { id: 'u1', username: 'alice', is_admin: false };
    const { load } = await import('./+layout');
    await expect(async () =>
-      // The SvelteKit `load` signature wants a LoadEvent; we don't read any of
-      // it inside the function, so an empty cast is sufficient for this gate.
      load({} as Parameters<typeof load>[0])
    ).rejects.toMatchObject({ status: 302, location: '/' });
  });
@@ -48,55 +61,165 @@ describe('admin/+layout.ts load gate', () => {
  test('admin user passes the gate', async () => {
    userState.current = { id: 'u1', username: 'root', is_admin: true };
    const { load } = await import('./+layout');
-    // load() is synchronous in the happy path; it only throws on the gate.
    expect(load({} as Parameters<typeof load>[0])).toEqual({});
  });
 });

+// Top-level imports — Svelte/Vite caches the module graph across tests,
+// and dynamic re-imports inside each test produced "stale-mock" symptoms
+// where the page picked up empty data despite `mockReturnValue` being set.
+// Importing at module load means the mocks attach to the same handles the
+// page binds against.
+import {
+  createAdminRequestsQuery,
+  createLidarrConfigQuery,
+  createAdminQuarantineQuery,
+  approveRequest,
+  rejectRequest,
+  resolveQuarantine,
+  deleteQuarantineFile,
+  deleteQuarantineViaLidarr
+} from '$lib/api/admin';
+import OverviewPage from './+page.svelte';
+
+function setupOverview(opts: {
+  requests?: unknown[];
+  quarantine?: unknown[];
+  lidarr?: { enabled: boolean };
+} = {}) {
+  (createAdminRequestsQuery as ReturnType<typeof vi.fn>).mockReturnValue(
+    mockQuery({ data: opts.requests ?? [] })
+  );
+  (createAdminQuarantineQuery as ReturnType<typeof vi.fn>).mockReturnValue(
+    mockQuery({ data: opts.quarantine ?? [] })
+  );
+  (createLidarrConfigQuery as ReturnType<typeof vi.fn>).mockReturnValue(
+    mockQuery({ data: opts.lidarr ?? { enabled: false } })
+  );
+}
+
 describe('admin Overview page', () => {
-  test('renders pending requests count and Lidarr status (connected)', async () => {
-    const { createAdminRequestsQuery, createLidarrConfigQuery } = await import('$lib/api/admin');
-    (createAdminRequestsQuery as ReturnType<typeof vi.fn>).mockReturnValue(
-      mockQuery({ data: [{ id: '1' }, { id: '2' }] })
-    );
-    (createLidarrConfigQuery as ReturnType<typeof vi.fn>).mockReturnValue(
-      mockQuery({ data: { enabled: true } })
-    );
-    const { default: OverviewPage } = await import('./+page.svelte');
+  test('renders top stat cards (requests, quarantine, lidarr)', async () => {
+    setupOverview({
+      requests: [{ id: '1' }, { id: '2' }],
+      quarantine: [{ track_id: 't1', report_count: 2, reports: [] }],
+      lidarr: { enabled: true }
+    });
    render(OverviewPage);
-    expect(screen.getByText('Pending requests')).toBeInTheDocument();
-    expect(screen.getByText('2')).toBeInTheDocument();
-    expect(screen.getByText('Lidarr')).toBeInTheDocument();
+    // "Pending requests" appears as both stat-card label and section
+    // header — assert via the link role to disambiguate to the stat card.
+    const reqLink = screen.getByRole('link', { name: /pending requests/i });
+    expect(reqLink).toHaveTextContent('2');
+    const quarLink = screen.getByRole('link', { name: /quarantined tracks/i });
+    expect(quarLink).toHaveTextContent('1');
    expect(screen.getByText(/connected/i)).toBeInTheDocument();
  });

-  test('shows zero pending and "Unset" when nothing is configured', async () => {
-    const { createAdminRequestsQuery, createLidarrConfigQuery } = await import('$lib/api/admin');
-    (createAdminRequestsQuery as ReturnType<typeof vi.fn>).mockReturnValue(
-      mockQuery({ data: [] })
-    );
-    (createLidarrConfigQuery as ReturnType<typeof vi.fn>).mockReturnValue(
-      mockQuery({ data: { enabled: false } })
-    );
-    const { default: OverviewPage } = await import('./+page.svelte');
+  test('shows empty-state copy when both queues are empty', async () => {
+    setupOverview({
+      requests: [],
+      quarantine: [],
+      lidarr: { enabled: false }
+    });
    render(OverviewPage);
-    expect(screen.getByText('0')).toBeInTheDocument();
+    expect(screen.getByText(/no pending requests/i)).toBeInTheDocument();
+    expect(screen.getByText(/no quarantined tracks/i)).toBeInTheDocument();
    expect(screen.getByText(/unset/i)).toBeInTheDocument();
  });

-  test('Overview cards link to their sub-pages', async () => {
-    const { createAdminRequestsQuery, createLidarrConfigQuery } = await import('$lib/api/admin');
-    (createAdminRequestsQuery as ReturnType<typeof vi.fn>).mockReturnValue(
-      mockQuery({ data: [] })
-    );
-    (createLidarrConfigQuery as ReturnType<typeof vi.fn>).mockReturnValue(
-      mockQuery({ data: { enabled: false } })
-    );
-    const { default: OverviewPage } = await import('./+page.svelte');
+  test('top stat cards link to their sub-pages', async () => {
+    setupOverview();
    render(OverviewPage);
-    const reqLink = screen.getByRole('link', { name: /pending requests/i });
-    expect(reqLink).toHaveAttribute('href', '/admin/requests');
-    const lidarrLink = screen.getByRole('link', { name: /lidarr/i });
-    expect(lidarrLink).toHaveAttribute('href', '/admin/integrations');
+    expect(screen.getByRole('link', { name: /pending requests/i }))
+      .toHaveAttribute('href', '/admin/requests');
+    expect(screen.getByRole('link', { name: /quarantined tracks/i }))
+      .toHaveAttribute('href', '/admin/quarantine');
+    expect(screen.getByRole('link', { name: /^lidarr/i }))
+      .toHaveAttribute('href', '/admin/integrations');
+  });
+
+  test('clicking Approve on a request row calls approveRequest', async () => {
+    setupOverview({
+      requests: [
+        { id: 'r1', kind: 'artist', artist_name: 'Boards of Canada', status: 'pending' }
+      ]
+    });
+    render(OverviewPage);
+    await fireEvent.click(screen.getByRole('button', { name: /approve boards of canada/i }));
+    expect(approveRequest).toHaveBeenCalledWith('r1');
+  });
+
+  test('clicking Reject on a request row calls rejectRequest', async () => {
+    setupOverview({
+      requests: [
+        { id: 'r1', kind: 'artist', artist_name: 'Boards of Canada', status: 'pending' }
+      ]
+    });
+    render(OverviewPage);
+    await fireEvent.click(screen.getByRole('button', { name: /reject boards of canada/i }));
+    expect(rejectRequest).toHaveBeenCalledWith('r1');
+  });
+
+  test('Resolve fires immediately on a quarantine row', async () => {
+    setupOverview({
+      quarantine: [
+        { track_id: 't1', track_title: 'Roygbiv', artist_name: 'BoC',
+          album_title: 'Geogaddi', report_count: 1, reports: [] }
+      ]
+    });
+    render(OverviewPage);
+    await fireEvent.click(screen.getByRole('button', { name: /resolve roygbiv/i }));
+    expect(resolveQuarantine).toHaveBeenCalledWith('t1');
+  });
+
+  test('Delete file requires two clicks (inline confirm)', async () => {
+    setupOverview({
+      quarantine: [
+        { track_id: 't1', track_title: 'Roygbiv', artist_name: 'BoC',
+          album_title: 'Geogaddi', report_count: 1, reports: [] }
+      ]
+    });
+    render(OverviewPage);
+    // First click: arms the confirm; no API call yet.
+    await fireEvent.click(screen.getByRole('button', { name: /^delete file roygbiv$/i }));
+    expect(deleteQuarantineFile).not.toHaveBeenCalled();
+    // Second click on the now-armed button fires the action.
+    await fireEvent.click(
+      screen.getByRole('button', { name: /confirm delete file roygbiv/i })
+    );
+    expect(deleteQuarantineFile).toHaveBeenCalledWith('t1');
+  });
+
+  test('Delete via Lidarr requires two clicks (inline confirm)', async () => {
+    setupOverview({
+      quarantine: [
+        { track_id: 't1', track_title: 'Roygbiv', artist_name: 'BoC',
+          album_title: 'Geogaddi', report_count: 1, reports: [] }
+      ]
+    });
+    render(OverviewPage);
+    await fireEvent.click(
+      screen.getByRole('button', { name: /^delete via lidarr roygbiv$/i })
+    );
+    expect(deleteQuarantineViaLidarr).not.toHaveBeenCalled();
+    await fireEvent.click(
+      screen.getByRole('button', { name: /confirm delete via lidarr roygbiv/i })
+    );
+    expect(deleteQuarantineViaLidarr).toHaveBeenCalledWith('t1');
+  });
+
+  test('"View all" link surfaces only when count exceeds the preview limit', async () => {
+    // 6 requests > limit of 5; quarantine empty.
+    const six = Array.from({ length: 6 }, (_, i) => ({
+      id: `r${i}`,
+      kind: 'artist' as const,
+      artist_name: `Artist ${i}`,
+      status: 'pending' as const
+    }));
+    setupOverview({ requests: six });
+    render(OverviewPage);
+    expect(screen.getByRole('link', { name: /view all 6/i })).toHaveAttribute(
+      'href', '/admin/requests'
+    );
  });
 });