From 89a26539959b3b97b7dadf8bf73cce6c0b123199 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 11 Oct 2026 10:53:58 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20Admin=20=E2=80=BA=20Reports=20is=20one?= =?UTF-8?q?=20queue=20of=20broken=20tracks,=20whether=20a=20listener=20fla?= =?UTF-8?q?gged=20it=20or=20it=20failed=20to=20play,=20one=20row=20per=20t?= =?UTF-8?q?rack=20with=20one=20set=20of=20actions=20(M503=20#5606)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - /admin/reports folds the quarantine queue and the unresolved playback errors into one row per track, newest first: who flagged it and why, what failed and for whom, the file, with Resolve (clears the reports and marks the errors fixed), Play, Copy details, Delete file and, for an album Lidarr holds, Delete via Lidarr. - One delete path: Delete file removes the track, and its reports and errors go with the row. - A Handled tab shows what was done: reports cleared, files deleted, errors marked fixed or ignored. - The summary counts reports by track across both sources, so the rail, the Overview and the page agree. - /admin/quarantine and /admin/playback-errors redirect to Reports: the notifications and the Android app link to them. Co-Authored-By: Claude Opus 5.5 --- internal/api/admin_summary.go | 22 +- internal/db/dbq/admin_summary.sql.go | 43 +- internal/db/queries/admin_summary.sql | 15 +- web/src/lib/admin/queues.ts | 3 +- web/src/lib/admin/reports.test.ts | 63 +++ web/src/lib/admin/reports.ts | 131 ++++++ web/src/lib/api/types.ts | 4 +- web/src/routes/admin/+layout.svelte | 3 +- web/src/routes/admin/admin.test.ts | 8 +- .../routes/admin/playback-errors/+page.svelte | 298 -------------- web/src/routes/admin/playback-errors/+page.ts | 9 + .../playback-errors/playback-errors.test.ts | 48 --- web/src/routes/admin/quarantine/+page.svelte | 385 ------------------ web/src/routes/admin/quarantine/+page.ts | 9 + .../admin/quarantine/quarantine.test.ts | 182 --------- web/src/routes/admin/reports/+page.svelte | 374 +++++++++++++++++ web/src/routes/admin/reports/reports.test.ts | 128 ++++++ 17 files changed, 765 insertions(+), 960 deletions(-) create mode 100644 web/src/lib/admin/reports.test.ts create mode 100644 web/src/lib/admin/reports.ts delete mode 100644 web/src/routes/admin/playback-errors/+page.svelte create mode 100644 web/src/routes/admin/playback-errors/+page.ts delete mode 100644 web/src/routes/admin/playback-errors/playback-errors.test.ts delete mode 100644 web/src/routes/admin/quarantine/+page.svelte create mode 100644 web/src/routes/admin/quarantine/+page.ts delete mode 100644 web/src/routes/admin/quarantine/quarantine.test.ts create mode 100644 web/src/routes/admin/reports/+page.svelte create mode 100644 web/src/routes/admin/reports/reports.test.ts diff --git a/internal/api/admin_summary.go b/internal/api/admin_summary.go index a3d0f790..f04aac48 100644 --- a/internal/api/admin_summary.go +++ b/internal/api/admin_summary.go @@ -19,12 +19,11 @@ type adminQueueView struct { // (#5605): every queue an admin acts on, in one response, so each admin page // pays one cheap read for its rail. type adminSummaryView struct { - Requests adminQueueView `json:"requests"` - Quarantined adminQueueView `json:"quarantined"` - PlaybackErrors adminQueueView `json:"playback_errors"` - Duplicates adminQueueView `json:"duplicates"` - Suspect adminQueueView `json:"suspect"` - Missing adminQueueView `json:"missing"` + Requests adminQueueView `json:"requests"` + Reports adminQueueView `json:"reports"` + Duplicates adminQueueView `json:"duplicates"` + Suspect adminQueueView `json:"suspect"` + Missing adminQueueView `json:"missing"` } func queueView(count int64, since pgtype.Timestamptz) adminQueueView { @@ -38,12 +37,11 @@ func queueView(count int64, since pgtype.Timestamptz) adminQueueView { func adminSummaryFrom(row dbq.AdminSummaryRow) adminSummaryView { return adminSummaryView{ - Requests: queueView(row.Requests, row.RequestsSince), - Quarantined: queueView(row.Quarantined, row.QuarantinedSince), - PlaybackErrors: queueView(row.PlaybackErrors, row.PlaybackErrorsSince), - Duplicates: queueView(row.Duplicates, row.DuplicatesSince), - Suspect: queueView(row.Suspect, row.SuspectSince), - Missing: queueView(row.Missing, row.MissingSince), + Requests: queueView(row.Requests, row.RequestsSince), + Reports: queueView(row.Reports, row.ReportsSince), + Duplicates: queueView(row.Duplicates, row.DuplicatesSince), + Suspect: queueView(row.Suspect, row.SuspectSince), + Missing: queueView(row.Missing, row.MissingSince), } } diff --git a/internal/db/dbq/admin_summary.sql.go b/internal/db/dbq/admin_summary.sql.go index 69dc1722..c8c0ed27 100644 --- a/internal/db/dbq/admin_summary.sql.go +++ b/internal/db/dbq/admin_summary.sql.go @@ -15,10 +15,17 @@ const adminSummary = `-- name: AdminSummary :one SELECT (SELECT count(*) FROM requests WHERE status = 'pending')::bigint AS requests, (SELECT min(requested_at) FROM requests WHERE status = 'pending')::timestamptz AS requests_since, - (SELECT count(DISTINCT track_id) FROM lidarr_quarantine)::bigint AS quarantined, - (SELECT min(created_at) FROM lidarr_quarantine)::timestamptz AS quarantined_since, - (SELECT count(*) FROM playback_errors WHERE resolved_at IS NULL)::bigint AS playback_errors, - (SELECT min(occurred_at) FROM playback_errors WHERE resolved_at IS NULL)::timestamptz AS playback_errors_since, + -- A broken track is one report however it was found: a listener flagged + -- it, or it failed to play (#5606). Counted by track, as the Reports + -- page lists them. + (SELECT count(*) FROM ( + SELECT track_id FROM lidarr_quarantine + UNION + SELECT track_id FROM playback_errors WHERE resolved_at IS NULL) r)::bigint AS reports, + (SELECT min(at) FROM ( + SELECT created_at AS at FROM lidarr_quarantine + UNION ALL + SELECT occurred_at FROM playback_errors WHERE resolved_at IS NULL) r)::timestamptz AS reports_since, (SELECT count(*) FROM duplicate_groups g WHERE g.status = 'pending' AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2)::bigint AS duplicates, @@ -32,18 +39,16 @@ SELECT ` type AdminSummaryRow struct { - Requests int64 - RequestsSince pgtype.Timestamptz - Quarantined int64 - QuarantinedSince pgtype.Timestamptz - PlaybackErrors int64 - PlaybackErrorsSince pgtype.Timestamptz - Duplicates int64 - DuplicatesSince pgtype.Timestamptz - Suspect int64 - SuspectSince pgtype.Timestamptz - Missing int64 - MissingSince pgtype.Timestamptz + Requests int64 + RequestsSince pgtype.Timestamptz + Reports int64 + ReportsSince pgtype.Timestamptz + Duplicates int64 + DuplicatesSince pgtype.Timestamptz + Suspect int64 + SuspectSince pgtype.Timestamptz + Missing int64 + MissingSince pgtype.Timestamptz } // What the admin rail and the Overview's verdict read (#5605): each queue @@ -57,10 +62,8 @@ func (q *Queries) AdminSummary(ctx context.Context) (AdminSummaryRow, error) { err := row.Scan( &i.Requests, &i.RequestsSince, - &i.Quarantined, - &i.QuarantinedSince, - &i.PlaybackErrors, - &i.PlaybackErrorsSince, + &i.Reports, + &i.ReportsSince, &i.Duplicates, &i.DuplicatesSince, &i.Suspect, diff --git a/internal/db/queries/admin_summary.sql b/internal/db/queries/admin_summary.sql index 0998ee3e..14ff1f24 100644 --- a/internal/db/queries/admin_summary.sql +++ b/internal/db/queries/admin_summary.sql @@ -7,10 +7,17 @@ SELECT (SELECT count(*) FROM requests WHERE status = 'pending')::bigint AS requests, (SELECT min(requested_at) FROM requests WHERE status = 'pending')::timestamptz AS requests_since, - (SELECT count(DISTINCT track_id) FROM lidarr_quarantine)::bigint AS quarantined, - (SELECT min(created_at) FROM lidarr_quarantine)::timestamptz AS quarantined_since, - (SELECT count(*) FROM playback_errors WHERE resolved_at IS NULL)::bigint AS playback_errors, - (SELECT min(occurred_at) FROM playback_errors WHERE resolved_at IS NULL)::timestamptz AS playback_errors_since, + -- A broken track is one report however it was found: a listener flagged + -- it, or it failed to play (#5606). Counted by track, as the Reports + -- page lists them. + (SELECT count(*) FROM ( + SELECT track_id FROM lidarr_quarantine + UNION + SELECT track_id FROM playback_errors WHERE resolved_at IS NULL) r)::bigint AS reports, + (SELECT min(at) FROM ( + SELECT created_at AS at FROM lidarr_quarantine + UNION ALL + SELECT occurred_at FROM playback_errors WHERE resolved_at IS NULL) r)::timestamptz AS reports_since, (SELECT count(*) FROM duplicate_groups g WHERE g.status = 'pending' AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2)::bigint AS duplicates, diff --git a/web/src/lib/admin/queues.ts b/web/src/lib/admin/queues.ts index 408d9b4b..c84cd9cd 100644 --- a/web/src/lib/admin/queues.ts +++ b/web/src/lib/admin/queues.ts @@ -17,8 +17,7 @@ export type QueueDef = { export const QUEUES: QueueDef[] = [ { key: 'requests', href: '/admin/requests', one: 'request to decide', many: 'requests to decide', none: 'requests' }, - { key: 'quarantined', href: '/admin/quarantine', one: 'reported track', many: 'reported tracks', none: 'reports' }, - { key: 'playback_errors', href: '/admin/playback-errors', one: 'playback error', many: 'playback errors', none: 'playback errors' }, + { key: 'reports', href: '/admin/reports', one: 'broken track reported', many: 'broken tracks reported', none: 'reports' }, { key: 'duplicates', href: '/admin/duplicates', one: 'set of duplicates to review', many: 'sets of duplicates to review', none: 'duplicates' }, { key: 'suspect', href: '/admin/suspect-sources', one: 'track from a suspect source', many: 'tracks from suspect sources', none: 'suspect sources' }, { key: 'missing', href: '/admin/missing-files', one: 'missing file', many: 'missing files', none: 'missing files' } diff --git a/web/src/lib/admin/reports.test.ts b/web/src/lib/admin/reports.test.ts new file mode 100644 index 00000000..8c3a75d8 --- /dev/null +++ b/web/src/lib/admin/reports.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, test } from 'vitest'; +import { errorCounts, mergeHandled, mergeReports } from './reports'; +import type { AdminPlaybackError, AdminQuarantineRow, LidarrQuarantineActionRow } from '#lib/api/types.js'; + +const flagged = (track: string, at: string): AdminQuarantineRow => ({ + track_id: track, + track_title: `T${track}`, + artist_name: 'A', + album_title: 'Al', + album_id: 'al', + lidarr_album_mbid: 'rg', + report_count: 1, + latest_at: at, + reason_counts: { bad_rip: 1 }, + reports: [{ user_id: 'u', username: 'alice', reason: 'bad_rip', created_at: at }] +}); + +const failed = (id: string, track: string, at: string, kind: AdminPlaybackError['kind'] = 'load_failed'): AdminPlaybackError => ({ + id, + track_id: track, + user_id: 'u', + username: 'bob', + client_id: 'android', + kind, + occurred_at: at, + track_title: `T${track}`, + track_file_path: `/music/${track}.flac`, + artist_name: 'A', + album_title: 'Al', + album_id: 'al' +}); + +describe('mergeReports', () => { + test('a track both flagged and failing to play is one row carrying both, newest first', () => { + const rows = mergeReports( + [flagged('1', '2026-10-01T00:00:00Z'), flagged('2', '2026-10-05T00:00:00Z')], + [failed('e1', '1', '2026-10-09T00:00:00Z'), failed('e2', '3', '2026-10-03T00:00:00Z', 'stalled')] + ); + expect(rows.map((r) => r.track_id)).toEqual(['1', '2', '3']); + expect(rows[0].reports).toHaveLength(1); + expect(rows[0].errors.map((e) => e.id)).toEqual(['e1']); + expect(rows[0].latest).toBe('2026-10-09T00:00:00Z'); + expect(rows[0].lidarr_album_mbid).toBe('rg'); + expect(rows[2].reports).toHaveLength(0); + expect(rows[2].file_path).toBe('/music/3.flac'); + }); + + test('errors are counted by kind', () => { + const [row] = mergeReports([], [failed('a', '1', '2026-10-01T00:00:00Z'), failed('b', '1', '2026-10-02T00:00:00Z'), failed('c', '1', '2026-10-03T00:00:00Z', 'stalled')]); + expect(errorCounts(row)).toEqual([['load_failed', 2], ['stalled', 1]]); + }); +}); + +describe('mergeHandled', () => { + test('cleared reports, deletions and resolved errors read as one history, newest first', () => { + const action: LidarrQuarantineActionRow = { + id: 'x', track_id: '1', track_title: 'T1', artist_name: 'A', action: 'deleted_file', + affected_users: 1, created_at: '2026-10-02T00:00:00Z' + }; + const fixed = { ...failed('e', '2', '2026-10-01T00:00:00Z'), resolved_at: '2026-10-04T00:00:00Z', resolution: 'fixed' as const }; + expect(mergeHandled([action], [fixed]).map((r) => r.what)).toEqual(['Playback error marked fixed', 'File deleted']); + }); +}); diff --git a/web/src/lib/admin/reports.ts b/web/src/lib/admin/reports.ts new file mode 100644 index 00000000..7b29aa95 --- /dev/null +++ b/web/src/lib/admin/reports.ts @@ -0,0 +1,131 @@ +import type { + AdminPlaybackError, + AdminQuarantineReport, + AdminQuarantineRow, + LidarrQuarantineActionRow, + LidarrQuarantineReason, + PlaybackErrorKind +} from '#lib/api/types.js'; + +// Admin › Reports (#5606): a broken track is one thing to decide about, +// whether a listener flagged it or it failed to play, so the two sources +// fold into one row per track. The rail's count (the summary's `reports`) +// counts the same way, by track. + +export type ReportRow = { + track_id: string; + title: string; + artist: string; + album: string; + album_id: string; + file_path: string | null; + lidarr_album_mbid: string | null; + reports: AdminQuarantineReport[]; + reason_counts: Partial>; + errors: AdminPlaybackError[]; + latest: string; +}; + +export const REASON_LABELS: Record = { + bad_rip: 'Bad rip', + wrong_file: 'Wrong file', + wrong_tags: 'Wrong tags', + duplicate: 'Duplicate', + other: 'Other' +}; + +export const ERROR_LABELS: Record = { + zero_duration: 'Zero duration', + load_failed: "Wouldn't load", + stalled: 'Stalled' +}; + +const later = (a: string, b: string) => (new Date(a) > new Date(b) ? a : b); + +export function mergeReports(flagged: AdminQuarantineRow[], errors: AdminPlaybackError[]): ReportRow[] { + const byTrack = new Map(); + for (const q of flagged) { + byTrack.set(q.track_id, { + track_id: q.track_id, + title: q.track_title, + artist: q.artist_name, + album: q.album_title, + album_id: q.album_id, + file_path: null, + lidarr_album_mbid: q.lidarr_album_mbid ?? null, + reports: q.reports, + reason_counts: q.reason_counts, + errors: [], + latest: q.latest_at + }); + } + for (const e of errors) { + let row = byTrack.get(e.track_id); + if (!row) { + row = { + track_id: e.track_id, + title: e.track_title, + artist: e.artist_name, + album: e.album_title, + album_id: e.album_id, + file_path: null, + lidarr_album_mbid: null, + reports: [], + reason_counts: {}, + errors: [], + latest: e.occurred_at + }; + byTrack.set(e.track_id, row); + } + row.errors.push(e); + row.file_path = e.track_file_path; + row.latest = later(row.latest, e.occurred_at); + } + return [...byTrack.values()].sort((a, b) => new Date(b.latest).getTime() - new Date(a.latest).getTime()); +} + +// How many of each playback-error kind, for the row's pills. +export function errorCounts(row: ReportRow): [PlaybackErrorKind, number][] { + const counts = new Map(); + for (const e of row.errors) counts.set(e.kind, (counts.get(e.kind) ?? 0) + 1); + return [...counts.entries()]; +} + +// The Handled view: what was done about past reports, newest first. +export type HandledRow = { key: string; title: string; artist: string; what: string; at: string }; + +const ACTION_LABELS: Record = { + resolved: 'Reports cleared', + deleted_file: 'File deleted', + deleted_via_lidarr: 'Deleted through Lidarr' +}; + +const RESOLUTION_LABELS: Record = { + fixed: 'Playback error marked fixed', + ignored: 'Playback error ignored', + hidden: 'Hidden after a playback error', + deleted: 'File deleted after a playback error', + requested: 'Re-requested after a playback error' +}; + +export function mergeHandled(actions: LidarrQuarantineActionRow[], resolved: AdminPlaybackError[]): HandledRow[] { + const rows: HandledRow[] = [ + ...actions.map((a) => ({ + key: `a:${a.id}`, + title: a.track_title, + artist: a.artist_name, + what: ACTION_LABELS[a.action] ?? a.action, + at: a.created_at + })), + ...resolved + .filter((e) => e.resolved_at) + .map((e) => ({ + key: `e:${e.id}`, + title: e.track_title, + artist: e.artist_name, + what: RESOLUTION_LABELS[e.resolution ?? ''] ?? 'Playback error resolved', + at: e.resolved_at as string + })) + ]; + return rows.sort((a, b) => new Date(b.at).getTime() - new Date(a.at).getTime()); +} diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 5f501626..2fac4486 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -906,8 +906,8 @@ export type ReleaseProfilesResponse = { export type AdminQueue = { count: number; since: string | null }; export type AdminSummary = { requests: AdminQueue; - quarantined: AdminQueue; - playback_errors: AdminQueue; + // Broken tracks: flagged by a listener or failed to play, counted by track. + reports: AdminQueue; duplicates: AdminQueue; suspect: AdminQueue; missing: AdminQueue; diff --git a/web/src/routes/admin/+layout.svelte b/web/src/routes/admin/+layout.svelte index 97ba0578..4ee0d968 100644 --- a/web/src/routes/admin/+layout.svelte +++ b/web/src/routes/admin/+layout.svelte @@ -28,8 +28,7 @@ const items: RailItem[] = $derived([ { href: '/admin', label: 'Overview', exact: true }, { href: '/admin/requests', label: 'Requests', attention: countOf(summary, 'requests') }, - { href: '/admin/quarantine', label: 'Quarantine', attention: countOf(summary, 'quarantined') }, - { href: '/admin/playback-errors', label: 'Playback errors', attention: countOf(summary, 'playback_errors') }, + { href: '/admin/reports', label: 'Reports', attention: countOf(summary, 'reports') }, { href: '/admin/duplicates', label: 'Duplicates', attention: countOf(summary, 'duplicates') }, { href: '/admin/suspect-sources', label: 'Suspect sources', attention: countOf(summary, 'suspect') }, { href: '/admin/missing-files', label: 'Missing files', attention: countOf(summary, 'missing') }, diff --git a/web/src/routes/admin/admin.test.ts b/web/src/routes/admin/admin.test.ts index 189b21d8..ece41c08 100644 --- a/web/src/routes/admin/admin.test.ts +++ b/web/src/routes/admin/admin.test.ts @@ -105,8 +105,7 @@ const empty = { count: 0, since: null }; function summary(over: Partial = {}): AdminSummary { return { requests: empty, - quarantined: empty, - playback_errors: empty, + reports: empty, duplicates: empty, suspect: empty, missing: empty, @@ -128,7 +127,7 @@ describe('admin Overview page', () => { render(OverviewPage); expect(screen.getByTestId('verdict').textContent).toMatch(/nothing needs you/i); expect(screen.queryAllByTestId('needs-you')).toHaveLength(0); - expect(screen.getByTestId('all-clear').textContent).toMatch(/no requests, reports, playback errors, duplicates, suspect sources, missing files/i); + expect(screen.getByTestId('all-clear').textContent).toMatch(/no requests, reports, duplicates, suspect sources, missing files/i); }); test('each queue that waits on you is listed with how long its oldest has waited, and opens its page', () => { @@ -213,8 +212,7 @@ describe('admin layout', () => { expect(links).toEqual([ ['/admin', 'Overview'], ['/admin/requests', 'Requests 2'], - ['/admin/quarantine', 'Quarantine'], - ['/admin/playback-errors', 'Playback errors'], + ['/admin/reports', 'Reports'], ['/admin/duplicates', 'Duplicates 5'], ['/admin/suspect-sources', 'Suspect sources'], ['/admin/missing-files', 'Missing files'], diff --git a/web/src/routes/admin/playback-errors/+page.svelte b/web/src/routes/admin/playback-errors/+page.svelte deleted file mode 100644 index 3c659e06..00000000 --- a/web/src/routes/admin/playback-errors/+page.svelte +++ /dev/null @@ -1,298 +0,0 @@ - - -{pageTitle('Admin · Playback errors')} - -
-
-
-

Playback errors

-

- Tracks that failed to play on a client. Reported automatically by - the Android player when a track loads with zero duration or - decode-fails — the player skips fast and logs here for triage. -

-
- {#if !query.isPending && !query.isError && !resolved} - - {rows.length} unresolved - - {/if} -
- - - (resolved ? 'resolved' : 'unresolved'), (v) => (resolved = v === 'resolved')} - items={[ - { id: 'unresolved', label: 'Unresolved' }, - { id: 'resolved', label: 'Resolved' } - ]} - /> - - {#if query.isError} -

Couldn't load: {errMessage(query.error)}

- {:else if query.isPending} -

Loading…

- {:else if rows.length === 0} -

- {resolved ? 'No resolved errors yet.' : 'No unresolved errors.'} -

- {:else} -
    - {#each rows as r (r.id)} -
  • -
    -
    - - {r.track_title} - - - {r.artist_name} · {r.album_title} - - - {kindLabel(r.kind)} - - {#if resolved && r.resolution} - - {resolutionLabel(r.resolution)} - - {/if} -
    -
    - by {r.username} · {relativeTime(r.occurred_at)} - {#if r.detail} · {r.detail}{/if} -
    -
    - {r.track_file_path} -
    -
    - {#if !resolved} - {@const a = actionsFor(r)} - - {/if} -
    • - {/each} -
- {/if} -
- - { deleteFileOpen = null; }} -> - {#if deleteFileOpen} -

- This removes {deleteFileOpen.track_title} - from the library and deletes the underlying file. Its error reports - go with it. -

-

{deleteFileOpen.track_file_path}

-
- - -
- {/if} - - - { resolveOpen = null; }} -> - {#if resolveOpen} -

- Marking {resolveOpen.track_title} resolved. - Pick how you handled it: -

- -
- - -
- {/if} - diff --git a/web/src/routes/admin/playback-errors/+page.ts b/web/src/routes/admin/playback-errors/+page.ts new file mode 100644 index 00000000..fe1692bc --- /dev/null +++ b/web/src/routes/admin/playback-errors/+page.ts @@ -0,0 +1,9 @@ +import { redirect } from '@sveltejs/kit'; + +// Quarantine and Playback errors are one page now, Admin › Reports (#5606). +// The old address stays because notifications and the Android app link to +// it, and a link already sent should still land somewhere useful. A +// universal load, as the app is adapter-static with ssr off. +export const load = () => { + throw redirect(308, '/admin/reports'); +}; diff --git a/web/src/routes/admin/playback-errors/playback-errors.test.ts b/web/src/routes/admin/playback-errors/playback-errors.test.ts deleted file mode 100644 index ae4f5105..00000000 --- a/web/src/routes/admin/playback-errors/playback-errors.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { fireEvent, render, screen, waitFor, within } from '@testing-library/svelte'; -import { mockQuery } from '../../../test-utils/query'; -import type { AdminPlaybackError } from '#lib/api/types.js'; - -vi.mock('#lib/api/admin.js', () => ({ - createAdminPlaybackErrorsQuery: vi.fn(), - resolvePlaybackError: vi.fn(), - deleteQuarantineFile: vi.fn() -})); - -import Page from './+page.svelte'; -import { createAdminPlaybackErrorsQuery, deleteQuarantineFile, resolvePlaybackError } from '#lib/api/admin.js'; - -const row = { - id: 'pe-1', - track_id: 't-1', - track_title: 'Roygbiv', - artist_name: 'Boards of Canada', - album_title: 'Geogaddi', - track_file_path: '/music/BoC/Geogaddi/05 Roygbiv.flac', - kind: 'load_failed', - detail: null, - username: 'alice', - client_id: 'android-1', - occurred_at: new Date().toISOString(), - resolution: null -} as unknown as AdminPlaybackError; - -afterEach(() => vi.clearAllMocks()); - -describe('/admin/playback-errors', () => { - // Deleting the track takes its error rows with it (ON DELETE CASCADE), so - // the page must not then try to resolve the row: that 404'd and said - // "Delete failed" for a file already gone (#5604). - test('Delete file deletes the track and says so, without resolving the row it took with it', async () => { - vi.mocked(createAdminPlaybackErrorsQuery).mockReturnValue( - mockQuery({ data: [row] }) as unknown as ReturnType - ); - vi.mocked(deleteQuarantineFile).mockResolvedValue({} as never); - render(Page); - await fireEvent.click(screen.getByRole('button', { name: /delete file for roygbiv/i })); - await fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: /^delete file$/i })); - await waitFor(() => expect(deleteQuarantineFile).toHaveBeenCalledWith('t-1')); - expect(resolvePlaybackError).not.toHaveBeenCalled(); - await waitFor(() => expect(screen.getByTestId('toast').textContent).toMatch(/deleted "roygbiv"/i)); - }); -}); diff --git a/web/src/routes/admin/quarantine/+page.svelte b/web/src/routes/admin/quarantine/+page.svelte deleted file mode 100644 index 9c544a07..00000000 --- a/web/src/routes/admin/quarantine/+page.svelte +++ /dev/null @@ -1,385 +0,0 @@ - - -{pageTitle('Admin · Quarantine')} - -
-
-
-

Quarantine

- {#if totalReports > 0} - - {totalReports} - - {/if} -
-

User-reported broken tracks waiting for triage.

-
- - {#if query.isPending} -

Reading the queue…

- {:else if query.isError} -

Couldn't load the quarantine queue.

- {:else if rows.length === 0} -

Nothing to triage right now.

- {:else} -
    - {#each rows as r (r.track_id)} - {@const isExpanded = !!expanded[r.track_id]} - {@const primary: RowAction = { - icon: RotateCcw, - label: 'Resolve', - ariaLabel: `Resolve ${r.track_title}`, - onclick: () => onResolve(r) - }} - {@const secondary: RowAction[] = [ - { - icon: Play, - label: 'Play', - ariaLabel: `Play ${r.track_title}`, - onclick: () => onPlay(r) - }, - { - icon: Trash2, - label: 'Delete file', - ariaLabel: `Delete file for ${r.track_title}`, - onclick: () => openDeleteFile(r), - danger: true - }, - // In the row's actions, so a phone reaches it from the menu too - // (#5604). A local-only track has no Lidarr album to remove, so - // it isn't offered. - ...(r.lidarr_album_mbid - ? [{ - icon: Cloud, - label: 'Delete via Lidarr', - ariaLabel: `Delete via Lidarr for ${r.track_title}`, - onclick: () => openDeleteLidarr(r), - danger: true - }] - : []) - ]} -
  • - - - -
    -
    - {r.track_title} -
    -
    - {r.artist_name} · {r.album_title} · {r.report_count} {r.report_count === 1 ? 'report' : 'reports'} — latest {relativeTime(r.latest_at)} -
    - - -
    - {#each Object.entries(r.reason_counts) as [reason, count] (reason)} - - {count}× {REASON_LABELS[reason as LidarrQuarantineReason] ?? reason} - - {/each} -
    - - - - - {#if isExpanded} -
      - {#each r.reports as rep, idx (rep.user_id + ':' + rep.created_at + ':' + idx)} -
    • - {rep.username} - · {REASON_LABELS[rep.reason] ?? rep.reason} - {#if rep.notes} - · {rep.notes} - {/if} - · {relativeTime(rep.created_at)} -
      • - {/each} -
    - {/if} -
    - - -
    - - -
    -
    • - {/each} -
- {/if} -
- - - {#if deleteFileRow} -

- Remove {deleteFileRow.track_title} - from disk and clear {deleteFileRow.report_count} {deleteFileRow.report_count === 1 ? 'report' : 'reports'}? Lidarr may auto-redownload depending on its monitor settings. -

-
- - -
- {/if} - - - - {#if deleteLidarrRow} -

- This will tell Lidarr to remove {deleteLidarrRow.album_title} - (artist {deleteLidarrRow.artist_name}) - and add it to the import-list exclusion. Affects all tracks on the album. Type DELETE to confirm. -

- - {#if deleteLidarrError} -

- {deleteLidarrError} -

- {/if} -
- - -
- {/if} - - - diff --git a/web/src/routes/admin/quarantine/+page.ts b/web/src/routes/admin/quarantine/+page.ts new file mode 100644 index 00000000..fe1692bc --- /dev/null +++ b/web/src/routes/admin/quarantine/+page.ts @@ -0,0 +1,9 @@ +import { redirect } from '@sveltejs/kit'; + +// Quarantine and Playback errors are one page now, Admin › Reports (#5606). +// The old address stays because notifications and the Android app link to +// it, and a link already sent should still land somewhere useful. A +// universal load, as the app is adapter-static with ssr off. +export const load = () => { + throw redirect(308, '/admin/reports'); +}; diff --git a/web/src/routes/admin/quarantine/quarantine.test.ts b/web/src/routes/admin/quarantine/quarantine.test.ts deleted file mode 100644 index fdf6534e..00000000 --- a/web/src/routes/admin/quarantine/quarantine.test.ts +++ /dev/null @@ -1,182 +0,0 @@ -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent, waitFor, within } from '@testing-library/svelte'; -import { mockQuery } from '../../../test-utils/query'; -import type { AdminQuarantineRow } from '#lib/api/types.js'; - -// Wrap useQueryClient so the page can call invalidateQueries() without a real -// QueryClient context. Everything else from svelte-query passes through. -vi.mock('#lib/api/admin.js', () => ({ - createAdminQuarantineQuery: vi.fn(), - resolveQuarantine: vi.fn(), - deleteQuarantineFile: vi.fn(), - deleteQuarantineViaLidarr: vi.fn() -})); - -vi.mock('#lib/player/store.svelte.js', () => ({ - playRadio: vi.fn() -})); - -import AdminQuarantinePage from './+page.svelte'; -import { - createAdminQuarantineQuery, - resolveQuarantine, - deleteQuarantineFile, - deleteQuarantineViaLidarr -} from '#lib/api/admin.js'; -import { playRadio } from '#lib/player/store.svelte.js'; - -const baseRow: AdminQuarantineRow = { - track_id: 't-001', - track_title: 'Roygbiv', - artist_name: 'Boards of Canada', - album_title: 'Geogaddi', - album_id: 'al-001', - lidarr_album_mbid: 'al-mbid-1', - report_count: 3, - latest_at: new Date(Date.now() - 2 * 3_600_000).toISOString(), - reason_counts: { bad_rip: 2, wrong_tags: 1 }, - reports: [ - { - user_id: 'u1', - username: 'alice', - reason: 'bad_rip', - notes: 'crackles at 1:24', - created_at: new Date(Date.now() - 2 * 3_600_000).toISOString() - }, - { - user_id: 'u2', - username: 'bob', - reason: 'bad_rip', - notes: null, - created_at: new Date(Date.now() - 5 * 3_600_000).toISOString() - }, - { - user_id: 'u3', - username: 'carol', - reason: 'wrong_tags', - notes: 'wrong year', - created_at: new Date(Date.now() - 7 * 3_600_000).toISOString() - } - ] -}; - -afterEach(() => vi.clearAllMocks()); - -function setup(rows: AdminQuarantineRow[] = [baseRow]) { - (createAdminQuarantineQuery as ReturnType).mockReturnValue( - mockQuery({ data: rows }) - ); - return render(AdminQuarantinePage); -} - -describe('/admin/quarantine', () => { - test('Empty state shows the spec copy', () => { - setup([]); - expect(screen.getByText('Nothing to triage right now.')).toBeInTheDocument(); - }); - - test('Aggregated row renders with reason distribution + report count', () => { - setup(); - expect(screen.getByText('Roygbiv')).toBeInTheDocument(); - expect( - screen.getByText(/Boards of Canada · Geogaddi · 3 reports — latest/i) - ).toBeInTheDocument(); - const pills = screen.getAllByTestId('reason-pill').map((el) => el.textContent?.trim()); - expect(pills).toContain('2× Bad rip'); - expect(pills).toContain('1× Wrong tags'); - expect(screen.getByTestId('report-count-pill')).toHaveTextContent('3'); - }); - - test('Resolve fires resolveQuarantine and invalidates the query', async () => { - setup(); - (resolveQuarantine as ReturnType).mockResolvedValueOnce({ - action_id: 'a1', - affected_users: 3 - }); - await fireEvent.click(screen.getByRole('button', { name: /resolve roygbiv/i })); - expect(resolveQuarantine).toHaveBeenCalledWith('t-001'); - }); - - test('Delete file → modal-confirm → fires deleteQuarantineFile', async () => { - setup(); - (deleteQuarantineFile as ReturnType).mockResolvedValueOnce({ - action_id: 'a2', - affected_users: 3 - }); - await fireEvent.click( - screen.getByRole('button', { name: /delete file for roygbiv/i }) - ); - const dialog = await screen.findByRole('dialog'); - expect(dialog).toHaveTextContent(/Remove/i); - expect(dialog).toHaveTextContent(/clear 3 reports/i); - await fireEvent.click( - screen.getByRole('button', { name: /confirm delete file/i }) - ); - expect(deleteQuarantineFile).toHaveBeenCalledWith('t-001'); - }); - - test('Delete via Lidarr → typed-confirm "DELETE" → fires deleteQuarantineViaLidarr', async () => { - setup(); - (deleteQuarantineViaLidarr as ReturnType).mockResolvedValueOnce({ - action_id: 'a3', - affected_users: 3, - deleted_track_count: 12 - }); - await fireEvent.click( - screen.getByRole('button', { name: /delete via lidarr for roygbiv/i }) - ); - await screen.findByRole('dialog'); - const confirmBtn = screen.getByRole('button', { - name: /confirm delete via lidarr/i - }) as HTMLButtonElement; - expect(confirmBtn).toBeDisabled(); - const input = screen.getByLabelText(/type delete to confirm/i) as HTMLInputElement; - await fireEvent.input(input, { target: { value: 'DELETE' } }); - expect(confirmBtn).not.toBeDisabled(); - await fireEvent.click(confirmBtn); - expect(deleteQuarantineViaLidarr).toHaveBeenCalledWith('t-001'); - }); - - test('Lidarr-unreachable error shows toast with spec copy', async () => { - setup(); - (resolveQuarantine as ReturnType).mockRejectedValueOnce({ - code: 'lidarr_unreachable', - message: 'unreachable', - status: 503 - }); - await fireEvent.click(screen.getByRole('button', { name: /resolve roygbiv/i })); - await waitFor(() => - expect( - screen.getByText( - 'Lidarr is unreachable right now. Try again, or check Admin → Integrations.' - ) - ).toBeInTheDocument() - ); - }); - - test('a local-only track (no Lidarr album) offers no Delete via Lidarr', () => { - setup([{ ...baseRow, lidarr_album_mbid: null }]); - expect( - screen.queryByRole('button', { name: /delete via lidarr for roygbiv/i }) - ).not.toBeInTheDocument(); - }); - - test('on a phone, Delete via Lidarr is in the row menu', async () => { - setup(); - await fireEvent.click(screen.getByRole('button', { name: /more actions/i })); - expect(within(screen.getByRole('menu')).getByText('Delete via Lidarr')).toBeInTheDocument(); - }); - - test('one report reads "1 report"', () => { - setup([{ ...baseRow, report_count: 1, reports: [baseRow.reports[0]] }]); - const row = screen.getByTestId('admin-quarantine-row'); - expect(row.textContent).toMatch(/· 1 report — latest/); - expect(row.textContent).toMatch(/Show 1 report\b/); - }); - - test('Inline play button calls playRadio with the track id', async () => { - setup(); - await fireEvent.click(screen.getByRole('button', { name: /play roygbiv/i })); - expect(playRadio).toHaveBeenCalledWith('t-001'); - }); -}); diff --git a/web/src/routes/admin/reports/+page.svelte b/web/src/routes/admin/reports/+page.svelte new file mode 100644 index 00000000..276067b0 --- /dev/null +++ b/web/src/routes/admin/reports/+page.svelte @@ -0,0 +1,374 @@ + + +{pageTitle('Admin · Reports')} + +
+
+

Reports

+

Tracks a listener flagged, or that failed to play.

+
+ + + + {#if view === 'open'} + {#if loading} +

Reading the reports…

+ {:else if failed} +

+ Couldn't load the reports. + +

+ {:else if rows.length === 0} +

No reports. Nothing is waiting on you here.

+ {:else} +

+ {plural(rows.length, 'track needs', 'tracks need')} a look. +

+
    + {#each rows as r (r.track_id)} + {@const a = actionsFor(r)} + {@const open = !!expanded[r.track_id]} +
  • + +
    +
    {r.title}
    +
    + {r.artist} · {r.album} · latest {relativeTime(r.latest)} +
    +
    + {#each Object.entries(r.reason_counts) as [reason, n] (reason)} + {n}× {REASON_LABELS[reason as LidarrQuarantineReason] ?? reason} + {/each} + {#each errorCounts(r) as [kind, n] (kind)} + {n}× {ERROR_LABELS[kind] ?? kind} + {/each} +
    + + {#if open} +
      + {#each r.reports as rep, i (rep.user_id + i)} +
    • + {rep.username} flagged it · + {REASON_LABELS[rep.reason] ?? rep.reason}{#if rep.notes} · {rep.notes}{/if} + · {relativeTime(rep.created_at)} +
      • + {/each} + {#each r.errors as e (e.id)} +
    • + Failed to play for {e.username} · + {ERROR_LABELS[e.kind] ?? e.kind}{#if e.detail} · {e.detail}{/if} · {relativeTime(e.occurred_at)} +
      • + {/each} + {#if r.file_path} +
    • {r.file_path}
      • + {/if} +
    + {/if} +
    + +
    • + {/each} +
+ {/if} + {:else if actions?.isPending || resolved?.isPending} +

Reading what was done…

+ {:else if actions?.isError || resolved?.isError} +

+ Couldn't load what was done. + +

+ {:else if handled.length === 0} +

Nothing has been handled yet.

+ {:else} +
    + {#each handled as h (h.key)} +
  • + {h.title} + {h.artist} + {h.what} · {relativeTime(h.at)} +
    • + {/each} +
+ {/if} +
+ + (deleteFileId = null)}> + {#if deleteFileRow} +

+ Remove {deleteFileRow.title} from the library and delete its + file. Its reports and playback errors go with it. +

+
+ + +
+ {/if} + + + (deleteLidarrId = null)}> + {#if deleteLidarrRow} +

+ Lidarr removes the whole album {deleteLidarrRow.album} by + {deleteLidarrRow.artist}, files included, and won't import it again. Type DELETE to confirm. +

+ + {#if deleteLidarrError} +

{deleteLidarrError}

+ {/if} +
+ + +
+ {/if} + + + diff --git a/web/src/routes/admin/reports/reports.test.ts b/web/src/routes/admin/reports/reports.test.ts new file mode 100644 index 00000000..120d9661 --- /dev/null +++ b/web/src/routes/admin/reports/reports.test.ts @@ -0,0 +1,128 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { fireEvent, render, screen, waitFor, within } from '@testing-library/svelte'; +import { mockQuery } from '../../../test-utils/query'; +import type { AdminPlaybackError, AdminQuarantineRow } from '#lib/api/types.js'; + +vi.mock('#lib/api/admin.js', () => ({ + createAdminQuarantineQuery: vi.fn(), + createAdminPlaybackErrorsQuery: vi.fn(), + createQuarantineActionsQuery: vi.fn(), + resolveQuarantine: vi.fn().mockResolvedValue({}), + resolvePlaybackError: vi.fn().mockResolvedValue({}), + deleteQuarantineFile: vi.fn().mockResolvedValue({}), + deleteQuarantineViaLidarr: vi.fn().mockResolvedValue({}) +})); +vi.mock('#lib/player/store.svelte.js', () => ({ playRadio: vi.fn() })); + +import Page from './+page.svelte'; +import { + createAdminPlaybackErrorsQuery, + createAdminQuarantineQuery, + createQuarantineActionsQuery, + deleteQuarantineFile, + resolvePlaybackError, + resolveQuarantine +} from '#lib/api/admin.js'; + +const hoursAgo = (h: number) => new Date(Date.now() - h * 3_600_000).toISOString(); + +const flagged: AdminQuarantineRow = { + track_id: 't1', + track_title: 'Roygbiv', + artist_name: 'Boards of Canada', + album_title: 'Geogaddi', + album_id: 'al1', + lidarr_album_mbid: 'rg1', + report_count: 1, + latest_at: hoursAgo(5), + reason_counts: { bad_rip: 1 }, + reports: [{ user_id: 'u1', username: 'alice', reason: 'bad_rip', notes: 'crackles', created_at: hoursAgo(5) }] +}; + +const failure = (id: string, track: string, title: string, h: number): AdminPlaybackError => ({ + id, + track_id: track, + user_id: 'u2', + username: 'bob', + client_id: 'android', + kind: 'load_failed', + occurred_at: hoursAgo(h), + track_title: title, + track_file_path: `/music/${title}.flac`, + artist_name: 'Annadale', + album_title: 'Dial Tone', + album_id: 'al2' +}); + +afterEach(() => vi.clearAllMocks()); + +function setup(q: AdminQuarantineRow[], e: AdminPlaybackError[], opts: { error?: boolean } = {}) { + vi.mocked(createAdminQuarantineQuery).mockReturnValue( + (opts.error ? mockQuery({ isError: true, error: new Error('x') }) : mockQuery({ data: q })) as never + ); + vi.mocked(createAdminPlaybackErrorsQuery).mockReturnValue(mockQuery({ data: e }) as never); + vi.mocked(createQuarantineActionsQuery).mockReturnValue(mockQuery({ data: [] }) as never); + return render(Page); +} + +describe('/admin/reports', () => { + test('a flagged track and a track that failed to play are one list, one row per track, newest first', () => { + setup([flagged], [failure('e1', 't1', 'Roygbiv', 1), failure('e2', 't2', 'Intro', 9)]); + const rows = screen.getAllByTestId('report-row'); + expect(rows).toHaveLength(2); + expect(screen.getByTestId('reports-verdict').textContent).toMatch(/2 tracks need a look/); + const first = within(rows[0]).getByTestId('report-kinds').textContent?.replace(/\s+/g, ' '); + expect(first).toMatch(/1× Bad rip/); + expect(first).toMatch(/1× Wouldn't load/); + expect(rows[1].textContent).toMatch(/Intro/); + }); + + test('Resolve clears the listeners\' reports and marks the playback errors fixed', async () => { + setup([flagged], [failure('e1', 't1', 'Roygbiv', 1)]); + await fireEvent.click(screen.getByRole('button', { name: /resolve roygbiv/i })); + await waitFor(() => expect(resolveQuarantine).toHaveBeenCalledWith('t1')); + await waitFor(() => expect(resolvePlaybackError).toHaveBeenCalledWith('e1', 'fixed')); + }); + + test('a track only failing to play is resolved without touching listener reports', async () => { + setup([], [failure('e2', 't2', 'Intro', 2)]); + await fireEvent.click(screen.getByRole('button', { name: /resolve intro/i })); + await waitFor(() => expect(resolvePlaybackError).toHaveBeenCalledWith('e2', 'fixed')); + expect(resolveQuarantine).not.toHaveBeenCalled(); + }); + + test('Delete file is one path, whatever the report came from', async () => { + setup([], [failure('e2', 't2', 'Intro', 2)]); + await fireEvent.click(screen.getByRole('button', { name: /delete file for intro/i })); + await fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: /confirm delete file/i })); + await waitFor(() => expect(deleteQuarantineFile).toHaveBeenCalledWith('t2')); + expect(resolvePlaybackError).not.toHaveBeenCalled(); + }); + + test('Delete via Lidarr is offered only for an album Lidarr holds, and on a phone it is in the row menu', async () => { + setup([flagged], [failure('e2', 't2', 'Intro', 2)]); + expect(screen.getByRole('button', { name: /delete via lidarr for roygbiv/i })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /delete via lidarr for intro/i })).toBeNull(); + await fireEvent.click(screen.getAllByRole('button', { name: /more actions/i })[0]); + expect(within(screen.getByRole('menu')).getByText('Delete via Lidarr')).toBeInTheDocument(); + }); + + test('details show who flagged it, who it failed for, and the file', async () => { + setup([flagged], [failure('e1', 't1', 'Roygbiv', 1)]); + await fireEvent.click(screen.getByRole('button', { name: /^details$/i })); + const details = screen.getByTestId('report-details').textContent?.replace(/\s+/g, ' '); + expect(details).toMatch(/alice flagged it · Bad rip · crackles/); + expect(details).toMatch(/Failed to play for bob · Wouldn't load/); + expect(details).toMatch(/\/music\/Roygbiv\.flac/); + }); + + test('a load failure says so with Try again, and an empty queue says nothing waits', () => { + setup([], [], { error: true }); + expect(screen.getByTestId('reports-error').textContent).toMatch(/couldn't load the reports/i); + }); + + test('with nothing reported it says so', () => { + setup([], []); + expect(screen.getByTestId('reports-empty').textContent).toMatch(/no reports/i); + }); +});