From aab90a7a39e6756858348ffc7665431b39faba0c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 16 Aug 2026 11:57:55 -0400 Subject: [PATCH] =?UTF-8?q?feat(android):=20name=20the=20missing=20file=20?= =?UTF-8?q?behind=20a=20greyed=20playlist=20row=20=E2=80=94=20#2527?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../fabledsword/minstrel/models/Playlist.kt | 20 +++++- .../minstrel/models/wire/PlaylistWire.kt | 9 +++ .../playlists/data/PlaylistsRepository.kt | 1 + .../playlists/ui/PlaylistDetailScreen.kt | 9 ++- .../data/PlaylistTrackAvailabilityTest.kt | 71 +++++++++++++++++++ 5 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 android/app/src/test/java/com/fabledsword/minstrel/playlists/data/PlaylistTrackAvailabilityTest.kt diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/Playlist.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/Playlist.kt index b690f911..7e918400 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/Playlist.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/Playlist.kt @@ -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` diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/PlaylistWire.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/PlaylistWire.kt index dd1b977f..a4fa6152 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/PlaylistWire.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/PlaylistWire.kt @@ -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, ) /** 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 d7b697ce..b62ada1a 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 @@ -326,4 +326,5 @@ private fun PlaylistTrackWire.toDomain(): PlaylistTrackRef = artistName = artistName, durationSec = durationSec, streamUrl = streamUrl, + unavailable = unavailable, ) 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 4e69ffd6..1106fdc0 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 @@ -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, diff --git a/android/app/src/test/java/com/fabledsword/minstrel/playlists/data/PlaylistTrackAvailabilityTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/playlists/data/PlaylistTrackAvailabilityTest.kt new file mode 100644 index 00000000..318e7e05 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/playlists/data/PlaylistTrackAvailabilityTest.kt @@ -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 }) + } +}