diff --git a/backend/library/remove_tracks.go b/backend/library/remove_tracks.go index 7e40ca1..903bf23 100644 --- a/backend/library/remove_tracks.go +++ b/backend/library/remove_tracks.go @@ -129,6 +129,15 @@ func (l *Library) RemoveFromLibrary(filePaths []string) (*RemovalResult, error) } } + // Deleting an audio_files row cascades to queue_tracks, so the + // queue's in-memory copy now holds tracks the database does not — + // including, possibly, the one playing. This is the same reload + // RemoveLibrary does, and it unloads the player if the current + // track was among them. + if l.removalHooks.CompactQueue != nil { + l.removalHooks.CompactQueue() + } + // An album, artist or genre whose last track just went is now a row // with nothing behind it, and the album list selects from // release_groups rather than from audio_files — so it would keep diff --git a/backend/library/remove_tracks_test.go b/backend/library/remove_tracks_test.go index cd764a9..ec7103b 100644 --- a/backend/library/remove_tracks_test.go +++ b/backend/library/remove_tracks_test.go @@ -257,6 +257,32 @@ func TestRemoveFromLibrary_EmitsPatchablePayload(t *testing.T) { } } +// TestRemoveFromLibrary_CompactsTheQueue pins the half that is invisible +// from the track list: deleting an audio_files row cascades to +// queue_tracks, so the queue's in-memory copy — and the player, if it +// was the track playing — has to be told. +func TestRemoveFromLibrary_CompactsTheQueue(t *testing.T) { + t.Parallel() + + lib, dir, paths, _, libID := setupScanLibrary(t, 2) + + lib.scanInternal(libID, "Test", dir) + + compacted := 0 + + lib.SetRemovalHooks(RemovalHooks{ + CompactQueue: func() { compacted++ }, + }) + + if _, err := lib.RemoveFromLibrary([]string{paths[0]}); err != nil { + t.Fatalf("RemoveFromLibrary: %v", err) + } + + if compacted != 1 { + t.Errorf("CompactQueue called %d times, want 1", compacted) + } +} + // TestRemoveFromLibrary_RejectsAnEmptyRequest keeps a stray Delete on // an empty selection from reaching the database at all. func TestRemoveFromLibrary_RejectsAnEmptyRequest(t *testing.T) { diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts index a82b071..7d2a8eb 100644 --- a/frontend/src/components/track-list/track-list.ts +++ b/frontend/src/components/track-list/track-list.ts @@ -62,6 +62,9 @@ import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js' import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js'; import '@awesome.me/webawesome/dist/components/icon/icon.js'; import { describeError } from '@utils/describe-error'; +import { notificationStore } from '@store/notification-store'; +import { confirmAction } from '@components/confirm-dialog/confirm-dialog'; +import { RemoveFromLibrary } from '@go/library/Library'; import { loadTrackDetails } from '@utils/lazy-track-details.js'; import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js'; import '@components/playlist-picker/playlist-picker.js'; @@ -1615,12 +1618,79 @@ export class TrackList void this.openBatchTrackDetails(filePaths); } break; + case 'remove-from-library': + // The only destructive command in this menu: it asks + // first, and it keeps the selection until the user has + // answered — the dialog names a count, and clearing the + // selection under it would make that count a claim + // about nothing. + this.ctxMenu.close(); + void this.removeFromLibrary(filePaths); + + return; } this.selection.clear(); this.ctxMenu.close(); } + /** + * "Remove from library", behind a confirmation that says what it + * does *and* what it does not. + * + * The second half is the point. This deletes the database rows and + * stops the scanner importing those paths again; the audio files + * are left exactly where they are. A user who reads "remove" as + * "delete" and finds their music gone would have been failed by the + * copy, not by the operation — so the copy says so in the impact + * line, where the consequence of every other destructive action in + * the app is written. + */ + private async removeFromLibrary(filePaths: string[]) { + const count = filePaths.length; + const only = + count === 1 + ? tracksByFilePath(this.tracks).get(filePaths[0]!) + : undefined; + + const ok = await confirmAction({ + title: + count === 1 + ? `Remove “${only?.TrackName ?? filePaths[0]!}” from the library?` + : `Remove ${count.toLocaleString()} tracks from the library?`, + message: + count === 1 + ? 'It is removed from YellowJacket and will not be added' + + ' back by a future scan.' + : 'They are removed from YellowJacket and will not be' + + ' added back by a future scan.', + impact: + count === 1 + ? 'The file is not deleted — it stays on disk exactly' + + ' where it is. A full rescan brings it back.' + : 'The files are not deleted — they stay on disk exactly' + + ' where they are. A full rescan brings them back.', + confirmLabel: + count === 1 + ? 'Remove track' + : `Remove ${count.toLocaleString()} tracks`, + danger: true, + }); + + if (!ok) return; + + try { + await RemoveFromLibrary(filePaths); + this.selection.clear(); + } catch (error) { + console.error('Error removing tracks from library:', error); + notificationStore.persistent({ + title: 'Could not remove from library', + text: `${count === 1 ? 'That track is' : `Those ${count.toLocaleString()} tracks are`} still in your library. ${describeError(error)}`, + }); + } + } + private onContextMenuFavoriteToggle() { const filePaths = this.selection.getSelectedKeysOrdered(); @@ -2081,6 +2151,20 @@ export class TrackList > Track Details + + this.onContextMenuAction( + 'remove-from-library', + )} + @mouseenter=${() => + this.ctxMenu.closePlaylistSubmenu()} + > + + Remove from Library + ` : nothing} diff --git a/frontend/src/store/library-store.ts b/frontend/src/store/library-store.ts index 0a72db9..0f79282 100644 --- a/frontend/src/store/library-store.ts +++ b/frontend/src/store/library-store.ts @@ -108,6 +108,9 @@ class LibraryStore { EventsOn(Events.TrackPlayCountChanged, (payload: unknown) => { this.applyPlayCount(payload); }); + EventsOn(Events.TracksRemovedFromLibrary, (payload: unknown) => { + this.applyTracksRemoved(payload); + }); this.loadCoverSize(); this.deferEagerFetch(); @@ -559,6 +562,71 @@ class LibraryStore { this.notify(); } + /** + * Splice removed tracks out in place, and refetch only the + * summaries whose counts changed. + * + * `invalidate()` would be correct and is the expensive answer: it + * nulls `tracks` and eagerly refetches it, which is ~37 MB across + * the IPC at 50 000 tracks for an operation that removed three + * rows. The event carries the paths precisely so this does not have + * to happen — the same bargain `TrackPlayCountChanged` makes. + * + * The album, artist and genre lists really do change (their track + * counts, and the row itself when its last track goes), so they are + * dropped and refetched. They are the small collections. + */ + private applyTracksRemoved(payload: unknown): void { + const p = payload as { filePaths?: string[] } | null; + const removed = p?.filePaths; + + if (!removed || removed.length === 0) return; + + // A tracks fetch already in flight would land holding the rows + // that were just deleted, and it captured the cache generation + // this patch is about to leave behind. There is no patch that + // is equivalent to that, so fall back. + if (this.inFlight.has('tracks')) { + this.invalidate(); + + return; + } + + if (this.tracks !== null) { + const gone = new Set(removed); + const kept = this.tracks.filter((t) => !gone.has(t.FilePath)); + + // A new array identity even when nothing matched would + // invalidate every memoized filter/sort cache keyed on it + // for no reason. + if (kept.length !== this.tracks.length) { + this.tracks = kept; + } + } + + this.albums = null; + this.artists = null; + this.genres = null; + // Bumping the cache generation is what stops an album fetch + // issued before the removal from committing its pre-removal + // answer. Safe for the tracks slot precisely because the guard + // above established there is nothing in flight for it. + this.cacheGen++; + this.inFlight.delete('albums'); + this.inFlight.delete('artists'); + this.inFlight.delete('genres'); + + this.changeGen++; + this.notify(); + + const logged = (what: string) => (err: unknown) => + console.error(`library: could not reload ${what}`, err); + + void this.getAlbums().catch(logged('albums')); + void this.getArtists().catch(logged('artists')); + void this.getGenres().catch(logged('genres')); + } + private invalidate(): void { this.tracks = null; this.albums = null; diff --git a/frontend/test/stores/library-store.test.ts b/frontend/test/stores/library-store.test.ts index 526407c..854be49 100644 --- a/frontend/test/stores/library-store.test.ts +++ b/frontend/test/stores/library-store.test.ts @@ -175,6 +175,55 @@ describe('library store: caching', () => { expect(calls()).toEqual([]); }); + /** + * Removing tracks is the same bargain the play count makes, one + * collection wider: the event carries the paths so the tracks array + * — the expensive one — is patched rather than refetched, while the + * album/artist/genre summaries, whose counts really did change, are + * dropped and reloaded. + */ + describe('tracks removed from the library', () => { + beforeEach(async () => { + emit(Events.TracksRemovedFromLibrary, { + filePaths: ['/a.mp3'], + count: 1, + }); + await flush(); + }); + + it('does not refetch the tracks', () => { + expect(calls('library.Library.GetAllTracks')).toHaveLength(0); + }); + + it('splices the removed track out in place', () => { + expect(libraryStore.getCachedTracks()?.map((t) => t.FilePath)).toEqual([ + '/b.mp3', + ]); + }); + + it('reloads the summaries, whose counts changed', () => { + expect( + [ + 'library.Library.GetAllAlbums', + 'library.Library.GetAllArtists', + 'library.Library.GetAllGenresWithCounts', + ].map((path) => calls(path).length), + ).toEqual([1, 1, 1]); + }); + }); + + it('ignores a removal naming a track it does not hold, without dropping the array', async () => { + const before = libraryStore.getCachedTracks(); + + emit(Events.TracksRemovedFromLibrary, { + filePaths: ['/not-in-this-library.mp3'], + count: 1, + }); + await flush(); + + expect(libraryStore.getCachedTracks()).toBe(before); + }); + it('resets scroll positions on invalidation, so a shorter list is not scrolled past its end', async () => { libraryStore.setScrollPosition('albums', 4200); emit(Events.LibraryScanComplete);