feat(web): a playlist entry whose file is missing greys out and is skipped — #2527
test-web / test (push) Successful in 33s
test-web / test (push) Successful in 33s
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.
This commit is contained in:
@@ -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 & {
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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}
|
||||
>
|
||||
<div class="truncate {isUnavailable ? 'line-through' : ''}">{row.title}</div>
|
||||
<div class="truncate {isRemoved ? 'line-through' : ''}">{row.title}</div>
|
||||
<div class="truncate text-xs text-text-muted">
|
||||
{row.artist_name} · {row.album_title}
|
||||
{#if isMissingFile}
|
||||
<span class="text-action-destructive"> · File missing</span>
|
||||
{/if}
|
||||
</div>
|
||||
</button>
|
||||
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
|
||||
@@ -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 }
|
||||
]
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user