diff --git a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationLinks.kt b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationLinks.kt index 7c49897d..420ef076 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationLinks.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationLinks.kt @@ -9,18 +9,23 @@ import com.fabledsword.minstrel.nav.Requests /** * The app screen for a notice's link. The server writes web paths, the same - * for every client; this maps them onto the app's routes. Admin pages the app - * has no screen for (missing files, duplicates, playback errors) open the - * Admin landing; anything unknown opens nothing. + * for every client; this maps them onto the app's routes. The library's + * health pages sit under Collection on the web (M504): Reports opens the + * app's report screen, and the ones the app has no screen for (duplicates, + * suspect sources, missing files) open the Admin landing, as do other Admin + * pages; anything unknown opens nothing. */ +private val ADMIN_COLLECTION_PAGES = setOf("duplicates", "suspect-sources", "missing") + internal fun routeForLink(link: String): Any? { - val parts = link.trim('/').split('/').filter { it.isNotEmpty() } + val parts = link.substringBefore('?').trim('/').split('/').filter { it.isNotEmpty() } return when { parts.size == 2 && parts[0] == "albums" -> AlbumDetail(parts[1]) parts.size == 2 && parts[0] == "artists" -> ArtistDetail(parts[1]) parts == listOf("requests") -> Requests parts == listOf("admin", "requests") -> AdminRequests - parts == listOf("admin", "quarantine") -> AdminQuarantine + parts == listOf("collection", "reports") -> AdminQuarantine + parts.firstOrNull() == "collection" && parts.getOrNull(1) in ADMIN_COLLECTION_PAGES -> Admin parts.firstOrNull() == "admin" -> Admin else -> null } diff --git a/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt index 763e13ba..1f21c4a3 100644 --- a/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt +++ b/android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationUiTest.kt @@ -23,9 +23,11 @@ class NotificationUiTest { assertEquals(ArtistDetail("ar-1"), routeForLink("/artists/ar-1")) assertEquals(Requests, routeForLink("/requests")) assertEquals(AdminRequests, routeForLink("/admin/requests")) - assertEquals(AdminQuarantine, routeForLink("/admin/quarantine")) - // Admin pages the app has no screen for open the Admin landing. - assertEquals(Admin, routeForLink("/admin/duplicates")) + assertEquals(AdminQuarantine, routeForLink("/collection/reports")) + // Library health pages the app has no screen for open the Admin landing. + assertEquals(Admin, routeForLink("/collection/duplicates")) + assertEquals(Admin, routeForLink("/collection/duplicates?view=resolved")) + assertEquals(Admin, routeForLink("/admin/missing-files")) assertEquals(Admin, routeForLink("/admin")) assertNull(routeForLink("/somewhere-new")) } diff --git a/internal/notifications/render.go b/internal/notifications/render.go index 77e52de9..3b1f511a 100644 --- a/internal/notifications/render.go +++ b/internal/notifications/render.go @@ -93,7 +93,7 @@ func Render(kind Kind, payload []byte) Rendered { if p.Reason != "" { body = who + " flagged " + name + ": " + p.Reason } - return Rendered{Title: "Track flagged", Body: body, Link: "/admin/quarantine"} + return Rendered{Title: "Track flagged", Body: body, Link: "/collection/reports"} case KindScanFailed: title := "Library scan failed" if p.Count > 1 { @@ -114,7 +114,7 @@ func Render(kind Kind, payload []byte) Rendered { return Rendered{ Title: plural(p.Count, "duplicate group to review", "duplicate groups to review"), Body: "The duplicate sweep found tracks holding the same recording.", - Link: "/admin/duplicates", + Link: "/collection/duplicates", } case KindDuplicatesResolved: body := "Extra copies of a song were merged into the one the album keeps." @@ -124,13 +124,13 @@ func Render(kind Kind, payload []byte) Rendered { return Rendered{ Title: plural(p.Count, "duplicate resolved automatically", "duplicates resolved automatically"), Body: body, - Link: "/admin/duplicates?view=resolved", + Link: "/collection/duplicates?view=resolved", } case KindPlaybackErrors: return Rendered{ Title: plural(p.Count, "playback error reported", "playback errors reported"), Body: "A track failed to play for someone.", - Link: "/admin/playback-errors", + Link: "/collection/reports", } default: return Rendered{Title: "Notification", Link: "/"} diff --git a/web/src/lib/admin/queues.ts b/web/src/lib/admin/queues.ts index c84cd9cd..110fb957 100644 --- a/web/src/lib/admin/queues.ts +++ b/web/src/lib/admin/queues.ts @@ -1,8 +1,9 @@ import type { AdminQueue, AdminSummary } from '#lib/api/types.js'; // The queues that wait on an admin (#5605), in the order the Overview lists -// them. The rail and the Overview read the same list, so a queue that -// needs you is counted the same in both places. +// them. The rails and the Overview read the same list, so a queue that +// needs you is counted the same in every place. Requests live under Admin; +// the library's health queues under Collection (M504). export type QueueKey = keyof AdminSummary; export type QueueDef = { @@ -17,9 +18,9 @@ export type QueueDef = { export const QUEUES: QueueDef[] = [ { key: 'requests', href: '/admin/requests', one: 'request to decide', many: 'requests to decide', none: 'requests' }, - { 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: 'reports', href: '/collection/reports', one: 'broken track reported', many: 'broken tracks reported', none: 'reports' }, + { key: 'duplicates', href: '/collection/duplicates', one: 'set of duplicates to review', many: 'sets of duplicates to review', none: 'duplicates' }, + { key: 'suspect', href: '/collection/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/components/wanted/CollectionRail.svelte b/web/src/lib/components/wanted/CollectionRail.svelte index 1152ff77..352ee263 100644 --- a/web/src/lib/components/wanted/CollectionRail.svelte +++ b/web/src/lib/components/wanted/CollectionRail.svelte @@ -1,14 +1,18 @@ diff --git a/web/src/routes/admin/+layout.svelte b/web/src/routes/admin/+layout.svelte index 03c384ba..35664b38 100644 --- a/web/src/routes/admin/+layout.svelte +++ b/web/src/routes/admin/+layout.svelte @@ -11,6 +11,7 @@ // the Collection are, so the sections sit in the same place everywhere // and fit a phone. Each queue carries how many wait on you, from one // summary read that is refreshed whenever you move between admin pages. + // The library's health queues are under Collection (M504). const summaryStore = createAdminSummaryQuery(); const summary = $derived($summaryStore.data); @@ -28,9 +29,6 @@ const items: RailItem[] = $derived([ { href: '/admin', label: 'Overview', exact: true }, { href: '/admin/requests', label: 'Requests', attention: countOf(summary, 'requests') }, - { 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') }, { href: '/admin/library', label: 'Library' }, { href: '/admin/acquisition', label: 'Acquisition' }, diff --git a/web/src/routes/admin/admin.test.ts b/web/src/routes/admin/admin.test.ts index 86a84c19..50c21466 100644 --- a/web/src/routes/admin/admin.test.ts +++ b/web/src/routes/admin/admin.test.ts @@ -116,6 +116,16 @@ describe('admin Overview page', () => { expect(screen.getByTestId('all-clear').textContent).not.toMatch(/requests/); }); + // The library's health queues live under Collection (M504); the Overview + // still lists them, linking there. + test('a library queue that waits on you opens its Collection page', () => { + setupOverview(summary({ duplicates: { count: 5, since: ago(4) } })); + render(OverviewPage); + expect(screen.getByRole('link', { name: /5 sets of duplicates to review/i }).getAttribute('href')).toBe( + '/collection/duplicates' + ); + }); + test('a failed scan needs you too', async () => { const { readable } = await import('svelte/store'); (createScanStatusQuery as ReturnType).mockReturnValueOnce( @@ -158,9 +168,6 @@ describe('admin layout', () => { expect(links).toEqual([ ['/admin', 'Overview'], ['/admin/requests', 'Requests 2'], - ['/admin/reports', 'Reports'], - ['/admin/duplicates', 'Duplicates 5'], - ['/admin/suspect-sources', 'Suspect sources'], ['/admin/missing-files', 'Missing files'], ['/admin/library', 'Library'], ['/admin/acquisition', 'Acquisition'], diff --git a/web/src/routes/admin/playback-errors/+page.ts b/web/src/routes/admin/playback-errors/+page.ts deleted file mode 100644 index fe1692bc..00000000 --- a/web/src/routes/admin/playback-errors/+page.ts +++ /dev/null @@ -1,9 +0,0 @@ -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/+page.ts b/web/src/routes/admin/quarantine/+page.ts deleted file mode 100644 index fe1692bc..00000000 --- a/web/src/routes/admin/quarantine/+page.ts +++ /dev/null @@ -1,9 +0,0 @@ -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/collection/+layout.svelte b/web/src/routes/collection/+layout.svelte index 3e70fa75..95cc3ec9 100644 --- a/web/src/routes/collection/+layout.svelte +++ b/web/src/routes/collection/+layout.svelte @@ -1,5 +1,7 @@
@@ -50,12 +61,21 @@ {/if}

{/if} + {#if health.length > 0} +

+ To look at: + {#each health as d, i (d.key)}{queueLabel(d, countOf(summary, d.key))}{i < health.length - 1 ? ', ' : '.'} + {/each} +

+ {/if}
- +
diff --git a/web/src/routes/collection/collection.test.ts b/web/src/routes/collection/collection.test.ts index 96623b25..a0e4ff13 100644 --- a/web/src/routes/collection/collection.test.ts +++ b/web/src/routes/collection/collection.test.ts @@ -3,22 +3,31 @@ import { render, screen } from '@testing-library/svelte'; import { createRawSnippet } from 'svelte'; import { mockInfiniteQuery } from '../../test-utils/query'; import { pageUrlModule } from '#test-utils/mocks/appState.js'; -import type { WantArtistsPage, WantCounts } from '#lib/api/types.js'; +import { mockQuery } from '../../test-utils/query'; +import type { AdminSummary, WantArtistsPage, WantCounts } from '#lib/api/types.js'; const state = vi.hoisted(() => ({ pageUrl: new URL('http://localhost/collection') })); vi.mock('$app/state', () => pageUrlModule(state)); vi.mock('#lib/api/wanted.js', () => ({ createWantArtistsQuery: vi.fn() })); +vi.mock('#lib/api/admin.js', () => ({ createAdminSummaryQuery: vi.fn() })); import WantedLayout from './+layout.svelte'; import { createWantArtistsQuery } from '#lib/api/wanted.js'; +import { createAdminSummaryQuery } from '#lib/api/admin.js'; const counts: WantCounts = { artists: 7, wanted: 5, missing: 1, held: 9, upgrades: 6, following: 2, activity: 3, attention: 0, blocked: 4, unmatched: 3 }; const children = createRawSnippet(() => ({ render: () => `

body

` })); afterEach(() => vi.clearAllMocks()); -function setup(c: WantCounts = counts, path = '/collection') { +const none = { count: 0, since: null }; +const clear: AdminSummary = { requests: none, reports: none, duplicates: none, suspect: none, missing: none }; + +function setup(c: WantCounts = counts, path = '/collection', summary: AdminSummary = clear) { state.pageUrl = new URL(`http://localhost${path}`); + vi.mocked(createAdminSummaryQuery).mockReturnValue( + mockQuery({ data: summary }) as unknown as ReturnType + ); const page: WantArtistsPage = { items: [], total: 0, @@ -47,7 +56,10 @@ describe('/collection layout', () => { ['/collection/activity', 'Activity 3'], ['/collection/following', 'Following 2'], ['/collection/blocked', 'Blocked 4'], - ['/collection/unmatched', 'Unmatched 3'] + ['/collection/unmatched', 'Unmatched 3'], + ['/collection/reports', 'Reports'], + ['/collection/duplicates', 'Duplicates'], + ['/collection/suspect-sources', 'Suspect sources'] ]); expect(screen.getByRole('link', { name: /activity/i })).toHaveAttribute('aria-current', 'page'); expect(screen.getByRole('link', { name: /^catalogue/i })).not.toHaveAttribute('aria-current'); @@ -76,6 +88,33 @@ describe('/collection layout', () => { expect(screen.getByRole('link', { name: /see unmatched/i }).getAttribute('href')).toBe('/collection/unmatched'); }); + // The library's health queues moved here from Admin (M504): their counts + // come from the admin summary, accented, and a line names the ones waiting. + test('reports, duplicates and suspect sources carry what waits on you, and a line links to each', () => { + setup(counts, '/collection', { + ...clear, + reports: { count: 2, since: null }, + duplicates: { count: 5, since: null }, + missing: { count: 9, since: null } + }); + const nav = screen.getByRole('navigation', { name: /collection sections/i }); + const dup = Array.from(nav.querySelectorAll('a')).find((a) => a.getAttribute('href') === '/collection/duplicates')!; + expect(dup.textContent?.replace(/\s+/g, ' ').trim()).toBe('Duplicates 5'); + const health = screen.getByTestId('collection-health'); + expect(health.textContent?.replace(/\s+/g, ' ').trim()).toBe( + 'To look at: 2 broken tracks reported, 5 sets of duplicates to review.' + ); + expect(Array.from(health.querySelectorAll('a')).map((a) => a.getAttribute('href'))).toEqual([ + '/collection/reports', + '/collection/duplicates' + ]); + }); + + test('with nothing waiting there is no health line', () => { + setup(); + expect(screen.queryByTestId('collection-health')).toBeNull(); + }); + test("an artist's page sits under Catalogue", () => { setup(counts, '/collection/artist/blur'); expect(screen.getByRole('link', { name: /^catalogue/i })).toHaveAttribute('aria-current', 'page'); diff --git a/web/src/routes/admin/duplicates/+page.svelte b/web/src/routes/collection/duplicates/+page.svelte similarity index 99% rename from web/src/routes/admin/duplicates/+page.svelte rename to web/src/routes/collection/duplicates/+page.svelte index c83c4679..38cb6aa7 100644 --- a/web/src/routes/admin/duplicates/+page.svelte +++ b/web/src/routes/collection/duplicates/+page.svelte @@ -187,7 +187,7 @@ } -{pageTitle('Admin · Duplicates')} +{pageTitle('Collection · Duplicates')}
diff --git a/web/src/routes/admin/duplicates/duplicates.test.ts b/web/src/routes/collection/duplicates/duplicates.test.ts similarity index 100% rename from web/src/routes/admin/duplicates/duplicates.test.ts rename to web/src/routes/collection/duplicates/duplicates.test.ts diff --git a/web/src/routes/admin/reports/+page.svelte b/web/src/routes/collection/reports/+page.svelte similarity index 99% rename from web/src/routes/admin/reports/+page.svelte rename to web/src/routes/collection/reports/+page.svelte index b4648356..688a60dd 100644 --- a/web/src/routes/admin/reports/+page.svelte +++ b/web/src/routes/collection/reports/+page.svelte @@ -147,7 +147,7 @@ const plural = (n: number, one: string, many: string) => `${n} ${n === 1 ? one : many}`; -{pageTitle('Admin · Reports')} +{pageTitle('Collection · Reports')}
diff --git a/web/src/routes/admin/reports/reports.test.ts b/web/src/routes/collection/reports/reports.test.ts similarity index 99% rename from web/src/routes/admin/reports/reports.test.ts rename to web/src/routes/collection/reports/reports.test.ts index a10201e9..6cc9c4cb 100644 --- a/web/src/routes/admin/reports/reports.test.ts +++ b/web/src/routes/collection/reports/reports.test.ts @@ -63,7 +63,7 @@ function setup(q: AdminQuarantineRow[], e: AdminPlaybackError[], opts: { error?: return render(Page); } -describe('/admin/reports', () => { +describe('/collection/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'); diff --git a/web/src/routes/admin/suspect-sources/+page.svelte b/web/src/routes/collection/suspect-sources/+page.svelte similarity index 99% rename from web/src/routes/admin/suspect-sources/+page.svelte rename to web/src/routes/collection/suspect-sources/+page.svelte index d1dfc53f..7e2192fc 100644 --- a/web/src/routes/admin/suspect-sources/+page.svelte +++ b/web/src/routes/collection/suspect-sources/+page.svelte @@ -82,7 +82,7 @@ } -{pageTitle('Admin · Suspect sources')} +{pageTitle('Collection · Suspect sources')}
diff --git a/web/src/routes/admin/suspect-sources/suspect-sources.test.ts b/web/src/routes/collection/suspect-sources/suspect-sources.test.ts similarity index 100% rename from web/src/routes/admin/suspect-sources/suspect-sources.test.ts rename to web/src/routes/collection/suspect-sources/suspect-sources.test.ts