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); + }); +});