From edf575e8fc4e4f2d496e1045f5a7e2c3d0b2740e Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:16:22 +0700 Subject: [PATCH] fix(playlists): preserve repeated tracks during imports --- lib/models/library_collections.dart | 26 ++++++++- .../library_collections_provider.dart | 6 +- lib/screens/playlist_screen.dart | 1 + lib/services/collection_track_batch.dart | 24 +++++++- lib/widgets/playlist_picker_sheet.dart | 21 ++++++- test/library_collections_mutation_test.dart | 55 +++++++++++++++++++ test/mornye_collection_sheets_test.dart | 5 +- 7 files changed, 125 insertions(+), 13 deletions(-) diff --git a/lib/models/library_collections.dart b/lib/models/library_collections.dart index 7bd51ae3..58809a35 100644 --- a/lib/models/library_collections.dart +++ b/lib/models/library_collections.dart @@ -23,6 +23,22 @@ String trackCollectionKey(Track track) => libraryTrackCollectionKey( isrc: track.isrc, ); +/// Stable entry keys preserve repeated positions when importing a playlist. +/// The first occurrence keeps the existing membership key, so re-importing +/// fills missing occurrences without appending the entire playlist again. +Iterable playlistImportKeys(Iterable tracks) sync* { + final occurrences = {}; + for (final track in tracks) { + final key = trackCollectionKey(track); + final count = occurrences.update( + key, + (value) => value + 1, + ifAbsent: () => 1, + ); + yield count == 1 ? key : 'occurrence:$count:$key'; + } +} + String collectionArtistId(String value) { final colonIndex = value.indexOf(':'); if (colonIndex <= 0 || colonIndex == value.length - 1) { @@ -255,10 +271,14 @@ class PlaylistPickerSummaryRequest { PlaylistPickerSummaryRequest._(this.trackKeys); - factory PlaylistPickerSummaryRequest.fromTracks(Iterable tracks) { + factory PlaylistPickerSummaryRequest.fromTracks( + Iterable tracks, { + bool preserveOccurrences = false, + }) { final keys = - tracks - .map(trackCollectionKey) + (preserveOccurrences + ? playlistImportKeys(tracks) + : tracks.map(trackCollectionKey)) .where((key) => key.trim().isNotEmpty) .toSet() .toList(growable: false) diff --git a/lib/providers/library_collections_provider.dart b/lib/providers/library_collections_provider.dart index 140bc99f..db35fdf9 100644 --- a/lib/providers/library_collections_provider.dart +++ b/lib/providers/library_collections_provider.dart @@ -519,8 +519,9 @@ class LibraryCollectionsNotifier extends Notifier { Future addTracksToPlaylist( String playlistId, - Iterable tracks, - ) => _mutate(() async { + Iterable tracks, { + bool preserveOccurrences = false, + }) => _mutate(() async { await ensurePlaylistLoaded(playlistId); if (!ref.mounted) { return const PlaylistAddBatchResult( @@ -541,6 +542,7 @@ class LibraryCollectionsNotifier extends Notifier { tracks: tracks, existingKeys: playlist.trackKeys, addedAt: now, + preserveOccurrences: preserveOccurrences, ); try { if (!ref.mounted || prepared.entries.isEmpty) { diff --git a/lib/screens/playlist_screen.dart b/lib/screens/playlist_screen.dart index 4dd1623b..61b488a3 100644 --- a/lib/screens/playlist_screen.dart +++ b/lib/screens/playlist_screen.dart @@ -558,6 +558,7 @@ class _PlaylistScreenState extends ConsumerState ref, _tracks, playlistNamePrefill: widget.playlistName, + preserveOccurrences: true, ), ); } diff --git a/lib/services/collection_track_batch.dart b/lib/services/collection_track_batch.dart index 96dc0ce5..d7ff5f7e 100644 --- a/lib/services/collection_track_batch.dart +++ b/lib/services/collection_track_batch.dart @@ -44,6 +44,7 @@ Future prepareCollectionTrackBatch({ required Iterable tracks, required Set existingKeys, required DateTime addedAt, + bool preserveOccurrences = false, bool? useNative, Directory? temporaryDirectory, }) async { @@ -58,10 +59,22 @@ Future prepareCollectionTrackBatch({ } try { if (snapshot.length < _workerThreshold) { - return _prepare(snapshot, existingKeys, addedAt, inputDirectory); + return _prepare( + snapshot, + existingKeys, + addedAt, + inputDirectory, + preserveOccurrences, + ); } return await Isolate.run( - () => _prepare(snapshot, existingKeys, addedAt, inputDirectory), + () => _prepare( + snapshot, + existingKeys, + addedAt, + inputDirectory, + preserveOccurrences, + ), ); } catch (_) { if (inputDirectory != null) await _removeDirectory(inputDirectory); @@ -74,12 +87,17 @@ PreparedCollectionTrackBatch _prepare( Set existingKeys, DateTime addedAt, String? inputDirectory, + bool preserveOccurrences, ) { final keys = {}; final entries = []; var duplicates = 0; + final importKeys = preserveOccurrences + ? playlistImportKeys(tracks).iterator + : null; for (final track in tracks) { - final key = trackCollectionKey(track); + importKeys?.moveNext(); + final key = importKeys?.current ?? trackCollectionKey(track); if (existingKeys.contains(key) || !keys.add(key)) { duplicates++; } else { diff --git a/lib/widgets/playlist_picker_sheet.dart b/lib/widgets/playlist_picker_sheet.dart index 2d0dde89..f035c295 100644 --- a/lib/widgets/playlist_picker_sheet.dart +++ b/lib/widgets/playlist_picker_sheet.dart @@ -29,6 +29,7 @@ Future showAddTracksToPlaylistSheet( WidgetRef ref, List tracks, { String? playlistNamePrefill, + bool preserveOccurrences = false, }) async { if (tracks.isEmpty) return; @@ -44,6 +45,7 @@ Future showAddTracksToPlaylistSheet( final content = _PlaylistPickerSheetContent( tracks: tracks, playlistNamePrefill: playlistNamePrefill, + preserveOccurrences: preserveOccurrences, ); return sheetContext.isMornye ? MornyeGlassPanel(tintOpacity: 0.78, child: content) @@ -55,10 +57,12 @@ Future showAddTracksToPlaylistSheet( class _PlaylistPickerSheetContent extends ConsumerStatefulWidget { final List tracks; final String? playlistNamePrefill; + final bool preserveOccurrences; const _PlaylistPickerSheetContent({ required this.tracks, this.playlistNamePrefill, + required this.preserveOccurrences, }); @override @@ -76,7 +80,10 @@ class _PlaylistPickerSheetContentState @override void initState() { super.initState(); - _summaryRequest = PlaylistPickerSummaryRequest.fromTracks(widget.tracks); + _summaryRequest = PlaylistPickerSummaryRequest.fromTracks( + widget.tracks, + preserveOccurrences: widget.preserveOccurrences, + ); } Future _handleDone(List playlists) async { @@ -96,7 +103,11 @@ class _PlaylistPickerSheetContentState try { for (final playlistId in idsToAdd) { - await notifier.addTracksToPlaylist(playlistId, widget.tracks); + await notifier.addTracksToPlaylist( + playlistId, + widget.tracks, + preserveOccurrences: widget.preserveOccurrences, + ); _committedPlaylistIds.add(playlistId); final playlistName = playlistNamesById[playlistId]; if (playlistName != null && playlistName.isNotEmpty) { @@ -217,7 +228,11 @@ class _PlaylistPickerSheetContentState return; } final playlistId = await notifier.createPlaylist(name.trim()); - await notifier.addTracksToPlaylist(playlistId, widget.tracks); + await notifier.addTracksToPlaylist( + playlistId, + widget.tracks, + preserveOccurrences: widget.preserveOccurrences, + ); if (!mounted) return; setState(() { _committedPlaylistIds.add(playlistId); diff --git a/test/library_collections_mutation_test.dart b/test/library_collections_mutation_test.dart index fa0a5c8e..6412ea0e 100644 --- a/test/library_collections_mutation_test.dart +++ b/test/library_collections_mutation_test.dart @@ -249,6 +249,61 @@ ProviderContainer _container(_CollectionsDatabase db) { } void main() { + test( + 'playlist import retains all 202 positions and re-import is idempotent', + () async { + final db = _CollectionsDatabase(); + db.playlists['playlist'] = { + 'id': 'playlist', + 'name': 'Playlist', + 'created_at': '2026-01-01T00:00:00Z', + 'updated_at': '2026-01-01T00:00:00Z', + }; + db.releasePlaylistRead.complete(); + db.releaseWrite.complete(); + final container = _container(db); + final notifier = container.read(libraryCollectionsProvider.notifier); + final tracks = [ + for (var i = 0; i < 202; i++) + _track('$i').copyWith(isrc: 'RECORDING${i % 198}'), + ]; + final first = await notifier.addTracksToPlaylist( + 'playlist', + tracks, + preserveOccurrences: true, + ); + expect(first.addedCount, 202); + final imported = container + .read(libraryCollectionsProvider) + .playlists + .single; + expect(imported.trackCount, 202); + expect( + imported.tracks.map((entry) => entry.track.id), + tracks.map((track) => track.id), + ); + expect(db.playlistTracks['playlist'], hasLength(202)); + final request = PlaylistPickerSummaryRequest.fromTracks( + tracks, + preserveOccurrences: true, + ); + expect(request.trackKeys.toSet(), imported.trackKeys); + final second = await notifier.addTracksToPlaylist( + 'playlist', + tracks, + preserveOccurrences: true, + ); + expect(second.addedCount, 0); + expect(second.alreadyInPlaylistCount, 202); + // Legacy playlists with the original 198 keys receive the four missing entries. + final legacyKeys = tracks.map(trackCollectionKey).toSet(); + expect( + request.trackKeys.where((key) => !legacyKeys.contains(key)), + hasLength(4), + ); + }, + ); + for (final failFirst in [false, true]) { test( 'playlist bulk deletion snapshots IDs and serializes retries (fail: $failFirst)', diff --git a/test/mornye_collection_sheets_test.dart b/test/mornye_collection_sheets_test.dart index 715f7b78..0bb18c17 100644 --- a/test/mornye_collection_sheets_test.dart +++ b/test/mornye_collection_sheets_test.dart @@ -51,8 +51,9 @@ class _Collections extends LibraryCollectionsNotifier { @override Future addTracksToPlaylist( String playlistId, - Iterable tracks, - ) async { + Iterable tracks, { + bool preserveOccurrences = false, + }) async { additions.add(playlistId); if (pendingAdditions.isNotEmpty) { await pendingAdditions.removeAt(0).future;