From d2713ff2f5294791ebe1441b5a4212c16b5145f1 Mon Sep 17 00:00:00 2001 From: Clint Bird Date: Sat, 31 Oct 2026 19:21:41 +0100 Subject: [PATCH] Can Select Artist or Album Artist to be displayed on Music > Artists Page (#2127) ## Description The Music / Artists view currently appears to use the artist metadata from individual tracks. While at a track level this makes perfect sense, it can result in a very noisy and fragmented artists view. For example, an album by Eminem might contain tracks with artists such as: - Eminem - Eminem feat. 50 Cent - Eminem ft. Rihanna - Eminem & Dr. Dre Each of these seem to appear as a separate artist entry (even though they are all tracks from the same album by Eminem) This becomes particularly noticeable in large music libraries, where featured artists, collaborations, remixes, and slightly different artist metadata ends up resulting in a large number of sparsely populated or duplicate / strange looking artist entries. This PR adds an option / toggle to determine which metadata field is used to populate the artists view. This option sits neatly within the current artist view settings e.g. Music > Artists > View options > Artist source - Artist (current behavior) - Album Artist (use the album artist metadata instead) That Artist setting stays as the default choice (so nobody sees a change unless they pick it) but now gives the option to filter down only to Album Artists if desired - this choice is saved per library and applies immediately. ### Related issues Closes #1868 ### Testing I've tested it extensively on an Amazon Fire TV Cube Gen 2 (using the firetv debug variant) the following manual tests were performed: - The list changes when you pick Album artist and importantly the selection survives a full app restart - It was tested on a large library and can confirm only the Album Artists remained (e.g. only one Eminem entry remained) Automated checks were run (testFiretvDebugUnitTest & pre-commit) As well as two new tests being added to the CollectionFolderViewModelTest.kt: - For letter counting its checking via the album artist endpoint (and not the standard one) when the source is set as ALBUM_ARTIST - When the source is changed via the saveViewOptions its triggering a load from the album artist endpoint ## Screenshots Please see the screenshots below showing the UI impact / modification: 1 2 3 ## AI or LLM usage The code was written / supported with Claude - the approach (e.g. design / storing the choice in ViewOptions) endpoint research etc was done by myself - I reviewed the approaches / refactoring and than deployed a debug.apk to my local device to confirm everything was working as expected. I've reviewed each line and can explain it / the thinking to approaches / logic etc and am comfortable submitting the PR. --------- Co-authored-by: Damontecres --- .../wholphin/data/model/GetItemsFilter.kt | 17 ++++ .../ui/components/CollectionFolderView.kt | 98 +++++++++++++++---- .../ui/components/ViewOptionsDialog.kt | 70 +++++++++++-- .../wholphin/util/ApiRequestPager.kt | 21 ++++ app/src/main/res/values/strings.xml | 5 + .../CollectionFolderViewModelTest.kt | 89 ++++++++++++----- 6 files changed, 251 insertions(+), 49 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/model/GetItemsFilter.kt b/app/src/main/java/com/github/damontecres/wholphin/data/model/GetItemsFilter.kt index 3746092b..91228969 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/model/GetItemsFilter.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/model/GetItemsFilter.kt @@ -9,6 +9,7 @@ import kotlinx.serialization.Serializable import kotlinx.serialization.UseSerializers import org.jellyfin.sdk.model.api.BaseItemKind import org.jellyfin.sdk.model.api.VideoType +import org.jellyfin.sdk.model.api.request.GetAlbumArtistsRequest import org.jellyfin.sdk.model.api.request.GetArtistsRequest import org.jellyfin.sdk.model.api.request.GetItemsRequest import org.jellyfin.sdk.model.api.request.GetPersonsRequest @@ -155,6 +156,22 @@ data class GetItemsFilter( }, ) + fun applyTo(req: GetAlbumArtistsRequest) = + req.copy( + minCommunityRating = minCommunityRating, + isFavorite = favorite, + genreIds = genres, + personIds = persons, + studioIds = studios, + tags = tags, + officialRatings = officialRatings, + years = + buildSet { + years?.letNotEmpty(::addAll) + decades?.forEach { addAll(it..<(it + 10)) } + }, + ) + /** * Merge another [GetItemsFilter] onto this one, replacing only unset values */ diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderView.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderView.kt index afd23273..8445dfa9 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderView.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderView.kt @@ -72,6 +72,7 @@ import com.github.damontecres.wholphin.util.ApiRequestPager import com.github.damontecres.wholphin.util.BlockingList import com.github.damontecres.wholphin.util.DataLoadingState import com.github.damontecres.wholphin.util.ExceptionHandler +import com.github.damontecres.wholphin.util.GetAlbumArtistsHandler import com.github.damontecres.wholphin.util.GetArtistsHandler import com.github.damontecres.wholphin.util.GetItemsRequestHandler import com.github.damontecres.wholphin.util.GetPersonsHandler @@ -100,6 +101,7 @@ import org.jellyfin.sdk.model.api.CollectionType import org.jellyfin.sdk.model.api.ImageType import org.jellyfin.sdk.model.api.ItemSortBy import org.jellyfin.sdk.model.api.SortOrder +import org.jellyfin.sdk.model.api.request.GetAlbumArtistsRequest import org.jellyfin.sdk.model.api.request.GetArtistsRequest import org.jellyfin.sdk.model.api.request.GetItemsRequest import org.jellyfin.sdk.model.api.request.GetPersonsRequest @@ -151,6 +153,9 @@ class CollectionFolderViewModel private val _state = MutableStateFlow(CollectionFolderState(viewOptions = defaultViewOptions)) val state: StateFlow = _state + private val artistSource: ArtistSource + get() = state.value.viewOptions.artistSource + var position: Int get() = savedStateHandle.get("position") ?: 0 set(value) { @@ -291,7 +296,17 @@ class CollectionFolderViewModel override fun saveViewOptions(viewOptions: ViewOptions) { position = 0 + val artistSourceChanged = viewOptions.artistSource != artistSource _state.update { it.copy(viewOptions = viewOptions) } + if (artistSourceChanged && state.value.filter.override == GetItemsFilterOverride.ARTIST) { + loadResults( + true, + state.value.sortAndDirection, + recursive, + state.value.filter, + useSeriesForPrimary, + ) + } viewModelScope.launch(ExceptionHandler() + WholphinDispatchers.IO) { saveLibraryDisplayInfo(viewOptions = viewOptions) if (!viewOptions.showBackdrop) { @@ -441,13 +456,27 @@ class CollectionFolderViewModel } GetItemsFilterOverride.ARTIST -> { - ApiRequestPager( - api, - createGetArtistsRequest(filter), - GetArtistsHandler, - viewModelScope, - useSeriesForPrimary = useSeriesForPrimary, - ) + when (artistSource) { + ArtistSource.ARTIST -> { + ApiRequestPager( + api, + createGetArtistsRequest(filter), + GetArtistsHandler, + viewModelScope, + useSeriesForPrimary = useSeriesForPrimary, + ) + } + + ArtistSource.ALBUM_ARTIST -> { + ApiRequestPager( + api, + createGetAlbumArtistsRequest(filter), + GetAlbumArtistsHandler, + viewModelScope, + useSeriesForPrimary = useSeriesForPrimary, + ) + } + } } } @@ -466,6 +495,17 @@ class CollectionFolderViewModel ) } + private fun createGetAlbumArtistsRequest(filter: GetItemsFilter): GetAlbumArtistsRequest { + val item = state.value.item.successValue + return filter.applyTo( + GetAlbumArtistsRequest( + parentId = item?.id, + enableImageTypes = listOf(ImageType.PRIMARY, ImageType.THUMB), + fields = SlimItemFields, + ), + ) + } + private fun createGetItemsRequest( sortAndDirection: SortAndDirection, recursive: Boolean, @@ -537,18 +577,37 @@ class CollectionFolderViewModel val filter = state.value.filter when (filter.override) { GetItemsFilterOverride.ARTIST -> { - GetArtistsHandler.countMatching( - api = api, - request = - createGetArtistsRequest(filter).copy( - enableImageTypes = null, - fields = null, - nameLessThan = letter.toString(), - limit = 0, - enableTotalRecordCount = true, - enableUserData = false, - ), - ) + when (artistSource) { + ArtistSource.ARTIST -> { + GetArtistsHandler.countMatching( + api = api, + request = + createGetArtistsRequest(filter).copy( + enableImageTypes = null, + fields = null, + nameLessThan = letter.toString(), + limit = 0, + enableTotalRecordCount = true, + enableUserData = false, + ), + ) + } + + ArtistSource.ALBUM_ARTIST -> { + GetAlbumArtistsHandler.countMatching( + api = api, + request = + createGetAlbumArtistsRequest(filter).copy( + enableImageTypes = null, + fields = null, + nameLessThan = letter.toString(), + limit = 0, + enableTotalRecordCount = true, + enableUserData = false, + ), + ) + } + } } // GetPersonsRequest has no nameLessThan or startIndex, so /Persons cannot be @@ -1127,6 +1186,7 @@ fun CollectionFolderViewContent( ViewOptionsDialog( viewOptions = state.viewOptions, defaultViewOptions = defaultViewOptions, + showArtistSource = state.filter.override == GetItemsFilterOverride.ARTIST, onDismissRequest = { showViewOptions = false viewActions.saveViewOptions(state.viewOptions) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/ViewOptionsDialog.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/ViewOptionsDialog.kt index a1237acd..df923a55 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/ViewOptionsDialog.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/ViewOptionsDialog.kt @@ -55,6 +55,7 @@ fun ViewOptionsDialog( onDismissRequest: () -> Unit, onViewOptionsChange: (ViewOptions) -> Unit, defaultViewOptions: ViewOptions = ViewOptions(), + showArtistSource: Boolean = false, ) { val focusRequester = remember { FocusRequester() } LaunchedEffect(Unit) { focusRequester.tryRequestFocus() } @@ -72,11 +73,11 @@ fun ViewOptionsDialog( window.setDimAmount(0f) } val options = - remember(viewOptions.type) { + remember(viewOptions.type, showArtistSource) { when (viewOptions.type) { - ViewOptionsType.GRID -> ViewOptions.GRID_OPTIONS - ViewOptionsType.LIST -> ViewOptions.LIST_OPTIONS - ViewOptionsType.DENSE_LIST -> ViewOptions.LIST_OPTIONS + ViewOptionsType.GRID -> ViewOptions.buildGridOptions(showArtistSource) + ViewOptionsType.LIST -> ViewOptions.buildListOptions(showArtistSource) + ViewOptionsType.DENSE_LIST -> ViewOptions.buildListOptions(showArtistSource) } } Column( @@ -130,6 +131,12 @@ fun ViewOptionsDialog( } } +/** Which Jellyfin artists list backs the Music > Artists tab */ +enum class ArtistSource { + ARTIST, + ALBUM_ARTIST, +} + /** * Stores the customizable view changes from the user * @@ -146,6 +153,7 @@ data class ViewOptions( val imageType: ViewOptionImageType = ViewOptionImageType.PRIMARY, val showTitles: Boolean = true, val type: ViewOptionsType = ViewOptionsType.GRID, + val extras: ViewOptionsExtras = ViewOptionsExtras.None, ) { companion object { val ViewOptionsColumns = @@ -250,12 +258,34 @@ data class ViewOptions( valueToIndex = { it.ordinal }, ) + val ViewOptionsArtistSource = + AppChoicePreference( + title = R.string.artist_source, + defaultValue = ArtistSource.ARTIST, + displayValues = R.array.artist_sources, + getter = { it.artistSource }, + setter = { viewOptions, value -> + viewOptions.copy(extras = ViewOptionsExtras.MusicLibrary(value)) + }, + indexToValue = { ArtistSource.entries[it] }, + valueToIndex = { it.ordinal }, + ) + val ViewOptionsReset = AppClickablePreference( title = R.string.reset, ) - val GRID_OPTIONS = + /** + * Get the options for a grid [ViewOptions] + */ + fun buildGridOptions(showArtistSource: Boolean) = + buildList { + if (showArtistSource) add(ViewOptionsArtistSource) + addAll(GRID_OPTIONS) + } + + private val GRID_OPTIONS = listOf( ViewOptionsTypePref, ViewOptionsImageType, @@ -269,7 +299,16 @@ data class ViewOptions( ViewOptionsReset, ) - val LIST_OPTIONS = + /** + * Get the options for a list [ViewOptions] + */ + fun buildListOptions(showArtistSource: Boolean) = + buildList { + if (showArtistSource) add(ViewOptionsArtistSource) + addAll(LIST_OPTIONS) + } + + private val LIST_OPTIONS = listOf( ViewOptionsTypePref, ViewOptionsDetailHeader, @@ -338,3 +377,22 @@ val BaseItemKind.defaultViewOptions: ViewOptions else -> ViewOptionsPoster } + +/** + * Extras [ViewOptions] info that is context or library specific + */ +@Serializable +sealed interface ViewOptionsExtras { + @Serializable + data object None : ViewOptionsExtras + + @Serializable + data class MusicLibrary( + val artistSource: ArtistSource = ArtistSource.ARTIST, + ) : ViewOptionsExtras +} + +/** + * The [ArtistSource] if the [ViewOptions.extras] is a [ViewOptionsExtras.MusicLibrary] + */ +val ViewOptions.artistSource: ArtistSource get() = (extras as? ViewOptionsExtras.MusicLibrary)?.artistSource ?: ArtistSource.ARTIST diff --git a/app/src/main/java/com/github/damontecres/wholphin/util/ApiRequestPager.kt b/app/src/main/java/com/github/damontecres/wholphin/util/ApiRequestPager.kt index 750ba711..a82201a9 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/util/ApiRequestPager.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/util/ApiRequestPager.kt @@ -22,6 +22,7 @@ import org.jellyfin.sdk.api.client.extensions.tvShowsApi import org.jellyfin.sdk.api.client.extensions.userLibraryApi import org.jellyfin.sdk.model.api.BaseItemDtoQueryResult import org.jellyfin.sdk.model.api.GetProgramsDto +import org.jellyfin.sdk.model.api.request.GetAlbumArtistsRequest import org.jellyfin.sdk.model.api.request.GetArtistsRequest import org.jellyfin.sdk.model.api.request.GetEpisodesRequest import org.jellyfin.sdk.model.api.request.GetGenresRequest @@ -401,6 +402,25 @@ object GetArtistsHandler : RequestHandler { ): Response = api.artistsApi.getArtists((request)) } +@Serializable +object GetAlbumArtistsHandler : RequestHandler { + override fun prepare( + request: GetAlbumArtistsRequest, + startIndex: Int, + limit: Int, + enableTotalRecordCount: Boolean, + ): GetAlbumArtistsRequest = + request.copy( + startIndex = startIndex, + limit = limit, + ) + + override suspend fun execute( + api: ApiClient, + request: GetAlbumArtistsRequest, + ): Response = api.artistsApi.getAlbumArtists(request) +} + val requestSerializersModule = SerializersModule { polymorphic(Any::class) { @@ -417,5 +437,6 @@ val requestSerializersModule = subclass(GetRecordingsRequest::class) subclass(GetLiveTvChannelsRequest::class) subclass(GetArtistsHandler::class) + subclass(GetAlbumArtistsHandler::class) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 189bf82d..cf49b425 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -806,6 +806,11 @@ Creator Artist Album artist + Artist source + + @string/artist + @string/album_artist + Album Popular songs Play next diff --git a/app/src/test/java/com/github/damontecres/wholphin/ui/components/CollectionFolderViewModelTest.kt b/app/src/test/java/com/github/damontecres/wholphin/ui/components/CollectionFolderViewModelTest.kt index bc879d24..ba7f1e14 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/ui/components/CollectionFolderViewModelTest.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/ui/components/CollectionFolderViewModelTest.kt @@ -9,6 +9,7 @@ import com.github.damontecres.wholphin.services.DeletedItem import com.github.damontecres.wholphin.services.MediaManagementService import com.github.damontecres.wholphin.ui.successQueryResult import com.github.damontecres.wholphin.ui.successResponse +import com.github.damontecres.wholphin.util.GetAlbumArtistsHandler import com.github.damontecres.wholphin.util.GetArtistsHandler import com.github.damontecres.wholphin.util.GetItemsRequestHandler import com.github.damontecres.wholphin.util.GetPersonsHandler @@ -34,6 +35,7 @@ import org.jellyfin.sdk.model.api.BaseItemDto import org.jellyfin.sdk.model.api.BaseItemKind import org.jellyfin.sdk.model.api.CollectionType import org.jellyfin.sdk.model.api.ItemFields +import org.jellyfin.sdk.model.api.request.GetAlbumArtistsRequest import org.jellyfin.sdk.model.api.request.GetArtistsRequest import org.jellyfin.sdk.model.api.request.GetItemsRequest import org.junit.After @@ -57,6 +59,7 @@ class CollectionFolderViewModelTest { private val mockUserLibraryApi = mockk() private val artistsRequest = slot() + private val albumArtistsRequest = slot() private val itemsRequest = slot() private val libraryId = UUID.randomUUID() @@ -83,11 +86,14 @@ class CollectionFolderViewModelTest { // against the relaxed ApiClient mockkObject(GetItemsRequestHandler) mockkObject(GetArtistsHandler) + mockkObject(GetAlbumArtistsHandler) mockkObject(GetPersonsHandler) coEvery { GetItemsRequestHandler.execute(mockApi, capture(itemsRequest)) } returns successQueryResult(totalRecordCount = LIBRARY_ITEM_COUNT) coEvery { GetArtistsHandler.execute(mockApi, capture(artistsRequest)) } returns successQueryResult(totalRecordCount = ARTIST_COUNT) + coEvery { GetAlbumArtistsHandler.execute(mockApi, capture(albumArtistsRequest)) } returns + successQueryResult(totalRecordCount = ALBUM_ARTIST_COUNT) coEvery { GetPersonsHandler.execute(mockApi, any()) } returns successQueryResult(totalRecordCount = PERSON_COUNT) } @@ -97,33 +103,36 @@ class CollectionFolderViewModelTest { WholphinDispatchers.reset() unmockkObject(GetItemsRequestHandler) unmockkObject(GetArtistsHandler) + unmockkObject(GetAlbumArtistsHandler) unmockkObject(GetPersonsHandler) } - private fun createViewModel(filter: GetItemsFilter) = - CollectionFolderViewModel( - savedStateHandle = SavedStateHandle(), - api = mockApi, - context = mockk(relaxed = true), - serverRepository = mockServerRepository, - libraryDisplayInfoDao = mockk(relaxed = true), - favoriteWatchManager = mockk(relaxed = true), - backdropService = mockk(relaxed = true), - navigationManager = mockk(relaxed = true), - themeSongPlayer = mockk(relaxed = true), - userPreferencesService = mockk(relaxed = true), - mediaManagementService = mockMediaManagementService, - musicService = mockk(relaxed = true), - streamChoiceService = mockk(relaxed = true), - serverReportService = mockk(relaxed = true), - filterOptionCache = mockk(relaxed = true), - itemId = libraryId.toString(), - initialSortAndDirection = null, - recursive = true, - collectionFilter = CollectionFolderFilter(filter = filter), - useSeriesForPrimary = false, - defaultViewOptions = ViewOptionsSquare, - ) + private fun createViewModel( + filter: GetItemsFilter, + viewOptions: ViewOptions = ViewOptionsSquare, + ) = CollectionFolderViewModel( + savedStateHandle = SavedStateHandle(), + api = mockApi, + context = mockk(relaxed = true), + serverRepository = mockServerRepository, + libraryDisplayInfoDao = mockk(relaxed = true), + favoriteWatchManager = mockk(relaxed = true), + backdropService = mockk(relaxed = true), + navigationManager = mockk(relaxed = true), + themeSongPlayer = mockk(relaxed = true), + userPreferencesService = mockk(relaxed = true), + mediaManagementService = mockMediaManagementService, + musicService = mockk(relaxed = true), + streamChoiceService = mockk(relaxed = true), + serverReportService = mockk(relaxed = true), + filterOptionCache = mockk(relaxed = true), + itemId = libraryId.toString(), + initialSortAndDirection = null, + recursive = true, + collectionFilter = CollectionFolderFilter(filter = filter), + useSeriesForPrimary = false, + defaultViewOptions = viewOptions, + ) /** Counting via /Items would return every song and album, overshooting the artist grid */ @Test @@ -143,6 +152,37 @@ class CollectionFolderViewModelTest { coVerify(exactly = 0) { GetItemsRequestHandler.execute(any(), any()) } } + @Test + fun `positionOfLetter counts album artists for the ALBUM_ARTIST source`() = + runTest(testDispatcher) { + val viewModel = + createViewModel( + GetItemsFilter(override = GetItemsFilterOverride.ARTIST), + ViewOptionsSquare.copy(extras = ViewOptionsExtras.MusicLibrary(ArtistSource.ALBUM_ARTIST)), + ) + advanceUntilIdle() + + val position = viewModel.positionOfLetter('K') + + assertEquals(ALBUM_ARTIST_COUNT, position) + assertEquals("K", albumArtistsRequest.captured.nameLessThan) + assertEquals(libraryId, albumArtistsRequest.captured.parentId) + coVerify(exactly = 0) { GetArtistsHandler.execute(any(), any()) } + } + + @Test + fun `changing the artist source reloads from the album artists endpoint`() = + runTest(testDispatcher) { + val viewModel = createViewModel(GetItemsFilter(override = GetItemsFilterOverride.ARTIST)) + advanceUntilIdle() + coVerify(exactly = 0) { GetAlbumArtistsHandler.execute(any(), any()) } + + viewModel.saveViewOptions(ViewOptionsSquare.copy(extras = ViewOptionsExtras.MusicLibrary(ArtistSource.ALBUM_ARTIST))) + advanceUntilIdle() + + coVerify { GetAlbumArtistsHandler.execute(any(), any()) } + } + /** * Without SortName, the alphabet highlight falls back to Name and highlights "T" for artists * like "The Beatles" even though the grid sorts them under "B". @@ -197,6 +237,7 @@ class CollectionFolderViewModelTest { companion object { private const val LIBRARY_ITEM_COUNT = 7315 private const val ARTIST_COUNT = 730 + private const val ALBUM_ARTIST_COUNT = 215 private const val PERSON_COUNT = 12 } }