From feb1c2eca8493acffd9fa0e02995c00c1bdf7460 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 5 Aug 2026 13:41:51 -0400 Subject: [PATCH] =?UTF-8?q?feat(web):=20genre=20and=20year=20browse=20page?= =?UTF-8?q?s=20=E2=80=94=20#367?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Client half of #367. Two new Library tabs, each an index plus a drill-down. Genres are ordered by track count rather than alphabetically. Raw ID3 carries a long tail of one-off tags, so alphabetical would bury the handful of genres you actually have a library's worth of. Years are grouped into decades — a flat list of every year in a decades-deep library is a wall of numbers, and the decade is how people actually think about it. ## Selection travels in the query string, not the path `?g=Rock%2FPop`, not `/library/genres/Rock%2FPop`. A slash-bearing genre cannot survive a path segment — the server sees two segments, and a hard reload wouldn't reconstruct it through the SPA fallback either. There's a test pinning the encoded href and another pinning that the DECODED value reaches the API. ## Why these two pages don't use svelte-query for their lists The indexes do — fetched once per mount, so static options suffice and the cache survives bouncing in and out of a drill-down. The drill-down lists deliberately don't. Their selection comes from the URL and changes WITHOUT remounting the page, and this codebase has no reactive-query-options pattern anywhere; inventing one here would be a larger change than the feature justifies, and one I can't exercise locally. So they use $effect keyed on the derived selection with an explicit Load more. The stale-response guard is a plain `let`, not $state, and that's load-bearing: as reactive state, reading the token inside the fetch path would make the effect depend on its own writes. Its job is to discard a late response for a previously selected genre instead of painting it over the current one. ## Also Added the year filter to /library/albums' contract but NOT to that page's UI — its infinite scroll is a svelte-query infinite query, and making it react to a filter is the same reactive-options problem. The dedicated pages cover the capability, which is the shape the task offered as its alternative. Library tab bar's comment claims it mirrors Android's LibraryScreen. These two tabs have no Android equivalent, so I noted that inline rather than leaving the claim quietly false. Parity remains an open call. Not yet done from #367's bullet list: genre/year quick-jump links on album and artist detail. Year is free (AlbumRef already carries it) but genre is exposed nowhere client-side — AlbumDetail is AlbumRef + tracks, and neither carries genre — so it needs a small API addition. Following as its own commit. --- web/src/lib/api/browse.ts | 82 ++++++++ web/src/lib/api/queries.ts | 5 + web/src/routes/library/+layout.svelte | 7 + web/src/routes/library/genres/+page.svelte | 210 +++++++++++++++++++ web/src/routes/library/genres/genres.test.ts | 171 +++++++++++++++ web/src/routes/library/years/+page.svelte | 206 ++++++++++++++++++ web/src/routes/library/years/years.test.ts | 179 ++++++++++++++++ 7 files changed, 860 insertions(+) create mode 100644 web/src/lib/api/browse.ts create mode 100644 web/src/routes/library/genres/+page.svelte create mode 100644 web/src/routes/library/genres/genres.test.ts create mode 100644 web/src/routes/library/years/+page.svelte create mode 100644 web/src/routes/library/years/years.test.ts diff --git a/web/src/lib/api/browse.ts b/web/src/lib/api/browse.ts new file mode 100644 index 00000000..70e5ff13 --- /dev/null +++ b/web/src/lib/api/browse.ts @@ -0,0 +1,82 @@ +import { createQuery } from '@tanstack/svelte-query'; +import { api } from './client'; +import { qk } from './queries'; +import type { AlbumRef, Page } from './types'; + +export const BROWSE_PAGE_SIZE = 50; + +// Genres are the raw ID3 strings, split on [;,] server-side but otherwise +// untouched — no case folding, no synonym mapping. So "Rock" and "rock" can +// both appear, as can "Rock/Pop" beside "Rock" and "Pop". Deliberate for v1: +// the raw spread has to be visible before anyone can judge whether it needs +// normalising. +export type GenreCount = { genre: string; track_count: number }; + +export type YearCount = { year: number; album_count: number }; + +export async function listGenres(): Promise { + return api.get('/api/library/genres'); +} + +export async function listAlbumYears(): Promise { + return api.get('/api/library/years'); +} + +// The indexes use svelte-query: they're fetched once per page mount, so static +// options are enough, and the cache means bouncing between browse and a +// drill-down doesn't refetch. The drill-down lists below deliberately do NOT — +// see the note on listAlbumsByGenre. +export function createGenresQuery() { + return createQuery({ + queryKey: qk.genres(), + queryFn: listGenres, + // Genres only change when the library is rescanned. + staleTime: 5 * 60_000 + }); +} + +export function createAlbumYearsQuery() { + return createQuery({ + queryKey: qk.albumYears(), + queryFn: listAlbumYears, + staleTime: 5 * 60_000 + }); +} + +// Genre travels as a QUERY parameter, never a path segment. Raw ID3 genres +// contain slashes ("Rock/Pop" is a real tag), which a path segment cannot +// carry — the server would see two segments, and a hard reload of such a URL +// would not survive the SPA fallback either. +// +// Called directly rather than through createInfiniteQuery because the selected +// genre comes from the URL and changes without remounting the page. This +// codebase has no reactive-query-options pattern, and introducing one here +// would be a larger change than the feature warrants. +export async function listAlbumsByGenre( + genre: string, + limit: number, + offset: number +): Promise> { + const params = new URLSearchParams({ + genre, + limit: String(limit), + offset: String(offset) + }); + return api.get>(`/api/library/albums?${params}`); +} + +// Single-year drill-down. The server takes an inclusive range, so one year is +// expressed as its own degenerate range rather than needing a separate shape. +export async function listAlbumsByYear( + year: number, + limit: number, + offset: number +): Promise> { + const params = new URLSearchParams({ + year_from: String(year), + year_to: String(year), + limit: String(limit), + offset: String(offset) + }); + return api.get>(`/api/library/albums?${params}`); +} diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index 73db1e60..584c234a 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -68,6 +68,11 @@ export const qk = { ['playlists', { kind: kind ?? 'user' }] as const, playlist: (id: string) => ['playlist', id] as const, systemPlaylistsStatus: () => ['systemPlaylistsStatus'] as const, + // Browse indexes (#367). Keys carry no arguments — both are whole-library + // indexes, and the per-genre / per-year album lists are fetched outside + // svelte-query because their selection comes from the URL. + genres: () => ['genres'] as const, + albumYears: () => ['albumYears'] as const, }; export function createArtistsQuery(sort: ArtistSort) { diff --git a/web/src/routes/library/+layout.svelte b/web/src/routes/library/+layout.svelte index b231858a..bec57e7d 100644 --- a/web/src/routes/library/+layout.svelte +++ b/web/src/routes/library/+layout.svelte @@ -7,9 +7,16 @@ // (artists / albums / liked / history / playlists) — Playlists lives // here so the operator can find their personal collection in one // place rather than tracking a separate top-level route. + // Genres and Years sit next to Albums — all three are ways of walking the + // same collection — rather than at the end beside the personal tabs (Liked, + // History, Playlists). NOTE: these two are web-only for now; Android's + // LibraryScreen has no equivalent, so the "mirrors Android" claim above is + // currently aspirational for this pair. Parity is an open call (#367). const tabs = [ { href: '/library/artists', label: 'Artists' }, { href: '/library/albums', label: 'Albums' }, + { href: '/library/genres', label: 'Genres' }, + { href: '/library/years', label: 'Years' }, { href: '/library/liked', label: 'Liked' }, { href: '/library/history', label: 'History' }, { href: '/library/playlists', label: 'Playlists' } diff --git a/web/src/routes/library/genres/+page.svelte b/web/src/routes/library/genres/+page.svelte new file mode 100644 index 00000000..069de967 --- /dev/null +++ b/web/src/routes/library/genres/+page.svelte @@ -0,0 +1,210 @@ + + + + {pageTitle(selected ? `Library · ${selected}` : 'Library · Genres')} + + +{#if selected} +
+
+ + +
+

