fix(playlists): batch multi-select adds into one playlist edit request#3658
fix(playlists): batch multi-select adds into one playlist edit request#3658Shrawan13-glitch wants to merge 6 commits intoMetrolistGroup:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds multi-item playlist operations and centralizes add-and-sync logic: dialog signature gains optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Menu as Menu/Caller
participant Dialog as AddToPlaylistDialog
participant VM as PlaylistsViewModel
participant DB as LocalDatabase
participant YT as YouTube/InnerTube
User->>Menu: select playlist & confirm
Menu->>Dialog: invoke onGetSong() -> songIds
activate Dialog
Dialog->>DB: transaction: insert/check local songs
DB-->>Dialog: localIds
Dialog->>Dialog: resolve & dedupe -> distinctIds
Dialog->>VM: addSongsAndSync(targetPlaylist, distinctIds, multiSelectParams)
activate VM
VM->>YT: getMultiSelectCommand(selectedItems, multiSelectParams) [optional]
YT-->>VM: resolvedVideoIds
alt resolvedVideoIds present
VM->>YT: addToPlaylist(playlistId, resolvedVideoIds) //-- batched
else
VM->>YT: addToPlaylist(playlistId, distinctIds) //-- fallback
end
VM->>VM: register/unregister pending-adds (finally)
YT-->>VM: success/failure
VM-->>Dialog: done
deactivate VM
Dialog-->>User: finished
deactivate Dialog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/kotlin/com/metrolist/music/ui/menu/SelectionSongsMenu.kt (1)
140-149:⚠️ Potential issue | 🔴 CriticalCritical:
addSongsAndSyncis launched on a dialog-scoped coroutine that will be cancelled when the dialog is dismissed.The implementation currently launches
addSongsAndSynconrememberCoroutineScope()(line 92, AddToPlaylistDialog.kt), which is tied to the dialog's composition lifecycle. At all three call sites (lines 322, 362, 377),onDismiss()is invoked before or within the samecoroutineScope.launchblock, causing the dialog to leave composition whileaddSongsAndSyncis still running. Once the composable is removed from the tree, the scope is cancelled, interrupting any pending database transactions and YouTube API calls.To properly fix
#3167("adds proceed continuously in background"),addSongsAndSyncmust be launched on a scope that outlives the dialog—such as a viewmodel scope or an application-scoped sync job—before the dialog is dismissed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/menu/SelectionSongsMenu.kt` around lines 140 - 149, The dialog currently calls addSongsAndSync from a dialog-scoped coroutine (via rememberCoroutineScope in AddToPlaylistDialog) so the job is cancelled when the dialog is dismissed; change the call sites to launch addSongsAndSync on a longer-lived scope (e.g. a ViewModelScope or an application-scoped sync job) before dismissing the dialog. Specifically, in AddToPlaylistDialog replace usage of rememberCoroutineScope-launched addSongsAndSync with a call into a ViewModel method (or inject a SyncManager) that performs addSongsAndSync using viewModelScope (or an app-level scope), and update the three callers (SelectionSongsMenu's usages where onDismiss and coroutine launch occur) to invoke that ViewModel/SyncManager method and only then set showChoosePlaylistDialog = false.
🧹 Nitpick comments (5)
app/src/main/kotlin/com/metrolist/music/ui/menu/PlayerMenu.kt (1)
168-177: Consider extracting a shared helper for the repeatedonGetSongtransaction+ID pattern.This exact lambda shape is now duplicated across several menus. A small helper (e.g.,
prepareSongForPlaylist(...)) would reduce drift when this contract changes again.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/menu/PlayerMenu.kt` around lines 168 - 177, Extract a shared helper to avoid duplicating the transaction+ID pattern used in AddToPlaylistDialog's onGetSong and similar lambdas: create a function (e.g., prepareSongForPlaylist(mediaMetadata: MediaMetadata, database: YourDatabaseType): List<String>) that runs database.withTransaction { insert(mediaMetadata) } and returns listOf(mediaMetadata.id); then replace the inline lambdas for onGetSong and onGetSongIds with calls to prepareSongForPlaylist (or a small wrapper for onGetSongIds if it only needs the IDs) so AddToPlaylistDialog, other menus, and any future call sites use the single helper.app/src/main/kotlin/com/metrolist/music/ui/menu/YouTubePlaylistMenu.kt (1)
147-149: Minor:onGetSongIdsmay return an empty list whensongsis empty.When
YouTubePlaylistMenuis invoked without a preloadedsongslist,onGetSonglazily fetches fromYouTube.playlist(playlist.id).completed(), butonGetSongIdsreturnssongs.map { it.id }directly — which yields an empty list. The dialog'splaylistsContainingSongLaunchedEffect (which only consumesonGetSongIdsfor read-only highlighting) will therefore not highlight any playlists in this code path, even though the songs would be retrievable.Since
onGetSongIdsmust remain side-effect-free (no network calls on dialog open per repo learnings), one option is to omitonGetSongIdshere so the dialog falls back to invokingonGetSong(with its full resolution) for highlighting. Optional — not a regression introduced by this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/menu/YouTubePlaylistMenu.kt` around lines 147 - 149, The onGetSongIds handler in YouTubePlaylistMenu currently returns songs.map { it.id } which yields an empty list when the local songs list is not preloaded; remove or omit setting onGetSongIds so the dialog will fallback to using onGetSong (which lazily resolves via YouTube.playlist(playlist.id).completed()) for highlighting, thereby avoiding returning an empty, misleading list to the playlistsContainingSong LaunchedEffect.innertube/src/main/kotlin/com/metrolist/innertube/InnerTube.kt (1)
511-528: LGTM — overload correctly batches multipleAddVideoActions in oneedit_playlistrequest.The new overload doesn't guard against
videoIds.isEmpty(), which would post anactions = emptyList()request. The current caller (addSongsAndSyncinAddToPlaylistDialog) only invokes this overload whenlocalIds.size > 1, so this is safe — butrequire(videoIds.isNotEmpty())would make the API more robust if reused later.Regarding batch size: no documented limit on
AddVideoActioncount per request exists. The ytmusicapi library (reverse-engineered YouTube Music API client) handles batch adds without hardcoded limits, and examples show the approach working for reasonable selections (e.g., albums). The 70+ songs use case should work without chunking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@innertube/src/main/kotlin/com/metrolist/innertube/InnerTube.kt` around lines 511 - 528, The addToPlaylist overload currently allows videoIds to be empty and would send an edit_playlist request with actions = emptyList(); add a precondition check in addToPlaylist (e.g., at start of the suspend fun addToPlaylist) such as require(videoIds.isNotEmpty()) with a clear message (or throw IllegalArgumentException) to prevent empty-batch requests; ensure the check references the function name addToPlaylist and leaves the existing body (httpClient.post(...) / EditPlaylistBody / Action.AddVideoAction) unchanged.app/src/main/kotlin/com/metrolist/music/ui/menu/YouTubeSelectionSongMenu.kt (1)
124-137: Remove themultiSelectParamsconcern; consider addingonGetSongIdsin a follow-up.The
multiSelectParamssuggestion is not applicable here.SongItemcarries nomultiSelectParamsfield, andOnlinePlaylistScreendoes not pass one toYouTubeSelectionSongMenu. There is no token available to thread through.Adding
onGetSongIds = { selectedSongs.map { it.id } }would enable highlight-only reads without side effects on dialog open. This aligns with the deferred refactor pattern noted for selection-based menus, so it can be addressed in a follow-up if the maintainer prioritizes splitting the read path from the write path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/menu/YouTubeSelectionSongMenu.kt` around lines 124 - 137, Remove references to the non-existent multiSelectParams concern in YouTubeSelectionSongMenu and instead add a new read-only callback to the AddToPlaylistDialog such as onGetSongIds = { selectedSongs.map { it.id } } so highlight-only reads can occur without side effects; keep the existing onGetSong lambda for the write path (database.withTransaction { selectedSongs.forEach(::insert) }) and preserve onDismiss handling, and update AddToPlaylistDialog usage/definition to accept this new onGetSongIds parameter while leaving SongItem and OnlinePlaylistScreen unchanged.innertube/src/main/kotlin/com/metrolist/innertube/YouTube.kt (1)
2600-2606: Nit: drop the redundant localresponsevariable.The intermediate
val response = ...; responsedoesn't add anything — the call expression itself is already the last expression of therunCatchinglambda.♻️ Optional simplification
suspend fun getMultiSelectCommand( selectedItems: List<String>, multiSelectParams: String? = null, ) = runCatching { - val response = innerTube.getMultiSelectCommand(WEB_REMIX, selectedItems, multiSelectParams).body<GetMultiSelectCommandResponse>() - response + innerTube.getMultiSelectCommand(WEB_REMIX, selectedItems, multiSelectParams) + .body<GetMultiSelectCommandResponse>() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@innertube/src/main/kotlin/com/metrolist/innertube/YouTube.kt` around lines 2600 - 2606, In getMultiSelectCommand, remove the redundant local val response and return the call directly; replace the block inside runCatching so it directly returns innerTube.getMultiSelectCommand(WEB_REMIX, selectedItems, multiSelectParams).body<GetMultiSelectCommandResponse>() instead of assigning to response and then returning it, keeping the suspend function signature and runCatching usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/kotlin/com/metrolist/music/ui/menu/AddToPlaylistDialog.kt`:
- Around line 130-141: The code calls YouTube.getMultiSelectCommand even when
multiSelectParams is null, causing unnecessary network requests; change the
condition that computes remoteIds to only call YouTube.getMultiSelectCommand
when both localIds.size > 1 and multiSelectParams != null (i.e., gate the call
on multiSelectParams), falling back to localIds otherwise; update the remoteIds
computation around getMultiSelectCommand/multiSelectParams to avoid the extra
network hop and preserve the existing orEmpty().ifEmpty { localIds } fallback
logic.
---
Outside diff comments:
In `@app/src/main/kotlin/com/metrolist/music/ui/menu/SelectionSongsMenu.kt`:
- Around line 140-149: The dialog currently calls addSongsAndSync from a
dialog-scoped coroutine (via rememberCoroutineScope in AddToPlaylistDialog) so
the job is cancelled when the dialog is dismissed; change the call sites to
launch addSongsAndSync on a longer-lived scope (e.g. a ViewModelScope or an
application-scoped sync job) before dismissing the dialog. Specifically, in
AddToPlaylistDialog replace usage of rememberCoroutineScope-launched
addSongsAndSync with a call into a ViewModel method (or inject a SyncManager)
that performs addSongsAndSync using viewModelScope (or an app-level scope), and
update the three callers (SelectionSongsMenu's usages where onDismiss and
coroutine launch occur) to invoke that ViewModel/SyncManager method and only
then set showChoosePlaylistDialog = false.
---
Nitpick comments:
In `@app/src/main/kotlin/com/metrolist/music/ui/menu/PlayerMenu.kt`:
- Around line 168-177: Extract a shared helper to avoid duplicating the
transaction+ID pattern used in AddToPlaylistDialog's onGetSong and similar
lambdas: create a function (e.g., prepareSongForPlaylist(mediaMetadata:
MediaMetadata, database: YourDatabaseType): List<String>) that runs
database.withTransaction { insert(mediaMetadata) } and returns
listOf(mediaMetadata.id); then replace the inline lambdas for onGetSong and
onGetSongIds with calls to prepareSongForPlaylist (or a small wrapper for
onGetSongIds if it only needs the IDs) so AddToPlaylistDialog, other menus, and
any future call sites use the single helper.
In `@app/src/main/kotlin/com/metrolist/music/ui/menu/YouTubePlaylistMenu.kt`:
- Around line 147-149: The onGetSongIds handler in YouTubePlaylistMenu currently
returns songs.map { it.id } which yields an empty list when the local songs list
is not preloaded; remove or omit setting onGetSongIds so the dialog will
fallback to using onGetSong (which lazily resolves via
YouTube.playlist(playlist.id).completed()) for highlighting, thereby avoiding
returning an empty, misleading list to the playlistsContainingSong
LaunchedEffect.
In `@app/src/main/kotlin/com/metrolist/music/ui/menu/YouTubeSelectionSongMenu.kt`:
- Around line 124-137: Remove references to the non-existent multiSelectParams
concern in YouTubeSelectionSongMenu and instead add a new read-only callback to
the AddToPlaylistDialog such as onGetSongIds = { selectedSongs.map { it.id } }
so highlight-only reads can occur without side effects; keep the existing
onGetSong lambda for the write path (database.withTransaction {
selectedSongs.forEach(::insert) }) and preserve onDismiss handling, and update
AddToPlaylistDialog usage/definition to accept this new onGetSongIds parameter
while leaving SongItem and OnlinePlaylistScreen unchanged.
In `@innertube/src/main/kotlin/com/metrolist/innertube/InnerTube.kt`:
- Around line 511-528: The addToPlaylist overload currently allows videoIds to
be empty and would send an edit_playlist request with actions = emptyList(); add
a precondition check in addToPlaylist (e.g., at start of the suspend fun
addToPlaylist) such as require(videoIds.isNotEmpty()) with a clear message (or
throw IllegalArgumentException) to prevent empty-batch requests; ensure the
check references the function name addToPlaylist and leaves the existing body
(httpClient.post(...) / EditPlaylistBody / Action.AddVideoAction) unchanged.
In `@innertube/src/main/kotlin/com/metrolist/innertube/YouTube.kt`:
- Around line 2600-2606: In getMultiSelectCommand, remove the redundant local
val response and return the call directly; replace the block inside runCatching
so it directly returns innerTube.getMultiSelectCommand(WEB_REMIX, selectedItems,
multiSelectParams).body<GetMultiSelectCommandResponse>() instead of assigning to
response and then returning it, keeping the suspend function signature and
runCatching usage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 645d32a4-50aa-4b3b-85af-23856c995ef5
📒 Files selected for processing (16)
app/src/main/kotlin/com/metrolist/music/ui/menu/AddToPlaylistDialog.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/AlbumMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/PlayerMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/QueueMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/SelectionSongsMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/SongMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/YouTubeAlbumMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/YouTubePlaylistMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/YouTubeSelectionSongMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/menu/YouTubeSongMenu.ktapp/src/main/kotlin/com/metrolist/music/ui/player/MiniPlayer.ktinnertube/src/main/kotlin/com/metrolist/innertube/InnerTube.ktinnertube/src/main/kotlin/com/metrolist/innertube/YouTube.ktinnertube/src/main/kotlin/com/metrolist/innertube/models/body/EditPlaylistBody.ktinnertube/src/main/kotlin/com/metrolist/innertube/models/body/GetMultiSelectCommandBody.ktinnertube/src/main/kotlin/com/metrolist/innertube/models/response/GetMultiSelectCommandResponse.kt
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/kotlin/com/metrolist/music/viewmodels/PlaylistsViewModel.kt (2)
82-92: Consider loggingYouTube.addToPlaylistfailures.Both overloads return
Result<…>(seeinnertube/.../YouTube.kt:2586-2598) but the result is discarded. On a network/API failure for a synced playlist, songs land in the local DB but never reach YouTube;unregisterPendingAddthen polls for ~30 s, finds nothing, and silently unregisters. That's a regression risk for the#2484reliability fix this PR targets — at minimum, log the failure so it's diagnosable from logs/Timber.♻️ Proposed change
try { - if (remoteIds.size == 1) { - YouTube.addToPlaylist(browseId, remoteIds.first()) - } else { - YouTube.addToPlaylist(browseId, remoteIds) - } + val result = if (remoteIds.size == 1) { + YouTube.addToPlaylist(browseId, remoteIds.first()) + } else { + YouTube.addToPlaylist(browseId, remoteIds) + } + result.onFailure { + Timber.e(it, "addSongsAndSync: addToPlaylist failed for browseId=$browseId, ${remoteIds.size} ids") + } } finally { remoteIds.forEach { songId -> syncUtils.unregisterPendingAdd(browseId, songId) } }(Add
import timber.log.Timberif not already present.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/viewmodels/PlaylistsViewModel.kt` around lines 82 - 92, The call to YouTube.addToPlaylist in PlaylistsViewModel discards its Result so failures are invisible; change the branches that call YouTube.addToPlaylist (both the overload taking a single id and the one taking a list) to capture the returned Result, check for failure, and log the error using Timber (include contextual info like browseId and song id(s)). After calling addToPlaylist, if result.isFailure (or result.exceptionOrNull() != null) call Timber.e(...) with the exception and a clear message; keep the existing finally block that calls syncUtils.unregisterPendingAdd.
62-94: Guard against emptyidsto avoid a wasteful YouTube edit call.
addSongsAndSyncis reachable from the dialog's "Skip duplicates" path with an empty filtered list (songIds!!.filter { !duplicates.contains(it) }when all selections are duplicates) and from anyonGetSongthat returns an empty list. In that casedatabase.addSongsToPlaylistis a no-op, but for synced playlists you still hitYouTube.addToPlaylist(browseId, emptyList())(line 86), which sends an emptyeditPlaylistrequest. An early return after resolvinglocalIdsis cheap insurance.♻️ Proposed change
fun addSongsAndSync( targetPlaylist: Playlist, ids: List<String>, multiSelectParams: String? = null, ) { viewModelScope.launch(Dispatchers.IO) { val localIds = ids.distinct() + if (localIds.isEmpty()) return@launch database.addSongsToPlaylist(targetPlaylist, localIds.map { it to null }, prepend = true) val browseId = targetPlaylist.playlist.browseId ?: return@launch🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/viewmodels/PlaylistsViewModel.kt` around lines 62 - 94, The addSongsAndSync handler should bail out when the computed localIds is empty to avoid sending an empty YouTube edit request; after computing val localIds = ids.distinct() add a guard like if (localIds.isEmpty()) return@launch so you skip database.addSongsToPlaylist, the sync registration loop, and the YouTube.addToPlaylist calls (refer to addSongsAndSync / viewModelScope.launch, localIds, database.addSongsToPlaylist, syncUtils.registerPendingAdd/unregisterPendingAdd, and YouTube.addToPlaylist).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/kotlin/com/metrolist/music/viewmodels/PlaylistsViewModel.kt`:
- Around line 82-92: The call to YouTube.addToPlaylist in PlaylistsViewModel
discards its Result so failures are invisible; change the branches that call
YouTube.addToPlaylist (both the overload taking a single id and the one taking a
list) to capture the returned Result, check for failure, and log the error using
Timber (include contextual info like browseId and song id(s)). After calling
addToPlaylist, if result.isFailure (or result.exceptionOrNull() != null) call
Timber.e(...) with the exception and a clear message; keep the existing finally
block that calls syncUtils.unregisterPendingAdd.
- Around line 62-94: The addSongsAndSync handler should bail out when the
computed localIds is empty to avoid sending an empty YouTube edit request; after
computing val localIds = ids.distinct() add a guard like if (localIds.isEmpty())
return@launch so you skip database.addSongsToPlaylist, the sync registration
loop, and the YouTube.addToPlaylist calls (refer to addSongsAndSync /
viewModelScope.launch, localIds, database.addSongsToPlaylist,
syncUtils.registerPendingAdd/unregisterPendingAdd, and YouTube.addToPlaylist).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25f6741f-c615-47cd-a414-3311700219b0
📒 Files selected for processing (2)
app/src/main/kotlin/com/metrolist/music/ui/menu/AddToPlaylistDialog.ktapp/src/main/kotlin/com/metrolist/music/viewmodels/PlaylistsViewModel.kt
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/kotlin/com/metrolist/music/ui/menu/AddToPlaylistDialog.kt (1)
322-334:⚠️ Potential issue | 🟡 MinorSkip-duplicates can dispatch an empty add when every song is a duplicate.
When
duplicates.size == songIds!!.size(all selected songs already exist in the target playlist), thefilter { !duplicates.contains(it) }produces an empty list.viewModel.addSongsAndSyncwill still be invoked, which in turn callsYouTube.addToPlaylist(browseId, emptyList())(theelsebranch of thesize == 1check in the ViewModel) — a wasted edit request to YouTube Music. Consider short‑circuiting when the filtered list is empty.♻️ Proposed fix
TextButton( onClick = { showDuplicateDialog = false onDismiss() - viewModel.addSongsAndSync( - selectedPlaylist!!, - songIds!!.filter { !duplicates.contains(it) }, - multiSelectParams, - ) + val toAdd = songIds!!.filter { it !in duplicates } + if (toAdd.isNotEmpty()) { + viewModel.addSongsAndSync( + selectedPlaylist!!, + toAdd, + multiSelectParams, + ) + } } ) { Text(stringResource(R.string.skip_duplicates)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/menu/AddToPlaylistDialog.kt` around lines 322 - 334, The skip-duplicates button can call viewModel.addSongsAndSync with an empty list when all selected songs are duplicates; change the onClick handler in AddToPlaylistDialog so it computes the filtered list (songIds!!.filter { !duplicates.contains(it) }) into a local val and only calls viewModel.addSongsAndSync(selectedPlaylist!!, filtered, multiSelectParams) if filtered.isNotEmpty(); otherwise just clear showDuplicateDialog and call onDismiss() (no YouTube.addToPlaylist request). Keep references to selectedPlaylist, songIds, duplicates, and viewModel.addSongsAndSync to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/src/main/kotlin/com/metrolist/music/ui/menu/AddToPlaylistDialog.kt`:
- Around line 322-334: The skip-duplicates button can call
viewModel.addSongsAndSync with an empty list when all selected songs are
duplicates; change the onClick handler in AddToPlaylistDialog so it computes the
filtered list (songIds!!.filter { !duplicates.contains(it) }) into a local val
and only calls viewModel.addSongsAndSync(selectedPlaylist!!, filtered,
multiSelectParams) if filtered.isNotEmpty(); otherwise just clear
showDuplicateDialog and call onDismiss() (no YouTube.addToPlaylist request).
Keep references to selectedPlaylist, songIds, duplicates, and
viewModel.addSongsAndSync to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33d0b299-0386-4e1f-af53-d4a564cdee6f
📒 Files selected for processing (1)
app/src/main/kotlin/com/metrolist/music/ui/menu/AddToPlaylistDialog.kt
2 participants
Problem
Multi-select add-to-playlist was slow and unreliable for large selections. The add work was tied too closely to the UI flow, and the app was issuing one request per song instead of using a single bulk playlist edit.
Cause
The playlist add flow was spread across multiple menu entry points and depended on the dialog/UI staying alive while each individual add request completed. For online selections, the flow also re-resolved songs unnecessarily instead of using the direct add path.
Solution
Testing
Manually verified that multi-select add-to-playlist completes correctly and is much faster for large selections.
Related Issues
Summary by CodeRabbit
New Features
Bug Fixes