From 80810cddf12acd421593ff007202ca27b1ea68ac Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 19 Sep 2025 16:59:46 +0200 Subject: [PATCH 1/3] Rename val --- .../io/element/android/features/ftue/impl/FtueFlowNode.kt | 6 +++--- .../android/features/ftue/impl/DefaultFtueEntryPointTest.kt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt index 1f4ca9e4ee..51414c70fd 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt @@ -49,7 +49,7 @@ import kotlinx.parcelize.Parcelize class FtueFlowNode( @Assisted buildContext: BuildContext, @Assisted plugins: List, - private val ftueState: DefaultFtueService, + private val defaultFtueService: DefaultFtueService, private val analyticsEntryPoint: AnalyticsEntryPoint, private val analyticsService: AnalyticsService, private val lockScreenEntryPoint: LockScreenEntryPoint, @@ -90,7 +90,7 @@ class FtueFlowNode( .onEach { moveToNextStepIfNeeded() } .launchIn(lifecycleScope) - ftueState.isVerificationStatusKnown + defaultFtueService.isVerificationStatusKnown .filter { it } .onEach { moveToNextStepIfNeeded() } .launchIn(lifecycleScope) @@ -134,7 +134,7 @@ class FtueFlowNode( } private fun moveToNextStepIfNeeded() = lifecycleScope.launch { - when (ftueState.getNextStep()) { + when (defaultFtueService.getNextStep()) { FtueStep.WaitingForInitialState -> { backstack.newRoot(NavTarget.Placeholder) } diff --git a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt index 50cc2eadce..2f832e3925 100644 --- a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt +++ b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt @@ -36,7 +36,7 @@ class DefaultFtueEntryPointTest { buildContext = buildContext, plugins = plugins, analyticsEntryPoint = { _, _ -> lambdaError() }, - ftueState = createDefaultFtueService(), + defaultFtueService = createDefaultFtueService(), analyticsService = FakeAnalyticsService(), lockScreenEntryPoint = object : LockScreenEntryPoint { override fun nodeBuilder( From f9d8c9a065a55c139abd51f931441f726f57a782 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 19 Sep 2025 17:21:00 +0200 Subject: [PATCH 2/3] Ensure we wait for user confirmation of session verified before going to next step. --- .../features/ftue/impl/FtueFlowNode.kt | 37 +++----- .../ftue/impl/state/DefaultFtueService.kt | 62 ++++++++----- .../ftue/impl/state/InternalFtueState.kt | 18 ++++ .../ftue/impl/DefaultFtueEntryPointTest.kt | 2 - .../ftue/impl/DefaultFtueServiceTest.kt | 90 ++++++++++--------- 5 files changed, 119 insertions(+), 90 deletions(-) create mode 100644 features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/InternalFtueState.kt diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt index 51414c70fd..a4aa9f18d7 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt @@ -14,7 +14,6 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.lifecycle.lifecycleScope -import com.bumble.appyx.core.lifecycle.subscribe import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.plugin.Plugin @@ -30,18 +29,16 @@ import io.element.android.features.ftue.impl.notifications.NotificationsOptInNod import io.element.android.features.ftue.impl.sessionverification.FtueSessionVerificationFlowNode import io.element.android.features.ftue.impl.state.DefaultFtueService import io.element.android.features.ftue.impl.state.FtueStep +import io.element.android.features.ftue.impl.state.InternalFtueState import io.element.android.features.lockscreen.api.LockScreenEntryPoint import io.element.android.libraries.architecture.BackstackView import io.element.android.libraries.architecture.BaseFlowNode import io.element.android.libraries.architecture.createNode import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator import io.element.android.libraries.di.SessionScope -import io.element.android.services.analytics.api.AnalyticsService -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize @ContributesNode(SessionScope::class) @@ -51,7 +48,6 @@ class FtueFlowNode( @Assisted plugins: List, private val defaultFtueService: DefaultFtueService, private val analyticsEntryPoint: AnalyticsEntryPoint, - private val analyticsService: AnalyticsService, private val lockScreenEntryPoint: LockScreenEntryPoint, ) : BaseFlowNode( backstack = BackStack( @@ -80,19 +76,11 @@ class FtueFlowNode( override fun onBuilt() { super.onBuilt() - - lifecycle.subscribe(onCreate = { - moveToNextStepIfNeeded() - }) - - analyticsService.didAskUserConsentFlow - .distinctUntilChanged() - .onEach { moveToNextStepIfNeeded() } - .launchIn(lifecycleScope) - - defaultFtueService.isVerificationStatusKnown - .filter { it } - .onEach { moveToNextStepIfNeeded() } + defaultFtueService.ftueStepStateFlow + .filterIsInstance(InternalFtueState.Incomplete::class) + .onEach { + showStep(it.nextStep) + } .launchIn(lifecycleScope) } @@ -104,7 +92,7 @@ class FtueFlowNode( is NavTarget.SessionVerification -> { val callback = object : FtueSessionVerificationFlowNode.Callback { override fun onDone() { - moveToNextStepIfNeeded() + defaultFtueService.onUserCompletedSessionVerification() } } createNode(buildContext, listOf(callback)) @@ -112,7 +100,7 @@ class FtueFlowNode( NavTarget.NotificationsOptIn -> { val callback = object : NotificationsOptInNode.Callback { override fun onNotificationsOptInFinished() { - moveToNextStepIfNeeded() + defaultFtueService.updateFtueStep() } } createNode(buildContext, listOf(callback)) @@ -123,7 +111,7 @@ class FtueFlowNode( NavTarget.LockScreenSetup -> { val callback = object : LockScreenEntryPoint.Callback { override fun onSetupDone() { - moveToNextStepIfNeeded() + defaultFtueService.updateFtueStep() } } lockScreenEntryPoint.nodeBuilder(this, buildContext, LockScreenEntryPoint.Target.Setup) @@ -133,8 +121,8 @@ class FtueFlowNode( } } - private fun moveToNextStepIfNeeded() = lifecycleScope.launch { - when (defaultFtueService.getNextStep()) { + private fun showStep(ftueStep: FtueStep) { + when (ftueStep) { FtueStep.WaitingForInitialState -> { backstack.newRoot(NavTarget.Placeholder) } @@ -150,7 +138,6 @@ class FtueFlowNode( FtueStep.LockscreenSetup -> { backstack.newRoot(NavTarget.LockScreenSetup) } - null -> Unit } } diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt index 43e85aa708..28a1ab25d9 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt @@ -9,13 +9,13 @@ package io.element.android.features.ftue.impl.state import android.Manifest import android.os.Build -import androidx.annotation.VisibleForTesting import dev.zacsweers.metro.ContributesBinding import dev.zacsweers.metro.Inject import dev.zacsweers.metro.SingleIn import io.element.android.features.ftue.api.state.FtueService import io.element.android.features.ftue.api.state.FtueState import io.element.android.features.lockscreen.api.LockScreenService +import io.element.android.libraries.core.coroutine.mapState import io.element.android.libraries.di.SessionScope import io.element.android.libraries.di.annotations.SessionCoroutineScope import io.element.android.libraries.matrix.api.verification.SessionVerificationService @@ -26,26 +26,39 @@ import io.element.android.services.analytics.api.AnalyticsService import io.element.android.services.toolbox.api.sdk.BuildVersionSdkIntProvider import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch @ContributesBinding(SessionScope::class) @SingleIn(SessionScope::class) @Inject class DefaultFtueService( private val sdkVersionProvider: BuildVersionSdkIntProvider, - @SessionCoroutineScope sessionCoroutineScope: CoroutineScope, + @SessionCoroutineScope private val sessionCoroutineScope: CoroutineScope, private val analyticsService: AnalyticsService, private val permissionStateProvider: PermissionStateProvider, private val lockScreenService: LockScreenService, private val sessionVerificationService: SessionVerificationService, private val sessionPreferencesStore: SessionPreferencesStore, ) : FtueService { - override val state = MutableStateFlow(FtueState.Unknown) + private val userNeedsToConfirmSessionVerificationSuccess = MutableStateFlow(false) + + val ftueStepStateFlow = MutableStateFlow(InternalFtueState.Unknown) + + override val state = ftueStepStateFlow + .mapState { + when (it) { + is InternalFtueState.Unknown -> FtueState.Unknown + is InternalFtueState.Incomplete -> FtueState.Incomplete + is InternalFtueState.Complete -> FtueState.Complete + } + } /** * This flow emits true when the FTUE flow is ready to be displayed. @@ -63,24 +76,37 @@ class DefaultFtueService( } init { - sessionVerificationService.sessionVerifiedStatus - .onEach { updateState() } - .launchIn(sessionCoroutineScope) - - analyticsService.didAskUserConsentFlow - .distinctUntilChanged() - .onEach { updateState() } + combine( + sessionVerificationService.sessionVerifiedStatus.onEach { sessionVerifiedStatus -> + if (sessionVerifiedStatus == SessionVerifiedStatus.NotVerified) { + // Ensure we wait for the user to confirm the session verified screen before going further + userNeedsToConfirmSessionVerificationSuccess.value = true + } + }, + userNeedsToConfirmSessionVerificationSuccess, + analyticsService.didAskUserConsentFlow.distinctUntilChanged(), + ) { + updateFtueStep() + } .launchIn(sessionCoroutineScope) } - suspend fun getNextStep(currentStep: FtueStep? = null): FtueStep? = - when (currentStep) { + fun updateFtueStep() = sessionCoroutineScope.launch { + val step = getNextStep(null) + ftueStepStateFlow.value = when (step) { + null -> InternalFtueState.Complete + else -> InternalFtueState.Incomplete(step) + } + } + + private suspend fun getNextStep(completedStep: FtueStep? = null): FtueStep? = + when (completedStep) { null -> if (!isSessionVerificationStateReady()) { FtueStep.WaitingForInitialState } else { getNextStep(FtueStep.WaitingForInitialState) } - FtueStep.WaitingForInitialState -> if (isSessionNotVerified()) { + FtueStep.WaitingForInitialState -> if (isSessionNotVerified() || userNeedsToConfirmSessionVerificationSuccess.value) { FtueStep.SessionVerification } else { getNextStep(FtueStep.SessionVerification) @@ -137,14 +163,8 @@ class DefaultFtueService( return lockScreenService.isSetupRequired().first() } - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal suspend fun updateState() { - val nextStep = getNextStep() - state.value = when { - // Final state, there aren't any more next steps - nextStep == null -> FtueState.Complete - else -> FtueState.Incomplete - } + fun onUserCompletedSessionVerification() { + userNeedsToConfirmSessionVerificationSuccess.value = false } } diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/InternalFtueState.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/InternalFtueState.kt new file mode 100644 index 0000000000..c620a2ca5b --- /dev/null +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/InternalFtueState.kt @@ -0,0 +1,18 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.ftue.impl.state + +sealed interface InternalFtueState { + data object Unknown : InternalFtueState + + data class Incomplete( + val nextStep: FtueStep, + ) : InternalFtueState + + data object Complete : InternalFtueState +} diff --git a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt index 2f832e3925..3a8ed11ea2 100644 --- a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt +++ b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPointTest.kt @@ -14,7 +14,6 @@ import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.testing.junit4.util.MainDispatcherRule import com.google.common.truth.Truth.assertThat import io.element.android.features.lockscreen.api.LockScreenEntryPoint -import io.element.android.services.analytics.test.FakeAnalyticsService import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.node.TestParentNode import kotlinx.coroutines.test.runTest @@ -37,7 +36,6 @@ class DefaultFtueEntryPointTest { plugins = plugins, analyticsEntryPoint = { _, _ -> lambdaError() }, defaultFtueService = createDefaultFtueService(), - analyticsService = FakeAnalyticsService(), lockScreenEntryPoint = object : LockScreenEntryPoint { override fun nodeBuilder( parentNode: com.bumble.appyx.core.node.Node, diff --git a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt index 1a4a8738dc..4ec2558c74 100644 --- a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt +++ b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt @@ -13,6 +13,7 @@ import com.google.common.truth.Truth.assertThat import io.element.android.features.ftue.api.state.FtueState import io.element.android.features.ftue.impl.state.DefaultFtueService import io.element.android.features.ftue.impl.state.FtueStep +import io.element.android.features.ftue.impl.state.InternalFtueState import io.element.android.features.lockscreen.api.LockScreenService import io.element.android.features.lockscreen.test.FakeLockScreenService import io.element.android.libraries.matrix.api.verification.SessionVerificationService @@ -69,9 +70,11 @@ class DefaultFtueServiceTest { analyticsService.setDidAskUserConsent() permissionStateProvider.setPermissionGranted() lockScreenService.setIsPinSetup(true) - service.updateState() - - assertThat(service.state.value).isEqualTo(FtueState.Complete) + service.updateFtueStep() + service.state.test { + assertThat(awaitItem()).isEqualTo(FtueState.Unknown) + assertThat(awaitItem()).isEqualTo(FtueState.Complete) + } } @Test @@ -90,9 +93,11 @@ class DefaultFtueServiceTest { sessionVerificationService.emitVerifiedStatus(SessionVerifiedStatus.Verified) permissionStateProvider.setPermissionGranted() lockScreenService.setIsPinSetup(true) - service.updateState() - - assertThat(service.state.value).isEqualTo(FtueState.Complete) + service.updateFtueStep() + service.state.test { + assertThat(awaitItem()).isEqualTo(FtueState.Unknown) + assertThat(awaitItem()).isEqualTo(FtueState.Complete) + } } @Test @@ -109,35 +114,30 @@ class DefaultFtueServiceTest { permissionStateProvider = permissionStateProvider, lockScreenService = lockScreenService, ) - val steps = mutableListOf() - // Session verification - steps.add(service.getNextStep(steps.lastOrNull())) - sessionVerificationService.emitVerifiedStatus(SessionVerifiedStatus.NotVerified) - - // Notifications opt in - steps.add(service.getNextStep(steps.lastOrNull())) - permissionStateProvider.setPermissionGranted() - - // Entering PIN code - steps.add(service.getNextStep(steps.lastOrNull())) - lockScreenService.setIsPinSetup(true) - - // Analytics opt in - steps.add(service.getNextStep(steps.lastOrNull())) - analyticsService.setDidAskUserConsent() - - // Final step (null) - steps.add(service.getNextStep(steps.lastOrNull())) - - assertThat(steps).containsExactly( - FtueStep.SessionVerification, - FtueStep.NotificationsOptIn, - FtueStep.LockscreenSetup, - FtueStep.AnalyticsOptIn, - // Final state - null, - ) + service.ftueStepStateFlow.test { + assertThat(awaitItem()).isEqualTo(InternalFtueState.Unknown) + // Session verification + assertThat(awaitItem()).isEqualTo(InternalFtueState.Incomplete(FtueStep.SessionVerification)) + sessionVerificationService.emitVerifiedStatus(SessionVerifiedStatus.Verified) + // User completes verification + service.onUserCompletedSessionVerification() + // Notifications opt in + assertThat(awaitItem()).isEqualTo(InternalFtueState.Incomplete(FtueStep.NotificationsOptIn)) + permissionStateProvider.setPermissionGranted() + // Simulate event from NotificationsOptInNode.Callback.onNotificationsOptInFinished + service.updateFtueStep() + // Entering PIN code + assertThat(awaitItem()).isEqualTo(InternalFtueState.Incomplete(FtueStep.LockscreenSetup)) + lockScreenService.setIsPinSetup(true) + // Simulate event from LockScreenEntryPoint.Callback.onSetupDone() + service.updateFtueStep() + // Analytics opt in + assertThat(awaitItem()).isEqualTo(InternalFtueState.Incomplete(FtueStep.AnalyticsOptIn)) + analyticsService.setDidAskUserConsent() + // Final step + assertThat(awaitItem()).isEqualTo(InternalFtueState.Complete) + } } @Test @@ -158,10 +158,13 @@ class DefaultFtueServiceTest { permissionStateProvider.setPermissionGranted() lockScreenService.setIsPinSetup(true) - assertThat(service.getNextStep()).isEqualTo(FtueStep.AnalyticsOptIn) - - analyticsService.setDidAskUserConsent() - assertThat(service.getNextStep(null)).isNull() + service.ftueStepStateFlow.test { + assertThat(awaitItem()).isEqualTo(InternalFtueState.Unknown) + // Analytics opt in + assertThat(awaitItem()).isEqualTo(InternalFtueState.Incomplete(FtueStep.AnalyticsOptIn)) + analyticsService.setDidAskUserConsent() + assertThat(awaitItem()).isEqualTo(InternalFtueState.Complete) + } } @Test @@ -180,10 +183,13 @@ class DefaultFtueServiceTest { sessionVerificationService.emitVerifiedStatus(SessionVerifiedStatus.Verified) lockScreenService.setIsPinSetup(true) - assertThat(service.getNextStep()).isEqualTo(FtueStep.AnalyticsOptIn) - - analyticsService.setDidAskUserConsent() - assertThat(service.getNextStep(null)).isNull() + service.ftueStepStateFlow.test { + assertThat(awaitItem()).isEqualTo(InternalFtueState.Unknown) + // Analytics opt in + assertThat(awaitItem()).isEqualTo(InternalFtueState.Incomplete(FtueStep.AnalyticsOptIn)) + analyticsService.setDidAskUserConsent() + assertThat(awaitItem()).isEqualTo(InternalFtueState.Complete) + } } @Test From dcec9581cde42c1a50c97125b40cab1341e5d30a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 22 Sep 2025 09:34:32 +0200 Subject: [PATCH 3/3] Simplify the code again. We do not need `isVerificationStatusKnown`. If `sessionVerificationService.sessionVerifiedStatus` is `Unknown`, `isSessionVerificationStateReady()` will return true and `isSessionNotVerified()` will not be called, since the `ftueState` will be `FtueStep.WaitingForInitialState`. Note that TU is still OK with this change. --- .../features/ftue/impl/state/DefaultFtueService.kt | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt index 28a1ab25d9..ae26ee28bc 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt @@ -28,10 +28,8 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch @@ -60,14 +58,6 @@ class DefaultFtueService( } } - /** - * This flow emits true when the FTUE flow is ready to be displayed. - * In this case, the FTUE flow is ready when the session verification status is known. - */ - val isVerificationStatusKnown = sessionVerificationService.sessionVerifiedStatus - .map { it != SessionVerifiedStatus.Unknown } - .distinctUntilChanged() - override suspend fun reset() { analyticsService.reset() if (sdkVersionProvider.isAtLeast(Build.VERSION_CODES.TIRAMISU)) { @@ -134,9 +124,6 @@ class DefaultFtueService( } private suspend fun isSessionNotVerified(): Boolean { - // Wait until the session verification status is known - isVerificationStatusKnown.filter { it }.first() - return sessionVerificationService.sessionVerifiedStatus.value == SessionVerifiedStatus.NotVerified && !canSkipVerification() }