{selected}

+ {#if !loading || albums.length > 0} +

+ {total} {total === 1 ? 'album' : 'albums'} +

+ {/if} +
+
+ + {#if failed} +

+ Couldn't load albums for this genre. + +

+ {:else if loading && albums.length === 0} +

Loading…

+ {:else if albums.length === 0} + + + {:else} +
+ {#each albums as album (album.id)} + + {/each} +
+ {#if albums.length < total} +
+ +
+ {:else} +

End of genre

+ {/if} + {/if} +
+{:else} +
+
+
+

Genres

+ {#if !index.isPending && !index.isError} +

+ {genres.length} {genres.length === 1 ? 'genre' : 'genres'}, straight from your file tags +

+ {/if} +
+ {#if genres.length > 0} + + {/if} +
+ + {#if index.isError} + + {:else if index.isPending} +

Loading…

+ {:else if genres.length === 0} + + {#snippet actions()} + + Open admin + + {/snippet} + + {:else if filter.trim() && filteredGenres.length === 0} +

+ No genres match '{filter.trim()}'. +

+ {:else} + + + {/if} +
+{/if} diff --git a/web/src/routes/library/genres/genres.test.ts b/web/src/routes/library/genres/genres.test.ts new file mode 100644 index 00000000..84cd7290 --- /dev/null +++ b/web/src/routes/library/genres/genres.test.ts @@ -0,0 +1,171 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/svelte'; +import { mockQuery } from '$test-utils/query'; +import { pageUrlModule } from '$test-utils/mocks/appState'; +import { apiClientMock } from '$test-utils/mocks/client'; +import { emptyLikesMock } from '$test-utils/mocks/likes'; +import type { AlbumRef } from '$lib/api/types'; + +const pageState = vi.hoisted(() => ({ + pageUrl: new URL('http://localhost/library/genres') +})); + +vi.mock('$app/state', () => pageUrlModule(pageState)); + +vi.mock('$lib/api/browse', () => ({ + BROWSE_PAGE_SIZE: 2, + createGenresQuery: vi.fn(), + listAlbumsByGenre: vi.fn() +})); + +vi.mock('$lib/api/client', () => apiClientMock()); +vi.mock('$lib/api/likes', () => emptyLikesMock()); +vi.mock('$lib/player/store.svelte', () => ({ + playQueue: vi.fn(), + playRadio: vi.fn(), + enqueueTrack: vi.fn(), + enqueueTracks: vi.fn(), + player: { current: undefined } +})); + +import GenresPage from './+page.svelte'; +import { createGenresQuery, listAlbumsByGenre } from '$lib/api/browse'; + +const asMock = (fn: unknown) => fn as ReturnType; + +function album(id: string, title: string): AlbumRef { + return { + id, + title, + sort_title: title, + artist_id: 'ar1', + artist_name: 'Someone', + year: 1999, + track_count: 1, + duration_sec: 100, + cover_url: '', + cover_art_source: null + }; +} + +afterEach(() => { + pageState.pageUrl = new URL('http://localhost/library/genres'); + vi.clearAllMocks(); +}); + +describe('/library/genres index', () => { + test('lists genres with track counts in server order', () => { + asMock(createGenresQuery).mockReturnValue( + mockQuery({ + data: [ + { genre: 'Rock', track_count: 120 }, + { genre: 'Jazz', track_count: 8 } + ] + }) + ); + render(GenresPage); + + expect(screen.getByText('Rock')).toBeInTheDocument(); + expect(screen.getByText('120')).toBeInTheDocument(); + expect(screen.getByText('Jazz')).toBeInTheDocument(); + expect(screen.getByText('8')).toBeInTheDocument(); + }); + + // The reason genre is a query parameter and not a route segment. If this + // regresses to a path, "Rock/Pop" silently becomes two segments. + test('encodes a slash-bearing genre into the query string', () => { + asMock(createGenresQuery).mockReturnValue( + mockQuery({ data: [{ genre: 'Rock/Pop', track_count: 3 }] }) + ); + render(GenresPage); + + const link = screen.getByRole('link', { name: /Rock\/Pop/ }); + expect(link.getAttribute('href')).toBe('/library/genres?g=Rock%2FPop'); + }); + + test('case variants appear separately — genres are exposed as-is', () => { + asMock(createGenresQuery).mockReturnValue( + mockQuery({ + data: [ + { genre: 'Rock', track_count: 10 }, + { genre: 'rock', track_count: 2 } + ] + }) + ); + render(GenresPage); + + expect(screen.getByRole('link', { name: /^Rock 10$/ })).toBeInTheDocument(); + expect(screen.getByRole('link', { name: /^rock 2$/ })).toBeInTheDocument(); + }); + + test('empty library explains where genres come from', () => { + asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); + render(GenresPage); + + expect(screen.getByText('No genres found')).toBeInTheDocument(); + expect(screen.getByText(/genre tag on your audio files/i)).toBeInTheDocument(); + }); + + test('surfaces an index error with a retry', () => { + const refetch = vi.fn(); + asMock(createGenresQuery).mockReturnValue( + mockQuery({ isError: true, error: { message: 'boom' }, refetch }) + ); + render(GenresPage); + + fireEvent.click(screen.getByRole('button', { name: /Try again/i })); + expect(refetch).toHaveBeenCalled(); + }); +}); + +describe('/library/genres drill-down', () => { + test('fetches and renders albums for the selected genre', async () => { + pageState.pageUrl = new URL('http://localhost/library/genres?g=Rock%2FPop'); + asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByGenre).mockResolvedValue({ + items: [album('a1', 'First Album')], + total: 1, + limit: 2, + offset: 0 + }); + render(GenresPage); + + // The decoded genre must reach the API, not the percent-encoded form. + await waitFor(() => expect(listAlbumsByGenre).toHaveBeenCalledWith('Rock/Pop', 2, 0)); + expect(await screen.findByText('First Album')).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Rock/Pop' })).toBeInTheDocument(); + }); + + test('load more appends the next page and then reports the end', async () => { + pageState.pageUrl = new URL('http://localhost/library/genres?g=Rock'); + asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByGenre) + .mockResolvedValueOnce({ + items: [album('a1', 'One'), album('a2', 'Two')], + total: 3, + limit: 2, + offset: 0 + }) + .mockResolvedValueOnce({ items: [album('a3', 'Three')], total: 3, limit: 2, offset: 2 }); + render(GenresPage); + + await screen.findByText('One'); + const more = await screen.findByRole('button', { name: /Load more \(1 left\)/ }); + await fireEvent.click(more); + + await waitFor(() => expect(listAlbumsByGenre).toHaveBeenLastCalledWith('Rock', 2, 2)); + expect(await screen.findByText('Three')).toBeInTheDocument(); + // Earlier pages are appended, not replaced. + expect(screen.getByText('One')).toBeInTheDocument(); + expect(await screen.findByText('End of genre')).toBeInTheDocument(); + }); + + test('a failed drill-down offers a retry rather than an empty grid', async () => { + pageState.pageUrl = new URL('http://localhost/library/genres?g=Rock'); + asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByGenre).mockRejectedValue(new Error('nope')); + render(GenresPage); + + expect(await screen.findByText(/Couldn't load albums for this genre/i)).toBeInTheDocument(); + }); +}); diff --git a/web/src/routes/library/years/+page.svelte b/web/src/routes/library/years/+page.svelte new file mode 100644 index 00000000..2f6f4478 --- /dev/null +++ b/web/src/routes/library/years/+page.svelte @@ -0,0 +1,206 @@ + + + + {pageTitle(selected !== null ? `Library · ${selected}` : 'Library · Years')} + + +{#if selected !== null} +
+
+ + +
+

{selected}

+ {#if !loading || albums.length > 0} +

+ {total} {total === 1 ? 'album' : 'albums'} +

+ {/if} +
+
+ + {#if failed} +

+ Couldn't load albums for {selected}. + +

+ {:else if loading && albums.length === 0} +

Loading…

+ {:else if albums.length === 0} + + {:else} +
+ {#each albums as album (album.id)} + + {/each} +
+ {#if albums.length < total} +
+ +
+ {:else} +

End of year

+ {/if} + {/if} +
+{:else} +
+
+

Years

+ {#if !index.isPending && !index.isError} +

+ {years.length} {years.length === 1 ? 'year' : 'years'} with dated releases +

+ {/if} +
+ + {#if index.isError} + + {:else if index.isPending} +

Loading…

+ {:else if years.length === 0} + + + {:else} +
    + {#each decades as d (d.decade)} +
  • +

    + {d.decade}s + · {d.albumCount} +

    + +
  • + {/each} +
+ {/if} +
+{/if} diff --git a/web/src/routes/library/years/years.test.ts b/web/src/routes/library/years/years.test.ts new file mode 100644 index 00000000..4a64ab0c --- /dev/null +++ b/web/src/routes/library/years/years.test.ts @@ -0,0 +1,179 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/svelte'; +import { mockQuery } from '$test-utils/query'; +import { pageUrlModule } from '$test-utils/mocks/appState'; +import { apiClientMock } from '$test-utils/mocks/client'; +import { emptyLikesMock } from '$test-utils/mocks/likes'; +import type { AlbumRef } from '$lib/api/types'; + +const pageState = vi.hoisted(() => ({ + pageUrl: new URL('http://localhost/library/years') +})); + +vi.mock('$app/state', () => pageUrlModule(pageState)); + +vi.mock('$lib/api/browse', () => ({ + BROWSE_PAGE_SIZE: 2, + createAlbumYearsQuery: vi.fn(), + listAlbumsByYear: vi.fn() +})); + +vi.mock('$lib/api/client', () => apiClientMock()); +vi.mock('$lib/api/likes', () => emptyLikesMock()); +vi.mock('$lib/player/store.svelte', () => ({ + playQueue: vi.fn(), + playRadio: vi.fn(), + enqueueTrack: vi.fn(), + enqueueTracks: vi.fn(), + player: { current: undefined } +})); + +import YearsPage from './+page.svelte'; +import { createAlbumYearsQuery, listAlbumsByYear } from '$lib/api/browse'; + +const asMock = (fn: unknown) => fn as ReturnType; + +function album(id: string, title: string): AlbumRef { + return { + id, + title, + sort_title: title, + artist_id: 'ar1', + artist_name: 'Someone', + year: 1999, + track_count: 1, + duration_sec: 100, + cover_url: '', + cover_art_source: null + }; +} + +afterEach(() => { + pageState.pageUrl = new URL('http://localhost/library/years'); + vi.clearAllMocks(); +}); + +describe('/library/years index', () => { + test('groups years into decades, newest decade first', () => { + asMock(createAlbumYearsQuery).mockReturnValue( + mockQuery({ + data: [ + { year: 2020, album_count: 3 }, + { year: 1995, album_count: 2 }, + { year: 1991, album_count: 1 } + ] + }) + ); + render(YearsPage); + + const headings = screen.getAllByRole('heading', { level: 2 }).map((h) => h.textContent ?? ''); + const decades = headings.map((t) => t.trim().split(/\s+/)[0]); + expect(decades).toEqual(['2020s', '1990s']); + }); + + test('sums album counts per decade', () => { + asMock(createAlbumYearsQuery).mockReturnValue( + mockQuery({ + data: [ + { year: 1995, album_count: 2 }, + { year: 1991, album_count: 5 } + ] + }) + ); + render(YearsPage); + + // 2 + 5 across the decade, not per-year. + const heading = screen.getByRole('heading', { level: 2 }); + expect(heading.textContent).toMatch(/1990s\s*·\s*7/); + }); + + test('years within a decade run newest first', () => { + asMock(createAlbumYearsQuery).mockReturnValue( + mockQuery({ + data: [ + { year: 1991, album_count: 1 }, + { year: 1997, album_count: 1 }, + { year: 1994, album_count: 1 } + ] + }) + ); + render(YearsPage); + + const links = screen.getAllByRole('link').map((a) => a.getAttribute('href')); + expect(links).toEqual([ + '/library/years?y=1997', + '/library/years?y=1994', + '/library/years?y=1991' + ]); + }); + + // Undated albums are excluded server-side rather than bucketed under a fake + // year, so an entirely undated library legitimately lands on the empty state. + test('empty index explains that undated albums are absent from this axis', () => { + asMock(createAlbumYearsQuery).mockReturnValue(mockQuery({ data: [] })); + render(YearsPage); + + expect(screen.getByText('No release years found')).toBeInTheDocument(); + expect(screen.getByText(/without one don't appear/i)).toBeInTheDocument(); + }); +}); + +describe('/library/years drill-down', () => { + test('requests the selected year as a degenerate range', async () => { + pageState.pageUrl = new URL('http://localhost/library/years?y=1995'); + asMock(createAlbumYearsQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByYear).mockResolvedValue({ + items: [album('a1', 'Mid Nineties')], + total: 1, + limit: 2, + offset: 0 + }); + render(YearsPage); + + await waitFor(() => expect(listAlbumsByYear).toHaveBeenCalledWith(1995, 2, 0)); + expect(await screen.findByText('Mid Nineties')).toBeInTheDocument(); + }); + + test('a non-numeric year is treated as no selection', () => { + pageState.pageUrl = new URL('http://localhost/library/years?y=nineteen'); + asMock(createAlbumYearsQuery).mockReturnValue( + mockQuery({ data: [{ year: 1999, album_count: 1 }] }) + ); + render(YearsPage); + + // Falls back to the index rather than fetching NaN. + expect(listAlbumsByYear).not.toHaveBeenCalled(); + expect(screen.getByRole('heading', { level: 1, name: 'Years' })).toBeInTheDocument(); + }); + + test('load more appends and then reports the end', async () => { + pageState.pageUrl = new URL('http://localhost/library/years?y=1995'); + asMock(createAlbumYearsQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByYear) + .mockResolvedValueOnce({ + items: [album('a1', 'One'), album('a2', 'Two')], + total: 3, + limit: 2, + offset: 0 + }) + .mockResolvedValueOnce({ items: [album('a3', 'Three')], total: 3, limit: 2, offset: 2 }); + render(YearsPage); + + await screen.findByText('One'); + await fireEvent.click(await screen.findByRole('button', { name: /Load more \(1 left\)/ })); + + await waitFor(() => expect(listAlbumsByYear).toHaveBeenLastCalledWith(1995, 2, 2)); + expect(await screen.findByText('Three')).toBeInTheDocument(); + expect(screen.getByText('One')).toBeInTheDocument(); + expect(await screen.findByText('End of year')).toBeInTheDocument(); + }); + + test('a failed drill-down offers a retry', async () => { + pageState.pageUrl = new URL('http://localhost/library/years?y=1995'); + asMock(createAlbumYearsQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByYear).mockRejectedValue(new Error('nope')); + render(YearsPage); + + expect(await screen.findByText(/Couldn't load albums for 1995/i)).toBeInTheDocument(); + }); +});