Skip to content

[TV] Return to the welcome screen after signing out #5744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sztomek wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[TV] Return to the welcome screen after signing out #5744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f2e127
Gate the TV onboarding start destination on login state
sztomek Aug 14, 2026
54a38f4
Return to the TV welcome screen when signing out
sztomek Aug 14, 2026
a204675
Install the RxJava undeliverable-exception handler on TV
sztomek Aug 14, 2026
f7ef258
Address PR comments
sztomek Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tv/src/main/java/au/com/shiftyjelly/pocketcasts/TvApplication.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.hilt.work.HiltWorkerFactory
import androidx.work.Configuration
import au.com.shiftyjelly.pocketcasts.repositories.playback.PlaybackManager
import au.com.shiftyjelly.pocketcasts.utils.TimberDebugTree
import au.com.shiftyjelly.pocketcasts.utils.log.RxJavaUncaughtExceptionHandling
import dagger.hilt.android.HiltAndroidApp
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
Expand All @@ -29,6 +30,7 @@ class TvApplication :
if (BuildConfig.DEBUG) {
Timber.plant(TimberDebugTree())
}
RxJavaUncaughtExceptionHandling.setUp()
// setup() subscribes the Up Next queue's sync pipeline itself, so there must be no
// separate UpNextQueue.setupBlocking() call on TV.
applicationScope.launch {
Expand Down
2 changes: 2 additions & 0 deletions tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvScaffold.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import au.com.shiftyjelly.pocketcasts.upnext.TvUpNextScreen
fun TvScaffold(
onLogIn: () -> Unit,
onCreateAccount: () -> Unit,
onSignedOut: () -> Unit,
modifier: Modifier = Modifier,
viewModel: TvScaffoldViewModel = hiltViewModel(),
) {
Expand Down Expand Up @@ -131,6 +132,7 @@ fun TvScaffold(
onLogOut = {
isProfileModalVisible = false
viewModel.signOut()
onSignedOut()

@claude claude Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

viewModel.signOut() is fire-and-forget (signOutManager.signOutAndWipeData() just launches on @ApplicationScope), and onSignedOut() fires synchronously right after. Two consequences worth thinking about:

  1. Navigation happens before the auth state actually flips. syncManager.isLoggedIn() / isLoggedInObservable only become false at the end of SyncManagerImpl.signOut() (SyncManagerImpl.kt:195). Harmless today because TvWelcomeScreen doesn't read login state, but any future screen on LANDING that does will see a stale SignedIn.

  2. Sign-in can now race the tail of the wipe. The wipe keeps running for up to ~10s+ after this returns and ends with settings.clearUserPreferences() + tvPreferences.clearAll(). Welcome auto-focuses Sign In, so the user is one click from starting a fresh device-auth flow while the old wipe is still in flight — if the new login lands first, those tail steps clear the new session's preferences. The hazard pre-dates this PR (the profile modal already offered Log In after logout), but dropping the user straight onto the Welcome CTA makes it much easier to hit.

If you want to close it, having TvSignOutManager expose the wipe Job/a StateFlow<Boolean> and navigating on completion (with the existing spinner/blocking UI) would be the tighter version.

},
)
}
Expand Down
14 changes: 7 additions & 7 deletions tv/src/main/java/au/com/shiftyjelly/pocketcasts/onboarding/TvOnboardingNavHost.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ fun TvOnboardingNavHost(
viewModel: TvOnboardingViewModel = hiltViewModel(),
) {
val navController = rememberNavController()
val navigateClearingBackStack: (String) -> Unit = { route ->
navController.navigate(route) {
popUpTo(navController.graph.id) { inclusive = true }
}
}
val toastHostState = remember { TvToastHostState() }
CompositionLocalProvider(LocalTvToastHostState provides toastHostState) {
Box(modifier = modifier.fillMaxSize()) {
Expand All @@ -41,7 +46,6 @@ fun TvOnboardingNavHost(
onSignIn = { navController.navigate(TvOnboardingRoutes.SIGN_IN) },
onCreateAccount = { navController.navigate(TvOnboardingRoutes.CREATE_ACCOUNT) },
onContinueWithoutAccount = {
viewModel.completeOnboarding()
navController.navigate(TvOnboardingRoutes.HOME) {
popUpTo(TvOnboardingRoutes.LANDING) { inclusive = true }
}
Expand All @@ -55,17 +59,12 @@ fun TvOnboardingNavHost(
}
composable(TvOnboardingRoutes.SIGN_IN) {
TvSignInScreen(
onSignInComplete = {
navController.navigate(TvOnboardingRoutes.SYNCING) {
popUpTo(navController.graph.id) { inclusive = true }
}
},
onSignInComplete = { navigateClearingBackStack(TvOnboardingRoutes.SYNCING) },
)
}
composable(TvOnboardingRoutes.SYNCING) {
TvSyncingScreen(
onSyncComplete = {
viewModel.completeOnboarding()
navController.navigate(TvOnboardingRoutes.HOME) {
popUpTo(TvOnboardingRoutes.SYNCING) { inclusive = true }
}
Expand All @@ -76,6 +75,7 @@ fun TvOnboardingNavHost(
TvScaffold(
onLogIn = { navController.navigate(TvOnboardingRoutes.SIGN_IN) },
onCreateAccount = { navController.navigate(TvOnboardingRoutes.CREATE_ACCOUNT) },
onSignedOut = { navigateClearingBackStack(TvOnboardingRoutes.LANDING) },
)
}
}
Expand Down
10 changes: 4 additions & 6 deletions tv/src/main/java/au/com/shiftyjelly/pocketcasts/onboarding/TvOnboardingViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,18 @@ package au.com.shiftyjelly.pocketcasts.onboarding

import androidx.lifecycle.ViewModel
import au.com.shiftyjelly.pocketcasts.preferences.Settings
import au.com.shiftyjelly.pocketcasts.repositories.sync.SyncManager
import dagger.hilt.android.lifecycle.HiltViewModel
import javax.inject.Inject

@HiltViewModel
class TvOnboardingViewModel @Inject constructor(
private val settings: Settings,
syncManager: SyncManager,
settings: Settings,
) : ViewModel() {
val startDestination: String = if (settings.hasCompletedOnboarding()) {
val startDestination: String = if (syncManager.isLoggedIn() && !settings.getFullySignedOut()) {

@claude claude Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interrupting the first sync now permanently skips it. This is the one case where dropping completeOnboarding() loses information the login state can't reconstruct.

The old flag was set after onSyncComplete, so it encoded "signed in and initial sync finished". isLoggedIn() && !getFullySignedOut() only encodes the first half — the account is added at SyncManagerImpl.kt:233-241 (loginWithDeviceAuth), i.e. before the user ever reaches SYNCING.

So:

  1. Sign in → account added, setFullySignedOut(false) written → navigate to SYNCING.
  2. User presses Back (which exits, since onSignInComplete popped the whole graph) or the process is killed mid-sync. TvSyncingViewModel.startSync() runs in viewModelScope (TvSyncingViewModel.kt:87), so the refresh is cancelled.
  3. Cold launch → gate resolves to HOME. SYNCING is only reachable from onSignInComplete, so the sync never runs again.

And nothing else heals it: :tv never calls RefreshPodcastsTask.scheduleOrCancel (only app, wear and AdvancedSettingsViewModel do), and TvHomeViewModel only reads local/discover data. refreshPodcastsAfterSignIn() has already called clearLastRefreshTime() + markAllPodcastsUnsynced() (PodcastManagerImpl.kt:218-222), so the DB is left mid-migration with no scheduled worker to finish it. The user sits on an empty Home, signed in, with no way back to SYNCING short of logging out and in again.

Under the old gate this interruption was also wrong (Welcome shown to a logged-in user) but it self-healed — signing in again re-ran the sync. The new gate turns a recoverable state into a stuck one.

Two ways out, either is fine:

  • Route to SYNCING rather than HOME when logged in but the initial sync hasn't been recorded — keeps the parity goal (signed-out ⇒ Welcome) while preserving the resume behaviour.
  • Or trigger RefreshPodcastsTask.scheduleOrCancel / a one-shot refresh when HOME is the start destination, which also fixes the broader "TV never refreshes in the background" gap.

Fix this →

TvOnboardingRoutes.HOME
} else {
TvOnboardingRoutes.LANDING
}

fun completeOnboarding() {
settings.setHasDoneInitialOnboarding()
}
}
29 changes: 16 additions & 13 deletions tv/src/test/java/au/com/shiftyjelly/pocketcasts/onboarding/TvOnboardingViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,37 @@
package au.com.shiftyjelly.pocketcasts.onboarding

import au.com.shiftyjelly.pocketcasts.preferences.Settings
import au.com.shiftyjelly.pocketcasts.repositories.sync.SyncManager
import org.junit.Assert.assertEquals
import org.junit.Test
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever

class TvOnboardingViewModelTest {

private val syncManager = mock<SyncManager>()
private val settings = mock<Settings>()

@Test
fun `start destination is landing when onboarding not completed`() {
whenever(settings.hasCompletedOnboarding()).thenReturn(false)
val viewModel = TvOnboardingViewModel(settings)
assertEquals(TvOnboardingRoutes.LANDING, viewModel.startDestination)
fun `start destination is landing when signed out`() {
whenever(syncManager.isLoggedIn()).thenReturn(false)
whenever(settings.getFullySignedOut()).thenReturn(true)

@claude claude Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this stub is dead. && short-circuits on isLoggedIn() == false, so getFullySignedOut() is never called here — the test passes with or without the line, and would also pass with .thenReturn(false). Harmless today (no MockitoJUnitRunner/strict-stubs rule in this class, so no UnnecessaryStubbingException), but it reads as though the assertion depends on both values when it only depends on one. Dropping it makes the signed-out case unambiguous.

assertEquals(TvOnboardingRoutes.LANDING, viewModel().startDestination)
}

@Test
fun `start destination is home when onboarding completed`() {
whenever(settings.hasCompletedOnboarding()).thenReturn(true)
val viewModel = TvOnboardingViewModel(settings)
assertEquals(TvOnboardingRoutes.HOME, viewModel.startDestination)
fun `start destination is home when signed in and not fully signed out`() {
whenever(syncManager.isLoggedIn()).thenReturn(true)
whenever(settings.getFullySignedOut()).thenReturn(false)
assertEquals(TvOnboardingRoutes.HOME, viewModel().startDestination)
}

@Test
fun `complete onboarding persists to settings`() {
val viewModel = TvOnboardingViewModel(settings)
viewModel.completeOnboarding()
verify(settings).setHasDoneInitialOnboarding()
fun `start destination is landing when logged in but sign-out is pending`() {
whenever(syncManager.isLoggedIn()).thenReturn(true)
whenever(settings.getFullySignedOut()).thenReturn(true)
assertEquals(TvOnboardingRoutes.LANDING, viewModel().startDestination)
}

private fun viewModel() = TvOnboardingViewModel(syncManager, settings)
}
Loading