diff --git a/CLAUDE.md b/CLAUDE.md index c40d3e56e2..b811a101c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -368,10 +368,10 @@ The message is a lambda, so build strings inline without guarding on build type logcat { "Plain info-level message" } // defaults to INFO logcat(LogPriority.DEBUG) { "GraphQL ${operation.name()} START" } // explicit priority logcat(LogPriority.ERROR, throwable) { "Failed to load X: ${throwable.message}" } -logcat(LogPriority.INFO, tag = DEEP_LINK_STACK_DEBUG_TAG) { "…" } // greppable custom tag +logcat(LogPriority.INFO, tag = SOMETHING_DEBUG_TAG) { "…" } // greppable custom tag ``` -Tag is optional — for a one-off log, just omit it (the caller's class name is used). When you want a greppable trace that spans several call sites or files, a shared `const val SOMETHING_DEBUG_TAG = "…"` passed as `tag` everywhere is a handy tool (see `DEEP_LINK_STACK_DEBUG_TAG` usage across `MainActivity`/`BackstackController`). It's a convenience, not a requirement; don't introduce a const for a single isolated log. +Tag is optional — for a one-off log, just omit it (the caller's class name is used). When you want a greppable trace that spans several call sites or files, a shared `const val SOMETHING_DEBUG_TAG = "…"` passed as `tag` everywhere is a handy tool. It's a convenience, not a requirement; don't introduce a const for a single isolated log. There is also an Apollo overload, `logcat(priority, operationError: ApolloOperationError, tag, message)`, which auto-downgrades to at most `WARN` for unauthenticated errors. Use it when logging a failed `safeExecute`/`safeFlow` result. diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt b/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt index 5933b9d267..e579f80631 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt @@ -121,9 +121,16 @@ class MainActivity : AppCompatActivity() { * top of [onCreate]; the controller, session reconciler and merged ViewModel factory are read off it. */ private val navRetainedViewModel: NavRetainedViewModel by lazy { + // Seed isOwnTask with isTaskRoot at construction so the controller never starts from a guessed + // default; it is refreshed authoritatively on every onResume (see refreshIsOwnTask). Captured as a + // value here because the initializer lambda's receiver is not this Activity. Only used on the + // genuine first creation — a config change reuses the retained instance and skips the initializer. + val isOwnTaskAtCreation = isTaskRoot ViewModelProvider( this, - viewModelFactory { initializer { NavRetainedViewModel((application as HedvigApplication).appGraph) } }, + viewModelFactory { + initializer { NavRetainedViewModel((application as HedvigApplication).appGraph, isOwnTaskAtCreation) } + }, )[NavRetainedViewModel::class.java] } @@ -197,6 +204,18 @@ class MainActivity : AppCompatActivity() { } } } + val launchedFromHistory = intent.flags and Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY != 0 + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "MainActivity.onCreate launch context: " + + "isColdStart(savedInstanceState==null)=${savedInstanceState == null} | " + + "isTaskRoot=$isTaskRoot | " + + "intent.action=${intent.action} | " + + "intent.categories=${intent.categories} | " + + "intent.data=${intent.data} | " + + "launchedFromHistory=$launchedFromHistory | " + + "intent.flags=0x${Integer.toHexString(intent.flags)} | " + + "isHostedInForeignTask=${isHostedInForeignTask(isTaskRoot, intent.flags)}" + } if (savedInstanceState == null) { handleDeepLinkIntent(intent) } @@ -267,8 +286,25 @@ class MainActivity : AppCompatActivity() { * in onCreate — there is no shared, process-wide controller that a backgrounded Activity could * steal, which is why no resume/top-resume re-attachment is needed. */ + override fun onResume() { + super.onResume() + refreshIsOwnTask() + } + + /** + * Pushes the current own-task value into the controller (see [isHostedInForeignTask] for why this is + * not just [isTaskRoot]). Unlike the one-time hooks in [attachBackstackTaskHooks], this is a *value* + * that can change over the Activity's life (e.g. an Activity below us finishes, or [isTaskRoot] reads + * more reliably once resumed than at onCreate), so it is refreshed on every onResume to keep the + * snapshot-backed value honest — otherwise the lone-deep-link chrome can latch a stale reading. See + * [BackstackController.isOwnTask]. + */ + private fun refreshIsOwnTask() { + backstackController.isOwnTask = !isHostedInForeignTask(isTaskRoot, intent.flags) + } + private fun attachBackstackTaskHooks() { - backstackController.isOwnTask = { isTaskRoot } + refreshIsOwnTask() backstackController.escapeToOwnTask = { parentStack -> NavigationStateBridge.escapeToOwnTask(this@MainActivity, parentStack, serializersModules) } @@ -282,6 +318,32 @@ class MainActivity : AppCompatActivity() { } } +/** + * Logcat tag for the per-launch navigation breadcrumb emitted in [MainActivity.onCreate]. Grep this to + * see how each launch was classified (action/flags/isTaskRoot -> isHostedInForeignTask) when diagnosing + * task/back-stack oddities like the deep-link-stack-on-Home report. + */ +internal const val DEEP_LINK_STACK_DEBUG_TAG = "DeepLinkStackDebug" + +/** + * Whether this Activity was launched into another app's task (genuinely foreign-hosted), given the + * task-root position and the [launchFlags] of the intent that started it. This is the one case that + * wants the lone-deep-link Up/escape affordance; everything else is our own task. + * + * Both conditions must hold to be foreign-hosted: + * - **not the task root** (`!isTaskRoot`): some other activity sits below us in the task; and + * - **launched without [Intent.FLAG_ACTIVITY_NEW_TASK]**: Android's default for that is to place us in + * the *caller's* task rather than one of our own, so we joined whoever started us. + * + * `NEW_TASK` being set means the system gave us our own task (a fresh one, or an existing one brought + * forward), so we are NOT foreign-hosted even when not its root. That is why a launcher relaunch or a + * notification tap that fronted our task (both carry `NEW_TASK`, both can be non-root) is correctly + * treated as own-task and does not render the Up bar on a normal Home. Only a real foreign deep link + * (e.g. an https link tapped in another app, launched with no flags) is foreign-hosted. + */ +internal fun isHostedInForeignTask(isTaskRoot: Boolean, launchFlags: Int): Boolean = + !isTaskRoot && (launchFlags and Intent.FLAG_ACTIVITY_NEW_TASK) == 0 + /** * Applies the theme in two ways: * 1. Uses UiModeManager to persist the last theme selected so that on new launches the splash screen matches the theme diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt b/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt index dc85033309..ecc4165c02 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt @@ -12,6 +12,7 @@ import com.hedvig.android.navigation.common.HedvigNavKey import com.hedvig.android.navigation.common.StashedSession import com.hedvig.android.navigation.common.TopLevelTab import com.hedvig.android.navigation.compose.merge +import kotlin.time.Duration.Companion.minutes import kotlinx.serialization.Polymorphic import kotlinx.serialization.PolymorphicSerializer import kotlinx.serialization.Serializable @@ -33,6 +34,14 @@ internal object NavigationStateBridge { private const val EXTRA_RESTORE_STACK = "com.hedvig.android.app.RESTORE_STACK" private const val NAV_STATE_REGISTRY_KEY = "com.hedvig.android.app.NAV_STATE" + /** + * How long a [BackstackController.pendingDeepLink] may survive a process death and still be landed at + * the next login. Covers a real "killed mid-login" window (switching to BankID, OTP, etc., is seconds + * to a few minutes) while discarding a link stashed long ago, which would otherwise bleed into an + * unrelated later login and strand the member on a lone deep-link stack. See [restoreAndPersist]. + */ + private val MAX_PENDING_DEEP_LINK_AGE = 30.minutes + private val handoffSerializer = ListSerializer(PolymorphicSerializer(HedvigNavKey::class)) /** @@ -65,16 +74,23 @@ internal object NavigationStateBridge { if (!handoff.isNullOrEmpty()) { backstackController.reseed(handoff) } else { - savedStateRegistry.consumeRestoredStateForKey(NAV_STATE_REGISTRY_KEY) + val snapshot = savedStateRegistry.consumeRestoredStateForKey(NAV_STATE_REGISTRY_KEY) ?.let { decodeFromSavedState(NavStateSnapshot.serializer(), it, savedStateConfiguration) } - ?.let { snapshot -> - backstackController.restoreFromSavedState( - entries = snapshot.entries, - parkedRuns = snapshot.parkedRuns, - pendingDeepLink = snapshot.pendingDeepLink, - stashedSession = snapshot.stashedSession, - ) - } + if (snapshot != null) { + // Discard a pending deep link that is too old to still belong to an in-flight login, so it + // can't bleed into an unrelated later login as a lone deep-link stack. + val now = System.currentTimeMillis() + val stashedAt = snapshot.pendingDeepLinkStashedAtEpochMs + val pendingStillValid = snapshot.pendingDeepLink != null && + isPendingDeepLinkStashTimeFresh(stashedAt, now) + backstackController.restoreFromSavedState( + entries = snapshot.entries, + parkedRuns = snapshot.parkedRuns, + pendingDeepLink = snapshot.pendingDeepLink.takeIf { pendingStillValid }, + pendingDeepLinkStashedAtEpochMs = stashedAt.takeIf { pendingStillValid }, + stashedSession = snapshot.stashedSession, + ) + } backstackController.seedIfEmpty(listOf(LoginKey)) } @@ -87,6 +103,7 @@ internal object NavigationStateBridge { entries = backstackController.entries.toList(), parkedRuns = backstackController.parkedRuns.toMap(), pendingDeepLink = backstackController.pendingDeepLink, + pendingDeepLinkStashedAtEpochMs = backstackController.pendingDeepLinkStashedAtEpochMs, stashedSession = backstackController.stashedSession, ), savedStateConfiguration, @@ -124,12 +141,28 @@ internal object NavigationStateBridge { private fun json(serializersModules: Set): Json = Json { serializersModule = serializersModules.merge() } + + /** + * Whether a pending deep link stashed at [stashedAtEpochMs] is recent enough (as of [nowEpochMs]) to + * still be landed after a process-death restore. `null` (no timestamp — e.g. a pre-upgrade snapshot) + * and a negative age (clock moved backwards) both count as not-fresh, so the link is dropped rather + * than risk landing a stale one. Pure and clock-injected so it is unit-testable without Android. + */ + internal fun isPendingDeepLinkStashTimeFresh(stashedAtEpochMs: Long?, nowEpochMs: Long): Boolean { + if (stashedAtEpochMs == null) return false + val ageMs = nowEpochMs - stashedAtEpochMs + return ageMs in 0..MAX_PENDING_DEEP_LINK_AGE.inWholeMilliseconds + } } /** * The full hoisted navigation state, serialized into the Activity's SavedStateRegistry so the - * in-memory [BackstackController] singleton can be re-hydrated after process death. Mirrors the four + * in-memory [BackstackController] singleton can be re-hydrated after process death. Mirrors the * holders the controller owns. + * + * [pendingDeepLinkStashedAtEpochMs] defaults to `null` so snapshots written before this field existed + * still decode; a missing timestamp means an age can't be computed, so such a restored pending link is + * treated as stale and dropped (see [NavigationStateBridge.restoreAndPersist]). */ @Serializable private data class NavStateSnapshot( @@ -137,4 +170,5 @@ private data class NavStateSnapshot( val parkedRuns: Map>, val pendingDeepLink: (@Polymorphic HedvigNavKey)?, val stashedSession: StashedSession?, + val pendingDeepLinkStashedAtEpochMs: Long? = null, ) diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt index e1c904655b..c9e0a59b0a 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt @@ -3,6 +3,7 @@ package com.hedvig.android.app.navigation import androidx.compose.runtime.MutableState import androidx.compose.runtime.Stable import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.runtime.snapshots.SnapshotStateList @@ -39,18 +40,10 @@ internal class BackstackController( internal val parkedRuns: SnapshotStateMap>, pendingDeepLinkState: MutableState, stashedSessionState: MutableState, - /** - * Whether this activity is the root of its own task. `false` means we were launched into the - * caller's task by an external deep link, so an Up press must escape into our own task rather than - * rebuilding the ancestry in place (which would leave our screens hosted under the foreign app). - * - * Attached by `MainActivity` in `onCreate` (see `attachBackstackTaskHooks`), not at construction: the - * controller is owned by this Activity's retained `ViewModel` and is 1:1 with the Activity, so there - * is no shared, process-wide controller a backgrounded Activity could steal — the hook is wired once - * at creation, with no resume re-attachment needed. Defaults to `true` so unit tests and any - * pre-attach use stay fully in-process. - */ - var isOwnTask: () -> Boolean = { true }, + // Defaulted (and placed after the required states) so the many positional test constructions need no + // change; the real construction in NavRetainedViewModel passes it explicitly by name. + pendingDeepLinkStashedAtState: MutableState = mutableStateOf(null), + initialIsOwnTask: Boolean = true, /** * Re-roots the app in its own task seeded with the given stack (see [navigateUp]). The Activity * owns the mechanics (relaunch with NEW_TASK|CLEAR_TASK + finish); the controller only supplies @@ -65,12 +58,50 @@ internal class BackstackController( */ var finishApp: () -> Unit = {}, ) : Backstack { + /** + * Whether this Activity is the root of its own task. `false` means we were launched into the + * caller's task by an external deep link, so an Up press must escape into our own task rather than + * rebuilding the ancestry in place (which would leave our screens hosted under the foreign app). + * + * Snapshot-backed on purpose, rather than a live lambda: [loneDeepLinkChrome] derives the whole + * back-arrow-vs-nav-bar decision partly from this, so it must (a) be observable so Compose recomposes + * the chrome when it changes, and (b) never sample a stale value. `MainActivity` pushes the computed + * own-task value here (see `isHostedInForeignTask`, which is *not* just `isTaskRoot`) in `onCreate` (see + * `attachBackstackTaskHooks`) AND on every `onResume` — onResume because the value can settle or + * change after creation (an Activity below us finishing, or an early-launch read returning a + * not-yet-settled value). Defaults to `true` so unit tests and any pre-attach use stay fully + * in-process. + */ + var isOwnTask: Boolean by mutableStateOf(initialIsOwnTask) + /** * A deep link resolved while logged out, held until [setLoggedIn] consumes it (so it can land - * alone). Persisted across rotation / process death (e.g. mid-OTP) — see the class KDoc. + * alone). Persisted across rotation / process death (e.g. mid-OTP) — see the class KDoc. Always set + * via [stashPendingDeepLink] so its companion timestamp [pendingDeepLinkStashedAtEpochMs] stays in + * sync; cleared (with the timestamp) when consumed by [setLoggedIn] or wiped by [reseed]. */ internal var pendingDeepLink: HedvigNavKey? by pendingDeepLinkState + /** + * Wall-clock time ([System.currentTimeMillis]) when [pendingDeepLink] was last stashed, or `null` + * when there is none. Persisted alongside [pendingDeepLink] so a process-death restore can discard a + * link that is too old to still belong to the in-flight login (see `NavigationStateBridge`), which + * stops a long-abandoned pending link from bleeding into an unrelated later login. + */ + internal var pendingDeepLinkStashedAtEpochMs: Long? by pendingDeepLinkStashedAtState + + /** Stashes [key] as the [pendingDeepLink], stamping [pendingDeepLinkStashedAtEpochMs] with now. */ + private fun stashPendingDeepLink(key: HedvigNavKey) { + pendingDeepLink = key + pendingDeepLinkStashedAtEpochMs = System.currentTimeMillis() + } + + /** Clears [pendingDeepLink] and its companion timestamp together. */ + private fun clearPendingDeepLink() { + pendingDeepLink = null + pendingDeepLinkStashedAtEpochMs = null + } + /** * The previous logged-in session, held between logout and the next login. Excluded from * [allLiveContentKeys] on purpose — see [StashedSession]. Persisted across process death. @@ -139,12 +170,11 @@ internal class BackstackController( val loneDeepLinkChrome: LoneDeepLinkChrome get() { val first = entries.firstOrNull() - val insideRunsModel = isOwnTask() && (first is HomeKey || first is LoginKey) - if (insideRunsModel) return LoneDeepLinkChrome.ShowSuite - return if (entries.lastOrNull()?.topLevelTabOrNull() != null) { - LoneDeepLinkChrome.ShowUpBar - } else { - LoneDeepLinkChrome.ShowNothing + val insideRunsModel = isOwnTask && (first is HomeKey || first is LoginKey) + return when { + insideRunsModel -> LoneDeepLinkChrome.ShowSuite + entries.lastOrNull()?.topLevelTabOrNull() != null -> LoneDeepLinkChrome.ShowUpBar + else -> LoneDeepLinkChrome.ShowNothing } } @@ -181,7 +211,7 @@ internal class BackstackController( */ fun navigateToInAppLink(key: HedvigNavKey) { if (!isLoggedIn) { - pendingDeepLink = key + stashPendingDeepLink(key) return } Snapshot.withMutableSnapshot { @@ -199,7 +229,7 @@ internal class BackstackController( */ fun navigateToExternalDeepLink(key: HedvigNavKey) { if (!isLoggedIn) { - pendingDeepLink = key + stashPendingDeepLink(key) return } reseed(listOf(key)) @@ -222,11 +252,11 @@ internal class BackstackController( val parentStack = synthetic.dropLast(1) when { parentStack.isNotEmpty() -> { - if (isOwnTask()) entries.replaceWith(parentStack) else escapeToOwnTask(parentStack) + if (isOwnTask) entries.replaceWith(parentStack) else escapeToOwnTask(parentStack) return true } - !isOwnTask() -> { + !isOwnTask -> { escapeToOwnTask(synthetic) return true } @@ -265,7 +295,7 @@ internal class BackstackController( fun setLoggedIn(memberId: String?) { Snapshot.withMutableSnapshot { val pending = pendingDeepLink - pendingDeepLink = null + clearPendingDeepLink() val stash = stashedSession?.takeIf { memberId != null && it.memberId == memberId } stashedSession = null parkedRuns.clear() @@ -328,7 +358,7 @@ internal class BackstackController( Snapshot.withMutableSnapshot { entries.replaceWith(stack) parkedRuns.clear() - pendingDeepLink = null + clearPendingDeepLink() stashedSession = null } } @@ -342,6 +372,7 @@ internal class BackstackController( entries: List, parkedRuns: Map>, pendingDeepLink: HedvigNavKey?, + pendingDeepLinkStashedAtEpochMs: Long?, stashedSession: StashedSession?, ) { if (this.entries.isNotEmpty()) return @@ -349,6 +380,7 @@ internal class BackstackController( this.entries.addAll(entries) this.parkedRuns.putAll(parkedRuns) this.pendingDeepLink = pendingDeepLink + this.pendingDeepLinkStashedAtEpochMs = pendingDeepLinkStashedAtEpochMs this.stashedSession = stashedSession } } diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt index 8a97bfd36a..16fd9fe8c7 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt @@ -20,13 +20,18 @@ import dev.zacsweers.metrox.viewmodel.MetroViewModelFactory * spins up a per-Activity [ActivityRetainedGraph] seeded with that controller. Everything that must * talk to this Activity's stack — the [sessionReconciler] and every back-stack ViewModel resolved by * [viewModelFactory] — comes from that extension, so two `MainActivity` instances never share state. + * + * [isOwnTask] seeds [BackstackController.isOwnTask] with the Activity's `isTaskRoot` at first creation + * (so it never starts from a guessed default); `MainActivity` refreshes it authoritatively on resume. */ -internal class NavRetainedViewModel(appGraph: AppGraph) : ViewModel() { +internal class NavRetainedViewModel(appGraph: AppGraph, isOwnTask: Boolean) : ViewModel() { val backstackController: BackstackController = BackstackController( entries = mutableStateListOf(), parkedRuns = mutableStateMapOf(), pendingDeepLinkState = mutableStateOf(null), + pendingDeepLinkStashedAtState = mutableStateOf(null), stashedSessionState = mutableStateOf(null), + initialIsOwnTask = isOwnTask, ) private val activityGraph: ActivityRetainedGraph = diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/IsHostedInForeignTaskTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/IsHostedInForeignTaskTest.kt new file mode 100644 index 0000000000..556cda383d --- /dev/null +++ b/app/app/src/test/kotlin/com/hedvig/android/app/IsHostedInForeignTaskTest.kt @@ -0,0 +1,40 @@ +package com.hedvig.android.app + +import android.content.Intent +import assertk.assertThat +import assertk.assertions.isFalse +import assertk.assertions.isTrue +import org.junit.Test + +internal class IsHostedInForeignTaskTest { + @Test + fun `a task-root launch is never foreign-hosted`() { + assertThat(isHostedInForeignTask(isTaskRoot = true, launchFlags = 0)).isFalse() + assertThat(isHostedInForeignTask(isTaskRoot = true, launchFlags = Intent.FLAG_ACTIVITY_NEW_TASK)).isFalse() + } + + @Test + fun `a non-root launch without NEW_TASK is foreign-hosted`() { + // The notes-app deep link case: launched into the caller's task with no flags (0x0) -> foreign, + // so we keep the Up/escape affordance. + assertThat(isHostedInForeignTask(isTaskRoot = false, launchFlags = 0)).isTrue() + } + + @Test + fun `a non-root launcher relaunch with NEW_TASK is not foreign-hosted`() { + // The reported bug: a second MainActivity stacked by a launcher relaunch (NEW_TASK | + // BROUGHT_TO_FRONT | RESET_TASK_IF_NEEDED) is in our own task, just not its root. + val launcherRelaunchFlags = Intent.FLAG_ACTIVITY_NEW_TASK or + Intent.FLAG_ACTIVITY_BROUGHT_TO_FRONT or + Intent.FLAG_ACTIVITY_RESET_TASK_IF_NEEDED + assertThat(isHostedInForeignTask(isTaskRoot = false, launchFlags = launcherRelaunchFlags)).isFalse() + } + + @Test + fun `a non-root notification tap that fronted our task with NEW_TASK is not foreign-hosted`() { + // The warm-notification case (1b): VIEW deep link with NEW_TASK | BROUGHT_TO_FRONT stacked on our + // own fronted task -> own-task, so Up navigates in place instead of escaping. + val warmNotificationFlags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_BROUGHT_TO_FRONT + assertThat(isHostedInForeignTask(isTaskRoot = false, launchFlags = warmNotificationFlags)).isFalse() + } +} diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/NavigationStateBridgeTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/NavigationStateBridgeTest.kt new file mode 100644 index 0000000000..31d3a46e9c --- /dev/null +++ b/app/app/src/test/kotlin/com/hedvig/android/app/NavigationStateBridgeTest.kt @@ -0,0 +1,50 @@ +package com.hedvig.android.app + +import assertk.assertThat +import assertk.assertions.isFalse +import assertk.assertions.isTrue +import org.junit.Test + +internal class NavigationStateBridgeTest { + private val thirtyMinutesMs = 30L * 60L * 1000L + + @Test + fun `a missing stash timestamp is treated as not fresh`() { + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(null, nowEpochMs = 1_000_000L)) + .isFalse() + } + + @Test + fun `a just-stashed pending link is fresh`() { + val now = 1_000_000L + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(now, nowEpochMs = now)).isTrue() + } + + @Test + fun `a link stashed just under the limit is fresh`() { + val now = 5_000_000L + val stashedAt = now - (thirtyMinutesMs - 1) + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isTrue() + } + + @Test + fun `a link stashed exactly at the limit is still fresh`() { + val now = 5_000_000L + val stashedAt = now - thirtyMinutesMs + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isTrue() + } + + @Test + fun `a link stashed past the limit is stale`() { + val now = 5_000_000L + val stashedAt = now - (thirtyMinutesMs + 1) + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isFalse() + } + + @Test + fun `a negative age from a backwards clock is treated as not fresh`() { + val now = 5_000_000L + val stashedAt = now + 60_000L // stashed "in the future" relative to now + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isFalse() + } +} diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt index 363de7ab55..8c748dbb51 100644 --- a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt +++ b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt @@ -9,6 +9,8 @@ import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.isEmpty import assertk.assertions.isEqualTo import assertk.assertions.isFalse +import assertk.assertions.isNotNull +import assertk.assertions.isNull import assertk.assertions.isTrue import com.hedvig.android.feature.help.center.navigation.HelpCenterKey import com.hedvig.android.feature.home.home.navigation.HomeKey @@ -16,13 +18,18 @@ import com.hedvig.android.feature.insurances.navigation.InsurancesKey import com.hedvig.android.feature.login.navigation.LoginKey import com.hedvig.android.feature.payments.navigation.PaymentsKey import com.hedvig.android.feature.profile.navigation.ProfileKey +import com.hedvig.android.logger.TestLogcatLoggingRule import com.hedvig.android.navigation.common.HedvigNavKey import com.hedvig.android.navigation.common.TopLevelTab import com.hedvig.android.navigation.compose.LoneDeepLinkChrome import com.hedvig.android.navigation.compose.popUpTo +import org.junit.Rule import org.junit.Test internal class BackstackControllerTest { + @get:Rule + val testLogcatLogger = TestLogcatLoggingRule() + private fun controllerWith(vararg keys: HedvigNavKey) = BackstackController( mutableStateListOf(*keys), mutableStateMapOf(), @@ -272,7 +279,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { false }, + initialIsOwnTask = false, escapeToOwnTask = { escaped = it }, ) assertThat(controller.navigateUp()).isTrue() @@ -289,7 +296,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { true }, + initialIsOwnTask = true, escapeToOwnTask = { escaped = it }, ) assertThat(controller.navigateUp()).isTrue() @@ -404,6 +411,41 @@ internal class BackstackControllerTest { assertThat(controller.pendingDeepLink).isEqualTo(HelpCenterKey) } + @Test + fun `stashing a pending deep link records a timestamp, and consuming it at login clears both`() { + val controller = controllerWith(LoginKey) + controller.navigateToExternalDeepLink(HelpCenterKey) + assertThat(controller.pendingDeepLink).isEqualTo(HelpCenterKey) + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNotNull() + controller.setLoggedIn("mem-1") + assertThat(controller.pendingDeepLink).isNull() + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNull() + } + + @Test + fun `reseed clears the pending deep link and its timestamp`() { + val controller = controllerWith(LoginKey) + controller.navigateToInAppLink(HelpCenterKey) + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNotNull() + controller.reseed(listOf(HomeKey)) + assertThat(controller.pendingDeepLink).isNull() + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNull() + } + + @Test + fun `restoreFromSavedState round-trips the pending deep link timestamp`() { + val controller = controllerWith() // empty so the restore applies (live state would otherwise win) + controller.restoreFromSavedState( + entries = listOf(LoginKey), + parkedRuns = emptyMap(), + pendingDeepLink = HelpCenterKey, + pendingDeepLinkStashedAtEpochMs = 123_456L, + stashedSession = null, + ) + assertThat(controller.pendingDeepLink).isEqualTo(HelpCenterKey) + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isEqualTo(123_456L) + } + @Test fun `loneDeepLinkChrome is ShowUpBar for a lone tab root`() { assertThat(controllerWith(InsurancesKey).loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowUpBar) @@ -445,7 +487,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { false }, + initialIsOwnTask = false, ) assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowUpBar) } @@ -457,11 +499,24 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { true }, + initialIsOwnTask = true, ) assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowSuite) } + @Test + fun `toggling isOwnTask flips a lone Home between suite and up bar`() { + // Guards the no-deep-link path: a normal lone Home renders as a deep-link stack (up bar) whenever + // isTaskRoot is false, and must self-correct once isOwnTask is refreshed back to true (the onResume + // fix). Also proves isOwnTask is read live, not frozen at construction. + val controller = controllerWith(HomeKey) + assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowSuite) + controller.isOwnTask = false + assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowUpBar) + controller.isOwnTask = true + assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowSuite) + } + @Test fun `navigateUp from a lone deep link drops the leaf and exposes the rebuilt ancestry`() { val controller = controllerWith(InsurancesKey) @@ -478,7 +533,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { false }, + initialIsOwnTask = false, escapeToOwnTask = { escaped = it }, ) assertThat(controller.navigateUp()).isTrue() diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt index c1f8e35e69..a1fe80faf4 100644 --- a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt +++ b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt @@ -8,7 +8,9 @@ import assertk.assertions.containsExactly import com.hedvig.android.feature.help.center.navigation.HelpCenterKey import com.hedvig.android.feature.home.home.navigation.HomeKey import com.hedvig.android.feature.insurances.navigation.InsurancesKey +import com.hedvig.android.logger.TestLogcatLoggingRule import com.hedvig.android.navigation.common.HedvigNavKey +import org.junit.Rule import org.junit.Test /** @@ -18,6 +20,9 @@ import org.junit.Test * to a key already on the stack must therefore never blind-append. */ internal class DeepLinkNavigationTest { + @get:Rule + val testLogcatLogger = TestLogcatLoggingRule() + private fun controllerWith(vararg keys: HedvigNavKey) = BackstackController( mutableStateListOf(*keys), mutableStateMapOf(), diff --git a/app/design-system/design-system-hedvig/src/commonMain/kotlin/com/hedvig/android/design/system/hedvig/StartClaimBottomSheet.kt b/app/design-system/design-system-hedvig/src/commonMain/kotlin/com/hedvig/android/design/system/hedvig/StartClaimBottomSheet.kt index 558f31da43..bdea2a83c2 100644 --- a/app/design-system/design-system-hedvig/src/commonMain/kotlin/com/hedvig/android/design/system/hedvig/StartClaimBottomSheet.kt +++ b/app/design-system/design-system-hedvig/src/commonMain/kotlin/com/hedvig/android/design/system/hedvig/StartClaimBottomSheet.kt @@ -41,10 +41,7 @@ import hedvig.resources.general_continue_button import org.jetbrains.compose.resources.stringResource @Composable -fun StartClaimBottomSheet( - state: HedvigBottomSheetState, - navigateToClaimChat: () -> Unit, -) { +fun StartClaimBottomSheet(state: HedvigBottomSheetState, navigateToClaimChat: () -> Unit) { HedvigBottomSheet( hedvigBottomSheetState = state, content = { @@ -62,16 +59,13 @@ fun StartClaimBottomSheet( ) } - @Composable -fun StartClaimPledgeScreen( - navigateUp: () -> Unit, - navigateToClaimChat: () -> Unit, - modifier: Modifier = Modifier, -) { +fun StartClaimPledgeScreen(navigateUp: () -> Unit, navigateToClaimChat: () -> Unit, modifier: Modifier = Modifier) { var isChecked by remember { mutableStateOf(false) } - Column(modifier - .verticalScroll(rememberScrollState())) { + Column( + modifier + .verticalScroll(rememberScrollState()), + ) { PledgeNotes() Spacer(Modifier.weight(1f)) Spacer(Modifier.height(8.dp)) @@ -89,10 +83,7 @@ fun StartClaimPledgeScreen( } @Composable -private fun StartClaimBottomSheetContent( - dismiss: () -> Unit, - navigateToClaimChat: () -> Unit, -) { +private fun StartClaimBottomSheetContent(dismiss: () -> Unit, navigateToClaimChat: () -> Unit) { var isChecked by remember { mutableStateOf(false) } Column { Spacer(Modifier.height(16.dp)) diff --git a/app/feature/feature-claim-chat/src/androidMain/kotlin/com/hedvig/feature/claim/chat/navigation/ClaimChatEntries.kt b/app/feature/feature-claim-chat/src/androidMain/kotlin/com/hedvig/feature/claim/chat/navigation/ClaimChatEntries.kt index 93494fd1bc..2c163cf3cc 100644 --- a/app/feature/feature-claim-chat/src/androidMain/kotlin/com/hedvig/feature/claim/chat/navigation/ClaimChatEntries.kt +++ b/app/feature/feature-claim-chat/src/androidMain/kotlin/com/hedvig/feature/claim/chat/navigation/ClaimChatEntries.kt @@ -37,7 +37,7 @@ internal data class ClaimOutcomeNewClaimKey( internal data object UpdateAppKey : HedvigNavKey @Serializable -internal data object StartClaimPledgeKey: HedvigNavKey +internal data object StartClaimPledgeKey : HedvigNavKey fun EntryProviderScope.claimChatEntries( backstack: Backstack, @@ -110,7 +110,7 @@ fun EntryProviderScope.claimChatEntries( ClaimChatKey(), inclusive = true, ) - } + }, ) } } diff --git a/app/feature/feature-claim-chat/src/commonMain/kotlin/com/hedvig/feature/claim/chat/ui/StartClaimPledgeScreen.kt b/app/feature/feature-claim-chat/src/commonMain/kotlin/com/hedvig/feature/claim/chat/ui/StartClaimPledgeScreen.kt index 543d1c9100..e0a9bc96f8 100644 --- a/app/feature/feature-claim-chat/src/commonMain/kotlin/com/hedvig/feature/claim/chat/ui/StartClaimPledgeScreen.kt +++ b/app/feature/feature-claim-chat/src/commonMain/kotlin/com/hedvig/feature/claim/chat/ui/StartClaimPledgeScreen.kt @@ -25,10 +25,7 @@ import hedvig.resources.Res import org.jetbrains.compose.resources.stringResource @Composable -internal fun StartClaimPledgeDestination( - navigateUp: () -> Unit, - navigateToClaimChat: () -> Unit -) { +internal fun StartClaimPledgeDestination(navigateUp: () -> Unit, navigateToClaimChat: () -> Unit) { Surface( color = HedvigTheme.colorScheme.backgroundPrimary, ) { @@ -47,21 +44,18 @@ internal fun StartClaimPledgeDestination( navigateToClaimChat = navigateToClaimChat, modifier = Modifier .weight(1f) - .padding(horizontal = 16.dp) + .padding(horizontal = 16.dp), ) } } } - @HedvigShortMultiScreenPreview @Composable -private fun PreviewStartClaimPledgeDestination( -) { +private fun PreviewStartClaimPledgeDestination() { HedvigTheme { Surface(color = HedvigTheme.colorScheme.backgroundPrimary) { - StartClaimPledgeDestination({},{} - ) + StartClaimPledgeDestination({}, {}) } } } diff --git a/app/feature/feature-help-center/src/commonMain/kotlin/com/hedvig/android/feature/help/center/HelpCenterPresenter.kt b/app/feature/feature-help-center/src/commonMain/kotlin/com/hedvig/android/feature/help/center/HelpCenterPresenter.kt index 0c168b1253..0f6e087b68 100644 --- a/app/feature/feature-help-center/src/commonMain/kotlin/com/hedvig/android/feature/help/center/HelpCenterPresenter.kt +++ b/app/feature/feature-help-center/src/commonMain/kotlin/com/hedvig/android/feature/help/center/HelpCenterPresenter.kt @@ -45,13 +45,13 @@ import com.hedvig.android.feature.help.center.data.QuickLinkDestination.OuterDes import com.hedvig.android.feature.help.center.data.QuickLinkDestination.OuterDestination.QuickLinkTermination import com.hedvig.android.feature.help.center.data.QuickLinkDestination.OuterDestination.QuickLinkTravelCertificate import com.hedvig.android.feature.help.center.model.QuickAction -import com.hedvig.android.featureflags.FeatureManager -import com.hedvig.android.featureflags.flags.Feature import com.hedvig.android.feature.help.center.navigation.EmergencyKey import com.hedvig.android.feature.help.center.navigation.FirstVetKey import com.hedvig.android.feature.movingflow.SelectContractForMovingKey import com.hedvig.android.feature.terminateinsurance.navigation.TerminateInsuranceKey import com.hedvig.android.feature.travelcertificate.navigation.TravelCertificateKey +import com.hedvig.android.featureflags.FeatureManager +import com.hedvig.android.featureflags.flags.Feature import com.hedvig.android.molecule.public.MoleculePresenter import com.hedvig.android.molecule.public.MoleculePresenterScope import com.hedvig.android.navigation.common.HedvigNavKey