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 1b7a33e8..2e0e73bf 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -942,6 +942,16 @@ interface LibraryDao { artistId: String, ): Flow + // A credited track artist may not have an artists row; its name only lives on the songs. + @Query( + "SELECT artist FROM songs WHERE sourceId = :sourceId AND artistId = :artistId " + + "AND artist IS NOT NULL AND artist != '' ORDER BY id LIMIT 1", + ) + fun artistName( + sourceId: Long, + artistId: String, + ): Flow + @Query("SELECT * FROM songs WHERE sourceId = :sourceId AND id = :songId") fun song( sourceId: Long, 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 694ceaa0..9d1fca58 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -348,6 +348,11 @@ class LibraryRepository( artistId: String, ): Flow = db.libraryDao().artist(sourceId, artistId) + fun artistName( + sourceId: Long, + artistId: String, + ): Flow = db.libraryDao().artistName(sourceId, artistId) + fun song( sourceId: Long, songId: String, 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 e88aa820..22ade453 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -152,6 +152,7 @@ fun ArtistDetailRoute( playbackController: PlaybackController = koinInject(), ) { val artist by viewModel.artist.collectAsStateWithLifecycle(initialValue = null) + val artistName by viewModel.artistName.collectAsStateWithLifecycle(initialValue = null) val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = emptyList()) val art by viewModel.art.collectAsStateWithLifecycle() val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle() @@ -184,10 +185,12 @@ fun ArtistDetailRoute( ) ArtistDetailScreen( artist = artist, + artistName = artistName.orEmpty(), + isAlbumArtist = albums.any { it.artistId == artistId }, albums = albums, art = art, artThumbnail = artThumbnail, - artwork = rememberArtworkColors(shortcutArt ?: artThumbnail ?: art, THEME_TRANSITION_MS, fallbackName = artist?.name), + artwork = rememberArtworkColors(shortcutArt ?: artThumbnail ?: art, THEME_TRANSITION_MS, fallbackName = artistName), coverArt = viewModel::coverArt, onBack = onBack, onAlbumClick = onAlbumClick, @@ -213,6 +216,8 @@ fun ArtistDetailRoute( @Composable fun ArtistDetailScreen( artist: Artist?, + artistName: String = "", + isAlbumArtist: Boolean = false, albums: List, art: CoverArtRef?, artThumbnail: CoverArtRef? = null, @@ -240,6 +245,8 @@ fun ArtistDetailScreen( val navBarBottom = with(density) { WindowInsets.navigationBars.getBottom(this).toDp() } val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight val albumCount = albums.size.toLong() + val displayName = artistName.ifBlank { artist?.name.orEmpty() } + val artistType = stringResource(if (isAlbumArtist) R.string.artist_type_album_artist else R.string.artist_type_artist) val imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() } val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() } @@ -309,7 +316,7 @@ fun ArtistDetailScreen( Box(Modifier.fillMaxWidth()) { CoverArt( ref = art, - name = artist?.name.orEmpty(), + name = displayName, thumbnailRef = artThumbnail, showPlaceholder = art == null, modifier = Modifier.fillMaxWidth().height(ART_HEIGHT), @@ -331,7 +338,7 @@ fun ArtistDetailScreen( } Box(Modifier.align(Alignment.BottomStart).fillMaxWidth()) { ArtistTitle( - text = artist?.name.orEmpty(), + text = displayName, style = imageNameStyle, color = Color.White, endReserve = SHUFFLE_RESERVE, @@ -360,12 +367,18 @@ fun ArtistDetailScreen( modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = 34.dp, bottom = 0.dp), verticalAlignment = Alignment.CenterVertically, ) { - Text( - text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount), - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.weight(1f), - ) + Column(Modifier.weight(1f)) { + Text( + text = artistType, + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + Text( + text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount), + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } IconButton( onClick = { onToggleStar?.invoke() }, enabled = onToggleStar != null, @@ -452,7 +465,7 @@ fun ArtistDetailScreen( TopAppBar( title = { Text( - text = artist?.name.orEmpty(), + text = displayName, maxLines = 1, overflow = TextOverflow.Ellipsis, modifier = Modifier.graphicsLayer { alpha = barFraction }, 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 5edc2e10..604c1e84 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt @@ -38,6 +38,14 @@ class ArtistDetailViewModel( val artist: Flow = sourceId.flatMapLatest { libraryRepository.artist(it, artistId) } + val artistName: Flow = + sourceId.flatMapLatest { id -> + combine( + libraryRepository.artist(id, artistId), + libraryRepository.artistName(id, artistId), + ) { loaded, fromSongs -> loaded?.name?.takeIf { it.isNotBlank() } ?: fromSongs } + } + val offline: StateFlow = sourceRepository.offline val albums: Flow> = diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 939b9dc7..39301538 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -29,6 +29,8 @@ Playlists Album Artist + Artist + Album artist 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 1a31140f..b8ad315a 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -558,6 +558,20 @@ class LibraryDaoTest { ) } + @Test + fun aCreditedTrackArtistsNameComesFromItsSongs() = + runTest { + val sourceId = source() + val dao = db.libraryDao() + dao.upsertSongs( + listOf( + song(sourceId, "s1", "One", starred = null).copy(artistId = "ar-guest", artist = "Guest"), + ), + ) + + assertEquals("Guest", dao.artistName(sourceId, "ar-guest").first()) + } + @Test fun downloadedFilterReportsTheDownloadedAlbumCount() = 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 new file mode 100644 index 00000000..d0cf3035 --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -0,0 +1,60 @@ +package com.subtracks.ui.library + +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithText +import androidx.compose.ui.test.onNodeWithText +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.data.model.Artist +import com.subtracks.ui.theme.SubtracksTheme +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.annotation.GraphicsMode + +@RunWith(AndroidJUnit4::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class ArtistDetailScreenTest { + @get:Rule + val composeRule = createAndroidComposeRule() + + private fun render( + artist: Artist?, + artistName: String, + isAlbumArtist: Boolean, + ) { + composeRule.setContent { + SubtracksTheme { + ArtistDetailScreen( + artist = artist, + artistName = artistName, + isAlbumArtist = isAlbumArtist, + albums = emptyList(), + art = null, + coverArt = { _, _ -> null }, + onBack = {}, + onAlbumClick = {}, + ) + } + } + } + + @Test + fun aCreditedTrackArtistShowsItsDerivedNameAndIsLabelledArtist() { + render(artist = null, artistName = "Guest", isAlbumArtist = false) + + composeRule.onAllNodesWithText("Guest")[0].assertExists() + composeRule.onNodeWithText("Artist").assertExists() + } + + @Test + fun anAlbumArtistIsLabelledAlbumArtist() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + isAlbumArtist = true, + ) + + composeRule.onNodeWithText("Album artist").assertExists() + } +}