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

Merged
sztomek merged 5 commits into from
Aug 24, 2026
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 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
1aca2da
Address PR comments
sztomek Aug 24, 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/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.

Comment thread
sztomek marked this conversation as resolved.
},
)
}
Expand Down
7 changes: 5 additions & 2 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 @@ -41,7 +41,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 @@ -65,7 +64,6 @@ fun TvOnboardingNavHost(
composable(TvOnboardingRoutes.SYNCING) {
TvSyncingScreen(
onSyncComplete = {
viewModel.completeOnboarding()
navController.navigate(TvOnboardingRoutes.HOME) {
popUpTo(TvOnboardingRoutes.SYNCING) { inclusive = true }
}
Expand All @@ -76,6 +74,11 @@ fun TvOnboardingNavHost(
TvScaffold(
onLogIn = { navController.navigate(TvOnboardingRoutes.SIGN_IN) },
onCreateAccount = { navController.navigate(TvOnboardingRoutes.CREATE_ACCOUNT) },
onSignedOut = {
navController.navigate(TvOnboardingRoutes.LANDING) {
popUpTo(navController.graph.id) { inclusive = 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.

The pop-whole-graph pattern is correct here (the start destination can be HOME, so popUpTo(LANDING) wouldn't work) and matches the SIGN_IN handler at line 58-60. Since it now appears twice, a small local helper would keep the two in sync:

val navigateClearingBackStack: (String) -> Unit = { route ->
    navController.navigate(route) {
        popUpTo(navController.graph.id) { inclusive = true }
    }
}

Nit only — no behavioural change requested.

One thing to be aware of for testing step 4: rememberNavController() saves the back stack, so a signed-out "browse without account" session that is restored after process death (rather than a genuine cold launch) will come back on HOME, not Welcome. Probably the desired state-restoration behaviour, just not quite the "every signed-out launch shows Welcome" invariant the description states.

)
}
}
Expand Down
10 changes: 3 additions & 7 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
@@ -1,21 +1,17 @@
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,
) : ViewModel() {
val startDestination: String = if (settings.hasCompletedOnboarding()) {
val startDestination: String = if (syncManager.isLoggedIn()) {

@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.

A failed/partial sign-out now strands the user in an empty Home instead of self-healing to Welcome.

The account removal is asynchronous and can fail independently of the data wipe:

  • TvSignOutManager.signOutAndWipeData() calls UserManager.signOutAndClearData(), which launches syncManager.signOut { … } on applicationScope (UserManager.kt:167).
  • SyncManagerImpl.signOut() runs syncServiceManager.signOut() → action() (network + analytics + experiment refresh) → only then syncAccountManager.signOut() (SyncManagerImpl.kt:191-196). If anything before that line throws, the account is never removed and syncManager.isLoggedIn() stays true.
  • Meanwhile TvSignOutManager gives up after SIGN_OUT_TIMEOUT (10s) and unconditionally proceeds to delete downloads, settings.clearUserPreferences() and tvPreferences.clearAll().

Under the old gate this was self-correcting: clearUserPreferences() does not preserve DONE_INITIAL_ONBOARDING_KEY (SettingsImpl.kt:592-613), so the next launch showed Welcome regardless of whether the token removal succeeded. With the new gate, "logged in + wiped database" resolves to HOME, which is exactly Apple TV's .dataLossResync state the description declares out of scope — on tvOS the keychain delete is synchronous, so it can't half-happen there; here it can.

PROCESSED_SIGNOUT_KEY is written synchronously (UserManager.kt:193), reset to false on every login (SyncManagerImpl.kt:241, :729), defaults to true, and is preserved across clearUserPreferences() — so it's a cheap durable guard:

Suggested change
val startDestination: String = if (syncManager.isLoggedIn()) {
val startDestination: String = if (syncManager.isLoggedIn() && !settings.getFullySignedOut()) {

(requires keeping the Settings injection). Alternatively, leave the gate as-is and accept the trade-off — but it'd be worth a LogBuffer entry so the state is diagnosable.

TvOnboardingRoutes.HOME
} else {
TvOnboardingRoutes.LANDING
}

fun completeOnboarding() {
settings.setHasDoneInitialOnboarding()
}
}
24 changes: 8 additions & 16 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,26 @@
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 settings = mock<Settings>()
private val syncManager = mock<SyncManager>()

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

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

@Test
fun `complete onboarding persists to settings`() {
val viewModel = TvOnboardingViewModel(settings)
viewModel.completeOnboarding()
verify(settings).setHasDoneInitialOnboarding()
}
}