From e70a76f48915cc129933a2cbcd547eb0cf16c299 Mon Sep 17 00:00:00 2001 From: Harvey Date: Sun, 19 Apr 2026 03:18:50 +0100 Subject: [PATCH] Failed to verify fix + reactivity & polling fixes for multi downloads --- .../components/BatchDownloadIndicator.svelte | 18 ++- frontend/src/lib/constants.ts | 3 +- frontend/src/lib/stores/musicSource.spec.ts | 117 ++++++++++++++++++ frontend/src/lib/stores/musicSource.ts | 33 ++++- frontend/src/routes/+layout.svelte | 3 + frontend/src/routes/+page.svelte | 15 +-- frontend/src/routes/artist/[id]/+page.svelte | 14 ++- frontend/src/routes/popular/+page.svelte | 15 +-- frontend/src/routes/trending/+page.svelte | 15 +-- frontend/src/routes/your-top/+page.svelte | 15 +-- 10 files changed, 210 insertions(+), 38 deletions(-) create mode 100644 frontend/src/lib/stores/musicSource.spec.ts diff --git a/frontend/src/lib/components/BatchDownloadIndicator.svelte b/frontend/src/lib/components/BatchDownloadIndicator.svelte index eec5dba..a8cf98e 100644 --- a/frontend/src/lib/components/BatchDownloadIndicator.svelte +++ b/frontend/src/lib/components/BatchDownloadIndicator.svelte @@ -4,12 +4,15 @@ import { libraryStore } from '$lib/stores/library'; import { playerStore } from '$lib/stores/player.svelte'; + const POLL_INTERVAL_MS = 15_000; + let completedPerJob = $derived.by(() => { + const { mbidSet } = $libraryStore; const counts: Record = {}; for (const job of batchDownloadStore.jobs) { let done = 0; for (const mbid of job.musicbrainzIds) { - if (libraryStore.isInLibrary(mbid)) done++; + if (mbidSet.has(mbid.toLowerCase())) done++; } counts[job.artistId] = done; } @@ -20,6 +23,19 @@ batchDownloadStore.jobs.every((job) => (completedPerJob[job.artistId] ?? 0) >= job.total) ); + // Poll library while batch downloads are in progress + $effect(() => { + if (!batchDownloadStore.hasActive || allComplete) return; + + libraryStore.refresh(); + + const interval = setInterval(() => { + libraryStore.refresh(); + }, POLL_INTERVAL_MS); + + return () => clearInterval(interval); + }); + $effect(() => { if (allComplete && batchDownloadStore.jobs.length > 0) { const timer = setTimeout(() => { diff --git a/frontend/src/lib/constants.ts b/frontend/src/lib/constants.ts index 642d4a6..be9e32a 100644 --- a/frontend/src/lib/constants.ts +++ b/frontend/src/lib/constants.ts @@ -182,7 +182,8 @@ export const API = { suggest: (query: string, limit = 5) => `/api/v1/search/suggest?q=${encodeURIComponent(query.trim())}&limit=${limit}` }, - home: (source: string) => `/api/v1/home?source=${encodeURIComponent(source)}`, + home: (source?: string) => + source ? `/api/v1/home?source=${encodeURIComponent(source)}` : '/api/v1/home', homeIntegrationStatus: () => '/api/v1/home/integration-status', discover: (source?: string) => source ? `/api/v1/discover?source=${encodeURIComponent(source)}` : '/api/v1/discover', diff --git a/frontend/src/lib/stores/musicSource.spec.ts b/frontend/src/lib/stores/musicSource.spec.ts new file mode 100644 index 0000000..b33a175 --- /dev/null +++ b/frontend/src/lib/stores/musicSource.spec.ts @@ -0,0 +1,117 @@ +import { describe, expect, it, beforeEach, vi } from 'vitest'; +import { PAGE_SOURCE_KEYS, API } from '$lib/constants'; + +vi.mock('$app/environment', () => ({ browser: true })); + +const storage = new Map(); +const mockLocalStorage = { + getItem: vi.fn((key: string) => storage.get(key) ?? null), + setItem: vi.fn((key: string, value: string) => storage.set(key, value)), + removeItem: vi.fn((key: string) => storage.delete(key)), + clear: vi.fn(() => storage.clear()), + get length() { + return storage.size; + }, + key: vi.fn((_i: number) => null) +}; + +vi.stubGlobal('localStorage', mockLocalStorage); +vi.stubGlobal('window', globalThis); + +describe('migratePageSourceKeys', () => { + let migratePageSourceKeys: (typeof import('$lib/stores/musicSource'))['migratePageSourceKeys']; + + beforeEach(async () => { + vi.clearAllMocks(); + storage.clear(); + vi.resetModules(); + const mod = await import('$lib/stores/musicSource'); + migratePageSourceKeys = mod.migratePageSourceKeys; + }); + + it('converts raw "listenbrainz" to JSON-encoded string', () => { + storage.set(PAGE_SOURCE_KEYS.home, 'listenbrainz'); + migratePageSourceKeys(); + expect(storage.get(PAGE_SOURCE_KEYS.home)).toBe('"listenbrainz"'); + }); + + it('converts raw "lastfm" to JSON-encoded string', () => { + storage.set(PAGE_SOURCE_KEYS.home, 'lastfm'); + migratePageSourceKeys(); + expect(storage.get(PAGE_SOURCE_KEYS.home)).toBe('"lastfm"'); + }); + + it('leaves already-JSON-encoded values unchanged', () => { + storage.set(PAGE_SOURCE_KEYS.home, '"listenbrainz"'); + migratePageSourceKeys(); + expect(storage.get(PAGE_SOURCE_KEYS.home)).toBe('"listenbrainz"'); + }); + + it('leaves invalid values unchanged', () => { + storage.set(PAGE_SOURCE_KEYS.home, 'plex'); + migratePageSourceKeys(); + expect(storage.get(PAGE_SOURCE_KEYS.home)).toBe('plex'); + }); + + it('handles absent keys without error', () => { + expect(() => migratePageSourceKeys()).not.toThrow(); + }); + + it('migrates all page source keys', () => { + for (const key of Object.values(PAGE_SOURCE_KEYS)) { + storage.set(key, 'lastfm'); + } + migratePageSourceKeys(); + for (const key of Object.values(PAGE_SOURCE_KEYS)) { + expect(storage.get(key)).toBe('"lastfm"'); + } + }); +}); + +describe('isMusicSource', () => { + let isMusicSource: (typeof import('$lib/stores/musicSource'))['isMusicSource']; + + beforeEach(async () => { + vi.resetModules(); + const mod = await import('$lib/stores/musicSource'); + isMusicSource = mod.isMusicSource; + }); + + it('returns true for listenbrainz', () => { + expect(isMusicSource('listenbrainz')).toBe(true); + }); + + it('returns true for lastfm', () => { + expect(isMusicSource('lastfm')).toBe(true); + }); + + it('returns false for undefined', () => { + expect(isMusicSource(undefined)).toBe(false); + }); + + it('returns false for invalid string', () => { + expect(isMusicSource('plex')).toBe(false); + }); + + it('returns false for null', () => { + expect(isMusicSource(null)).toBe(false); + }); +}); + +describe('API.home', () => { + it('includes source param when provided', () => { + expect(API.home('listenbrainz')).toBe('/api/v1/home?source=listenbrainz'); + }); + + it('omits source param when undefined', () => { + expect(API.home(undefined)).toBe('/api/v1/home'); + }); + + it('omits source param when called with no arguments', () => { + expect(API.home()).toBe('/api/v1/home'); + }); + + it('encodes special characters in source', () => { + expect(API.home('listen brainz')).toBe('/api/v1/home?source=listen%20brainz'); + }); +}); diff --git a/frontend/src/lib/stores/musicSource.ts b/frontend/src/lib/stores/musicSource.ts index f1eeafd..836760f 100644 --- a/frontend/src/lib/stores/musicSource.ts +++ b/frontend/src/lib/stores/musicSource.ts @@ -18,6 +18,23 @@ export function isMusicSource(value: unknown): value is MusicSource { return value === 'listenbrainz' || value === 'lastfm'; } +/** + * Migrate old raw-string localStorage source values to JSON format. + * Before v1.3.0, setPageSource() stored raw strings (e.g. `listenbrainz`). + * PersistedState expects JSON (e.g. `"listenbrainz"`). Must run before + * any PersistedState constructor reads these keys. + */ +export function migratePageSourceKeys(): void { + if (!browser) return; + for (const key of Object.values(PAGE_SOURCE_KEYS)) { + const raw = localStorage.getItem(key); + if (raw === null) continue; + if (isMusicSource(raw)) { + localStorage.setItem(key, JSON.stringify(raw)); + } + } +} + function readCachedSource(): MusicSource { if (!browser) return DEFAULT_SOURCE; const stored = localStorage.getItem(CACHED_SOURCE_KEY); @@ -105,13 +122,23 @@ function createMusicSourceStore() { function getPageSource(page: MusicSourcePage): MusicSource { const fallbackSource = getSource(); if (!browser) return fallbackSource; - const storedSource = localStorage.getItem(getPageStorageKey(page)); - return isMusicSource(storedSource) ? storedSource : fallbackSource; + const raw = localStorage.getItem(getPageStorageKey(page)); + if (raw === null) return fallbackSource; + // Handle JSON-encoded values (new format) + try { + const parsed: unknown = JSON.parse(raw); + if (isMusicSource(parsed)) return parsed; + } catch { + // Fall through to raw check + } + // Handle raw string values (old format) + if (isMusicSource(raw)) return raw; + return fallbackSource; } function setPageSource(page: MusicSourcePage, source: MusicSource): void { if (!browser) return; - localStorage.setItem(getPageStorageKey(page), source); + localStorage.setItem(getPageStorageKey(page), JSON.stringify(source)); } return { diff --git a/frontend/src/routes/+layout.svelte b/frontend/src/routes/+layout.svelte index 82fc04b..6a10175 100644 --- a/frontend/src/routes/+layout.svelte +++ b/frontend/src/routes/+layout.svelte @@ -3,6 +3,7 @@ import { browser } from '$app/environment'; import { goto, beforeNavigate, afterNavigate } from '$app/navigation'; import { resolve } from '$app/paths'; + import { migratePageSourceKeys } from '$lib/stores/musicSource'; import { errorModal } from '$lib/stores/errorModal'; import { libraryStore } from '$lib/stores/library'; import { integrationStore } from '$lib/stores/integration'; @@ -61,6 +62,8 @@ import type { Snippet } from 'svelte'; import QueryProvider from '$lib/queries/QueryProvider.svelte'; + migratePageSourceKeys(); + let { children }: { children: Snippet } = $props(); let query = $state(''); diff --git a/frontend/src/routes/+page.svelte b/frontend/src/routes/+page.svelte index f414790..07dadcb 100644 --- a/frontend/src/routes/+page.svelte +++ b/frontend/src/routes/+page.svelte @@ -9,7 +9,7 @@ HomeSection as HomeSectionType, WeeklyExplorationSection as WeeklyExplorationSectionType } from '$lib/types'; - import { type MusicSource } from '$lib/stores/musicSource'; + import { type MusicSource, isMusicSource } from '$lib/stores/musicSource'; import CarouselSkeleton from '$lib/components/CarouselSkeleton.svelte'; import PageHeader from '$lib/components/PageHeader.svelte'; import { getGreeting } from '$lib/utils/homeCache'; @@ -27,7 +27,11 @@ // svelte-ignore state_referenced_locally let activeSource = new PersistedState(PAGE_SOURCE_KEYS['home'], data.primarySource); - const homeQuery = getHomeQuery(() => activeSource.current); + let validSource = $derived( + isMusicSource(activeSource.current) ? activeSource.current : data.primarySource + ); + + const homeQuery = getHomeQuery(() => validSource); const homeData = $derived(homeQuery.data); const loading = $derived(homeQuery.isLoading); const isUpdating = $derived(homeQuery.isRefetching); @@ -68,7 +72,7 @@ }); } if ( - activeSource.current === 'listenbrainz' && + validSource === 'listenbrainz' && homeData.weekly_exploration && homeData.weekly_exploration.tracks.length > 0 ) { @@ -178,10 +182,7 @@
- +
{#if homeQuery.error && !homeData} diff --git a/frontend/src/routes/artist/[id]/+page.svelte b/frontend/src/routes/artist/[id]/+page.svelte index b34be93..96ba7e6 100644 --- a/frontend/src/routes/artist/[id]/+page.svelte +++ b/frontend/src/routes/artist/[id]/+page.svelte @@ -17,7 +17,7 @@ import { requestAlbum } from '$lib/utils/albumRequest'; import { integrationStore } from '$lib/stores/integration'; import { libraryStore } from '$lib/stores/library'; - import { type MusicSource } from '$lib/stores/musicSource'; + import { type MusicSource, isMusicSource } from '$lib/stores/musicSource'; import { getArtistLastFmEnrichmentQuery, getArtistReleasesInfiniteQuery, @@ -49,6 +49,10 @@ data.primarySource ); + let validSource = $derived( + isMusicSource(activeSource.current) ? activeSource.current : data.primarySource + ); + let showToast = $state(false); let toastMessage = 'Added to Library'; let showArtistRemovedModal = $state(false); @@ -73,21 +77,21 @@ const similarArtistsQuery = getSimilarArtistsQuery(() => ({ artistId: data.artistId, - source: activeSource.current + source: validSource })); const similarArtists = $derived(similarArtistsQuery.data); const loadingSimilar = $derived(similarArtistsQuery.isLoading); const topSongsQuery = getArtistTopSongsQuery(() => ({ artistId: data.artistId, - source: activeSource.current + source: validSource })); const topSongs = $derived(topSongsQuery.data); const loadingTopSongs = $derived(topSongsQuery.isLoading); const topAlbumsQuery = getArtistTopAlbumsQuery(() => ({ artistId: data.artistId, - source: activeSource.current + source: validSource })); const topAlbums = $derived(topAlbumsQuery.data); const loadingTopAlbums = $derived(topAlbumsQuery.isLoading); @@ -351,7 +355,7 @@
{ activeSource.current = newSource; }} diff --git a/frontend/src/routes/popular/+page.svelte b/frontend/src/routes/popular/+page.svelte index 0ff1a8d..c5f6520 100644 --- a/frontend/src/routes/popular/+page.svelte +++ b/frontend/src/routes/popular/+page.svelte @@ -1,6 +1,6 @@ @@ -28,17 +32,14 @@
- +
diff --git a/frontend/src/routes/trending/+page.svelte b/frontend/src/routes/trending/+page.svelte index f0662f7..a6ca633 100644 --- a/frontend/src/routes/trending/+page.svelte +++ b/frontend/src/routes/trending/+page.svelte @@ -1,6 +1,6 @@ @@ -28,17 +32,14 @@
- +
diff --git a/frontend/src/routes/your-top/+page.svelte b/frontend/src/routes/your-top/+page.svelte index 3542d41..57ff7d0 100644 --- a/frontend/src/routes/your-top/+page.svelte +++ b/frontend/src/routes/your-top/+page.svelte @@ -1,6 +1,6 @@ @@ -28,17 +32,14 @@
- +