From 8cdbe64942a6f711ac3955547fca3858b3324460 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 19 Oct 2026 02:57:56 +0000 Subject: [PATCH] ui: split artist album credits from track appearances Put the artist type and total album count on the single meta row beside the star and more buttons, then list albums where the artist is an album artist separately from albums where they only have a track credit. The hero shuffle button and page-level actions stay where they were. The split comes from an albumCredit flag on the shared credit rows, so the online and downloaded album lists are partitioned by the same rule and the two sections still sum to the stored album count. --- .../com/subtracks/data/db/LibraryDao.kt | 18 +- .../kotlin/com/subtracks/data/model/Models.kt | 7 + .../subtracks/data/repo/LibraryRepository.kt | 21 +- .../ui/library/ArtistDetailScreen.kt | 180 +++++++++++------- .../ui/library/ArtistDetailViewModel.kt | 6 +- app/src/main/res/values/strings.xml | 2 + .../com/subtracks/data/db/LibraryDaoTest.kt | 48 +++++ .../data/repo/LibraryRepositoryTest.kt | 28 +++ .../com/subtracks/ui/AppScreenshotTest.kt | 6 +- .../com/subtracks/ui/DemoScreenshotTest.kt | 16 +- .../ui/library/AlbumArtistLabelTest.kt | 40 ---- .../ui/library/ArtistDetailScreenTest.kt | 62 +++++- 12 files changed, 306 insertions(+), 128 deletions(-) delete mode 100644 app/src/test/kotlin/com/subtracks/ui/library/AlbumArtistLabelTest.kt 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 16fce628..de5ae935 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -93,20 +93,21 @@ internal const val CREDITED_SONG_ROWS = "WHERE album_artists.sourceId = :sourceId" // May repeat an (artistId, albumId) pair when several credit paths reach it; consumers dedupe. +// albumCredit is 1 for album-level credits and 0 for track-level ones. internal const val CREDITED_ALBUM_ROWS = - "SELECT albums.artistId AS artistId, albums.id AS albumId " + + "SELECT albums.artistId AS artistId, albums.id AS albumId, 1 AS albumCredit " + "FROM albums WHERE albums.sourceId = :sourceId AND albums.artistId IS NOT NULL " + "UNION ALL " + - "SELECT album_artists.artistId, albums.id " + + "SELECT album_artists.artistId, albums.id, 1 " + "FROM album_artists JOIN albums " + "ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " + "WHERE album_artists.sourceId = :sourceId " + "UNION ALL " + - "SELECT songs.artistId, songs.albumId " + + "SELECT songs.artistId, songs.albumId, 0 " + "FROM songs JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + "WHERE songs.sourceId = :sourceId AND songs.artistId IS NOT NULL " + "UNION ALL " + - "SELECT song_artists.artistId, songs.albumId " + + "SELECT song_artists.artistId, songs.albumId, 0 " + "FROM song_artists JOIN songs " + "ON songs.sourceId = song_artists.sourceId AND songs.id = song_artists.songId " + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + @@ -1065,6 +1066,15 @@ interface LibraryDao { artistId: String, ): Flow> + @Query( + "SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) " + + "WHERE artistId = :artistId AND albumCredit = 1", + ) + fun albumCreditAlbumIds( + sourceId: Long, + artistId: String, + ): Flow> + @Query("SELECT * FROM discs WHERE sourceId = :sourceId AND albumId = :albumId ORDER BY disc") fun discs( sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/data/model/Models.kt b/app/src/main/kotlin/com/subtracks/data/model/Models.kt index 80db8821..37e83546 100644 --- a/app/src/main/kotlin/com/subtracks/data/model/Models.kt +++ b/app/src/main/kotlin/com/subtracks/data/model/Models.kt @@ -167,6 +167,13 @@ data class Album( } } +data class ArtistAlbums( + val albumCredits: List = emptyList(), + val appearances: List = emptyList(), +) { + val count: Int get() = albumCredits.size + appearances.size +} + @Entity( tableName = "discs", primaryKeys = ["sourceId", "albumId", "disc"], 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 ef911286..aadcb685 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -14,6 +14,7 @@ import com.subtracks.data.db.rediscoverAlbumsQuery import com.subtracks.data.model.Album import com.subtracks.data.model.AlbumSongItem import com.subtracks.data.model.Artist +import com.subtracks.data.model.ArtistAlbums import com.subtracks.data.model.Disc import com.subtracks.data.model.Playlist import com.subtracks.data.model.PlaylistSongItem @@ -31,6 +32,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex @@ -376,12 +378,21 @@ class LibraryRepository( sourceId: Long, artistId: String, downloaded: Boolean = false, - ): Flow> = - if (downloaded) { - db.libraryDao().albumsForArtistDownloaded(sourceId, artistId) - } else { - db.libraryDao().albumsForArtist(sourceId, artistId) + ): Flow { + val albums = + if (downloaded) { + db.libraryDao().albumsForArtistDownloaded(sourceId, artistId) + } else { + db.libraryDao().albumsForArtist(sourceId, artistId) + } + 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 }, + ) } + } fun albumDiscs( sourceId: Long, 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 ae4fe2c7..71b353a9 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -80,6 +80,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.subtracks.R import com.subtracks.data.model.Album import com.subtracks.data.model.Artist +import com.subtracks.data.model.ArtistAlbums import com.subtracks.data.model.BulkDownloadAction import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.ListDownloadStatus @@ -140,11 +141,6 @@ private const val THEME_TRANSITION_MS = 100 private const val SCRIM_FADE_MS = 180 internal const val ARTIST_NAME_SCRIM_TAG = "artistNameScrim" -internal fun isAlbumArtist( - albums: List, - artistId: String, -): Boolean = albums.any { album -> album.artistId == artistId || album.artistRefs.any { it.id == artistId } } - @Composable fun ArtistDetailRoute( artistId: String, @@ -192,8 +188,7 @@ fun ArtistDetailRoute( ArtistDetailScreen( artist = artist, artistName = artistName.orEmpty(), - isAlbumArtist = albums?.let { isAlbumArtist(it, artistId) }, - albums = albums.orEmpty(), + albums = albums ?: ArtistAlbums(), albumsLoaded = albums != null, art = art, artThumbnail = artThumbnail, @@ -225,8 +220,7 @@ fun ArtistDetailScreen( artist: Artist?, modifier: Modifier = Modifier, artistName: String = "", - isAlbumArtist: Boolean? = null, - albums: List, + albums: ArtistAlbums = ArtistAlbums(), albumsLoaded: Boolean = true, art: CoverArtRef?, artThumbnail: CoverArtRef? = null, @@ -252,9 +246,19 @@ fun ArtistDetailScreen( val statusBarTop = with(density) { WindowInsets.statusBars.getTop(this).toDp() } val navBarBottom = with(density) { WindowInsets.navigationBars.getBottom(this).toDp() } val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight - val albumCount = albums.size.toLong() + val albumCount = albums.count.toLong() val displayName = artistName.ifBlank { artist?.name.orEmpty() } - val artistType = stringResource(if (isAlbumArtist == true) R.string.artist_type_album_artist else R.string.artist_type_artist) + val artistType = + 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 "" var titleHeightPx by remember { mutableFloatStateOf(0f) } val titleHeight = if (titleHeightPx > 0f) with(density) { titleHeightPx.toDp() } else lineHeight val titleMaxLines = @@ -386,24 +390,12 @@ fun ArtistDetailScreen( Column( modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = TITLE_INSET), ) { - Text( - text = if (albumsLoaded) artistType else "", - style = MaterialTheme.typography.labelLarge, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - // Hold the row at the button height even without the buttons, so the - // label-to-count gap is the same for every artist type. Row( modifier = Modifier.fillMaxWidth().defaultMinSize(minHeight = 48.dp), verticalAlignment = Alignment.CenterVertically, ) { Text( - text = - if (albumsLoaded) { - pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount) - } else { - "" - }, + text = metaText, style = MaterialTheme.typography.bodyLarge, color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.weight(1f), @@ -435,49 +427,39 @@ fun ArtistDetailScreen( } } } - itemsIndexed(albums, key = { _, album -> album.id }) { index, album -> - val art = coverArt(album.coverArt, false) - val thumbnail = coverArt(album.coverArt, true) - PrefetchArtworkSeeds(thumbnail) - Column( - modifier = - Modifier - .padding( - start = if (index % 2 == 0) 16.dp else 0.dp, - end = if (index % 2 == 1) 16.dp else 0.dp, - ).combinedClickable( - onClick = { onAlbumClick(album) }, - onLongClick = { onAlbumLongClick(MenuTarget.Album(album, thumbnail, albumDownloads[album.id])) }, - ).onSizeChanged { albumHeightPx = it.height.toFloat() }, - ) { - Box( - modifier = Modifier.fillMaxWidth().aspectRatio(1f).clip(RoundedCornerShape(2.dp)), - ) { - CoverArt( - ref = art, - name = album.name, - thumbnailRef = thumbnail, - showPlaceholder = art == null, - modifier = Modifier.fillMaxSize(), - ) - ListDownloadIndicator( - status = albumDownloads[album.id], - modifier = Modifier.align(Alignment.BottomEnd).padding(4.dp), - scrim = true, - ) - } - Text( - text = album.name, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onBackground, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.padding(top = 6.dp), + if (albums.albumCredits.isNotEmpty()) { + item(span = { GridItemSpan(maxLineSpan) }) { + ArtistSectionHeader(stringResource(R.string.artist_section_albums, albums.albumCredits.size)) + } + itemsIndexed(albums.albumCredits, key = { _, album -> album.id }) { index, album -> + ArtistAlbumCard( + album = album, + index = index, + coverArt = coverArt, + downloadStatus = albumDownloads[album.id], + onClick = { onAlbumClick(album) }, + onLongClick = { thumbnail -> + onAlbumLongClick(MenuTarget.Album(album, thumbnail, albumDownloads[album.id])) + }, + onSizeChanged = { albumHeightPx = it }, ) - Text( - text = album.year?.takeIf { it > 0 }?.toString() ?: "\u00A0", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, + } + } + if (albums.appearances.isNotEmpty()) { + item(span = { GridItemSpan(maxLineSpan) }) { + ArtistSectionHeader(stringResource(R.string.artist_section_appearances, albums.appearances.size)) + } + itemsIndexed(albums.appearances, key = { _, album -> album.id }) { index, album -> + ArtistAlbumCard( + album = album, + index = index, + coverArt = coverArt, + downloadStatus = albumDownloads[album.id], + onClick = { onAlbumClick(album) }, + onLongClick = { thumbnail -> + onAlbumLongClick(MenuTarget.Album(album, thumbnail, albumDownloads[album.id])) + }, + onSizeChanged = { albumHeightPx = it }, ) } } @@ -523,6 +505,72 @@ fun ArtistDetailScreen( } } +@Composable +private fun ArtistSectionHeader(text: String) { + Text( + text = text, + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 16.dp), + ) +} + +@Composable +private fun ArtistAlbumCard( + album: Album, + index: Int, + coverArt: (String?, Boolean) -> CoverArtRef?, + downloadStatus: ListDownloadStatus?, + onClick: () -> Unit, + onLongClick: (CoverArtRef?) -> Unit, + onSizeChanged: (Float) -> Unit, +) { + val art = coverArt(album.coverArt, false) + val thumbnail = coverArt(album.coverArt, true) + PrefetchArtworkSeeds(thumbnail) + Column( + modifier = + Modifier + .padding( + start = if (index % 2 == 0) 16.dp else 0.dp, + end = if (index % 2 == 1) 16.dp else 0.dp, + ).combinedClickable( + onClick = onClick, + onLongClick = { onLongClick(thumbnail) }, + ).onSizeChanged { onSizeChanged(it.height.toFloat()) }, + ) { + Box( + modifier = Modifier.fillMaxWidth().aspectRatio(1f).clip(RoundedCornerShape(2.dp)), + ) { + CoverArt( + ref = art, + name = album.name, + thumbnailRef = thumbnail, + showPlaceholder = art == null, + modifier = Modifier.fillMaxSize(), + ) + ListDownloadIndicator( + status = downloadStatus, + modifier = Modifier.align(Alignment.BottomEnd).padding(4.dp), + scrim = true, + ) + } + Text( + text = album.name, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onBackground, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.padding(top = 6.dp), + ) + Text( + text = album.year?.takeIf { it > 0 }?.toString() ?: "\u00A0", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} + @Composable private fun ArtistTitle( text: String, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt index 9d280906..faedd579 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt @@ -2,8 +2,8 @@ package com.subtracks.ui.library import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.subtracks.data.model.Album import com.subtracks.data.model.Artist +import com.subtracks.data.model.ArtistAlbums import com.subtracks.data.model.BulkDownloadAction import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.DownloadList @@ -50,10 +50,10 @@ class ArtistDetailViewModel( val offline: StateFlow = sourceRepository.offline - val albums: Flow?> = + val albums: Flow = combine(sourceId, sourceRepository.offline) { id, offline -> id to offline } .flatMapLatest { (id, offline) -> libraryRepository.artistAlbums(id, artistId, offline) } - .map, List?> { it } + .map { it } .onStart { emit(null) } val downloadStatus: StateFlow = diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 7ca0dbed..83be1aed 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -35,6 +35,8 @@ Artist Artist Album artist + Albums (%1$d) + Appears on (%1$d) 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 5beeea79..758ebdb7 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -515,6 +515,54 @@ class LibraryDaoTest { assertEquals(listOf("al-1"), dao.albumsForArtistDownloaded(sourceId, "ar-guest").first().map { it.id }) } + @Test + fun albumCreditsAreSeparatedFromTrackAppearances() = + runTest { + val sourceId = source() + val dao = db.libraryDao() + dao.upsertAlbums( + listOf( + album(sourceId, "al-1", "Own", year = null, starred = null), + album(sourceId, "al-2", "Other", year = null, starred = null).copy(artistId = "ar-2"), + ), + ) + dao.upsertSongs( + listOf( + song(sourceId, "s1", "One", starred = null), + song(sourceId, "s2", "Two", starred = null).copy(albumId = "al-2", artistId = "ar-1"), + ), + ) + + assertEquals(listOf("al-1"), dao.albumCreditAlbumIds(sourceId, "ar-1").first()) + assertEquals(listOf("al-2"), dao.albumCreditAlbumIds(sourceId, "ar-2").first()) + assertEquals( + listOf("al-1", "al-2"), + dao + .albumsForArtist(sourceId, "ar-1") + .first() + .map { it.id } + .sorted(), + ) + } + + @Test + fun albumCreditIdsFindCreditsThroughArtistIndexes() = + runTest { + val plan = + plan( + "SELECT DISTINCT albumId FROM ($CREDITED_ALBUM_ROWS) " + + "WHERE artistId = :artistId AND albumCredit = 1", + ) + + 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=?)"), + ) + assertFalse(plan, plan.contains("SCAN albums")) + assertFalse(plan, plan.contains("SCAN songs")) + } + @Test fun downloadedArtistsIncludeOneCreditedOnlyOnADownloadedTrack() = runTest { 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 18557e4e..a650ebad 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt @@ -13,6 +13,7 @@ import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.model.Album import com.subtracks.data.model.Song +import com.subtracks.data.model.Source import com.subtracks.data.prefs.AlbumSort import com.subtracks.data.prefs.StarredFilter import com.subtracks.data.prefs.fakeUserPreferences @@ -203,6 +204,33 @@ class LibraryRepositoryTest { } } + @Test + fun artistAlbumsSeparatesAlbumCreditsFromTrackAppearances() = + 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"), + ), + ) + + val albums = repository.artistAlbums(sourceId, "ar1").first() + + assertEquals(listOf("al-0001"), albums.albumCredits.map { it.id }) + assertEquals(listOf("al-0002"), albums.appearances.map { it.id }) + assertEquals(2, albums.count) + } + private suspend fun awaitRefreshes( presenter: RecordingPresenter, target: Int, diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index 3f0fce0e..1036cb9f 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -37,6 +37,7 @@ import com.github.takahirom.roborazzi.RobolectricDeviceQualifiers import com.github.takahirom.roborazzi.captureRoboImage import com.subtracks.data.model.Album import com.subtracks.data.model.Artist +import com.subtracks.data.model.ArtistAlbums import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.ListDownloadStatus import com.subtracks.data.model.Playlist @@ -306,7 +307,10 @@ class AppScreenshotTest { SubtracksTheme { ArtistDetailScreen( artist = Fixtures.artists.first(), - albums = Fixtures.albums.filter { it.artistId == Fixtures.artists.first().id }, + albums = + ArtistAlbums( + albumCredits = Fixtures.albums.filter { it.artistId == Fixtures.artists.first().id }, + ), art = CoverArtRef("art-ar-radiohead", "test:art-ar-radiohead"), artwork = artworkColorsFromSeed(Color.rgb(120, 80, 200)), coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } }, diff --git a/app/src/test/kotlin/com/subtracks/ui/DemoScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/DemoScreenshotTest.kt index 3ce54ea0..7abbe3fb 100644 --- a/app/src/test/kotlin/com/subtracks/ui/DemoScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/DemoScreenshotTest.kt @@ -32,6 +32,7 @@ import com.github.takahirom.roborazzi.captureRoboImage import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.model.Album import com.subtracks.data.model.Artist +import com.subtracks.data.model.ArtistAlbums import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.Playlist import com.subtracks.data.model.Song @@ -177,8 +178,19 @@ class DemoScreenshotTest { ArtistDetailScreen( artist = demo.artist, artistName = demo.artist.name, - isAlbumArtist = demo.artistAlbums.any { album -> album.artistRefs.any { it.id == demo.artist.id } }, - albums = demo.artistAlbums, + albums = + ArtistAlbums( + albumCredits = + demo.artistAlbums.filter { album -> + album.artistId == demo.artist.id || + album.artistRefs.any { it.id == demo.artist.id } + }, + appearances = + demo.artistAlbums.filterNot { album -> + album.artistId == demo.artist.id || + album.artistRefs.any { it.id == demo.artist.id } + }, + ), art = demo.coverArt(demo.artist.coverArt, false), artThumbnail = demo.coverArt(demo.artist.coverArt, true), artwork = demo.artwork(demo.artist.coverArt), diff --git a/app/src/test/kotlin/com/subtracks/ui/library/AlbumArtistLabelTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/AlbumArtistLabelTest.kt deleted file mode 100644 index 4cbb9591..00000000 --- a/app/src/test/kotlin/com/subtracks/ui/library/AlbumArtistLabelTest.kt +++ /dev/null @@ -1,40 +0,0 @@ -package com.subtracks.ui.library - -import com.subtracks.data.model.Album -import com.subtracks.data.model.ArtistRef -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Test - -class AlbumArtistLabelTest { - @Test - fun creditsBothTheSingularIdAndEveryPluralRef() { - val album = - album( - artistId = "a1", - artists = listOf(ArtistRef("a2", "B")), - ) - - assertTrue(isAlbumArtist(listOf(album), "a1")) - assertTrue(isAlbumArtist(listOf(album), "a2")) - assertFalse(isAlbumArtist(listOf(album), "a3")) - } - - private fun album( - artistId: String?, - artists: List, - ) = Album( - sourceId = 1, - id = "al1", - artistId = artistId, - name = "Album", - albumArtist = "Various Artists", - created = 0, - coverArt = null, - genre = null, - year = null, - starred = null, - songCount = 1, - artists = artists, - ) -} 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 745d2f8c..a6ca5ecd 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -2,13 +2,18 @@ package com.subtracks.ui.library import androidx.activity.ComponentActivity import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.hasScrollAction +import androidx.compose.ui.test.hasText import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule import androidx.compose.ui.test.onAllNodesWithContentDescription import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performScrollToNode import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.data.model.Album import com.subtracks.data.model.Artist +import com.subtracks.data.model.ArtistAlbums import com.subtracks.ui.theme.SubtracksTheme import org.junit.Rule import org.junit.Test @@ -24,7 +29,7 @@ class ArtistDetailScreenTest { private fun render( artist: Artist?, artistName: String, - isAlbumArtist: Boolean? = null, + albums: ArtistAlbums = ArtistAlbums(), albumsLoaded: Boolean = true, ) { composeRule.setContent { @@ -32,8 +37,7 @@ class ArtistDetailScreenTest { ArtistDetailScreen( artist = artist, artistName = artistName, - isAlbumArtist = isAlbumArtist, - albums = emptyList(), + albums = albums, albumsLoaded = albumsLoaded, art = null, coverArt = { _, _ -> null }, @@ -46,10 +50,14 @@ class ArtistDetailScreenTest { @Test fun aCreditedTrackArtistShowsItsDerivedNameAndIsLabelledArtist() { - render(artist = null, artistName = "Guest", isAlbumArtist = false) + render( + artist = null, + artistName = "Guest", + albums = ArtistAlbums(appearances = listOf(album("al-1", "Guest Album"))), + ) composeRule.onAllNodesWithText("Guest")[0].assertExists() - composeRule.onNodeWithText("Artist").assertExists() + composeRule.onNodeWithText("Artist · 1 album").assertExists() composeRule.onAllNodesWithContentDescription("More options").assertCountEquals(0) } @@ -58,13 +66,36 @@ class ArtistDetailScreenTest { render( artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), artistName = "Radiohead", - isAlbumArtist = true, + albums = ArtistAlbums(albumCredits = listOf(album("al-1", "Kid A"))), ) - composeRule.onNodeWithText("Album artist").assertExists() + composeRule.onNodeWithText("Album artist · 1 album").assertExists() composeRule.onNodeWithContentDescription("More options").assertExists() } + @Test + fun mixedCreditsShowAlbumCreditsAndAppearancesInSeparateSections() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 2, starred = null), + artistName = "Radiohead", + albums = + ArtistAlbums( + albumCredits = listOf(album("al-1", "Kid A")), + appearances = listOf(album("al-2", "Guest Album")), + ), + ) + + composeRule.onNodeWithText("Album artist · 2 albums").assertExists() + + composeRule.onNode(hasScrollAction()).performScrollToNode(hasText("Albums (1)")) + composeRule.onNodeWithText("Albums (1)").assertExists() + composeRule.onNodeWithText("Kid A").assertExists() + + composeRule.onNode(hasScrollAction()).performScrollToNode(hasText("Appears on (1)")) + composeRule.onNodeWithText("Appears on (1)").assertExists() + composeRule.onNodeWithText("Guest Album").assertExists() + } + @Test fun theTypeAndCountStayBlankUntilTheAlbumsLoad() { render( @@ -77,4 +108,21 @@ class ArtistDetailScreenTest { composeRule.onNodeWithText("Album artist").assertDoesNotExist() composeRule.onNodeWithText("0 albums").assertDoesNotExist() } + + private fun album( + id: String, + name: String, + ) = Album( + sourceId = 1, + id = id, + artistId = "ar-1", + name = name, + albumArtist = "Radiohead", + created = 0, + coverArt = null, + genre = null, + year = null, + starred = null, + songCount = 1, + ) }