fix(android): share playlist play conversion + filter unplayable rows
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<PlaylistTrackRef>.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.
This commit is contained in:
@@ -69,6 +69,7 @@ import com.fabledsword.minstrel.nav.ArtistDetail
|
|||||||
import com.fabledsword.minstrel.nav.Home
|
import com.fabledsword.minstrel.nav.Home
|
||||||
import com.fabledsword.minstrel.nav.PlaylistDetail
|
import com.fabledsword.minstrel.nav.PlaylistDetail
|
||||||
import com.fabledsword.minstrel.playlists.data.PlaylistsRepository
|
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.OfflinePoolCard
|
||||||
import com.fabledsword.minstrel.playlists.widgets.PlaylistCard
|
import com.fabledsword.minstrel.playlists.widgets.PlaylistCard
|
||||||
import com.fabledsword.minstrel.playlists.widgets.PlaylistPlaceholderCard
|
import com.fabledsword.minstrel.playlists.widgets.PlaylistPlaceholderCard
|
||||||
@@ -274,20 +275,10 @@ class HomeViewModel @Inject constructor(
|
|||||||
)
|
)
|
||||||
return@launch
|
return@launch
|
||||||
}
|
}
|
||||||
val tracks = detail.tracks.mapNotNull { row ->
|
// Shared with PlaylistDetailViewModel.play - filters out
|
||||||
row.trackId?.let { trackId ->
|
// unplayable rows (missing trackId or empty streamUrl) so the
|
||||||
TrackRef(
|
// queue can't end up with tracks Media3 silently rejects.
|
||||||
id = trackId,
|
val tracks = detail.tracks.toPlayableTrackRefs()
|
||||||
title = row.title,
|
|
||||||
albumId = row.albumId.orEmpty(),
|
|
||||||
albumTitle = row.albumTitle,
|
|
||||||
artistId = row.artistId.orEmpty(),
|
|
||||||
artistName = row.artistName,
|
|
||||||
durationSec = row.durationSec,
|
|
||||||
streamUrl = row.streamUrl.orEmpty(),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (tracks.isEmpty()) {
|
if (tracks.isEmpty()) {
|
||||||
poolMessages.trySend("Mix isn't ready yet - try again in a moment")
|
poolMessages.trySend("Mix isn't ready yet - try again in a moment")
|
||||||
return@launch
|
return@launch
|
||||||
|
|||||||
+26
@@ -12,6 +12,7 @@ import com.fabledsword.minstrel.cache.db.entities.CachedPlaylistTrackEntity
|
|||||||
import com.fabledsword.minstrel.cache.mutations.MutationQueue
|
import com.fabledsword.minstrel.cache.mutations.MutationQueue
|
||||||
import com.fabledsword.minstrel.models.PlaylistRef
|
import com.fabledsword.minstrel.models.PlaylistRef
|
||||||
import com.fabledsword.minstrel.models.PlaylistTrackRef
|
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.PlaylistDetailWire
|
||||||
import com.fabledsword.minstrel.models.wire.PlaylistTrackWire
|
import com.fabledsword.minstrel.models.wire.PlaylistTrackWire
|
||||||
import com.fabledsword.minstrel.models.wire.PlaylistWire
|
import com.fabledsword.minstrel.models.wire.PlaylistWire
|
||||||
@@ -204,6 +205,31 @@ data class PlaylistDetailRef(
|
|||||||
val tracks: List<PlaylistTrackRef>,
|
val tracks: List<PlaylistTrackRef>,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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<PlaylistTrackRef>.toPlayableTrackRefs(): List<TrackRef> =
|
||||||
|
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) ──
|
// ── Mappers (internal — keep Room + wire types out of the UI layer) ──
|
||||||
|
|
||||||
private fun CachedPlaylistEntity.toDomain(): PlaylistRef =
|
private fun CachedPlaylistEntity.toDomain(): PlaylistRef =
|
||||||
|
|||||||
+3
-3
@@ -69,6 +69,7 @@ import com.fabledsword.minstrel.player.PlayerController
|
|||||||
import com.fabledsword.minstrel.likes.data.LikesRepository
|
import com.fabledsword.minstrel.likes.data.LikesRepository
|
||||||
import com.fabledsword.minstrel.playlists.data.PlaylistDetailRef
|
import com.fabledsword.minstrel.playlists.data.PlaylistDetailRef
|
||||||
import com.fabledsword.minstrel.playlists.data.PlaylistsRepository
|
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.formatDuration
|
||||||
import com.fabledsword.minstrel.shared.widgets.TrackRow
|
import com.fabledsword.minstrel.shared.widgets.TrackRow
|
||||||
import com.fabledsword.minstrel.shared.widgets.LikeButton
|
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). */
|
/** Play the available tracks starting at [startTrackId] (or first available). */
|
||||||
fun play(tracks: List<PlaylistTrackRef>, startTrackId: String?) {
|
fun play(tracks: List<PlaylistTrackRef>, startTrackId: String?) {
|
||||||
val playable = tracks.filter { it.isAvailable && !it.streamUrl.isNullOrEmpty() }
|
val refs = tracks.toPlayableTrackRefs()
|
||||||
if (playable.isEmpty()) return
|
if (refs.isEmpty()) return
|
||||||
val refs = playable.map { it.toTrackRef() }
|
|
||||||
val startIndex = refs.indexOfFirst { it.id == startTrackId }.coerceAtLeast(0)
|
val startIndex = refs.indexOfFirst { it.id == startTrackId }.coerceAtLeast(0)
|
||||||
player.setQueue(refs, initialIndex = startIndex, source = "playlist:$playlistId")
|
player.setQueue(refs, initialIndex = startIndex, source = "playlist:$playlistId")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user