Merge pull request #5379 from element-hq/feature/bma/cleanupFtueCode

Cleanup ftue code and ensure verification confirmation is displayed

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)
@@ -49,9 +46,8 @@ import kotlinx.parcelize.Parcelize
 class FtueFlowNode(
     @Assisted buildContext: BuildContext,
     @Assisted plugins: List<Plugin>,
-    private val ftueState: DefaultFtueService,
+    private val defaultFtueService: DefaultFtueService,
     private val analyticsEntryPoint: AnalyticsEntryPoint,
-    private val analyticsService: AnalyticsService,
     private val lockScreenEntryPoint: LockScreenEntryPoint,
 ) : BaseFlowNode<FtueFlowNode.NavTarget>(
     backstack = BackStack(
@@ -80,19 +76,11 @@ class FtueFlowNode(
 
     override fun onBuilt() {
         super.onBuilt()
-
-        lifecycle.subscribe(onCreate = {
-            moveToNextStepIfNeeded()
-        })
-
-        analyticsService.didAskUserConsentFlow
-            .distinctUntilChanged()
-            .onEach { moveToNextStepIfNeeded() }
-            .launchIn(lifecycleScope)
-
-        ftueState.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<FtueSessionVerificationFlowNode>(buildContext, listOf(callback))
@@ -112,7 +100,7 @@ class FtueFlowNode(
             NavTarget.NotificationsOptIn -> {
                 val callback = object : NotificationsOptInNode.Callback {
                     override fun onNotificationsOptInFinished() {
-                        moveToNextStepIfNeeded()
+                        defaultFtueService.updateFtueStep()
                     }
                 }
                 createNode<NotificationsOptInNode>(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 (ftueState.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
         }
     }
 

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,34 +26,37 @@ 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>(FtueState.Unknown)
+    private val userNeedsToConfirmSessionVerificationSuccess = MutableStateFlow(false)
 
-    /**
-     * 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()
+    val ftueStepStateFlow = MutableStateFlow<InternalFtueState>(InternalFtueState.Unknown)
+
+    override val state = ftueStepStateFlow
+        .mapState {
+            when (it) {
+                is InternalFtueState.Unknown -> FtueState.Unknown
+                is InternalFtueState.Incomplete -> FtueState.Incomplete
+                is InternalFtueState.Complete -> FtueState.Complete
+            }
+        }
 
     override suspend fun reset() {
         analyticsService.reset()
@@ -63,24 +66,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)
@@ -108,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()
     }
 
@@ -137,14 +150,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
     }
 }
 

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
+}

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
@@ -36,8 +35,7 @@ class DefaultFtueEntryPointTest {
                 buildContext = buildContext,
                 plugins = plugins,
                 analyticsEntryPoint = { _, _ -> lambdaError() },
-                ftueState = createDefaultFtueService(),
-                analyticsService = FakeAnalyticsService(),
+                defaultFtueService = createDefaultFtueService(),
                 lockScreenEntryPoint = object : LockScreenEntryPoint {
                     override fun nodeBuilder(
                         parentNode: com.bumble.appyx.core.node.Node,

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<FtueStep?>()
 
-        // 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