app/src/main/kotlin/org/libremail/ui/AppViewModel.kt

@@ -6,6 +6,7 @@ import androidx.lifecycle.viewModelScope
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.flow.take
@@ -15,33 +16,51 @@ import org.libremail.ui.navigation.Routes
 import javax.inject.Inject
 
 /**
- * Decides the app's start destination from the stored account count: no accounts → the onboarding
- * welcome flow; otherwise the mailbox.
+ * Decides the app's start destination from two launch-time facts: whether any account is stored and
+ * whether the GPL-3.0 license has been accepted (#172). The license gate comes first — an un-accepted
+ * license routes to onboarding (which begins at the license screen) even when accounts already exist,
+ * so a user upgrading from a pre-#172 install can't skip the license by virtue of already having mail.
+ * Only a user who has accepted AND has at least one account lands straight on the mailbox.
  *
- * [startDestination] is `null` until the first account snapshot loads — the UI holds (renders
- * nothing) during that window so a cold start never flashes the wrong screen. Only the *first*
- * determination is used ([take]), so adding the first account mid-onboarding does not later flip the
- * start destination and tear down the in-progress flow.
+ * Each exposed flow is `null` until the first snapshot loads — the UI holds (renders nothing) during
+ * that window so a cold start never flashes the wrong screen. Only the *first* determination is used
+ * ([take]), so adding the first account (or accepting the license) mid-onboarding does not later flip
+ * the start destination and tear down the in-progress flow.
  */
 @HiltViewModel
 class AppViewModel @Inject constructor(accountRepository: AccountRepository, settingsRepository: SettingsRepository) :
     ViewModel() {
 
-    val startDestination: StateFlow<String?> = accountRepository.observeAccounts()
-        .map { accounts -> if (accounts.isEmpty()) Routes.ONBOARDING else Routes.MAILBOX }
+    val startDestination: StateFlow<String?> = combine(
+        accountRepository.observeAccounts(),
+        settingsRepository.settings,
+    ) { accounts, settings ->
+        if (settings.licenseAccepted && accounts.isNotEmpty()) Routes.MAILBOX else Routes.ONBOARDING
+    }
         .take(1)
         .stateIn(viewModelScope, SharingStarted.Eagerly, null)
 
     /**
-     * Whether the user has already agreed to the GPL-3.0 license (#172), resolved once at launch
-     * exactly like [startDestination] (same "hold until known, then never re-decide" contract).
-     * [LibreMailApp][org.libremail.ui.LibreMailApp] holds off building its `NavHost` until this is
-     * known too, because the onboarding graph — and therefore its choice of start destination between
-     * `Routes.ONBOARDING_LICENSE` and `Routes.ONBOARDING_WELCOME` — is registered as part of that same
-     * `NavHost` regardless of whether [startDestination] itself resolves to onboarding or the mailbox.
+     * Whether the user has already agreed to the GPL-3.0 license (#172), resolved once at launch.
+     * [LibreMailApp][org.libremail.ui.LibreMailApp] uses it to pick the onboarding graph's own start
+     * destination between `Routes.ONBOARDING_LICENSE` and `Routes.ONBOARDING_WELCOME`, and holds off
+     * building its `NavHost` until this is known — the onboarding graph is registered as part of that
+     * `NavHost` regardless of whether [startDestination] resolves to onboarding or the mailbox.
      */
     val licenseAccepted: StateFlow<Boolean?> = settingsRepository.settings
         .map { it.licenseAccepted }
         .take(1)
         .stateIn(viewModelScope, SharingStarted.Eagerly, null)
+
+    /**
+     * Whether at least one account already exists, resolved once at launch. Lets
+     * [LibreMailApp][org.libremail.ui.LibreMailApp] route correctly *after* an upgrade user accepts the
+     * license: someone who already had accounts goes straight to the mailbox rather than the "add your
+     * first account" welcome screen (which would be a dead-end for them), while a fresh install with no
+     * accounts continues into that welcome flow as before.
+     */
+    val hasAccounts: StateFlow<Boolean?> = accountRepository.observeAccounts()
+        .map { it.isNotEmpty() }
+        .take(1)
+        .stateIn(viewModelScope, SharingStarted.Eagerly, null)
 }

app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt

