From 1062b7c0bc5ce3d477354ce85f5e8d547b9684d4 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Tue, 18 Aug 2026 08:04:39 -0400 Subject: [PATCH] fix(explore): tell Lit that a track request changed something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The album page's tracklist badges read `libraryStatusFor(false, track.mbid)` at render time, which is a dependency on `downloadStore` that Lit cannot see. The page did subscribe to that store, but its callback only assigned `canDownload` and `isRequested` — neither of which a *track* request changes — so no reactive field moved and the component never re-rendered. The request was filed, the plus stayed a plus, and clicking again cancelled it. The other three hosts rendering these badges have always asked for the repaint in the same place, which is what made this one look correct on inspection. Closes #33 Co-Authored-By: Claude Opus 5 (1M context) --- .../explore-album-details.ts | 14 ++ .../components/album-track-request.test.ts | 149 ++++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 frontend/test/components/album-track-request.test.ts diff --git a/frontend/src/components/explore-album-details/explore-album-details.ts b/frontend/src/components/explore-album-details/explore-album-details.ts index 41abd21..b1736ba 100644 --- a/frontend/src/components/explore-album-details/explore-album-details.ts +++ b/frontend/src/components/explore-album-details/explore-album-details.ts @@ -720,14 +720,28 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost { // The download button only appears once a client is connected, // so this tracks the provider list rather than assuming. + // + // The `requestUpdate` is what makes the *tracklist's* badges + // move. Both assignments below are reactive fields, so Lit + // repaints when either changes — but a track request changes + // neither: `canDownload` is about providers and `isRequested` + // is about this album's own release group. Each row's badge + // reads `libraryStatusFor(false, track.mbid)` at render time, + // which is a dependency on the store that Lit cannot see, so + // clicking one filed the request and left the plus exactly + // where it was. The other three hosts rendering these badges + // (`explore-artist-details`, `explore-view`, `top-results-row`) + // have always asked for the repaint here; this one did not. this.downloadUnsub = downloadStore.subscribe(() => { this.canDownload = downloadStore.available; this.syncRequested(); + this.requestUpdate(); }); void downloadStore.init().then(() => { this.canDownload = downloadStore.available; this.syncRequested(); + this.requestUpdate(); }); void this.resolveTargetLibraryId(); diff --git a/frontend/test/components/album-track-request.test.ts b/frontend/test/components/album-track-request.test.ts new file mode 100644 index 0000000..28af27f --- /dev/null +++ b/frontend/test/components/album-track-request.test.ts @@ -0,0 +1,149 @@ +/** + * Issue #33: "Want track" filed the request and left the badge alone. + * + * The tracklist's badges read `libraryStatusFor(false, track.mbid)` at + * render time, which is a dependency on `downloadStore` that Lit cannot + * see. `explore-album-details` did subscribe to that store, but its + * callback only assigned `canDownload` and `isRequested` — neither of + * which a *track* request changes — so nothing in the component's + * reactive state moved and the page never re-rendered. The request was + * real, the plus stayed a plus, and clicking again cancelled it. + * + * The other three hosts rendering these badges (`explore-artist- + * details`, `explore-view`, `top-results-row`) have always asked for + * the repaint in the same place, which is what made this one look + * correct on inspection. + */ +import { describe, expect, it, beforeEach } from 'vitest'; +import type { LitElement } from 'lit'; + +import '@components/explore-album-details/explore-album-details'; +import type { Request } from '@store/download-store'; +import { Events } from '../../src/events'; +import { stub, flush, resetHarness, emit } from '@test/support/harness'; +import { fixture, shadowAll } from '@test/support/render'; + +type RequestOverrides = Partial> & { + state?: `${Request['state']}`; + entity?: `${Request['entity']}`; +}; + +function request(overrides: RequestOverrides): Request { + return { + id: 1, + mbid: 'mbid-1', + entity: 'recording', + libraryId: 1, + artist: 'An Artist', + title: 'Track 1', + scope: 'future', + secondary: false, + state: 'wanted', + attempts: 0, + ...overrides, + } as Request; +} + +/** Put a request list into the store the way the backend does. */ +async function withRequests(rows: Request[]): Promise { + stub('download.Service.ListRequests', rows); + emit(Events.RequestsChanged); + await flush(); +} + +function track(n: number) { + return { + position: n, + discNumber: 1, + title: `Track ${n}`, + length: 200000, + mbid: `mbid-${n}`, + inLibrary: false, + }; +} + +/** An unowned catalog tracklist, which is the only case with badges: + * an owned row renders none, there being nothing left to ask for. */ +async function withTracklist(count: number): Promise { + const el = await fixture('explore-album-details', { + albumName: 'Glass Harbour', + }); + + Object.assign(el, { + versionEntries: [ + { + key: 'v1', + label: '2019', + sublabel: `${count} tracks`, + tracks: Array.from({ length: count }, (_, i) => track(i + 1)), + }, + ], + selectedVersionKey: 'v1', + loadingReleases: false, + loadingInfo: false, + }); + el.requestUpdate(); + await flush(); + await el.updateComplete; + + return el; +} + +/** The status of each track badge, in tracklist order. */ +function badgeStatuses(el: LitElement): string[] { + return shadowAll(el, 'library-status-indicator.track-request').map( + (b) => b.getAttribute('status') ?? '', + ); +} + +describe('the album tracklist’s request badges', () => { + beforeEach(async () => { + resetHarness(); + stub('library.Library.GetFilePathsByRecordingMBIDs', {}); + stub('library.Library.GetFilePathsByAlbums', {}); + stub('library.Library.GetAlbumTracks', []); + stub('library.Library.GetAllLibrariesWithTrackCounts', []); + await withRequests([]); + }); + + it('starts as a plus on every unowned row', async () => { + const el = await withTracklist(3); + + expect(badgeStatuses(el)).toEqual([ + 'not-in-library', + 'not-in-library', + 'not-in-library', + ]); + }); + + it('repaints the row whose track has been requested', async () => { + const el = await withTracklist(3); + + await withRequests([request({ mbid: 'mbid-2' })]); + await el.updateComplete; + + // Only the requested row moves. A request is by MBID, so the two + // rows either side of it are still a plus. + expect(badgeStatuses(el)).toEqual([ + 'not-in-library', + 'queued', + 'not-in-library', + ]); + }); + + it('repaints again when the request is cancelled', async () => { + const el = await withTracklist(3); + + await withRequests([request({ mbid: 'mbid-2' })]); + await el.updateComplete; + + await withRequests([]); + await el.updateComplete; + + expect(badgeStatuses(el)).toEqual([ + 'not-in-library', + 'not-in-library', + 'not-in-library', + ]); + }); +});