From 7c89e5953675ecbb754b3042ac390ce3702559e6 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 11 Oct 2026 09:40:11 -0400 Subject: [PATCH] fix: admin bugs from the audit: a playback-error delete no longer reports failure, a failed Overview load no longer reads as all-clear, and email settings can't be saved over with defaults (M503 #5604) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Playback errors: Delete file stops resolving the error row the track deletion already took with it (ON DELETE CASCADE), which 404'd into "Delete failed". - Overview: a queue that fails to load says so with Try again, and its tile shows a dash, not 0. Delete via Lidarr shows only for an album Lidarr holds while Lidarr is connected. An approve refreshes the Collection. - Quarantine: Delete via Lidarr is in the row menu, so a phone reaches it; "1 report", not "1 reports". - Duplicates: the Keep badge follows the copy picked. - Integrations: provider save failures and load errors are said, not swallowed or shown as "No providers registered"; the Lidarr test catches a failed call; SMTP fills once and Save stays off until it has loaded. Lidarr's blurb says what it is still used for. - Requests: release profiles point at Collection › Following. - Library: an empty library sends an admin to Admin to run a scan, and tells anyone else the admin scans it, instead of opening Settings. Co-Authored-By: Claude Opus 5.5 --- web/src/routes/admin/+page.svelte | 29 +++++++++-- web/src/routes/admin/admin.test.ts | 42 ++++++++++++++- web/src/routes/admin/duplicates/+page.svelte | 12 +++-- .../admin/duplicates/duplicates.test.ts | 9 ++++ .../routes/admin/integrations/+page.svelte | 52 +++++++++++++++---- .../admin/integrations/integrations.test.ts | 37 ++++++++++++- .../routes/admin/playback-errors/+page.svelte | 19 ++++--- .../playback-errors/playback-errors.test.ts | 48 +++++++++++++++++ web/src/routes/admin/quarantine/+page.svelte | 44 ++++++---------- .../admin/quarantine/quarantine.test.ts | 24 +++++---- web/src/routes/admin/requests/+page.svelte | 2 +- web/src/routes/library/albums/+page.svelte | 37 ++++++++----- web/src/routes/library/albums/empty.test.ts | 46 ++++++++++++++++ web/src/routes/library/artists/+page.svelte | 37 ++++++++----- 14 files changed, 343 insertions(+), 95 deletions(-) create mode 100644 web/src/routes/admin/playback-errors/playback-errors.test.ts create mode 100644 web/src/routes/library/albums/empty.test.ts diff --git a/web/src/routes/admin/+page.svelte b/web/src/routes/admin/+page.svelte index 7f04dfbf..acec5dec 100644 --- a/web/src/routes/admin/+page.svelte +++ b/web/src/routes/admin/+page.svelte @@ -56,6 +56,12 @@ const quarantineCount = $derived(quarantine.data?.length ?? 0); const lidarrConnected = $derived(config.data?.enabled === true); + // A tile whose query failed shows a dash, not 0: a failed load must not + // read as an all-clear (#5604). + function tile(q: { isPending: boolean; isError: boolean }, n: number): string { + return q.isPending || q.isError ? '—' : String(n); + } + const requestPreview = $derived( (requests.data ?? []).slice(0, PREVIEW_LIMIT) ); @@ -110,6 +116,9 @@ // ---- Action handlers ---- async function invalidateRequests() { await Promise.all([ + // An approved request becomes a want, so the Collection's lists and + // counts change too (#5604). + client.invalidateQueries({ queryKey: ['adminWants'] }), client.invalidateQueries({ queryKey: qk.adminRequests('pending') }), client.invalidateQueries({ queryKey: qk.adminRequests('approved') }), client.invalidateQueries({ queryKey: qk.adminRequests('rejected') }), @@ -265,14 +274,14 @@ class="block rounded-xl border border-border bg-surface p-5 transition-colors hover:bg-surface-hover" >
Pending requests
-
{pendingCount}
+
{tile(requests, pendingCount)}
Quarantined tracks
-
{quarantineCount}
+
{tile(quarantine, quarantineCount)}
Lidarr
- {lidarrConnected ? 'Connected' : 'Unset'} + {config.isError ? '—' : lidarrConnected ? 'Connected' : 'Unset'}
@@ -296,6 +305,11 @@ {#if requests.isPending}

Loading…

+ {:else if requests.isError} +

+ Couldn't load the requests. + +

{:else if requestPreview.length === 0}

No pending requests.

{:else} @@ -529,6 +543,11 @@ {#if quarantine.isPending}

Loading…

+ {:else if quarantine.isError} +

+ Couldn't load the quarantine. + +

{:else if quarantinePreview.length === 0}

No quarantined tracks.

{:else} @@ -568,6 +587,9 @@ > {fileArmed ? 'Confirm?' : 'Delete file'} + + {#if lidarrConnected && row.lidarr_album_mbid} + {/if} { setupOverview({ quarantine: [ { track_id: 't1', track_title: 'Roygbiv', artist_name: 'BoC', - album_title: 'Geogaddi', report_count: 1, reports: [] } - ] + album_title: 'Geogaddi', lidarr_album_mbid: 'rg-1', report_count: 1, reports: [] } + ], + lidarr: { enabled: true } }); render(OverviewPage); await fireEvent.click( @@ -235,6 +236,43 @@ describe('admin Overview page', () => { expect(deleteQuarantineViaLidarr).toHaveBeenCalledWith('t1'); }); + test('Delete via Lidarr is offered only for an album Lidarr holds, while Lidarr is connected', async () => { + const local = { track_id: 't1', track_title: 'Dial Tone', artist_name: 'Annadale', + album_title: 'Dial Tone', lidarr_album_mbid: null, report_count: 1, reports: [] }; + const held = { track_id: 't2', track_title: 'Roygbiv', artist_name: 'BoC', + album_title: 'Geogaddi', lidarr_album_mbid: 'rg-1', report_count: 1, reports: [] }; + setupOverview({ quarantine: [local, held], lidarr: { enabled: false } }); + render(OverviewPage); + expect(screen.queryByRole('button', { name: /delete via lidarr/i })).toBeNull(); + cleanup(); + + setupOverview({ quarantine: [local, held], lidarr: { enabled: true } }); + render(OverviewPage); + expect(screen.queryByRole('button', { name: /delete via lidarr dial tone/i })).toBeNull(); + expect(screen.getByRole('button', { name: /delete via lidarr roygbiv/i })).toBeInTheDocument(); + }); + + test('a queue that fails to load says so, with Try again, and its tile shows a dash, not 0', async () => { + const refetch = vi.fn(); + (createAdminRequestsQuery as ReturnType).mockReturnValue( + mockQuery({ isError: true, error: new Error('boom'), refetch }) + ); + (createAdminQuarantineQuery as ReturnType).mockReturnValue( + mockQuery({ isError: true, error: new Error('boom') }) + ); + (createLidarrConfigQuery as ReturnType).mockReturnValue( + mockQuery({ data: { enabled: false } }) + ); + render(OverviewPage); + expect(screen.queryByText(/no pending requests/i)).toBeNull(); + expect(screen.queryByText(/no quarantined tracks/i)).toBeNull(); + expect(screen.getByTestId('tile-requests').textContent).toBe('—'); + expect(screen.getByTestId('tile-quarantine').textContent).toBe('—'); + expect(screen.getByTestId('requests-error').textContent).toMatch(/couldn't load the requests/i); + await fireEvent.click(screen.getAllByRole('button', { name: /try again/i })[0]); + expect(refetch).toHaveBeenCalled(); + }); + 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) => ({ diff --git a/web/src/routes/admin/duplicates/+page.svelte b/web/src/routes/admin/duplicates/+page.svelte index 35f39834..a9506f99 100644 --- a/web/src/routes/admin/duplicates/+page.svelte +++ b/web/src/routes/admin/duplicates/+page.svelte @@ -432,7 +432,8 @@