From 3198a94df328be1394f8848eca96cfe5c5cc1206 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 19 Oct 2026 03:14:13 +0000 Subject: [PATCH] ui: cover the artist section split in tests Assert the combined meta line while loading, cover the downloaded partition and the plural album/song credit branches, share the album-credit id query with its plan test, and put the meta line through a translatable string. --- .../com/subtracks/data/db/LibraryDao.kt | 9 +-- .../subtracks/data/repo/LibraryRepository.kt | 6 +- .../ui/library/ArtistDetailScreen.kt | 9 +-- app/src/main/res/values/strings.xml | 1 + .../com/subtracks/data/db/LibraryDaoTest.kt | 11 ++-- .../data/repo/LibraryRepositoryTest.kt | 33 ++++++++++ .../subtracks/data/sync/SyncServiceTest.kt | 65 +++++++++++++++++++ .../ui/library/ArtistDetailScreenTest.kt | 5 +- 8 files changed, 116 insertions(+), 23 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 de5ae935..cedf4ede 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -118,6 +118,10 @@ internal const val ALBUMS_FOR_ARTIST = "JOIN albums ON albums.sourceId = :sourceId AND albums.id = credits.albumId " + "ORDER BY year DESC, sortName COLLATE NOCASE, id" +internal const val ALBUM_CREDIT_ALBUM_IDS = + "SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) " + + "WHERE artistId = :artistId AND albumCredit = 1" + // Download visibility follows the credit path: an album credit includes the album when any song is // downloaded, while a track credit includes it only when the credited track is downloaded. private const val DOWNLOADED_ARTIST_ALBUMS = @@ -1066,10 +1070,7 @@ interface LibraryDao { artistId: String, ): Flow> - @Query( - "SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) " + - "WHERE artistId = :artistId AND albumCredit = 1", - ) + @Query(ALBUM_CREDIT_ALBUM_IDS) fun albumCreditAlbumIds( sourceId: Long, artistId: String, diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index aadcb685..3bc088df 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -387,10 +387,8 @@ class LibraryRepository( } return combine(albums, db.libraryDao().albumCreditAlbumIds(sourceId, artistId)) { list, creditIds -> val credits = creditIds.toHashSet() - ArtistAlbums( - albumCredits = list.filter { it.id in credits }, - appearances = list.filterNot { it.id in credits }, - ) + val (albumCredits, appearances) = list.partition { it.id in credits } + ArtistAlbums(albumCredits = albumCredits, appearances = appearances) } } diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt index 71b353a9..910f88a2 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -252,13 +252,8 @@ fun ArtistDetailScreen( stringResource( if (albums.albumCredits.isNotEmpty()) R.string.artist_type_album_artist else R.string.artist_type_artist, ) - val albumCountText = - if (albumsLoaded) { - pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount) - } else { - "" - } - val metaText = if (albumsLoaded) "$artistType · $albumCountText" else "" + val albumCountText = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount) + val metaText = if (albumsLoaded) stringResource(R.string.artist_meta, artistType, albumCountText) else "" var titleHeightPx by remember { mutableFloatStateOf(0f) } val titleHeight = if (titleHeightPx > 0f) with(density) { titleHeightPx.toDp() } else lineHeight val titleMaxLines = diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 83be1aed..d1f5080b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -37,6 +37,7 @@ Album artist Albums (%1$d) Appears on (%1$d) + %1$s · %2$s Genre Starred %1$s\n\nDeleting will free %2$s 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 758ebdb7..93d87b24 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -548,17 +548,18 @@ class LibraryDaoTest { @Test fun albumCreditIdsFindCreditsThroughArtistIndexes() = runTest { - val plan = - plan( - "SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) " + - "WHERE artistId = :artistId AND albumCredit = 1", - ) + val plan = plan(ALBUM_CREDIT_ALBUM_IDS) assertTrue(plan, plan.contains("USING INDEX index_albums_sourceId_artistId (sourceId=? AND artistId=?)")) assertTrue( plan, plan.contains("USING INDEX index_album_artists_sourceId_artistId (sourceId=? AND artistId=?)"), ) + assertTrue(plan, plan.contains("USING INDEX index_songs_sourceId_artistId (sourceId=? AND artistId=?)")) + assertTrue( + plan, + plan.contains("USING INDEX index_song_artists_sourceId_artistId (sourceId=? AND artistId=?)"), + ) assertFalse(plan, plan.contains("SCAN albums")) assertFalse(plan, plan.contains("SCAN songs")) } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt index a650ebad..3e6a7a59 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt @@ -12,7 +12,9 @@ import com.subtracks.cancelAndJoinBlocking import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.model.Album +import com.subtracks.data.model.DownloadStatus import com.subtracks.data.model.Song +import com.subtracks.data.model.SongDownload import com.subtracks.data.model.Source import com.subtracks.data.prefs.AlbumSort import com.subtracks.data.prefs.StarredFilter @@ -231,6 +233,37 @@ class LibraryRepositoryTest { assertEquals(2, albums.count) } + @Test + fun downloadedArtistAlbumsKeepPerCreditVisibility() = + runBlocking { + val sourceId = 1L + db.sourcesDao().upsertSource( + Source(id = sourceId, name = "test", address = "http://localhost", isActive = true, createdAt = 0), + ) + db.libraryDao().upsertAlbums( + listOf( + seedAlbum(sourceId, 1), + seedAlbum(sourceId, 2).copy(artistId = "ar2"), + ), + ) + db.libraryDao().upsertSongs( + listOf( + seedSong(sourceId).copy(albumId = "al-0001"), + seedSong(sourceId).copy(id = "s2", albumId = "al-0002"), + ), + ) + + db.downloadDao().upsert(SongDownload(sourceId, "s1", DownloadStatus.Completed)) + val before = repository.artistAlbums(sourceId, "ar1", downloaded = true).first() + assertEquals(listOf("al-0001"), before.albumCredits.map { it.id }) + assertEquals(emptyList(), before.appearances.map { it.id }) + + db.downloadDao().upsert(SongDownload(sourceId, "s2", DownloadStatus.Completed)) + val after = repository.artistAlbums(sourceId, "ar1", downloaded = true).first() + assertEquals(listOf("al-0001"), after.albumCredits.map { it.id }) + assertEquals(listOf("al-0002"), after.appearances.map { it.id }) + } + private suspend fun awaitRefreshes( presenter: RecordingPresenter, target: Int, diff --git a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt index 9e446faa..58694af6 100644 --- a/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/sync/SyncServiceTest.kt @@ -1640,6 +1640,71 @@ class SyncServiceTest { ) } + @Test + fun albumCreditIdsFollowPluralCredits() = + runTest { + insertSource() + val source = + FakeMusicSource( + artists = listOf(artist("a1", "A"), artist("a2", "B")), + albums = + listOf( + album("al1").copy(artistId = "a1", artists = listOf(ArtistRef("a1", "A"))), + album("al2").copy(artistId = "a2", artists = listOf(ArtistRef("a2", "B"))), + album("al3").copy( + artistId = "a2", + artists = listOf(ArtistRef("a1", "A"), ArtistRef("a2", "B")), + ), + ), + songs = + listOf( + song("s1").copy( + albumId = "al1", + artistId = "a1", + artists = listOf(ArtistRef("a1", "A")), + ), + song("s2").copy( + albumId = "al2", + artistId = "a2", + artists = listOf(ArtistRef("a2", "B"), ArtistRef("a1", "A")), + ), + song("s3").copy( + albumId = "al3", + artistId = "a2", + artists = listOf(ArtistRef("a1", "A"), ArtistRef("a2", "B")), + ), + ), + ) + + SyncService(db, source).sync() + + assertEquals( + listOf("al1", "al3"), + db + .libraryDao() + .albumCreditAlbumIds(1, "a1") + .first() + .sorted(), + ) + assertEquals( + listOf("al2", "al3"), + db + .libraryDao() + .albumCreditAlbumIds(1, "a2") + .first() + .sorted(), + ) + assertEquals( + listOf("al1", "al2", "al3"), + db + .libraryDao() + .albumsForArtist(1, "a1") + .first() + .map { it.id } + .sorted(), + ) + } + @Test fun everyArtistConsumerAgreesOnTheCreditedSet() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt index a6ca5ecd..a208f3ca 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -104,9 +104,8 @@ class ArtistDetailScreenTest { albumsLoaded = false, ) - composeRule.onNodeWithText("Artist").assertDoesNotExist() - composeRule.onNodeWithText("Album artist").assertDoesNotExist() - composeRule.onNodeWithText("0 albums").assertDoesNotExist() + composeRule.onNodeWithText("Artist · 0 albums").assertDoesNotExist() + composeRule.onNodeWithText("Album artist · 0 albums").assertDoesNotExist() } private fun album(