Give each screen its own view models so the profile editor loads

app/build.gradle

@@ -191,6 +191,7 @@ dependencies {
     implementation libs.androidx.activity.compose
     // Optional - Integration with ViewModels
     implementation libs.androidx.lifecycle.viewmodel.compose
+    implementation libs.androidx.lifecycle.viewmodel.navigation3
 
     // Firebase
     implementation libs.firebase.analytics

app/src/main/java/de/psdev/devdrawer/DevDrawerHost.kt

@@ -72,7 +72,8 @@ fun DevDrawerHost(
         entry<WidgetProfileEditorRoute> { key ->
             WidgetProfileEditor(
                 profileId = key.id,
-                isNew = key.isNew
+                isNew = key.isNew,
+                onBack = { navigator.goBack() }
             )
         }
     }

app/src/main/java/de/psdev/devdrawer/NavigationState.kt

@@ -9,6 +9,7 @@ import androidx.compose.runtime.saveable.rememberSerializable
 import androidx.compose.runtime.setValue
 import androidx.compose.runtime.snapshots.SnapshotStateList
 import androidx.compose.runtime.toMutableStateList
+import androidx.lifecycle.viewmodel.navigation3.rememberViewModelStoreNavEntryDecorator
 import androidx.navigation3.runtime.NavBackStack
 import androidx.navigation3.runtime.NavEntry
 import androidx.navigation3.runtime.NavKey
@@ -77,6 +78,9 @@ fun NavigationState.toEntries(
     val decoratedEntries = backStacks.mapValues { (_, stack) ->
         val decorators = listOf(
             rememberSaveableStateHolderNavEntryDecorator<NavKey>(),
+            // Each entry gets its own view models, cleared when it leaves the back stack; without this they were
+            // shared through the activity, so a second editor reused the first one's view model and arguments.
+            rememberViewModelStoreNavEntryDecorator<NavKey>(),
         )
         rememberDecoratedNavEntries(
             backStack = stack,

app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditor.kt

@@ -33,6 +33,7 @@ import androidx.compose.material3.Text
 import androidx.compose.material3.TextButton
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.DisposableEffect
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
@@ -64,6 +65,7 @@ private const val COLLAPSED_APP_COUNT = 5
 fun WidgetProfileEditor(
     profileId: String,
     isNew: Boolean = false,
+    onBack: () -> Unit = {},
     modifier: Modifier = Modifier,
     viewModel: WidgetProfileEditorViewModel = hiltViewModel(
         creationCallback = { factory: WidgetProfileEditorViewModel.Factory ->
@@ -79,7 +81,7 @@ fun WidgetProfileEditor(
 
     // Changes save as they are made; a name still being typed is saved when the editor closes.
     DisposableEffect(viewModel) {
-        onDispose { viewModel.onEditorClosed() }
+        onDispose { viewModel.onEditorHidden() }
     }
 
     Box(modifier = modifier.fillMaxSize()) {
@@ -89,6 +91,7 @@ fun WidgetProfileEditor(
             onNameChange = viewModel::onNameChanged,
             onAddFilterClick = { currentDialog = WidgetProfileEditorDialogs.AddFilter },
             onPackageFilterClick = { currentDialog = WidgetProfileEditorDialogs.PackageFilterInfo(it) },
+            onMissing = onBack,
             onRemoveFilterClick = { filter ->
                 viewModel.deleteFilter(filter)
                 scope.launch {
@@ -176,8 +179,13 @@ internal fun WidgetProfileEditor(
     onNameChange: (String) -> Unit = {},
     onAddFilterClick: () -> Unit = {},
     onPackageFilterClick: (PackageFilter) -> Unit = {},
-    onRemoveFilterClick: (PackageFilter) -> Unit = {}
+    onRemoveFilterClick: (PackageFilter) -> Unit = {},
+    onMissing: () -> Unit = {}
 ) {
+    if (viewState.isMissing) {
+        LaunchedEffect(Unit) { onMissing() }
+        return
+    }
     val widgetProfile = viewState.widgetProfile
     if (widgetProfile == null) {
         // Loading

app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewModel.kt

@@ -16,6 +16,8 @@ import de.psdev.devdrawer.database.DevDrawerDatabase
 import de.psdev.devdrawer.database.FilterType
 import de.psdev.devdrawer.database.PackageFilter
 import de.psdev.devdrawer.profiles.IPackageFilterRepository
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.delay
@@ -80,7 +82,8 @@ class WidgetProfileEditorViewModel @AssistedInject constructor(
             packageFilters = filters,
             matchingApps = matchingApps,
             filterAppCounts = filterAppCounts,
-            usedByWidgets = usedByWidgets
+            usedByWidgets = usedByWidgets,
+            isMissing = widgetProfile == null
         )
     }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), WidgetProfileEditorViewState.Empty)
 
@@ -111,19 +114,26 @@ class WidgetProfileEditorViewModel @AssistedInject constructor(
         }
     }
 
-    /**
-     * Saves a name still waiting for typing to pause, and discards a new profile that was left untouched
-     * (no filters, no name typed) so backing out of "Create new profile" leaves nothing behind.
-     */
-    fun onEditorClosed() {
+    /** Saves a name still waiting for typing to pause; called whenever the editor leaves the screen. */
+    fun onEditorHidden() {
         val name = widgetNameState.value
         if (pendingNameSave?.isActive == true && name != null) {
             pendingNameSave?.cancel()
             saveName(name)
         }
-        if (isNew && name.isNullOrBlank()) {
-            write {
-                val profile = database.widgetProfileDao().findById(profileId) ?: return@write
+    }
+
+    /**
+     * The editor was closed for good (its back stack entry is gone): discard a new profile that was left untouched
+     * (no filters, no name typed), so backing out of "Create new profile" leaves nothing behind. Not done when the
+     * editor is merely hidden, e.g. by switching tabs, since it comes back.
+     */
+    override fun onCleared() {
+        if (!isNew || !widgetNameState.value.isNullOrBlank()) return
+        // viewModelScope is already cancelled here; the write must still happen, after any pending ones.
+        CoroutineScope(Dispatchers.IO).launch {
+            writeMutex.withLock {
+                val profile = database.widgetProfileDao().findById(profileId) ?: return@withLock
                 if (database.packageFilterDao().findAllByProfile(profileId).isEmpty()) {
                     database.widgetProfileDao().delete(profile)
                 }

app/src/main/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorViewState.kt

@@ -15,7 +15,9 @@ data class WidgetProfileEditorViewState(
     /** How many installed apps each filter matches on its own, by filter id. */
     val filterAppCounts: Map<String, Int> = emptyMap(),
     /** Names of the widgets showing this profile, which every edit affects. */
-    val usedByWidgets: List<String> = emptyList()
+    val usedByWidgets: List<String> = emptyList(),
+    /** The profile was deleted (e.g. a new one discarded earlier); the editor leaves instead of loading forever. */
+    val isMissing: Boolean = false
 ) {
     companion object {
         val Empty = WidgetProfileEditorViewState()

app/src/test/java/de/psdev/devdrawer/NavigationViewModelScopeTest.kt

@@ -0,0 +1,92 @@
+package de.psdev.devdrawer
+
+import androidx.compose.material3.Text
+import androidx.compose.runtime.remember
+import androidx.compose.ui.test.junit4.createComposeRule
+import androidx.compose.ui.test.onNodeWithText
+import androidx.lifecycle.ViewModel
+import androidx.lifecycle.viewmodel.compose.viewModel
+import androidx.navigation3.runtime.NavKey
+import androidx.navigation3.runtime.entryProvider
+import androidx.navigation3.ui.NavDisplay
+import kotlinx.serialization.Serializable
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+@Serializable
+data class TestDetailRoute(val id: String) : NavKey
+
+class TestDetailViewModel(val id: String) : ViewModel() {
+    var cleared = false
+    override fun onCleared() {
+        cleared = true
+    }
+}
+
+/** Each screen on the back stack must get its own view models, cleared when the screen is popped. */
+@RunWith(RobolectricTestRunner::class)
+class NavigationViewModelScopeTest {
+
+    @get:Rule
+    val composeTestRule = createComposeRule()
+
+    private lateinit var navigator: Navigator
+    private val created = mutableListOf<TestDetailViewModel>()
+
+    private fun showApp() {
+        composeTestRule.setContent {
+            val state = rememberNavigationState(startRoute = WidgetListRoute, topLevelRoutes = setOf(WidgetListRoute, WidgetProfilesRoute))
+            navigator = remember { Navigator(state) }
+            NavDisplay(
+                entries = state.toEntries(
+                    entryProvider {
+                        entry<WidgetListRoute> { Text("widgets") }
+                        entry<WidgetProfilesRoute> { Text("profiles") }
+                        entry<TestDetailRoute> { key ->
+                            val viewModel = viewModel { TestDetailViewModel(key.id).also { created += it } }
+                            Text("detail ${viewModel.id}")
+                        }
+                    }
+                ),
+                onBack = { navigator.goBack() }
+            )
+        }
+    }
+
+    @Test
+    fun `given one detail screen was closed, when another opens, then it gets its own view model`() {
+        // Given
+        showApp()
+        composeTestRule.runOnIdle { navigator.navigate(TestDetailRoute("a")) }
+        composeTestRule.onNodeWithText("detail a").assertExists()
+        composeTestRule.runOnIdle { navigator.goBack() }
+
+        // When
+        composeTestRule.runOnIdle { navigator.navigate(TestDetailRoute("b")) }
+
+        // Then
+        composeTestRule.onNodeWithText("detail b").assertExists()
+        assertTrue(created.first { it.id == "a" }.cleared)
+    }
+
+    @Test
+    fun `given a detail screen, when switching tabs and back, then its view model survives`() {
+        // Given
+        showApp()
+        composeTestRule.runOnIdle { navigator.navigate(WidgetProfilesRoute) }
+        composeTestRule.runOnIdle { navigator.navigate(TestDetailRoute("a")) }
+        composeTestRule.onNodeWithText("detail a").assertExists()
+
+        // When
+        composeTestRule.runOnIdle { navigator.navigate(WidgetListRoute) }
+        composeTestRule.onNodeWithText("widgets").assertExists()
+        composeTestRule.runOnIdle { navigator.navigate(WidgetProfilesRoute) }
+
+        // Then
+        composeTestRule.onNodeWithText("detail a").assertExists()
+        assertTrue(created.single().cleared.not())
+    }
+}

app/src/test/java/de/psdev/devdrawer/profiles/ui/editor/WidgetProfileEditorScreenTest.kt

@@ -5,6 +5,7 @@ import androidx.compose.ui.test.onNodeWithText
 import androidx.compose.ui.test.performTextReplacement
 import de.psdev.devdrawer.database.WidgetProfile
 import de.psdev.devdrawer.ui.theme.DevDrawerTheme
+import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -53,4 +54,20 @@ class WidgetProfileEditorScreenTest {
         // Then
         composeTestRule.onNodeWithText("Work apps").assertExists()
     }
+
+    @Test
+    fun `given the profile no longer exists, when the editor shows, then it leaves instead of loading forever`() {
+        // Given
+        var left = false
+
+        // When
+        composeTestRule.setContent {
+            DevDrawerTheme {
+                WidgetProfileEditor(viewState = WidgetProfileEditorViewState(isMissing = true), onMissing = { left = true })
+            }
+        }
+
+        // Then
+        composeTestRule.runOnIdle { assertTrue(left) }
+    }
 }