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 1f4ca9e4eea..a4aa9f18d7a 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) @@ -49,9 +46,8 @@ 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, ) : 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) - - 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(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 (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 } } 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 43e85aa7087..ae26ee28bc8 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,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.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.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() } + 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) + } - analyticsService.didAskUserConsentFlow - .distinctUntilChanged() - .onEach { updateState() } - .launchIn(sessionCoroutineScope) + fun updateFtueStep() = sessionCoroutineScope.launch { + val step = getNextStep(null) + ftueStepStateFlow.value = when (step) { + null -> InternalFtueState.Complete + else -> InternalFtueState.Incomplete(step) + } } - suspend fun getNextStep(currentStep: FtueStep? = null): FtueStep? = - when (currentStep) { + 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 } } 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 00000000000..c620a2ca5b3 --- /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 50cc2eadced..3a8ed11ea28 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 @@ -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, 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 1a4a8738dc7..4ec2558c74c 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