Fix person credits: movie release dates, full filmography, sorted order (#1950)
<!-- By submitting this pull request, you acknowledge that you have read the [contributing guide](https://github.com/damontecres/Wholphin/blob/main/CONTRIBUTING.md, including the AI/LLM policy, and [developer's guide](https://github.com/damontecres/Wholphin/blob/main/DEVELOPMENT.md) --> ## Description Person pages that get a filmography from Seerr had three problems, all in the same place. The release date was taken from `firstAirDate` alone — the field TMDB fills for series — so movie cards showed no year at all. The library person page capped the row at 25 cast + 25 crew while `DiscoverPersonPage` had no cap, because each screen carried its own copy of the same logic. And `cast + crew` left the row in no order a viewer can read, with a title appearing twice when a person is both cast and crew on it. This PR: * maps credit dates as `releaseDate ?: firstAirDate` — the expression the search result mapping already uses; * moves the credits logic into a single `SeerrService.personCredits()` used by both screens, so they cannot drift apart again; * drops the 25 + 25 cap, de-duplicates by TMDB id and sorts by release date descending, with undated entries last. Rows on movie and series pages are untouched: only the `BaseItemKind.PERSON` branch of `similar()` changed. Two things worth your opinion: * Without the cap, the row for a person with a long filmography is long — 135 cards for Cameron Diaz, for example. If you would rather keep a cap, the row could keep the newest 25 and gain a `ViewMoreCard` opening the full grid — `Destination.DiscoveredItem` with a `PERSON` item already routes to `DiscoverPersonPage`. I am happy to add that in a second commit. * The sort is fixed to release date descending. It now lives in one function, so a different key or a user-facing choice would be cheap to add later. ### Related issues Fixes #1873 ### Testing CI in my fork ran the repository's own PR workflow on this branch: `pre-commit` (ktlint) and `assembleDefaultDebug` with `testDefaultDebugUnitTest` are both green. I then installed a debug build of the same code on an NVIDIA Shield TV Pro 2019 (Android 11), against Jellyfin 10.11.11 with Jellyseerr, and opened Cameron Diaz from the cast row of *Charlie's Angels* before and after the change: * movie cards show a year now; before, the only two cards in the row with a year were the two TV entries; * the row is ordered by release date descending, movies and series interleaved, instead of the API order of all cast followed by all crew; * the row is no longer cut at 25 + 25 — it now runs back to her 1952 credit, and the entries TMDB has no date for sit at the very end of the row. Not tested on any other device; nothing in the change is device specific. ## Screenshots Person page of Cameron Diaz, reached from the cast row of *Charlie's Angels*. The library rows above the Discover row are the same in every shot and are useful as a reference: they always had their years. Before, start of the Discover row — no year under any card: <img width="1920" height="1080" alt="pr1873-1-before-start" src="https://github.com/user-attachments/assets/585d0670-ae39-4214-9004-0c92262991da" /> Before, end of the Discover row — the row stops here, and the only two years in it belong to the two TV entries: <img width="1920" height="1080" alt="pr1873-2-before-end" src="https://github.com/user-attachments/assets/85c58575-2ad0-4369-a103-60f04c8d2350" /> After, start of the row — years on the movie cards, ordered by date descending: <img width="1920" height="1080" alt="pr1873-3-after-start" src="https://github.com/user-attachments/assets/804745c5-e1a6-4fe4-93bd-dc6c97a7a876" /> After, end of the row — the list now reaches 1952, with the undated entries last: <img width="1920" height="1080" alt="pr1873-4-after-end" src="https://github.com/user-attachments/assets/2c32389c-ec0b-4df5-b9d1-f564ef10d8b3" /> ## AI or LLM usage The code was written with Claude Opus 5 (Claude Code): the two mapping lines, the extracted `personCredits()` function and the two call sites. I reviewed every line and can explain it. This description was written by me in Russian and translated into English with the same assistant. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Damontecres <damontecres@gmail.com>
This commit is contained in:
parent
9c56965c82
commit
6486bd19ed
5 changed files with 77 additions and 32 deletions
|
|
@ -124,21 +124,7 @@ class SeerrService
|
||||||
}
|
}
|
||||||
|
|
||||||
BaseItemKind.PERSON -> {
|
BaseItemKind.PERSON -> {
|
||||||
api.personApi
|
personCredits(it)
|
||||||
.personPersonIdCombinedCreditsGet(personId = it)
|
|
||||||
.let { credits ->
|
|
||||||
val cast =
|
|
||||||
credits.cast
|
|
||||||
?.take(25)
|
|
||||||
?.map { createDiscoverItem(it) }
|
|
||||||
.orEmpty()
|
|
||||||
val crew =
|
|
||||||
credits.crew
|
|
||||||
?.take(25)
|
|
||||||
?.map { createDiscoverItem(it) }
|
|
||||||
.orEmpty()
|
|
||||||
cast + crew
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
else -> {
|
else -> {
|
||||||
|
|
@ -150,6 +136,50 @@ class SeerrService
|
||||||
null
|
null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get the combined movie & TV credits of a person as [DiscoverItem]s
|
||||||
|
*
|
||||||
|
* The two lists returned by the API are merged, de-duplicated by id since a person can be
|
||||||
|
* both cast and crew on the same title, and sorted by release date descending with undated
|
||||||
|
* entries last
|
||||||
|
*/
|
||||||
|
suspend fun personCredits(personId: Int): List<DiscoverItem> =
|
||||||
|
api.personApi
|
||||||
|
.personPersonIdCombinedCreditsGet(personId = personId)
|
||||||
|
.let { credits ->
|
||||||
|
val cast = credits.cast?.map { createDiscoverItem(it) }.orEmpty()
|
||||||
|
val crew = credits.crew?.map { createDiscoverItem(it) }.orEmpty()
|
||||||
|
(cast + crew)
|
||||||
|
.distinctBy { it.id }
|
||||||
|
.sortedByDescending { it.releaseDate }
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a [DiscoverItem] for a person in the library so that the full grid of their
|
||||||
|
* credits can be opened from their page
|
||||||
|
*
|
||||||
|
* @return the item or null if the person has no TMDB id
|
||||||
|
*/
|
||||||
|
fun discoverPerson(item: BaseItem): DiscoverItem? =
|
||||||
|
item.data.providerIds
|
||||||
|
?.get("Tmdb")
|
||||||
|
?.toIntOrNull()
|
||||||
|
?.let {
|
||||||
|
DiscoverItem(
|
||||||
|
id = it,
|
||||||
|
type = SeerrItemType.PERSON,
|
||||||
|
title = item.name,
|
||||||
|
subtitle = null,
|
||||||
|
overview = item.data.overview,
|
||||||
|
availability = SeerrAvailability.UNKNOWN,
|
||||||
|
releaseDate = null,
|
||||||
|
posterUrl = null,
|
||||||
|
backDropUrl = null,
|
||||||
|
logoUrl = null,
|
||||||
|
jellyfinItemId = null,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
suspend fun getTvSeries(item: BaseItem): TvDetails? =
|
suspend fun getTvSeries(item: BaseItem): TvDetails? =
|
||||||
if (active.first()) {
|
if (active.first()) {
|
||||||
item.data.providerIds
|
item.data.providerIds
|
||||||
|
|
@ -424,7 +454,7 @@ class SeerrService
|
||||||
availability =
|
availability =
|
||||||
SeerrAvailability.from(credit.mediaInfo?.status)
|
SeerrAvailability.from(credit.mediaInfo?.status)
|
||||||
?: SeerrAvailability.UNKNOWN,
|
?: SeerrAvailability.UNKNOWN,
|
||||||
releaseDate = toLocalDate(credit.firstAirDate),
|
releaseDate = toLocalDate(credit.releaseDate ?: credit.firstAirDate),
|
||||||
posterUrl =
|
posterUrl =
|
||||||
createImageUrl(
|
createImageUrl(
|
||||||
ImageType.PRIMARY,
|
ImageType.PRIMARY,
|
||||||
|
|
@ -451,7 +481,7 @@ class SeerrService
|
||||||
availability =
|
availability =
|
||||||
SeerrAvailability.from(credit.mediaInfo?.status)
|
SeerrAvailability.from(credit.mediaInfo?.status)
|
||||||
?: SeerrAvailability.UNKNOWN,
|
?: SeerrAvailability.UNKNOWN,
|
||||||
releaseDate = toLocalDate(credit.firstAirDate),
|
releaseDate = toLocalDate(credit.releaseDate ?: credit.firstAirDate),
|
||||||
posterUrl =
|
posterUrl =
|
||||||
createImageUrl(
|
createImageUrl(
|
||||||
ImageType.PRIMARY,
|
ImageType.PRIMARY,
|
||||||
|
|
|
||||||
|
|
@ -157,6 +157,7 @@ class PersonViewModel
|
||||||
_state.update {
|
_state.update {
|
||||||
it.copy(
|
it.copy(
|
||||||
discovered = results,
|
discovered = results,
|
||||||
|
discoverPerson = seerrService.discoverPerson(person),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -221,6 +222,7 @@ data class PersonState(
|
||||||
val series: RowLoadingState = RowLoadingState.Pending,
|
val series: RowLoadingState = RowLoadingState.Pending,
|
||||||
val episodes: RowLoadingState = RowLoadingState.Pending,
|
val episodes: RowLoadingState = RowLoadingState.Pending,
|
||||||
val discovered: List<DiscoverItem> = emptyList(),
|
val discovered: List<DiscoverItem> = emptyList(),
|
||||||
|
val discoverPerson: DiscoverItem? = null,
|
||||||
)
|
)
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
|
|
@ -257,6 +259,12 @@ fun PersonPage(
|
||||||
imageType = ImageType.PRIMARY,
|
imageType = ImageType.PRIMARY,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
// A long filmography is hard to navigate as a row, so it is cut at the same
|
||||||
|
// limit as the home page rows and the rest is left to the full grid
|
||||||
|
val maxItems = preferences.appPreferences.homePagePreferences.maxItemsPerRow
|
||||||
|
val discovered =
|
||||||
|
remember(state.discovered, maxItems) { state.discovered.take(maxItems) }
|
||||||
|
|
||||||
PersonPageContent(
|
PersonPageContent(
|
||||||
preferences = preferences,
|
preferences = preferences,
|
||||||
name = name,
|
name = name,
|
||||||
|
|
@ -276,10 +284,18 @@ fun PersonPage(
|
||||||
favoriteOnClick = {
|
favoriteOnClick = {
|
||||||
viewModel.setFavorite(!person.favorite)
|
viewModel.setFavorite(!person.favorite)
|
||||||
},
|
},
|
||||||
discovered = state.discovered,
|
discovered = discovered,
|
||||||
onClickDiscover = { index, item ->
|
onClickDiscover = { index, item ->
|
||||||
viewModel.navigationManager.navigateTo(item.destination)
|
viewModel.navigationManager.navigateTo(item.destination)
|
||||||
},
|
},
|
||||||
|
onClickViewMoreDiscover = {
|
||||||
|
state.discoverPerson?.let {
|
||||||
|
viewModel.navigationManager.navigateTo(
|
||||||
|
Destination.DiscoveredItem(it, maxItems),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
enableViewMoreDiscover = state.discovered.size > discovered.size,
|
||||||
modifier = modifier,
|
modifier = modifier,
|
||||||
)
|
)
|
||||||
AnimatedVisibility(showOverviewDialog) {
|
AnimatedVisibility(showOverviewDialog) {
|
||||||
|
|
@ -323,6 +339,8 @@ fun PersonPageContent(
|
||||||
overviewOnClick: () -> Unit,
|
overviewOnClick: () -> Unit,
|
||||||
favoriteOnClick: () -> Unit,
|
favoriteOnClick: () -> Unit,
|
||||||
onClickDiscover: (Int, DiscoverItem) -> Unit,
|
onClickDiscover: (Int, DiscoverItem) -> Unit,
|
||||||
|
onClickViewMoreDiscover: () -> Unit,
|
||||||
|
enableViewMoreDiscover: Boolean,
|
||||||
modifier: Modifier = Modifier,
|
modifier: Modifier = Modifier,
|
||||||
) {
|
) {
|
||||||
val focusRequester = remember { FocusRequester() }
|
val focusRequester = remember { FocusRequester() }
|
||||||
|
|
@ -437,6 +455,11 @@ fun PersonPageContent(
|
||||||
onLongClickItem = { _, _ -> },
|
onLongClickItem = { _, _ -> },
|
||||||
onCardFocus = {},
|
onCardFocus = {},
|
||||||
focusRequester = focusRequester,
|
focusRequester = focusRequester,
|
||||||
|
enableViewMore = enableViewMoreDiscover,
|
||||||
|
onClickViewMore = {
|
||||||
|
position = RowColumn(DISCOVER_ROW, discovered.size)
|
||||||
|
onClickViewMoreDiscover.invoke()
|
||||||
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -61,20 +61,7 @@ class DiscoverPersonViewModel
|
||||||
viewModelScope.launchIO {
|
viewModelScope.launchIO {
|
||||||
backdropService.clearBackdrop()
|
backdropService.clearBackdrop()
|
||||||
|
|
||||||
val credits =
|
val credits = seerrService.personCredits(item.id)
|
||||||
seerrService.api.personApi
|
|
||||||
.personPersonIdCombinedCreditsGet(personId = item.id)
|
|
||||||
.let { credits ->
|
|
||||||
val cast =
|
|
||||||
credits.cast
|
|
||||||
?.map { seerrService.createDiscoverItem(it) }
|
|
||||||
.orEmpty()
|
|
||||||
val crew =
|
|
||||||
credits.crew
|
|
||||||
?.map { seerrService.createDiscoverItem(it) }
|
|
||||||
.orEmpty()
|
|
||||||
cast + crew
|
|
||||||
}
|
|
||||||
this@DiscoverPersonViewModel.credits.update {
|
this@DiscoverPersonViewModel.credits.update {
|
||||||
DataLoadingState.Success(credits)
|
DataLoadingState.Success(credits)
|
||||||
}
|
}
|
||||||
|
|
@ -85,6 +72,7 @@ class DiscoverPersonViewModel
|
||||||
@Composable
|
@Composable
|
||||||
fun DiscoverPersonPage(
|
fun DiscoverPersonPage(
|
||||||
person: DiscoverItem,
|
person: DiscoverItem,
|
||||||
|
startIndex: Int,
|
||||||
modifier: Modifier = Modifier,
|
modifier: Modifier = Modifier,
|
||||||
viewModel: DiscoverPersonViewModel =
|
viewModel: DiscoverPersonViewModel =
|
||||||
hiltViewModel<DiscoverPersonViewModel, DiscoverPersonViewModel.Factory>(
|
hiltViewModel<DiscoverPersonViewModel, DiscoverPersonViewModel.Factory>(
|
||||||
|
|
@ -137,6 +125,7 @@ fun DiscoverPersonPage(
|
||||||
},
|
},
|
||||||
onClickPlay = { _, item ->
|
onClickPlay = { _, item ->
|
||||||
},
|
},
|
||||||
|
initialPosition = startIndex,
|
||||||
letterPosition = { c: Char -> 0 },
|
letterPosition = { c: Char -> 0 },
|
||||||
gridFocusRequester = focusRequester,
|
gridFocusRequester = focusRequester,
|
||||||
showJumpButtons = false,
|
showJumpButtons = false,
|
||||||
|
|
|
||||||
|
|
@ -151,6 +151,8 @@ sealed class Destination(
|
||||||
@Serializable
|
@Serializable
|
||||||
data class DiscoveredItem(
|
data class DiscoveredItem(
|
||||||
val item: DiscoverItem,
|
val item: DiscoverItem,
|
||||||
|
// Index to scroll to on the person page grid
|
||||||
|
val startIndex: Int = 0,
|
||||||
) : Destination(false)
|
) : Destination(false)
|
||||||
|
|
||||||
@Serializable
|
@Serializable
|
||||||
|
|
|
||||||
|
|
@ -405,6 +405,7 @@ fun DestinationContent(
|
||||||
LaunchedEffect(Unit) { onClearBackdrop.invoke() }
|
LaunchedEffect(Unit) { onClearBackdrop.invoke() }
|
||||||
DiscoverPersonPage(
|
DiscoverPersonPage(
|
||||||
person = destination.item,
|
person = destination.item,
|
||||||
|
startIndex = destination.startIndex,
|
||||||
modifier = modifier,
|
modifier = modifier,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue