feat(android): name the missing file behind a greyed playlist row — #2527
android / Build + lint + test (push) Successful in 3m42s
android / Build + lint + test (push) Successful in 3m42s
Android was already skipping these by accident: toPlayableTrackRefs
filters on a non-empty streamUrl, and the server stopped emitting one
for a missing file, so they never reached the queue. Correct behaviour,
no idea why -- the row just sat there greyed with the same treatment as
a track deleted from the library, which is a different and permanent
thing.
isAvailable now covers both cases explicitly rather than inferring one
from an empty URL, so every reader (row alpha, click gating, queue
building) gets the same answer from one place. The flag stands on its
own deliberately: a detail fetched before the file went missing can
still carry a stale streamUrl from cache, and that must not resurrect
the row.
The row says which kind of dead it is. A missing file gets "· File
missing" on the subtitle line, because that one can fix itself -- the
scanner clears the mark when the file returns and adopts the row if it
returns renamed (#2528) -- so it is worth telling the user about. A
removed track keeps its bare greyed treatment; there is nothing to act
on once it's gone from the library.
Matches the web treatment landed in 4c49ee2c (rules #23/#27: parity,
not web-only).
This commit is contained in:
@@ -47,7 +47,10 @@ data class PlaylistRef(
|
||||
* `trackId` and `streamUrl` are nullable because the upstream track can
|
||||
* be removed from the library while the row stays in the playlist —
|
||||
* those tiles render grey + unplayable per Flutter's `isAvailable`
|
||||
* convention.
|
||||
* convention. [unavailable] is the second, softer case: the track is
|
||||
* still there but its file is missing. Both render grey and refuse to
|
||||
* play; only the second is worth explaining to the user, because it
|
||||
* can fix itself.
|
||||
*/
|
||||
data class PlaylistTrackRef(
|
||||
val position: Int,
|
||||
@@ -59,8 +62,21 @@ data class PlaylistTrackRef(
|
||||
val artistName: String = "",
|
||||
val durationSec: Int = 0,
|
||||
val streamUrl: String? = null,
|
||||
/**
|
||||
* The track is still in the library but its file is missing from
|
||||
* disk (#2527). Unlike a null [trackId] this is expected to be
|
||||
* temporary — the scanner clears it when the file returns, and
|
||||
* adopts the row if it returns under a new name (#2528) — so the
|
||||
* row keeps its identity, its likes and its play history.
|
||||
*/
|
||||
val unavailable: Boolean = false,
|
||||
) {
|
||||
val isAvailable: Boolean get() = trackId != null
|
||||
/**
|
||||
* Playable-ness, covering both ways a row can outlive its audio.
|
||||
* Everything that greys a row or refuses to queue it reads this, so
|
||||
* neither concern has to be re-derived at a call site.
|
||||
*/
|
||||
val isAvailable: Boolean get() = trackId != null && !unavailable
|
||||
|
||||
/**
|
||||
* Cover URL derived from the parent album's `/api/albums/{id}/cover`
|
||||
|
||||
@@ -41,6 +41,14 @@ data class PlaylistsListWire(
|
||||
* / `artistId` / `streamUrl` are nullable because the upstream track
|
||||
* may have been removed from the library while the row stays in the
|
||||
* playlist with its display fields preserved.
|
||||
*
|
||||
* [unavailable] is the other way a row outlives its audio (#2527): the
|
||||
* track is still in the library, with its history and likes, but its
|
||||
* file is missing from disk. The server withholds `stream_url` in that
|
||||
* case too, so a client that only checked the URL would already skip
|
||||
* it — the flag is what lets the UI say WHY instead of rendering a
|
||||
* mysteriously dead row. Defaults false so a server that predates the
|
||||
* field deserialises cleanly.
|
||||
*/
|
||||
@Serializable
|
||||
data class PlaylistTrackWire(
|
||||
@@ -53,6 +61,7 @@ data class PlaylistTrackWire(
|
||||
@SerialName("artist_name") val artistName: String = "",
|
||||
@SerialName("duration_sec") val durationSec: Int = 0,
|
||||
@SerialName("stream_url") val streamUrl: String? = null,
|
||||
val unavailable: Boolean = false,
|
||||
)
|
||||
|
||||
/**
|
||||
|
||||
+1
@@ -326,4 +326,5 @@ private fun PlaylistTrackWire.toDomain(): PlaylistTrackRef =
|
||||
artistName = artistName,
|
||||
durationSec = durationSec,
|
||||
streamUrl = streamUrl,
|
||||
unavailable = unavailable,
|
||||
)
|
||||
|
||||
+8
-1
@@ -577,9 +577,16 @@ private fun TrackRow(
|
||||
) {
|
||||
val enabled = row.isAvailable
|
||||
val rowAlpha = if (enabled) 1f else UNAVAILABLE_ALPHA
|
||||
// A greyed row with no explanation reads as a bug. Say which kind of
|
||||
// dead it is: a missing file (#2527) is the library's problem and can
|
||||
// fix itself when the file returns, so it earns a line the user can
|
||||
// act on. A removed track needs no note — grey and unplayable is the
|
||||
// whole story once it's gone from the library.
|
||||
val subtitle = row.artistName.ifEmpty { row.albumTitle }
|
||||
val artistLine = if (row.unavailable) "$subtitle · File missing" else subtitle
|
||||
TrackRow(
|
||||
title = row.title,
|
||||
artist = row.artistName.ifEmpty { row.albumTitle },
|
||||
artist = artistLine,
|
||||
trackId = row.trackId.orEmpty(),
|
||||
onClick = onClick,
|
||||
nowPlaying = nowPlaying,
|
||||
|
||||
+71
@@ -0,0 +1,71 @@
|
||||
package com.fabledsword.minstrel.playlists.data
|
||||
|
||||
import com.fabledsword.minstrel.models.PlaylistTrackRef
|
||||
import org.junit.jupiter.api.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* A playlist row can outlive its audio two ways — the track was removed
|
||||
* from the library, or the track is still there but its file is missing
|
||||
* (#2527). Both must be unplayable, and neither may be silently dropped
|
||||
* from the list the user curated.
|
||||
*/
|
||||
class PlaylistTrackAvailabilityTest {
|
||||
|
||||
private fun row(
|
||||
position: Int = 0,
|
||||
trackId: String? = "t-$position",
|
||||
streamUrl: String? = "/api/tracks/t-$position/stream",
|
||||
unavailable: Boolean = false,
|
||||
) = PlaylistTrackRef(
|
||||
position = position,
|
||||
trackId = trackId,
|
||||
title = "Track $position",
|
||||
albumId = "a-1",
|
||||
albumTitle = "Album",
|
||||
artistId = "ar-1",
|
||||
artistName = "Artist",
|
||||
durationSec = 137,
|
||||
streamUrl = streamUrl,
|
||||
unavailable = unavailable,
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `a playable row is available`() {
|
||||
assertTrue(row().isAvailable)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a removed track is unavailable`() {
|
||||
assertFalse(row(trackId = null, streamUrl = null).isAvailable)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a missing file is unavailable even though the track id survives`() {
|
||||
assertFalse(row(unavailable = true, streamUrl = null).isAvailable)
|
||||
}
|
||||
|
||||
/**
|
||||
* The flag has to stand on its own. The server also withholds the
|
||||
* stream URL, but a cached detail fetched before the file went missing
|
||||
* can still carry a stale URL, and that must not resurrect the row.
|
||||
*/
|
||||
@Test
|
||||
fun `the flag disqualifies a row that still has a stream url`() {
|
||||
assertFalse(row(unavailable = true).isAvailable)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `queue building drops both kinds of dead row and keeps the rest`() {
|
||||
val queue = listOf(
|
||||
row(position = 0),
|
||||
row(position = 1, trackId = null, streamUrl = null),
|
||||
row(position = 2, unavailable = true, streamUrl = null),
|
||||
row(position = 3),
|
||||
).toPlayableTrackRefs()
|
||||
|
||||
assertEquals(listOf("t-0", "t-3"), queue.map { it.id })
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user