mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-15 00:42:54 +01:00
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.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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") } },
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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<ArtistRef>,
|
||||
) = 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,
|
||||
)
|
||||
}
|
||||
@@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user