From d8ac7fefc747624227790a6eced0ee155475bced Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:04:46 +0000 Subject: [PATCH] db: keep playlist owner and sharing in filtered lists The hand-written playlist projection used by the Downloaded filter and offline mode did not select the new owner and shared columns, so the playlist Info dialog lost them there while unfiltered lists had them. Select both columns and pin the filtered path in LibraryDaoTest. Also share the artist-name extraction between the subtitle and the plural label so duplicate or blank refs cannot make them disagree, accept the XML boolean form "1" for the public flag, and strengthen the date-adjacency and pluralisation tests. --- .../com/subtracks/data/db/LibraryDao.kt | 2 +- .../data/source/subsonic/SubsonicXml.kt | 2 +- .../com/subtracks/ui/components/SongArtist.kt | 14 +++++++-- .../subtracks/ui/components/SongInfoDialog.kt | 7 +---- .../com/subtracks/data/db/LibraryDaoTest.kt | 9 +++++- .../data/source/subsonic/SubsonicXmlTest.kt | 2 ++ .../ui/components/LibraryInfoDialogTest.kt | 30 +++++++++++++++++++ 7 files changed, 55 insertions(+), 11 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 06db5ee9..39f43b16 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -365,7 +365,7 @@ internal const val PLAYLISTS_SELECT = "JOIN song_downloads sd ON sd.sourceId = ps.sourceId AND sd.songId = ps.songId AND sd.status = 'Completed' " + "WHERE ps.sourceId = playlists.sourceId AND ps.playlistId = playlists.id " + ") ELSE playlists.songCount END AS songCount, " + - "playlists.created, playlists.changed, playlists.duration " + "playlists.created, playlists.changed, playlists.duration, playlists.owner, playlists.shared " internal const val PLAYLIST_ORDER_BY_NAME = "name COLLATE NOCASE, id" internal const val PLAYLIST_ORDER_BY_NAME_REVERSED = "name COLLATE NOCASE DESC, id DESC" diff --git a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt index ee6ccd37..070841b0 100644 --- a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt +++ b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt @@ -367,7 +367,7 @@ internal fun Attributes.attr(name: String): String = getValue(name) ?: "" internal fun Attributes.longAttr(name: String): Long? = attr(name).takeIf { it.isNotEmpty() }?.toLongOrNull() -internal fun Attributes.boolAttr(name: String): Boolean = attr(name).equals("true", ignoreCase = true) +internal fun Attributes.boolAttr(name: String): Boolean = attr(name).let { it == "1" || it.equals("true", ignoreCase = true) } internal fun Attributes.floatAttr(name: String): Float? = attr(name).takeIf { it.isNotEmpty() }?.toFloatOrNull()?.takeIf { it.isFinite() } diff --git a/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt b/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt index 178fe290..18945502 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt @@ -7,12 +7,22 @@ private const val ARTIST_SEPARATOR = " \u00B7 " internal fun Song.artistSubtitle(): String? = artistSubtitle(artist, artists) +internal fun Song.artistNames(): List = artistNames(artist, artists) + +internal fun artistNames( + artist: String?, + artists: List, +): List { + val names = artists.map { it.name }.filter { it.isNotBlank() }.distinct() + return names.ifEmpty { listOfNotNull(artist?.takeIf { it.isNotBlank() }) } +} + internal fun artistSubtitle( artist: String?, artists: List, ): String? { - val names = artists.map { it.name }.filter { it.isNotBlank() }.distinct() - if (names.isEmpty()) return artist?.takeIf { it.isNotBlank() } + val names = artistNames(artist, artists) + if (names.isEmpty()) return null val primary = artist?.takeIf { it in names } return buildList { primary?.let(::add) diff --git a/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt b/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt index a731690d..1d8c583b 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt @@ -46,12 +46,7 @@ fun SongInfoDialog( title = { Text(stringResource(R.string.info)) }, text = { Column(verticalArrangement = Arrangement.spacedBy(6.dp)) { - val multipleArtists = - song.artistRefs - .map { it.name } - .filter { it.isNotBlank() } - .distinct() - .size > 1 + val multipleArtists = song.artistNames().size > 1 val regularArtistSort = song.artist?.takeIf { it.isNotBlank() } ?: song.artistRefs.firstOrNull()?.name InfoRow(stringResource(R.string.resources_sort_by_title), song.title) InfoRow(stringResource(R.string.info_sort_title), song.sortName.takeIfDifferentFrom(song.title)) diff --git a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt index 527ef321..75c0de2a 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -1016,7 +1016,12 @@ class LibraryDaoTest { runTest { val sourceId = source() val dao = db.libraryDao() - dao.upsertPlaylists(listOf(playlist(sourceId, "pl-1", "List", created = 0, changed = 0).copy(songCount = 5))) + dao.upsertPlaylists( + listOf( + playlist(sourceId, "pl-1", "List", created = 0, changed = 0) + .copy(songCount = 5, owner = "austin", shared = true), + ), + ) dao.upsertSongs( listOf( song(sourceId, "s1", "One", starred = null), @@ -1034,6 +1039,8 @@ class LibraryDaoTest { val filtered = dao.playlistsByName(sourceId, "", downloadedFilter = 1).page() assertEquals(1, filtered.size) assertEquals(1L, filtered.first().songCount) + assertEquals("austin", filtered.first().owner) + assertEquals(true, filtered.first().shared) assertEquals( 5L, diff --git a/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicXmlTest.kt b/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicXmlTest.kt index 08146f7b..86b1babb 100644 --- a/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicXmlTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicXmlTest.kt @@ -754,6 +754,7 @@ class SubsonicXmlTest { + """.trimIndent(), @@ -764,6 +765,7 @@ class SubsonicXmlTest { assertEquals("austin", playlists[0].owner) assertTrue(playlists[0].shared) assertEquals(false, playlists[1].shared) + assertTrue(playlists[2].shared) } private class CountingInputStream( diff --git a/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt index a7baf30b..de49b7ad 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt @@ -11,6 +11,7 @@ import com.subtracks.data.model.ArtistRef import com.subtracks.data.model.Playlist import com.subtracks.data.model.Song import com.subtracks.ui.theme.SubtracksTheme +import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test @@ -171,6 +172,12 @@ class LibraryInfoDialogTest { val plays = composeRule.onNodeWithText("Plays").fetchSemanticsNode().boundsInRoot assertTrue("last played must follow added", lastPlayed.top >= added.bottom) assertTrue("plays must not sit between the dates", plays.top >= lastPlayed.bottom) + assertEquals( + "no row may sit between added and last played", + lastPlayed.top - added.top, + plays.top - lastPlayed.top, + 0.5f, + ) } @Test @@ -228,6 +235,29 @@ class LibraryInfoDialogTest { composeRule.onNodeWithText("Artists sort").assertDoesNotExist() } + @Test + fun duplicateAndBlankArtistRefsStaySingular() { + composeRule.setContent { + SubtracksTheme { + SongInfoDialog( + song = + song( + artists = listOf(ArtistRef("ar1", "Alpha"), ArtistRef("ar2", "Alpha"), ArtistRef("ar3", "")), + artistSort = "Beta", + ), + download = null, + localFile = null, + streamEncoding = null, + onDismiss = {}, + genres = emptyList(), + ) + } + } + + composeRule.onNodeWithText("Artist").assertIsDisplayed() + composeRule.onNodeWithText("Artists").assertDoesNotExist() + } + @Test fun aMixedArtistTrackUsesPluralLabelsForBothRows() { composeRule.setContent {