a9bcf85ca3c9
refactor: extract routeOnUnlock pure function + UnlockRoute sealed class (v2.14.10)
- time
- 2026-08-06 00:52 UTC · 2 months ago
- machine
- koh2
- parent
- 3165a9822d41
↓ peach-android-a9bcf85c.tar.gzatlas at this save
files changed4 files
VERSION+1 −1app/src/main/java/com/peachpasswords/ui/lock/LockScreen.kt+42 −8app/src/test/java/com/peachpasswords/ui/lock/LockScreenRoutingTest.kt+79 −0releases/v2.14.10.md+2 −0app/src/main/java/com/peachpasswords/ui/lock/LockScreen.kt+42 −8
@@ -70,6 +70,43 @@7070 import com.peachpasswords.ui.theme.PeachType7171 import com.peachpasswords.ui.theme.ShellBackground7272 73+/**74+ * Where the unlock transition should land. Extracted from the LaunchedEffect body so75+ * the priority order (created > autofill > postUnlock) is unit-testable without a76+ * Compose runtime; the LaunchedEffect is a thin dispatcher that calls77+ * [routeOnUnlock] once per unlock transition.78+ *79+ * [Defer] means "let the NavHost handle it" — used when an autofill-driven navigation80+ * is queued at the moment of unlock so we don't clobber it with popUpTo(0).81+ */82+internal sealed class UnlockRoute {83+ /** A freshly created vault must walk through recovery-word setup before the list. */84+ data object RecoverySetup : UnlockRoute()85+ /** An autofill nav request is in flight; the NavHost owns the destination. */86+ data object Defer : UnlockRoute()87+ /** Ordinary unlock: route to the post-unlock destination the ViewModel computed. */88+ data class PostUnlock(val destination: PeachNav) : UnlockRoute()89+}90+91+/**92+ * Decide what the lock screen does when [LockUiState.unlocked] flips true. The93+ * priority order matters: a newly-created vault always goes to recovery-word setup,94+ * an autofill-pending unlock defers to the NavHost, everything else lands on the95+ * ViewModel-computed post-unlock destination.96+ *97+ * `postUnlock` is a lambda so the function is pure and the lazy destination lookup98+ * is observable in tests (a [UnlockRoute.Defer] must never invoke it).99+ */100+internal fun routeOnUnlock(101+ state: LockUiState,102+ autofillPending: Boolean,103+ postUnlock: () -> PeachNav,104+): UnlockRoute = when {105+ state.created -> UnlockRoute.RecoverySetup106+ autofillPending -> UnlockRoute.Defer107+ else -> UnlockRoute.PostUnlock(postUnlock())108+}109+73110 @Composable74111 fun LockScreen(75112 onUnlocked: (PeachNav) -> Unit,@@ -109,14 +146,11 @@109146 // transition (rather than re-reading it on every key change) makes the two effects110147 // cooperative instead of racing.111148 LaunchedEffect(state.unlocked) {112- if (state.unlocked) {113- // A freshly created vault routes through recovery-word setup before the list;114- // an ordinary unlock lands on the last-viewed entry (brief Fix 3). The autofill115- // nav host handles its own destinations when autofillPending is set; stepping in116- // here would clobber that navigation with `popUpTo(0)`.117- if (state.created) onNavigate(PeachNav.RecoverySetup)118- else if (autofillPending) Unit119- else onUnlocked(vm.postUnlockDestination())149+ if (!state.unlocked) return@LaunchedEffect150+ when (val route = routeOnUnlock(state, autofillPending, vm::postUnlockDestination)) {151+ UnlockRoute.RecoverySetup -> onNavigate(PeachNav.RecoverySetup)152+ UnlockRoute.Defer -> Unit153+ is UnlockRoute.PostUnlock -> onUnlocked(route.destination)120154 }121155 }122156 app/src/test/java/com/peachpasswords/ui/lock/LockScreenRoutingTest.kt+79 −0
@@ -0,0 +1,79 @@1+package com.peachpasswords.ui.lock2+3+import com.peachpasswords.ui.PeachNav4+import org.junit.Assert.assertEquals5+import org.junit.Assert.assertTrue6+import org.junit.Test7+8+/**9+ * Pins the unlock-routing decision in [routeOnUnlock]. The LaunchedEffect body in10+ * [LockScreen] is a thin dispatcher over this function, so the priority order11+ * (created > autofill > postUnlock) and the lazy `postUnlock` semantics are the12+ * only thing worth locking down here.13+ *14+ * Background: a previous v2.14.5 guard put the autofill-pending branch inside15+ * LockScreen and was keyed on both `state.unlocked` and `autofillPending`. The16+ * NavHost's autofill LaunchedEffect consumed `newEntryRequest` synchronously,17+ * which flipped `autofillPending` to false mid-unlock and re-fired LockScreen's18+ * effect — landing the user on the vault list instead of the new-entry editor.19+ * The fix keyed LockScreen's effect on `state.unlocked` only; these tests pin20+ * the decision so a future reorder can't silently re-introduce the regression.21+ */22+class LockScreenRoutingTest {23+24+ private fun state(unlocked: Boolean = true, created: Boolean = false) =25+ LockUiState(unlocked = unlocked, created = created)26+27+ @Test fun created_vault_always_routes_to_recovery_setup_regardless_of_autofill() {28+ // A freshly created vault must walk through recovery-word setup before29+ // the list — even if an autofill nav request is queued, the user just30+ // made the vault and the recovery phrase is the immediate priority.31+ val dest = PeachNav.VaultList // any sentinel; must not be consulted32+ assertEquals(33+ UnlockRoute.RecoverySetup,34+ routeOnUnlock(state(created = true), autofillPending = false) { dest },35+ )36+ assertEquals(37+ UnlockRoute.RecoverySetup,38+ routeOnUnlock(state(created = true), autofillPending = true) { dest },39+ )40+ }41+42+ @Test fun autofill_pending_unlock_defers_to_the_navhost() {43+ // The regression: when autofillPending was true at the unlock transition,44+ // LockScreen must NOT route to the post-unlock destination or the NavHost's45+ // editor navigation gets clobbered with popUpTo(0).46+ var postUnlockCalls = 047+ val route = routeOnUnlock(state(created = false), autofillPending = true) {48+ postUnlockCalls++49+ PeachNav.VaultList50+ }51+ assertEquals(UnlockRoute.Defer, route)52+ assertEquals(53+ "Defer must not consult postUnlock — lazy lookup matters when the " +54+ "destination is expensive (lastViewed → resolve → firstActive)",55+ 0, postUnlockCalls,56+ )57+ }58+59+ @Test fun ordinary_unlock_routes_through_the_viewmodel_destination() {60+ // The base case: a normal unlock lands on the destination the ViewModel61+ // computed (last-viewed entry, or the vault list, or recovery setup).62+ val expected = PeachNav.EntryDetail("abc-123")63+ val route = routeOnUnlock(state(created = false), autofillPending = false) { expected }64+ assertTrue(route is UnlockRoute.PostUnlock)65+ assertEquals(expected, (route as UnlockRoute.PostUnlock).destination)66+ }67+68+ @Test fun postUnlock_lookup_runs_exactly_once_on_ordinary_unlock() {69+ // Sanity: postUnlock is invoked exactly once per ordinary unlock so the70+ // ViewModel-side resolution work (which can be non-trivial) is bounded.71+ var calls = 072+ val route = routeOnUnlock(state(created = false), autofillPending = false) {73+ calls++74+ PeachNav.VaultList75+ }76+ assertTrue(route is UnlockRoute.PostUnlock)77+ assertEquals(1, calls)78+ }79+}releases/v2.14.10.md+2 −0
@@ -0,0 +1,2 @@1+## Notes2+- Internal-only. No user-visible changes from v2.14.9. The lock-screen unlock routing logic is now a pure testable function (`routeOnUnlock`) backed by four unit tests pinning the priority order (created-vault → recovery setup, autofill-pending → defer to the NavHost, otherwise → post-unlock destination). APK is functionally identical to v2.14.9. Skipping publication unless explicitly requested.