From 84f60351032eb1c305affdde05ab9099613a4151 Mon Sep 17 00:00:00 2001 From: Damontecres Date: Wed, 15 Jul 2026 18:45:32 -0400 Subject: [PATCH] Fix bug finding the episode to scroll to on Series Overview (#1708) ## Description Fixes an off-by-one error when searching for the right episode (or season) to scroll to on the series overview page. While the code works most of the time, on cases where a special appears _after_ a season is included, the final episode in the season not be scrolled to. For example, if Special S00E04 comes after Season 2 (which has 10 episodes), clicking on S02E10 on the home page won't scroll to that episode on the series overview, instead S02E01 will be focused. And S00E04 will appear at the end of the list. This should also be a very, very tiny performance boost since the "search the whole list" fallback won't be triggered as often. I also added some comments to explain the function better. ### Related issues N/A, just noticed it yesterday ### Testing Emulator in the situation described above ## Screenshots N/A ## AI or LLM usage None --- .../ui/detail/series/SeriesViewModel.kt | 71 ++++++---- .../test/TestFindIndexByNumberOrId.kt | 129 ++++++++++++++++++ 2 files changed, 177 insertions(+), 23 deletions(-) create mode 100644 app/src/test/java/com/github/damontecres/wholphin/test/TestFindIndexByNumberOrId.kt diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt index 499dc44f..033ae5f6 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesViewModel.kt @@ -37,6 +37,7 @@ import com.github.damontecres.wholphin.ui.lt import com.github.damontecres.wholphin.ui.nav.Destination import com.github.damontecres.wholphin.ui.showToast 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.GetEpisodesRequestHandler @@ -165,7 +166,7 @@ class SeriesViewModel viewModelScope.launchIO { val index = (seasons as? ApiRequestPager<*>)?.let { - findIndexOf( + findIndexByNumberOrId( seasonEpisodeIds.seasonNumber, seasonEpisodeIds.seasonId, it, @@ -361,7 +362,7 @@ class SeriesViewModel pager.init(episodeNumber ?: 0) val initialIndex = if (episodeId != null || episodeNumber != null) { - findIndexOf(episodeNumber, episodeId, pager) + findIndexByNumberOrId(episodeNumber, episodeId, pager) .coerceAtLeast(0) } else { // Force the first page to to be fetched @@ -704,43 +705,67 @@ enum class SeriesPageType { OVERVIEW, } -private suspend fun findIndexOf( +private fun checkNumberOrId( targetNum: Int?, targetId: UUID?, - pager: ApiRequestPager<*>, + indexNumber: Int?, + id: UUID?, +): Boolean = + if (targetId != null) { + equalsNotNull(targetId, id) + } else { + equalsNotNull(indexNumber, targetNum) + } + +/** + * Find the index in the [list] where the item's `indexNumber`==[targetNum] or `id`==[targetId] + * + * This is necessary in cases where items are missing. E.g. if looking for episode 4 but + * episodes 2 & 3 is missing, then the index in the list for episode 4 will be `1`. + * + * @param targetNum the 1-index season or episode number + * @param targetId the season or episode ID + * + * @return the index within [list] that matches, or zero if no match is found + */ +suspend fun findIndexByNumberOrId( + targetNum: Int?, + targetId: UUID?, + list: BlockingList, ): Int { + // Adjust for 1-based numbers + val listIndex = targetNum?.minus(1)?.coerceAtLeast(0) val index = - if (targetId != null && (targetNum == null || targetNum !in pager.indices)) { + if (targetId != null && (targetNum == null || listIndex !in list.indices)) { // No hint info, so have to check everything - pager.indexOfBlocking { - equalsNotNull(it?.indexNumber, targetNum) || - equalsNotNull(it?.id, targetId) - } - } else if (targetNum != null && targetNum in pager.indices) { - // Start searching from the season number and choose direction from there - val num = pager.getBlocking(targetNum)?.indexNumber + list + .indexOfBlocking { + checkNumberOrId(targetNum, targetId, it?.indexNumber, it?.id) + }.coerceAtLeast(0) + } else if (listIndex != null && listIndex in list.indices) { + // Start searching from the target number and choose direction from there + val num = list.getBlocking(listIndex)?.indexNumber if (num.lt(targetNum)) { - for (i in targetNum + 1 until pager.lastIndex) { - val season = pager.getBlocking(i) - if (equalsNotNull(season?.indexNumber, targetNum) || - equalsNotNull(season?.id, targetId) - ) { + for (i in listIndex + 1 until list.lastIndex) { + val item = list.getBlocking(i) + if (checkNumberOrId(targetNum, targetId, item?.indexNumber, item?.id)) { return i } } return 0 } else if (num.gt(targetNum)) { - for (i in targetNum - 1 downTo 0) { - val season = pager.getBlocking(i) - if (equalsNotNull(season?.indexNumber, targetNum) || - equalsNotNull(season?.id, targetId) - ) { + for (i in listIndex - 1 downTo 0) { + val item = list.getBlocking(i) + if (checkNumberOrId(targetNum, targetId, item?.indexNumber, item?.id)) { return i } } return 0 } else { - targetNum + list + .indexOfBlocking { + checkNumberOrId(targetNum, targetId, it?.indexNumber, it?.id) + }.coerceAtLeast(0) } } else { 0 diff --git a/app/src/test/java/com/github/damontecres/wholphin/test/TestFindIndexByNumberOrId.kt b/app/src/test/java/com/github/damontecres/wholphin/test/TestFindIndexByNumberOrId.kt new file mode 100644 index 00000000..f50ed223 --- /dev/null +++ b/app/src/test/java/com/github/damontecres/wholphin/test/TestFindIndexByNumberOrId.kt @@ -0,0 +1,129 @@ +package com.github.damontecres.wholphin.test + +import com.github.damontecres.wholphin.data.model.BaseItem +import com.github.damontecres.wholphin.ui.detail.series.findIndexByNumberOrId +import com.github.damontecres.wholphin.util.BlockingList +import kotlinx.coroutines.test.runTest +import org.jellyfin.sdk.model.UUID +import org.jellyfin.sdk.model.api.BaseItemDto +import org.jellyfin.sdk.model.api.BaseItemKind +import org.junit.Assert +import org.junit.Test + +class TestFindIndexByNumberOrId { + fun create( + indexNumber: Int, + parentIndexNumber: Int, + ) = BaseItem( + BaseItemDto( + id = UUID.randomUUID(), + type = BaseItemKind.EPISODE, + indexNumber = indexNumber, + parentIndexNumber = parentIndexNumber, + ), + ) + + val epS02E01 = create(1, 2) + val epS02E02 = create(2, 2) + val epS02E03 = create(3, 2) + val epS02E04 = create(4, 2) + val epS02E05 = create(5, 2) + val epS00E08 = create(8, 0) + val epS00E02 = create(2, 0) + + private val episodes = + listOf( + epS02E01, + epS02E02, + epS02E03, + ) + + private val missingEpisodes = + listOf( + epS02E01, + epS02E04, + epS02E05, + ) + + private val withSpecials = + listOf( + epS02E01, + epS02E02, + epS02E03, + epS00E08, + ) + + @Test + fun `Basic tests`() = + runTest { + findIndexByNumberOrId(targetNum = 1, targetId = epS02E01.id, list = BlockingList.of(episodes)).let { index -> + Assert.assertEquals(0, index) + } + findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(episodes)).let { index -> + Assert.assertEquals(1, index) + } + findIndexByNumberOrId(targetNum = 3, targetId = epS02E03.id, list = BlockingList.of(episodes)).let { index -> + Assert.assertEquals(2, index) + } + findIndexByNumberOrId(targetNum = 100, targetId = UUID.randomUUID(), list = BlockingList.of(episodes)).let { index -> + Assert.assertEquals(0, index) + } + } + + @Test + fun `Test missing episodes`() = + runTest { + findIndexByNumberOrId(targetNum = 1, targetId = epS02E01.id, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(0, index) + } + findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(0, index) + } + findIndexByNumberOrId(targetNum = 4, targetId = epS02E04.id, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(1, index) + } + findIndexByNumberOrId(targetNum = 5, targetId = epS02E05.id, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(2, index) + } + } + + @Test + fun `Test missing episodes without ID`() = + runTest { + findIndexByNumberOrId(targetNum = 1, targetId = null, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(0, index) + } + findIndexByNumberOrId(targetNum = 2, targetId = null, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(0, index) + } + findIndexByNumberOrId(targetNum = 4, targetId = null, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(0, index) + } + findIndexByNumberOrId(targetNum = 5, targetId = null, list = BlockingList.of(missingEpisodes)).let { index -> + Assert.assertEquals(0, index) + } + } + + @Test + fun `Test with special`() = + runTest { + findIndexByNumberOrId(targetNum = 1, targetId = epS02E01.id, list = BlockingList.of(withSpecials)).let { index -> + Assert.assertEquals(0, index) + } + findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(withSpecials)).let { index -> + Assert.assertEquals(1, index) + } + findIndexByNumberOrId(targetNum = 3, targetId = epS02E03.id, list = BlockingList.of(withSpecials)).let { index -> + Assert.assertEquals(2, index) + } + } + + @Test + fun `Test with special matching number`() = + runTest { + val list = listOf(epS02E01, epS00E02, epS02E02) + findIndexByNumberOrId(targetNum = 2, targetId = epS02E02.id, list = BlockingList.of(list)).let { index -> + Assert.assertEquals(2, index) + } + } +}