From 66e8f0f3cb962110e9e64a0736e128a54669a00b Mon Sep 17 00:00:00 2001 From: Aswin C Date: Sun, 8 Nov 2026 23:59:01 +0530 Subject: [PATCH 1/2] fix(app): keep the onboarding view model in the activity's store The app root created OnboardingViewModel with remember, outside any ViewModelStore, so onCleared never ran: Echo and deck requests were not cancelled, the clip pool was not released, and an unfinished onboarding was never reported as abandoned. rememberOnboardingViewModel now keeps the one instance in the activity's store under a fixed key, so every onboarding flow still shares it (picks carry over between search, Echo, genres and the import sheet), it survives activity recreation, and it is cleared when the activity finishes. --- app/README.md | 1 + .../cx/aswin/boxlore/ui/BoxLoreAppRoot.kt | 3 +- .../boxlore/ui/OnboardingViewModelScope.kt | 38 +++++ .../ui/OnboardingViewModelScopeTest.kt | 142 ++++++++++++++++++ feature/onboarding/README.md | 2 +- 5 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 app/src/main/java/cx/aswin/boxlore/ui/OnboardingViewModelScope.kt create mode 100644 app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelScopeTest.kt diff --git a/app/README.md b/app/README.md index e2da3c40a..6b354c1bd 100644 --- a/app/README.md +++ b/app/README.md @@ -16,6 +16,7 @@ ## Composition root and startup - `AppContainer` is created once in `BoxLoreApplication.onCreate` and wires database, network, auth, RSS, catalog, ranking, playback, queue, downloads, prefs, support and analytics in the [object graph order](../ARCHITECTURE.md#identity-and-storage). +- `ui/rememberOnboardingViewModel` keeps the one `OnboardingViewModel` in the activity's `ViewModelStore`, so every onboarding flow shares it and it is cleared when the activity finishes; never create it with `remember`. - The one `PlaybackRepository` is built lazily. Cast state waits in `DeferredCastSessionState` until it exists, so widget, worker and notification starts never build playback on the main thread. - The container gives core modules ports instead of reverse dependencies: `feedTelemetry` for feed events, `rolloutSwitches`, and a `DownloadServiceLauncherHolder` that starts `MediaDownloadService`. - `onCreate` order: API sessions first, then the Coil image loader (before widgets and services), Media3 Cast from `BoxLoreCastOptionsProvider`, the shared `UserPreferencesRepository`, the container and its holders. PostHog starts later on a background thread through `PostHogStartup`, after `AnalyticsHelper.initPolicy`. diff --git a/app/src/main/java/cx/aswin/boxlore/ui/BoxLoreAppRoot.kt b/app/src/main/java/cx/aswin/boxlore/ui/BoxLoreAppRoot.kt index ade05ebf2..910cc9be6 100644 --- a/app/src/main/java/cx/aswin/boxlore/ui/BoxLoreAppRoot.kt +++ b/app/src/main/java/cx/aswin/boxlore/ui/BoxLoreAppRoot.kt @@ -240,8 +240,9 @@ fun BoxLoreAppRoot( FcmTopicHelper.reconcileAfterRestoreIfNeeded(activity, subscriptionRepository) } + // One instance for every onboarding flow, held by the activity so it is cleared when the activity finishes. val onboardingViewModel = - remember { + rememberOnboardingViewModel(owner = activity) { cx.aswin.boxlore.feature.onboarding.OnboardingViewModel( application, podcastRepository, diff --git a/app/src/main/java/cx/aswin/boxlore/ui/OnboardingViewModelScope.kt b/app/src/main/java/cx/aswin/boxlore/ui/OnboardingViewModelScope.kt new file mode 100644 index 000000000..b56671322 --- /dev/null +++ b/app/src/main/java/cx/aswin/boxlore/ui/OnboardingViewModelScope.kt @@ -0,0 +1,38 @@ +package cx.aswin.boxlore.ui + +import androidx.compose.runtime.Composable +import androidx.lifecycle.ViewModel +import androidx.lifecycle.ViewModelProvider +import androidx.lifecycle.ViewModelStoreOwner +import androidx.lifecycle.viewmodel.compose.viewModel +import cx.aswin.boxlore.feature.onboarding.OnboardingViewModel + +/** The key the activity's onboarding view model is stored under. */ +internal const val ONBOARDING_VIEW_MODEL_KEY = "boxlore-onboarding" + +/** + * The activity's one [OnboardingViewModel], kept in [owner]'s view model store rather than in + * composition. + * + * Every onboarding path shares this instance: search, Echo (AI), genre picks, the import sheet, + * Home's AI onboarding entry and the sign-in detour through Settings. Shows picked in one flow + * therefore carry over to the others, and the instance survives the activity being recreated. + * + * Because the store owns it, `onCleared` runs when the activity finishes for good: Echo requests + * are cancelled, deck audio is released, and an onboarding left unfinished is reported as + * abandoned. Created with `remember`, it was never cleared, so none of that happened. + * + * [build] runs once per store; later calls return the stored instance. + */ +@Composable +internal fun rememberOnboardingViewModel( + owner: ViewModelStoreOwner, + build: () -> OnboardingViewModel, +): OnboardingViewModel = viewModel( + viewModelStoreOwner = owner, + key = ONBOARDING_VIEW_MODEL_KEY, + factory = object : ViewModelProvider.Factory { + @Suppress("UNCHECKED_CAST") + override fun create(modelClass: Class): T = build() as T + }, +) diff --git a/app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelScopeTest.kt b/app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelScopeTest.kt new file mode 100644 index 000000000..2aefb6445 --- /dev/null +++ b/app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelScopeTest.kt @@ -0,0 +1,142 @@ +package cx.aswin.boxlore.ui + +import android.app.Application +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.lifecycle.ViewModelStore +import androidx.lifecycle.ViewModelStoreOwner +import androidx.lifecycle.viewModelScope +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import cx.aswin.boxlore.core.analytics.AnalyticsHelper +import cx.aswin.boxlore.core.catalog.PodcastRepository +import cx.aswin.boxlore.core.catalog.SubscriptionRepository +import cx.aswin.boxlore.core.database.BoxLoreDatabase +import cx.aswin.boxlore.core.prefs.BoxcastPrefs +import cx.aswin.boxlore.core.prefs.UserPreferencesRepository +import cx.aswin.boxlore.core.rss.RssPodcastRepository +import cx.aswin.boxlore.feature.onboarding.OnboardingViewModel +import cx.aswin.boxlore.feature.onboarding.markOnboardingCompletedSilent +import kotlinx.coroutines.isActive +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * The onboarding view model lives in the activity's view model store: every onboarding flow shares + * one instance (so picks carry over between search, Echo and the import sheet), and finishing the + * activity clears it. Created with `remember` it was never cleared, so an unfinished onboarding was + * never reported as abandoned and its Echo work was never cancelled. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class) +class OnboardingViewModelScopeTest { + @get:Rule val composeRule = createComposeRule() + + private val context: Application = ApplicationProvider.getApplicationContext() + private lateinit var database: BoxLoreDatabase + private val store = ViewModelStore() + private val activityOwner = object : ViewModelStoreOwner { + override val viewModelStore: ViewModelStore = store + } + private val events = mutableListOf>>() + private lateinit var restoreAnalytics: () -> Unit + private var builds = 0 + + @Before + fun setUp() { + database = Room.inMemoryDatabaseBuilder(context, BoxLoreDatabase::class.java).allowMainThreadQueries().build() + RssPodcastRepository.clearInstanceForTests() + BoxcastPrefs(context).setOnboardingCompleted(false) + restoreAnalytics = AnalyticsHelper.installRecordingSink(events) + } + + @After + fun tearDown() { + store.clear() + restoreAnalytics() + database.close() + RssPodcastRepository.clearInstanceForTests() + } + + private fun build(): OnboardingViewModel { + builds++ + val podcastRepository = + PodcastRepository( + baseUrl = "http://localhost/", + publicKey = "test-app-key", + context = context, + rssRepository = RssPodcastRepository.createForTests(context = context, database = database), + ) + return OnboardingViewModel(context, podcastRepository, SubscriptionRepository(database.podcastDao()), UserPreferencesRepository(context)) + } + + @Test + fun `every onboarding flow gets the same instance while the activity lives`() { + var showImportSheet by mutableStateOf(false) + var showOnboarding by mutableStateOf(true) + val seen = mutableListOf() + composeRule.setContent { + // Separate call sites stand for the onboarding route, the import sheet and Home's AI entry. + if (showOnboarding) seen += rememberOnboardingViewModel(activityOwner, ::build) + if (showImportSheet) seen += rememberOnboardingViewModel(activityOwner, ::build) + } + + composeRule.runOnIdle { showImportSheet = true } + // Leaving onboarding (e.g. for the sign-in detour) and coming back keeps the same instance. + composeRule.runOnIdle { showOnboarding = false } + composeRule.runOnIdle { showOnboarding = true } + composeRule.waitForIdle() + + assertEquals(1, builds) + assertTrue(seen.size >= 3) + seen.forEach { assertSame(seen.first(), it) } + } + + @Test + fun `finishing the activity mid-onboarding reports it abandoned once and stops its work`() { + lateinit var viewModel: OnboardingViewModel + composeRule.setContent { viewModel = rememberOnboardingViewModel(activityOwner, ::build) } + composeRule.runOnIdle { viewModel.onWelcomeScreenViewed() } + + composeRule.runOnIdle { store.clear() } + + val abandoned = events.filter { it.first == "onboarding_abandoned" } + assertEquals(1, abandoned.size) + assertTrue(abandoned.single().second.containsKey("last_step")) + assertFalse(viewModel.viewModelScope.isActive) + } + + @Test + fun `a finished onboarding is not reported when the activity finishes`() { + lateinit var viewModel: OnboardingViewModel + composeRule.setContent { viewModel = rememberOnboardingViewModel(activityOwner, ::build) } + composeRule.runOnIdle { + viewModel.onWelcomeScreenViewed() + viewModel.markOnboardingCompletedSilent {} + } + + composeRule.runOnIdle { store.clear() } + + assertTrue(events.none { it.first == "onboarding_abandoned" }) + } + + @Test + fun `an onboarding never started is not reported when the activity finishes`() { + composeRule.setContent { rememberOnboardingViewModel(activityOwner, ::build) } + + composeRule.runOnIdle { store.clear() } + + assertTrue(events.none { it.first == "onboarding_abandoned" }) + } +} diff --git a/feature/onboarding/README.md b/feature/onboarding/README.md index e54e5e812..a806d2e9e 100644 --- a/feature/onboarding/README.md +++ b/feature/onboarding/README.md @@ -72,7 +72,7 @@ src/main/res/values/ # echo_onboarding.xml, onboarding_suggest ## Threading / lifecycle -- `OnboardingViewModel` is constructed directly in the app root's composition (`remember`), not through a `ViewModelStore`, so it outlives the onboarding route. Its `onCleared` cancels Echo and deck requests, releases the clip pool and records abandonment. +- `OnboardingViewModel` is one instance per activity, kept in the activity's `ViewModelStore` by `:app` (`rememberOnboardingViewModel`). Every onboarding flow shares it (search, Echo, genres, the import sheet, Home's AI entry), so picks carry over between them. Its `onCleared` runs when the activity finishes: it cancels Echo and deck requests, releases the clip pool and reports an unfinished onboarding as abandoned. - Echo turn, synthesis, deck and similar-shows jobs cancel on Back, Search, the topic fallback, restart and skip. Cancelled work never replaces a restored question or shows an error; cancellation is rethrown, not reported. - Genre synthesis runs `async` on `Dispatchers.IO` and catches failure inside the child (`orFallbackOnFailure`), because an exception escaping an `async` child fails its parent. - Subscribing a show asks the application-scoped `subscriptionForegroundSync` for a catalog ingest, so leaving onboarding cannot cancel it. From 9a3218f4987e7ca7923067f5da70631d27b65af2 Mon Sep 17 00:00:00 2001 From: Aswin C Date: Mon, 9 Nov 2026 00:15:23 +0530 Subject: [PATCH 2/2] test(app): cover onboarding view model retention across activity recreation --- .../ui/OnboardingViewModelRecreationTest.kt | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelRecreationTest.kt diff --git a/app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelRecreationTest.kt b/app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelRecreationTest.kt new file mode 100644 index 000000000..6308258f6 --- /dev/null +++ b/app/src/test/java/cx/aswin/boxlore/ui/OnboardingViewModelRecreationTest.kt @@ -0,0 +1,111 @@ +package cx.aswin.boxlore.ui + +import android.app.Application +import android.os.Looper +import androidx.activity.ComponentActivity +import androidx.activity.compose.setContent +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import cx.aswin.boxlore.core.analytics.AnalyticsHelper +import cx.aswin.boxlore.core.catalog.PodcastRepository +import cx.aswin.boxlore.core.catalog.SubscriptionRepository +import cx.aswin.boxlore.core.database.BoxLoreDatabase +import cx.aswin.boxlore.core.prefs.BoxcastPrefs +import cx.aswin.boxlore.core.prefs.UserPreferencesRepository +import cx.aswin.boxlore.core.rss.RssPodcastRepository +import cx.aswin.boxlore.core.testing.TestFixtures +import cx.aswin.boxlore.feature.onboarding.OnboardingViewModel +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.android.controller.ActivityController +import org.robolectric.annotation.Config + +/** + * A real activity recreated mid-onboarding keeps its onboarding view model and the shows picked so + * far, and reports nothing; only finishing it for good clears the view model and reports the + * unfinished onboarding as abandoned. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class) +class OnboardingViewModelRecreationTest { + private val context: Application = ApplicationProvider.getApplicationContext() + private lateinit var database: BoxLoreDatabase + private lateinit var controller: ActivityController + private val events = mutableListOf>>() + private lateinit var restoreAnalytics: () -> Unit + private var builds = 0 + + @Before + fun setUp() { + database = Room.inMemoryDatabaseBuilder(context, BoxLoreDatabase::class.java).allowMainThreadQueries().build() + RssPodcastRepository.clearInstanceForTests() + BoxcastPrefs(context).setOnboardingCompleted(false) + restoreAnalytics = AnalyticsHelper.installRecordingSink(events) + controller = Robolectric.buildActivity(ComponentActivity::class.java).setup() + } + + @After + fun tearDown() { + restoreAnalytics() + database.close() + RssPodcastRepository.clearInstanceForTests() + } + + private fun build(): OnboardingViewModel { + builds++ + val podcastRepository = + PodcastRepository( + baseUrl = "http://localhost/", + publicKey = "test-app-key", + context = context, + rssRepository = RssPodcastRepository.createForTests(context = context, database = database), + ) + return OnboardingViewModel(context, podcastRepository, SubscriptionRepository(database.podcastDao()), UserPreferencesRepository(context)) + } + + /** Sets the activity's content the way the app root does and returns the view model it gets. */ + private fun showOnboarding(): OnboardingViewModel { + var viewModel: OnboardingViewModel? = null + val activity = controller.get() + activity.setContent { viewModel = rememberOnboardingViewModel(owner = activity, build = ::build) } + shadowOf(Looper.getMainLooper()).idle() + assertNotNull(viewModel) + return viewModel!! + } + + @Test + fun `recreating the activity mid-onboarding keeps the view model and its picks and reports nothing`() { + val before = showOnboarding() + before.onWelcomeScreenViewed() + before.setShowsSelected(listOf(TestFixtures.podcast(id = "show-1"), TestFixtures.podcast(id = "show-2")), selected = true) + + controller.recreate() + val after = showOnboarding() + + assertSame(before, after) + assertEquals(1, builds) + assertEquals(setOf("show-1", "show-2"), after.uiState.value.selectedPodcasts.keys) + assertTrue(events.none { it.first == "onboarding_abandoned" }) + } + + @Test + fun `finishing the activity after a recreation reports the unfinished onboarding once`() { + showOnboarding().onWelcomeScreenViewed() + controller.recreate() + showOnboarding() + + controller.get().finish() + controller.pause().stop().destroy() + + assertEquals(1, events.count { it.first == "onboarding_abandoned" }) + } +}