Skip to content

Commit 4f623b8

Browse files
sztomeksztomek
committed
Address PR comments
1 parent c573497 commit 4f623b8

6 files changed

Lines changed: 102 additions & 20 deletions

File tree

‎modules/services/localization/src/main/res/values/strings.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,10 @@
265265
<item quantity="one">The only episode of this playlist has been archived.</item>
266266
<item quantity="other">All %1$d episodes of this playlist have been archived.</item>
267267
</plurals>
268+
<plurals name="tv_podcast_all_archived">
269+
<item quantity="one">The only episode of this podcast has been archived.</item>
270+
<item quantity="other">All %1$d episodes of this podcast have been archived.</item>
271+
</plurals>
268272
<string name="tv_playlist_empty_title">No episodes</string>
269273
<string name="tv_playlist_empty_subtitle">Either it’s time to celebrate completing this list or edit your rules in the mobile app to get some more.</string>
270274
<string name="tv_playlist_empty_subtitle_manual">Add episodes to this playlist from the mobile app and they’ll show up here.</string>

‎tv/src/main/java/au/com/shiftyjelly/pocketcasts/component/TvEpisodeListControls.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ fun <T> TvSortButton(
123123
isSelected = option == selected,
124124
onClick = {
125125
isExpanded = false
126-
onSelect(option)
126+
if (option != selected) {
127+
onSelect(option)
128+
}
127129
},
128130
)
129131
}