@@ -63,11 +63,13 @@ fun LibreMailApp(
 ) {
     val startDestination by appViewModel.startDestination.collectAsStateWithLifecycle()
     val licenseAccepted by appViewModel.licenseAccepted.collectAsStateWithLifecycle()
+    val hasAccounts by appViewModel.hasAccounts.collectAsStateWithLifecycle()
     // Hold (render nothing) until the account count AND the license-acceptance flag are known, so a
     // cold start never flashes the wrong screen before onboarding-vs-mailbox — and, within onboarding,
     // license-vs-welcome (#172) — is decided.
     val start = startDestination ?: return
     val licenseAlreadyAccepted = licenseAccepted ?: return
+    val hasExistingAccounts = hasAccounts ?: return
     val navController = rememberNavController()
     val pendingCrash by startupViewModel.pendingCrash.collectAsStateWithLifecycle()
 
@@ -75,15 +77,20 @@ fun LibreMailApp(
     // it fires once per intent (and again for a new intent delivered while the app is alive).
     LaunchedEffect(pendingCompose) {
         val prefill = pendingCompose ?: return@LaunchedEffect
-        navController.navigate(
-            Routes.compose(
-                to = prefill.to,
-                subject = prefill.subject,
-                cc = prefill.cc,
-                bcc = prefill.bcc,
-                body = prefill.body,
-            ),
-        )
+        // Don't let a deep-link jump past onboarding — including the license gate (#172), which is why
+        // start == ONBOARDING now also covers an upgrade user who hasn't accepted yet. Consume the
+        // request either way so it isn't replayed. Mirrors the pendingOpenMessageId guard below.
+        if (start != Routes.ONBOARDING) {
+            navController.navigate(
+                Routes.compose(
+                    to = prefill.to,
+                    subject = prefill.subject,
+                    cc = prefill.cc,
+                    bcc = prefill.bcc,
+                    body = prefill.body,
+                ),
+            )
+        }
         onComposeHandled()
     }
 
@@ -100,7 +107,7 @@ fun LibreMailApp(
         navController = navController,
         startDestination = start,
     ) {
-        onboardingGraph(navController, licenseAlreadyAccepted)
+        onboardingGraph(navController, licenseAlreadyAccepted, hasExistingAccounts)
 
         composable(
             route = Routes.MAILBOX_PATTERN,
@@ -295,8 +302,15 @@ private fun CrashReportDialog(onReview: () -> Unit, onLater: () -> Unit, onDisca
  * @param licenseAlreadyAccepted decides the graph's start destination (#172): false routes through
  *   [Routes.ONBOARDING_LICENSE] first; true (the user already agreed on a prior run) skips straight to
  *   [Routes.ONBOARDING_WELCOME], matching this graph's pre-#172 behavior.
+ * @param hasExistingAccounts where to go once the license is accepted from [Routes.ONBOARDING_LICENSE]:
+ *   true (an upgrade from a pre-#172 install that already has accounts) goes straight to the mailbox;
+ *   false (a fresh install) continues into the welcome/add-account flow.
  */
-private fun NavGraphBuilder.onboardingGraph(navController: NavHostController, licenseAlreadyAccepted: Boolean) {
+private fun NavGraphBuilder.onboardingGraph(
+    navController: NavHostController,
+    licenseAlreadyAccepted: Boolean,
+    hasExistingAccounts: Boolean,
+) {
     val onboardingStart = if (licenseAlreadyAccepted) Routes.ONBOARDING_WELCOME else Routes.ONBOARDING_LICENSE
     navigation(startDestination = onboardingStart, route = Routes.ONBOARDING) {
         composable(Routes.ONBOARDING_LICENSE) { entry ->
@@ -305,10 +319,19 @@ private fun NavGraphBuilder.onboardingGraph(navController: NavHostController, li
             LicenseScreen(
                 onAgree = {
                     onboarding.markLicenseAccepted()
-                    // Pop LICENSE off the back stack: it is a one-time gate, so a later back-press
-                    // from the welcome screen must not be able to return to it.
-                    navController.navigate(Routes.ONBOARDING_WELCOME) {
-                        popUpTo(Routes.ONBOARDING_LICENSE) { inclusive = true }
+                    if (hasExistingAccounts) {
+                        // Upgrade path (#172 follow-up): a pre-#172 install with accounts is now forced
+                        // through the license gate too (see AppViewModel.startDestination). Once they
+                        // accept, send them straight to their inbox — ONBOARDING_WELCOME ("add your
+                        // first account") would strand a user who already has accounts. finishOnboarding
+                        // pops the whole onboarding graph, so the one-time license gate is left behind.
+                        navController.finishOnboarding(firstAccountId = null)
+                    } else {
+                        // Fresh install: continue into the add-account flow. Pop LICENSE off the back
+                        // stack so a later back-press from welcome can't return to this one-time gate.
+                        navController.navigate(Routes.ONBOARDING_WELCOME) {
+                            popUpTo(Routes.ONBOARDING_LICENSE) { inclusive = true }
+                        }
                     }
                 },
                 // MainActivity is this app's only Activity (single-activity Compose app), so finish()

app/src/test/kotlin/org/libremail/ui/AppViewModelTest.kt

@@ -24,12 +24,12 @@ import org.libremail.ui.navigation.Routes
 import kotlin.test.assertEquals
 
 /**
- * [AppViewModel] decides two independent things, each resolved once at launch: the top-level start
- * destination (onboarding vs. mailbox, from the stored account count) and, since #172, whether the
- * user has already agreed to the license (consulted by `onboardingGraph()` in `LibreMailApp.kt` to
- * pick between `Routes.ONBOARDING_LICENSE` and `Routes.ONBOARDING_WELCOME`). Both repositories are
- * mocked: [AccountRepository] and [SettingsRepository] are interfaces/classes this ViewModel only
- * reads from, and `SettingsRepository` itself needs a real `Context` it doesn't get in a JVM test.
+ * [AppViewModel] resolves, once at launch, the top-level start destination plus the two facts it
+ * derives from: whether any account exists and whether the license has been accepted (#172). The
+ * start destination is onboarding unless the license is accepted AND an account exists — so an upgrade
+ * user with accounts but no acceptance is still routed through the license gate, not the mailbox.
+ * Both repositories are mocked: [AccountRepository] and [SettingsRepository] are types this ViewModel
+ * only reads from, and `SettingsRepository` itself needs a real `Context` it doesn't get in a JVM test.
  */
 @OptIn(ExperimentalCoroutinesApi::class)
 class AppViewModelTest {
@@ -60,19 +60,32 @@ class AppViewModelTest {
     }
 
     @Test
-    fun `no accounts resolves onboarding as the start destination`() = runTest(testDispatcher) {
-        val vm = AppViewModel(accountRepository(emptyList()), settingsRepository())
+    fun `no accounts routes to onboarding regardless of license state`() = runTest(testDispatcher) {
+        val unaccepted = AppViewModel(accountRepository(emptyList()), settingsRepository(licenseAccepted = false))
+        val accepted = AppViewModel(accountRepository(emptyList()), settingsRepository(licenseAccepted = true))
 
-        assertEquals(Routes.ONBOARDING, vm.startDestination.value)
+        assertEquals(Routes.ONBOARDING, unaccepted.startDestination.value)
+        assertEquals(Routes.ONBOARDING, accepted.startDestination.value)
     }
 
     @Test
-    fun `an existing account resolves the mailbox as the start destination`() = runTest(testDispatcher) {
-        val vm = AppViewModel(accountRepository(listOf(account())), settingsRepository())
+    fun `existing accounts route to the mailbox once the license is accepted`() = runTest(testDispatcher) {
+        val vm = AppViewModel(accountRepository(listOf(account())), settingsRepository(licenseAccepted = true))
 
         assertEquals(Routes.MAILBOX, vm.startDestination.value)
     }
 
+    @Test
+    fun `existing accounts with an unaccepted license still route to onboarding (upgrade license gate)`() =
+        runTest(testDispatcher) {
+            // Regression for the post-batch review finding: a pre-#172 install already has accounts, so
+            // the old account-count-only rule sent it straight to the mailbox and skipped the license.
+            // The license gate must win, forcing onboarding (which begins at ONBOARDING_LICENSE).
+            val vm = AppViewModel(accountRepository(listOf(account())), settingsRepository(licenseAccepted = false))
+
+            assertEquals(Routes.ONBOARDING, vm.startDestination.value)
+        }
+
     @Test
     fun `license acceptance defaults to not accepted`() = runTest(testDispatcher) {
         val vm = AppViewModel(accountRepository(), settingsRepository(licenseAccepted = false))
@@ -87,4 +100,13 @@ class AppViewModelTest {
 
             assertEquals(true, vm.licenseAccepted.value)
         }
+
+    @Test
+    fun `hasAccounts reflects whether any account is stored`() = runTest(testDispatcher) {
+        val empty = AppViewModel(accountRepository(emptyList()), settingsRepository())
+        val withAccount = AppViewModel(accountRepository(listOf(account())), settingsRepository())
+
+        assertEquals(false, empty.hasAccounts.value)
+        assertEquals(true, withAccount.hasAccounts.value)
+    }
 }