From 487400fab65b6a35ccb371c4e273518853ef1aea Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 1 Jun 2026 08:15:29 -0400 Subject: [PATCH] fix(android): share playlist play conversion + filter unplayable rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HomeViewModel.playPlaylist (the Home play-button overlay path) and PlaylistDetailViewModel.play (the detail-screen path) both converted PlaylistTrackRef -> TrackRef before player.setQueue, but only the detail-screen path filtered out unplayable rows (missing trackId or empty streamUrl). Home's path passed them through; Media3 then silently no-op'd on setUri("") and the queue appeared loaded but nothing started. Operator reported "Songs-like playlists queue music that never plays" via the Home play-overlay 2026-06-01. The same conversion in two places with different rules is exactly the foot-gun the §1-§3 DRY pass was supposed to head off; this commit adds it as a follow-up since the playlist-play helper wasn't covered by that sweep. Extracts `List.toPlayableTrackRefs()` in playlists/data/PlaylistsRepository.kt. Filters (isAvailable && streamUrl non-empty) THEN maps to TrackRef, in one pass. Both call sites now use it. The remaining private `toTrackRef()` in PlaylistDetailScreen is kept for the per-row TrackActionsButton wiring - the actions menu only needs id+display fields and doesn't queue anything itself. --- .../minstrel/home/ui/HomeScreen.kt | 19 ++++---------- .../playlists/data/PlaylistsRepository.kt | 26 +++++++++++++++++++ .../playlists/ui/PlaylistDetailScreen.kt | 6 ++--- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt index 03d8f3de..e9048195 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt @@ -69,6 +69,7 @@ import com.fabledsword.minstrel.nav.ArtistDetail import com.fabledsword.minstrel.nav.Home import com.fabledsword.minstrel.nav.PlaylistDetail import com.fabledsword.minstrel.playlists.data.PlaylistsRepository +import com.fabledsword.minstrel.playlists.data.toPlayableTrackRefs import com.fabledsword.minstrel.playlists.widgets.OfflinePoolCard import com.fabledsword.minstrel.playlists.widgets.PlaylistCard import com.fabledsword.minstrel.playlists.widgets.PlaylistPlaceholderCard @@ -274,20 +275,10 @@ class HomeViewModel @Inject constructor( ) return@launch } - val tracks = detail.tracks.mapNotNull { row -> - row.trackId?.let { trackId -> - TrackRef( - id = trackId, - title = row.title, - albumId = row.albumId.orEmpty(), - albumTitle = row.albumTitle, - artistId = row.artistId.orEmpty(), - artistName = row.artistName, - durationSec = row.durationSec, - streamUrl = row.streamUrl.orEmpty(), - ) - } - } + // Shared with PlaylistDetailViewModel.play - filters out + // unplayable rows (missing trackId or empty streamUrl) so the + // queue can't end up with tracks Media3 silently rejects. + val tracks = detail.tracks.toPlayableTrackRefs() if (tracks.isEmpty()) { poolMessages.trySend("Mix isn't ready yet - try again in a moment") return@launch diff --git a/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt index c97012fc..3f744ef2 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt @@ -12,6 +12,7 @@ import com.fabledsword.minstrel.cache.db.entities.CachedPlaylistTrackEntity import com.fabledsword.minstrel.cache.mutations.MutationQueue import com.fabledsword.minstrel.models.PlaylistRef import com.fabledsword.minstrel.models.PlaylistTrackRef +import com.fabledsword.minstrel.models.TrackRef import com.fabledsword.minstrel.models.wire.PlaylistDetailWire import com.fabledsword.minstrel.models.wire.PlaylistTrackWire import com.fabledsword.minstrel.models.wire.PlaylistWire @@ -204,6 +205,31 @@ data class PlaylistDetailRef( val tracks: List, ) +/** + * Convert each [PlaylistTrackRef] to a player-ready [TrackRef], dropping + * any row that isn't actually playable (trackId missing OR streamUrl + * missing). The two call sites (PlaylistDetailViewModel.play and + * HomeViewModel.playPlaylist) used to hand-roll this conversion and + * differed on whether they applied the filter, which let unplayable + * tracks reach `player.setQueue` and silently no-op inside Media3 + * (setUri("") never errors). Centralise both the conversion AND the + * defensive filter so the queue is always playable. + */ +fun List.toPlayableTrackRefs(): List = + filter { it.isAvailable && !it.streamUrl.isNullOrEmpty() } + .map { row -> + TrackRef( + id = row.trackId.orEmpty(), + title = row.title, + albumId = row.albumId.orEmpty(), + artistId = row.artistId.orEmpty(), + albumTitle = row.albumTitle, + artistName = row.artistName, + durationSec = row.durationSec, + streamUrl = row.streamUrl.orEmpty(), + ) + } + // ── Mappers (internal — keep Room + wire types out of the UI layer) ── private fun CachedPlaylistEntity.toDomain(): PlaylistRef = diff --git a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt index 5b262c7f..1504c0a4 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt @@ -69,6 +69,7 @@ import com.fabledsword.minstrel.player.PlayerController import com.fabledsword.minstrel.likes.data.LikesRepository import com.fabledsword.minstrel.playlists.data.PlaylistDetailRef import com.fabledsword.minstrel.playlists.data.PlaylistsRepository +import com.fabledsword.minstrel.playlists.data.toPlayableTrackRefs import com.fabledsword.minstrel.shared.formatDuration import com.fabledsword.minstrel.shared.widgets.TrackRow import com.fabledsword.minstrel.shared.widgets.LikeButton @@ -223,9 +224,8 @@ class PlaylistDetailViewModel @Inject constructor( /** Play the available tracks starting at [startTrackId] (or first available). */ fun play(tracks: List, startTrackId: String?) { - val playable = tracks.filter { it.isAvailable && !it.streamUrl.isNullOrEmpty() } - if (playable.isEmpty()) return - val refs = playable.map { it.toTrackRef() } + val refs = tracks.toPlayableTrackRefs() + if (refs.isEmpty()) return val startIndex = refs.indexOfFirst { it.id == startTrackId }.coerceAtLeast(0) player.setQueue(refs, initialIndex = startIndex, source = "playlist:$playlistId") }