From 4c49ee2cc6fefe9f1ef3c901ca95852a3182de1e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 16 Aug 2026 11:55:36 -0400 Subject: [PATCH] =?UTF-8?q?feat(web):=20a=20playlist=20entry=20whose=20fil?= =?UTF-8?q?e=20is=20missing=20greys=20out=20and=20is=20skipped=20=E2=80=94?= =?UTF-8?q?=20#2527?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row treatment for a dead playlist entry already existed -- muted text, no play on click, no drag, no kebab, never "now playing" -- but it only fired for track_id === null, the track-deleted case. A missing file kept a live-looking row that failed on click. The behavioural gate now covers both, and the presentation distinguishes them, because they mean different things to the person reading the list. A removed track is gone for good and keeps the strikethrough. A missing file is a track we still have -- history, likes, the lot -- whose bytes aren't on disk right now, so it gets an explicit "File missing" and a title explaining it stays in the playlist and comes back on its own if the file does. A strikethrough there would claim it was deleted, which is a lie about a file the scanner may well adopt back tomorrow (#2528). Skipping routes through playlistTrackToRef, which already returned null for removed tracks and whose callers already filter nulls. Adding the unavailable check there means every queue builder -- PlaylistCard, systemRefetch, the detail page -- skips a missing file without any of them learning what missing_since is. Remove stays available on a dead row: the owner must still be able to take it out of their own list. --- web/src/lib/api/types.ts | 6 +++ web/src/lib/components/PlaylistCard.test.ts | 6 ++- .../lib/components/PlaylistTrackRow.svelte | 18 ++++++- .../lib/components/PlaylistTrackRow.test.ts | 39 +++++++++++++- .../lib/playlists/playlistTrackToRef.test.ts | 51 +++++++++++++++++++ web/src/lib/playlists/playlistTrackToRef.ts | 12 +++-- .../routes/playlists/[id]/playlist.test.ts | 4 +- 7 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 web/src/lib/playlists/playlistTrackToRef.test.ts diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index bb1645c3..34f5d364 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -87,6 +87,12 @@ export type PlaylistTrack = { duration_sec: number; stream_url: string | null; added_at: string; + // True when the track still exists but its file is missing from disk + // (#2527). Distinct from track_id === null: that one is gone for good, + // this one keeps its play history and likes and may come back — the + // scanner un-marks it, or adopts it if it returns renamed. Both are + // unplayable; only this one is worth telling the user about. + unavailable: boolean; }; export type PlaylistDetail = Playlist & { diff --git a/web/src/lib/components/PlaylistCard.test.ts b/web/src/lib/components/PlaylistCard.test.ts index c5d3bf3c..ef859b29 100644 --- a/web/src/lib/components/PlaylistCard.test.ts +++ b/web/src/lib/components/PlaylistCard.test.ts @@ -35,7 +35,8 @@ vi.mock('$lib/api/playlists', () => ({ album_title: 'Test Album', duration_sec: 60, stream_url: '/s/t-1', - added_at: '2026-01-01T00:00:00Z' + added_at: '2026-01-01T00:00:00Z', + unavailable: false } ] } as PlaylistDetail), @@ -66,7 +67,8 @@ vi.mock('$lib/api/playlists', () => ({ album_title: 'Test Album', duration_sec: 60, stream_url: '/s/t-9', - added_at: '2026-01-01T00:00:00Z' + added_at: '2026-01-01T00:00:00Z', + unavailable: false } ] } as PlaylistDetail), diff --git a/web/src/lib/components/PlaylistTrackRow.svelte b/web/src/lib/components/PlaylistTrackRow.svelte index d3fb6d0c..9238da84 100644 --- a/web/src/lib/components/PlaylistTrackRow.svelte +++ b/web/src/lib/components/PlaylistTrackRow.svelte @@ -22,7 +22,15 @@ onMove?: (fromPos: number, toPos: number) => void; } = $props(); - const isUnavailable = $derived(row.track_id === null); + // Two different ways a row can be dead, with the same behaviour and + // deliberately different copy. A removed track is gone for good; a + // missing file is a track we still have — history, likes and all — + // whose bytes aren't on disk right now (#2527). Telling the user + // "file missing" invites them to fix it; a strikethrough would say + // "deleted", which is a lie about a file that may well come back. + const isMissingFile = $derived(row.unavailable); + const isRemoved = $derived(row.track_id === null); + const isUnavailable = $derived(isRemoved || isMissingFile); // Reconstruct a minimal TrackRef for the kebab menu when the // upstream track still exists. When unavailable, the menu is hidden. @@ -108,10 +116,16 @@ class="min-w-0 flex-1 text-left" onclick={() => !isUnavailable && onPlay(row.position)} disabled={isUnavailable} + title={isMissingFile + ? 'This track’s file is missing from the library, so it can’t be played. It stays in the playlist, and returns automatically if the file comes back.' + : undefined} > -
{row.title}
+
{row.title}
{row.artist_name} · {row.album_title} + {#if isMissingFile} + · File missing + {/if}
diff --git a/web/src/lib/components/PlaylistTrackRow.test.ts b/web/src/lib/components/PlaylistTrackRow.test.ts index 5ea34d6b..f743db15 100644 --- a/web/src/lib/components/PlaylistTrackRow.test.ts +++ b/web/src/lib/components/PlaylistTrackRow.test.ts @@ -41,7 +41,8 @@ const live: PlaylistTrack = { album_title: 'MHTRTC', duration_sec: 137, stream_url: '/api/tracks/t-1/stream', - added_at: '' + added_at: '', + unavailable: false }; const removed: PlaylistTrack = { @@ -52,6 +53,15 @@ const removed: PlaylistTrack = { stream_url: null }; +// Missing file (#2527): the track row still exists — ids intact, history +// intact — but the server withholds the stream URL because the bytes are +// not on disk. Deliberately distinct from `removed` above. +const missingFile: PlaylistTrack = { + ...live, + stream_url: null, + unavailable: true +}; + afterEach(() => vi.clearAllMocks()); describe('PlaylistTrackRow', () => { @@ -119,4 +129,31 @@ describe('PlaylistTrackRow', () => { await fireEvent.click(screen.getByText('Roygbiv')); expect(onPlay).toHaveBeenCalledWith(2); }); + + // A missing file is NOT a removed track: the row says so, and says it + // without the strikethrough that would imply the track is gone for good. + test('missing file is labelled and not struck through', () => { + render(PlaylistTrackRow, { + props: { row: missingFile, isOwner: true, onRemove: vi.fn(), onPlay: vi.fn() } + }); + expect(screen.getByText(/file missing/i)).toBeTruthy(); + expect(screen.getByText('Roygbiv').className).not.toContain('line-through'); + }); + + test('clicking a missing-file row does not play it', async () => { + const onPlay = vi.fn(); + render(PlaylistTrackRow, { + props: { row: missingFile, isOwner: true, onRemove: vi.fn(), onPlay } + }); + await fireEvent.click(screen.getByText('Roygbiv')); + expect(onPlay).not.toHaveBeenCalled(); + }); + + // The owner must still be able to take the dead entry out of their list. + test('remove stays available on a missing-file row', () => { + render(PlaylistTrackRow, { + props: { row: missingFile, isOwner: true, onRemove: vi.fn(), onPlay: vi.fn() } + }); + expect(screen.getByLabelText(/remove roygbiv from playlist/i)).toBeTruthy(); + }); }); diff --git a/web/src/lib/playlists/playlistTrackToRef.test.ts b/web/src/lib/playlists/playlistTrackToRef.test.ts new file mode 100644 index 00000000..3bee4760 --- /dev/null +++ b/web/src/lib/playlists/playlistTrackToRef.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from 'vitest'; +import type { PlaylistTrack } from '$lib/api/types'; +import { playlistTrackToRef } from './playlistTrackToRef'; + +const row: PlaylistTrack = { + position: 0, + track_id: 't-1', + album_id: 'a-1', + artist_id: 'ar-1', + title: 'Roygbiv', + artist_name: 'Boards of Canada', + album_title: 'MHTRTC', + duration_sec: 137, + stream_url: '/api/tracks/t-1/stream', + added_at: '', + unavailable: false +}; + +describe('playlistTrackToRef', () => { + test('maps a playable row onto a TrackRef', () => { + expect(playlistTrackToRef(row)).toEqual({ + id: 't-1', + title: 'Roygbiv', + album_id: 'a-1', + album_title: 'MHTRTC', + artist_id: 'ar-1', + artist_name: 'Boards of Canada', + duration_sec: 137, + stream_url: '/api/tracks/t-1/stream' + }); + }); + + test('returns null when the upstream track was removed', () => { + expect(playlistTrackToRef({ ...row, track_id: null, stream_url: null })).toBeNull(); + }); + + // This is what makes a missing file "get skipped": every queue builder + // filters the nulls out of this mapper, so nothing downstream has to know + // about missing_since. If this stops returning null, the player will try + // to stream a file that is not there. + test('returns null when the file is missing, even with ids intact', () => { + expect(playlistTrackToRef({ ...row, unavailable: true, stream_url: null })).toBeNull(); + }); + + // Belt and braces: the server withholds stream_url for a missing file, but + // the flag alone must be enough — a client that trusted only the URL would + // happily queue a stale one from cache. + test('the flag alone disqualifies a row', () => { + expect(playlistTrackToRef({ ...row, unavailable: true })).toBeNull(); + }); +}); diff --git a/web/src/lib/playlists/playlistTrackToRef.ts b/web/src/lib/playlists/playlistTrackToRef.ts index 929c1d4e..2ff34dd6 100644 --- a/web/src/lib/playlists/playlistTrackToRef.ts +++ b/web/src/lib/playlists/playlistTrackToRef.ts @@ -2,13 +2,19 @@ // by play queues + track menus. Centralizes the mapping so server // additions (new TrackRef fields) only update one site. // -// Returns null when the upstream track has been removed (track_id -// is null on the playlist row); callers should filter nulls. +// Returns null when the row cannot be played, for either of the two +// reasons a playlist entry can outlive its track: +// - track_id is null — the track was removed from the library and only +// the playlist's text snapshot remains. +// - unavailable — the track is still there, with its history intact, +// but its file is missing from disk (#2527). +// Callers filter nulls, which is what makes a missing file "get skipped" +// everywhere a queue is built rather than at each call site. import type { PlaylistTrack, TrackRef } from '$lib/api/types'; export function playlistTrackToRef(row: PlaylistTrack): TrackRef | null { - if (!row.track_id) return null; + if (!row.track_id || row.unavailable) return null; return { id: row.track_id, title: row.title, diff --git a/web/src/routes/playlists/[id]/playlist.test.ts b/web/src/routes/playlists/[id]/playlist.test.ts index fd1fbfd6..21b638d5 100644 --- a/web/src/routes/playlists/[id]/playlist.test.ts +++ b/web/src/routes/playlists/[id]/playlist.test.ts @@ -52,8 +52,8 @@ const ownDetail: PlaylistDetail = { description: '', is_public: false, kind: 'user', system_variant: null, refreshable: false, seed_artist_id: null, cover_url: '', track_count: 2, duration_sec: 274, created_at: '', updated_at: '', tracks: [ - { position: 0, track_id: 't1', album_id: 'a1', artist_id: 'ar1', title: 'A', artist_name: 'X', album_title: 'Y', duration_sec: 137, stream_url: '/api/tracks/t1/stream', added_at: '' }, - { position: 1, track_id: 't2', album_id: 'a1', artist_id: 'ar1', title: 'B', artist_name: 'X', album_title: 'Y', duration_sec: 137, stream_url: '/api/tracks/t2/stream', added_at: '' } + { position: 0, track_id: 't1', album_id: 'a1', artist_id: 'ar1', title: 'A', artist_name: 'X', album_title: 'Y', duration_sec: 137, stream_url: '/api/tracks/t1/stream', added_at: '', unavailable: false }, + { position: 1, track_id: 't2', album_id: 'a1', artist_id: 'ar1', title: 'B', artist_name: 'X', album_title: 'Y', duration_sec: 137, stream_url: '/api/tracks/t2/stream', added_at: '', unavailable: false } ] };