‎tv/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/TvPodcastDetailsScreen.kt‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import androidx.compose.ui.focus.focusRequester
3131
import androidx.compose.ui.graphics.Color
3232
import androidx.compose.ui.platform.LocalContext
3333
import androidx.compose.ui.res.painterResource
34+
import androidx.compose.ui.res.pluralStringResource
3435
import androidx.compose.ui.res.stringResource
3536
import androidx.compose.ui.text.style.TextAlign
3637
import androidx.compose.ui.text.style.TextOverflow
@@ -220,6 +221,20 @@ private fun EpisodeList(
220221
) {
221222
val context = LocalContext.current
222223
val dateFormatter = remember(context) { RelativeDateFormatter(context) }
224+
val firstEpisodeFocusRequester = remember { FocusRequester() }
225+
val listState = rememberLazyListState()
226+
LaunchedEffect(podcast.episodesSortType) {
227+
listState.scrollToItem(0)
228+
}
229+
var hasRequestedInitialFocus by remember { mutableStateOf(episodes.isEmpty()) }
230+
LaunchedEffect(episodes.isNotEmpty()) {
231+
if (episodes.isNotEmpty() && !hasRequestedInitialFocus) {
232+
snapshotFlow { listState.layoutInfo.visibleItemsInfo.any { it.index == 0 } }.first { it }
233+
runCatching { firstEpisodeFocusRequester.requestFocus() }
234+
.onFailure { Timber.e(it, "Failed to focus the first podcast episode") }
235+
hasRequestedInitialFocus = true
236+
}
237+
}
223238
var actionsEpisode by remember { mutableStateOf<PodcastEpisode?>(null) }
224239
Column(modifier = modifier) {
225240
Row(
@@ -253,13 +268,6 @@ private fun EpisodeList(
253268
modifier = Modifier.weight(1f).fillMaxWidth(),
254269
)
255270
} else {
256-
val firstEpisodeFocusRequester = remember { FocusRequester() }
257-
val listState = rememberLazyListState()
258-
LaunchedEffect(Unit) {
259-
snapshotFlow { listState.layoutInfo.visibleItemsInfo.any { it.index == 0 } }.first { it }
260-
runCatching { firstEpisodeFocusRequester.requestFocus() }
261-
.onFailure { Timber.e(it, "Failed to focus the first podcast episode") }
262-
}
263271
LazyColumn(
264272
state = listState,
265273
verticalArrangement = Arrangement.spacedBy(12.dp),
@@ -299,7 +307,7 @@ private fun AllEpisodesArchived(
299307
modifier = modifier,
300308
) {
301309
Text(
302-
text = stringResource(LR.string.podcast_no_episodes_all_archived, episodeCount),
310+
text = pluralStringResource(LR.plurals.tv_podcast_all_archived, episodeCount, episodeCount),
303311
style = MaterialTheme.typography.bodyLarge,
304312
color = TvColors.TextSecondary,
305313
textAlign = TextAlign.Center,

‎tv/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/TvPodcastDetailsViewModel.kt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import au.com.shiftyjelly.pocketcasts.models.entity.PodcastEpisode
77
import au.com.shiftyjelly.pocketcasts.models.type.EpisodesSortType
88
import au.com.shiftyjelly.pocketcasts.preferences.TvPreferences
99
import au.com.shiftyjelly.pocketcasts.repositories.di.DefaultDispatcher
10+
import au.com.shiftyjelly.pocketcasts.repositories.di.IoDispatcher
1011
import au.com.shiftyjelly.pocketcasts.repositories.podcast.EpisodeManager
1112
import au.com.shiftyjelly.pocketcasts.repositories.podcast.PodcastManager
1213
import au.com.shiftyjelly.pocketcasts.utils.log.LogBuffer
@@ -24,12 +25,12 @@ import kotlinx.coroutines.flow.SharingStarted
2425
import kotlinx.coroutines.flow.StateFlow
2526
import kotlinx.coroutines.flow.WhileSubscribed
2627
import kotlinx.coroutines.flow.combine
28+
import kotlinx.coroutines.flow.distinctUntilChangedBy
2729
import kotlinx.coroutines.flow.emitAll
2830
import kotlinx.coroutines.flow.filterNotNull
2931
import kotlinx.coroutines.flow.flatMapLatest
3032
import kotlinx.coroutines.flow.flow
3133
import kotlinx.coroutines.flow.flowOn
32-
import kotlinx.coroutines.flow.map
3334
import kotlinx.coroutines.flow.stateIn
3435
import kotlinx.coroutines.launch
3536
import kotlinx.coroutines.rx2.await
@@ -42,6 +43,7 @@ class TvPodcastDetailsViewModel @AssistedInject constructor(
4243
private val episodeManager: EpisodeManager,
4344
private val preferences: TvPreferences,
4445
@DefaultDispatcher private val defaultDispatcher: CoroutineDispatcher,
46+
@IoDispatcher private val ioDispatcher: CoroutineDispatcher,
4547
) : ViewModel() {
4648

4749
private val isShowingArchivedFlow = MutableStateFlow(preferences.isPodcastShowingArchived(podcastUuid))
@@ -58,14 +60,12 @@ class TvPodcastDetailsViewModel @AssistedInject constructor(
5860
if (podcast == null) {
5961
emit(TvPodcastDetailsUiState.NotFound)
6062
} else {
61-
val podcastEpisodesFlow = podcastManager.podcastByUuidFlow(podcastUuid)
62-
.filterNotNull()
63-
.flatMapLatest { updatedPodcast ->
64-
episodeManager.findEpisodesByPodcastOrderedFlow(updatedPodcast)
65-
.map { episodes -> updatedPodcast to episodes }
66-
}
63+
val podcastFlow = podcastManager.podcastByUuidFlow(podcastUuid).filterNotNull()
64+
val episodesFlow = podcastFlow
65+
.distinctUntilChangedBy { it.episodesSortType }
66+
.flatMapLatest { episodeManager.findEpisodesByPodcastOrderedFlow(it) }
6767
emitAll(
68-
combine(podcastEpisodesFlow, isShowingArchivedFlow) { (loadedPodcast, episodes), isShowingArchived ->
68+
combine(podcastFlow, episodesFlow, isShowingArchivedFlow) { loadedPodcast, episodes, isShowingArchived ->
6969
TvPodcastDetailsUiState.Loaded(
7070
podcast = loadedPodcast,
7171
episodes = if (isShowingArchived) episodes else episodes.filterNot(PodcastEpisode::isArchived),
@@ -84,7 +84,7 @@ class TvPodcastDetailsViewModel @AssistedInject constructor(
8484

8585
fun changeSortType(sortType: EpisodesSortType) {
8686
val podcast = (uiState.value as? TvPodcastDetailsUiState.Loaded)?.podcast ?: return
87-
viewModelScope.launch(defaultDispatcher) {
87+
viewModelScope.launch(ioDispatcher) {
8888
podcastManager.updateEpisodesSortTypeBlocking(podcast, sortType)
8989
}
9090
}

‎tv/src/test/java/au/com/shiftyjelly/pocketcasts/auth/TvSignOutManagerTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class TvSignOutManagerTest {
5757
manager.signOutAndWipeData()
5858
advanceUntilIdle()
5959

60-
inOrder(userManager, settings, playlistPreferences) {
60+
inOrder(userManager, settings, tvPreferences) {
6161
verify(userManager).signOutAndClearData(
6262
playbackManager = eq(playbackManager),
6363
upNextQueue = eq(upNextQueue),
@@ -67,7 +67,7 @@ class TvSignOutManagerTest {
6767
wasInitiatedByUser = eq(true),
6868
)
6969
verify(settings).clearUserPreferences()
70-
verify(playlistPreferences).clearAll()
70+
verify(tvPreferences).clearAll()
7171
}
7272
}
7373

‎tv/src/test/java/au/com/shiftyjelly/pocketcasts/podcasts/TvPodcastDetailsViewModelTest.kt‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import org.mockito.kotlin.any
2121
import org.mockito.kotlin.doReturn
2222
import org.mockito.kotlin.eq
2323
import org.mockito.kotlin.mock
24+
import org.mockito.kotlin.times
2425
import org.mockito.kotlin.verify
2526

2627
@OptIn(ExperimentalCoroutinesApi::class)
@@ -112,6 +113,72 @@ class TvPodcastDetailsViewModelTest {
112113
}
113114
}
114115

116+
@Test
117+
fun `toggling the archive filter off hides archived episodes again`() = runTest {
118+
val preferences = mock<TvPreferences> {
119+
on { isPodcastShowingArchived("podcast-uuid") } doReturn true
120+
}
121+
val viewModel = createViewModel(preferences)
122+
123+
viewModel.uiState.test {
124+
assertEquals(TvPodcastDetailsUiState.Loading, awaitItem())
125+
126+
episodes.emit(listOf(availableEpisode, archivedEpisode))
127+
assertEquals(listOf(availableEpisode, archivedEpisode), (awaitItem() as TvPodcastDetailsUiState.Loaded).episodes)
128+
129+
viewModel.toggleArchiveFilter()
130+
131+
val state = awaitItem() as TvPodcastDetailsUiState.Loaded
132+
assertEquals(listOf(availableEpisode), state.episodes)
133+
assertEquals(false, state.isShowingArchived)
134+
verify(preferences).setPodcastShowingArchived(eq("podcast-uuid"), eq(false))
135+
}
136+
}
137+
138+
@Test
139+
fun `a sort order change re-runs the episodes query`() = runTest {
140+
val podcastFlow = MutableStateFlow(podcast.copy(episodesSortType = EpisodesSortType.EPISODES_SORT_BY_DATE_DESC))
141+
val podcastManager = mock<PodcastManager> {
142+
on { findOrDownloadPodcastRxSingle(any(), any()) } doReturn Single.just(podcast)
143+
on { podcastByUuidFlow(any()) } doReturn podcastFlow
144+
}
145+
val viewModel = createViewModel(podcastManager = podcastManager)
146+
147+
viewModel.uiState.test {
148+
assertEquals(TvPodcastDetailsUiState.Loading, awaitItem())
149+
episodes.emit(listOf(availableEpisode))
150+
awaitItem() as TvPodcastDetailsUiState.Loaded
151+
152+
podcastFlow.value = podcastFlow.value.copy(episodesSortType = EpisodesSortType.EPISODES_SORT_BY_TITLE_ASC)
153+
154+
cancelAndConsumeRemainingEvents()
155+
}
156+
157+
verify(episodeManager, times(2)).findEpisodesByPodcastOrderedFlow(any())
158+
}
159+
160+
@Test
161+
fun `a non-sort podcast change does not re-run the episodes query`() = runTest {
162+
val podcastFlow = MutableStateFlow(podcast.copy(episodesSortType = EpisodesSortType.EPISODES_SORT_BY_DATE_DESC))
163+
val podcastManager = mock<PodcastManager> {
164+
on { findOrDownloadPodcastRxSingle(any(), any()) } doReturn Single.just(podcast)
165+
on { podcastByUuidFlow(any()) } doReturn podcastFlow
166+
}
167+
val viewModel = createViewModel(podcastManager = podcastManager)
168+
169+
viewModel.uiState.test {
170+
assertEquals(TvPodcastDetailsUiState.Loading, awaitItem())
171+
episodes.emit(listOf(availableEpisode))
172+
awaitItem() as TvPodcastDetailsUiState.Loaded
173+
174+
podcastFlow.value = podcastFlow.value.copy(title = "Renamed podcast")
175+
176+
cancelAndConsumeRemainingEvents()
177+
}
178+
179+
verify(episodeManager, times(1)).findEpisodesByPodcastOrderedFlow(any())
180+
}
181+
115182
@Test
116183
fun `changing the sort type persists it to the podcast`() = runTest {
117184
val viewModel = createViewModel()
@@ -148,6 +215,7 @@ class TvPodcastDetailsViewModelTest {
148215
episodeManager = episodeManager,
149216
preferences = prefs,
150217
defaultDispatcher = coroutineRule.testDispatcher,
218+
ioDispatcher = coroutineRule.testDispatcher,
151219
)
152220

153221
private fun episode(uuid: String, isArchived: Boolean) = PodcastEpisode(

0 commit comments

Comments
 (0)