Properly skip the FTUE verification screen if verification is not needed.

2024-09-04 16:28:35 +02:00
parent 4fbf98700a
commit ec4f2dcfbf
9 changed files with 139 additions and 52 deletions
--- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/sessionverification/FtueSessionVerificationFlowNode.kt
+++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/sessionverification/FtueSessionVerificationFlowNode.kt
@@ -46,7 +46,7 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
    private val secureBackupEntryPoint: SecureBackupEntryPoint,
 ) : BaseFlowNode<FtueSessionVerificationFlowNode.NavTarget>(
    backstack = BackStack(
-        initialElement = NavTarget.Root,
+        initialElement = NavTarget.Root(showDeviceVerifiedScreen = false),
        savedStateMap = buildContext.savedStateMap,
    ),
    buildContext = buildContext,
@@ -54,7 +54,7 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
 ) {
    sealed interface NavTarget : Parcelable {
        @Parcelize
-        data object Root : NavTarget
+        data class Root(val showDeviceVerifiedScreen: Boolean) : NavTarget

        @Parcelize
        data object EnterRecoveryKey : NavTarget
@@ -71,7 +71,7 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
        override fun onDone() {
            lifecycleScope.launch {
                // Move to the completed state view in the verification flow
-                backstack.newRoot(NavTarget.Root)
+                backstack.newRoot(NavTarget.Root(showDeviceVerifiedScreen = true))
            }
        }
    }
@@ -80,6 +80,7 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
        return when (navTarget) {
            is NavTarget.Root -> {
                verifySessionEntryPoint.nodeBuilder(this, buildContext)
+                    .params(VerifySessionEntryPoint.Params(navTarget.showDeviceVerifiedScreen))
                    .callback(object : VerifySessionEntryPoint.Callback {
                        override fun onEnterRecoveryKey() {
                            backstack.push(NavTarget.EnterRecoveryKey)
--- a/features/verifysession/api/src/main/kotlin/io/element/android/features/verifysession/api/VerifySessionEntryPoint.kt
+++ b/features/verifysession/api/src/main/kotlin/io/element/android/features/verifysession/api/VerifySessionEntryPoint.kt
@@ -20,12 +20,16 @@ import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.plugin.Plugin
 import io.element.android.libraries.architecture.FeatureEntryPoint
+import io.element.android.libraries.architecture.NodeInputs

 interface VerifySessionEntryPoint : FeatureEntryPoint {
+    data class Params(val showDeviceVerifiedScreen: Boolean) : NodeInputs
+
    fun nodeBuilder(parentNode: Node, buildContext: BuildContext): NodeBuilder

    interface NodeBuilder {
        fun callback(callback: Callback): NodeBuilder
+        fun params(params: Params): NodeBuilder
        fun build(): Node
    }

--- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/DefaultVerifySessionEntryPoint.kt
+++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/DefaultVerifySessionEntryPoint.kt
@@ -36,6 +36,11 @@ class DefaultVerifySessionEntryPoint @Inject constructor() : VerifySessionEntryP
                return this
            }

+            override fun params(params: VerifySessionEntryPoint.Params): VerifySessionEntryPoint.NodeBuilder {
+                plugins += params
+                return this
+            }
+
            override fun build(): Node {
                return parentNode.createNode<VerifySelfSessionNode>(buildContext, plugins)
            }
--- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionNode.kt
+++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionNode.kt
@@ -30,16 +30,21 @@ import io.element.android.anvilannotations.ContributesNode
 import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.logout.api.util.onSuccessLogout
 import io.element.android.features.verifysession.api.VerifySessionEntryPoint
+import io.element.android.libraries.architecture.inputs
 import io.element.android.libraries.di.SessionScope

@ContributesNode(SessionScope::class)
 class VerifySelfSessionNode @AssistedInject constructor(
    @Assisted buildContext: BuildContext,
    @Assisted plugins: List<Plugin>,
-    private val presenter: VerifySelfSessionPresenter,
+    presenterFactory: VerifySelfSessionPresenter.Factory,
 ) : Node(buildContext, plugins = plugins) {
    private val callback = plugins<VerifySessionEntryPoint.Callback>().first()

+    private val presenter = presenterFactory.create(
+        showDeviceVerifiedScreen = inputs<VerifySessionEntryPoint.Params>().showDeviceVerifiedScreen,
+    )
+
    @Composable
    override fun View(modifier: Modifier) {
        val state = presenter.present()
--- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt
+++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenter.kt
@@ -28,6 +28,9 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
 import com.freeletics.flowredux.compose.rememberStateAndDispatch
+import dagger.assisted.Assisted
+import dagger.assisted.AssistedFactory
+import dagger.assisted.AssistedInject
 import io.element.android.features.logout.api.LogoutUseCase
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
@@ -37,6 +40,7 @@ import io.element.android.libraries.core.meta.BuildMeta
 import io.element.android.libraries.matrix.api.encryption.EncryptionService
 import io.element.android.libraries.matrix.api.encryption.RecoveryState
 import io.element.android.libraries.matrix.api.verification.SessionVerificationService
+import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
 import io.element.android.libraries.matrix.api.verification.VerificationFlowState
 import io.element.android.libraries.preferences.api.store.SessionPreferencesStore
 import kotlinx.coroutines.CoroutineScope
@@ -44,11 +48,11 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
-import javax.inject.Inject
 import io.element.android.features.verifysession.impl.VerifySelfSessionStateMachine.Event as StateMachineEvent
 import io.element.android.features.verifysession.impl.VerifySelfSessionStateMachine.State as StateMachineState

-class VerifySelfSessionPresenter @Inject constructor(
+class VerifySelfSessionPresenter @AssistedInject constructor(
+    @Assisted private val showDeviceVerifiedScreen: Boolean,
    private val sessionVerificationService: SessionVerificationService,
    private val encryptionService: EncryptionService,
    private val stateMachine: VerifySelfSessionStateMachine,
@@ -56,6 +60,11 @@ class VerifySelfSessionPresenter @Inject constructor(
    private val sessionPreferencesStore: SessionPreferencesStore,
    private val logoutUseCase: LogoutUseCase,
 ) : Presenter<VerifySelfSessionState> {
+    @AssistedFactory
+    interface Factory {
+        fun create(showDeviceVerifiedScreen: Boolean): VerifySelfSessionPresenter
+    }
+
    @Composable
    override fun present(): VerifySelfSessionState {
        val coroutineScope = rememberCoroutineScope()
@@ -66,18 +75,32 @@ class VerifySelfSessionPresenter @Inject constructor(
        val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState()
        val stateAndDispatch = stateMachine.rememberStateAndDispatch()
        val skipVerification by sessionPreferencesStore.isSessionVerificationSkipped().collectAsState(initial = false)
-        val needsVerification by sessionVerificationService.needsSessionVerification.collectAsState(initial = true)
+        val sessionVerifiedStatus by sessionVerificationService.sessionVerifiedStatus.collectAsState()
        val signOutAction = remember {
            mutableStateOf<AsyncAction<String?>>(AsyncAction.Uninitialized)
        }
        val verificationFlowStep by remember {
            derivedStateOf {
-                when {
-                    skipVerification -> VerifySelfSessionState.VerificationStep.Skipped
-                    needsVerification -> stateAndDispatch.state.value.toVerificationStep(
-                        canEnterRecoveryKey = recoveryState == RecoveryState.INCOMPLETE
-                    )
-                    else -> VerifySelfSessionState.VerificationStep.Completed
+                if (skipVerification) {
+                    VerifySelfSessionState.VerificationStep.Skipped
+                } else {
+                    when (sessionVerifiedStatus) {
+                        SessionVerifiedStatus.Unknown -> VerifySelfSessionState.VerificationStep.Loading
+                        SessionVerifiedStatus.NotVerified -> {
+                            stateAndDispatch.state.value.toVerificationStep(
+                                canEnterRecoveryKey = recoveryState == RecoveryState.INCOMPLETE
+                            )
+                        }
+                        SessionVerifiedStatus.Verified -> {
+                            if (stateAndDispatch.state.value != StateMachineState.Initial || showDeviceVerifiedScreen) {
+                                // The user has verified the session, we need to show the success screen
+                                VerifySelfSessionState.VerificationStep.Completed
+                            } else {
+                                // Automatic verification, which can happen on freshly created account, in this case, skip the screen
+                                VerifySelfSessionState.VerificationStep.Skipped
+                            }
+                        }
+                    }
                }
            }
        }
--- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionState.kt
+++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionState.kt
@@ -31,6 +31,7 @@ data class VerifySelfSessionState(
 ) {
    @Stable
    sealed interface VerificationStep {
+        data object Loading : VerificationStep
        data class Initial(val canEnterRecoveryKey: Boolean, val isLastDevice: Boolean = false) : VerificationStep
        data object Canceled : VerificationStep
        data object AwaitingOtherDeviceResponse : VerificationStep
--- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionStateProvider.kt
+++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionStateProvider.kt
@@ -59,6 +59,12 @@ open class VerifySelfSessionStateProvider : PreviewParameterProvider<VerifySelfS
                signOutAction = AsyncAction.Loading,
                displaySkipButton = true,
            ),
+            aVerifySelfSessionState(
+                verificationFlowStep = VerificationStep.Loading
+            ),
+            aVerifySelfSessionState(
+                verificationFlowStep = VerificationStep.Skipped
+            ),
            // Add other state here
        )
 }
--- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionView.kt
+++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionView.kt
@@ -19,11 +19,13 @@ package io.element.android.features.verifysession.impl
 import androidx.activity.compose.BackHandler
 import androidx.compose.foundation.Image
 import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.ColumnScope
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxHeight
+import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
@@ -57,6 +59,7 @@ import io.element.android.libraries.designsystem.components.ProgressDialog
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.designsystem.theme.components.Button
+import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
 import io.element.android.libraries.designsystem.theme.components.OutlinedButton
 import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.designsystem.theme.components.TextButton
@@ -99,43 +102,55 @@ fun VerifySelfSessionView(
        }
    }
    val verificationFlowStep = state.verificationFlowStep
-    HeaderFooterPage(
-        modifier = modifier,
-        topBar = {
-            TopAppBar(
-                title = {},
-                actions = {
-                    if (state.verificationFlowStep != FlowStep.Completed &&
-                        state.displaySkipButton &&
-                        LocalInspectionMode.current.not()) {
-                        TextButton(
-                            text = stringResource(CommonStrings.action_skip),
-                            onClick = { state.eventSink(VerifySelfSessionViewEvents.SkipVerification) }
-                        )
-                    }
-                    if (state.verificationFlowStep is FlowStep.Initial) {
-                        TextButton(
-                            text = stringResource(CommonStrings.action_signout),
-                            onClick = { state.eventSink(VerifySelfSessionViewEvents.SignOut) }
-                        )
-                    }
-                }
-            )
-        },
-        header = {
-            HeaderContent(verificationFlowStep = verificationFlowStep)
-        },
-        footer = {
-            BottomMenu(
-                screenState = state,
-                goBack = ::resetFlow,
-                onEnterRecoveryKey = onEnterRecoveryKey,
-                onFinish = onFinish,
-                onResetKey = onResetKey,
-            )
+
+    if (state.verificationFlowStep is FlowStep.Loading ||
+        state.verificationFlowStep is FlowStep.Skipped) {
+        // Just display a loader in this case, to avoid UI glitch.
+        Box(
+            modifier = Modifier.fillMaxSize(),
+            contentAlignment = Alignment.Center,
+        ) {
+            CircularProgressIndicator()
+        }
+    } else {
+        HeaderFooterPage(
+            modifier = modifier,
+            topBar = {
+                TopAppBar(
+                    title = {},
+                    actions = {
+                        if (state.verificationFlowStep !is FlowStep.Completed &&
+                            state.displaySkipButton &&
+                            LocalInspectionMode.current.not()) {
+                            TextButton(
+                                text = stringResource(CommonStrings.action_skip),
+                                onClick = { state.eventSink(VerifySelfSessionViewEvents.SkipVerification) }
+                            )
+                        }
+                        if (state.verificationFlowStep is FlowStep.Initial) {
+                            TextButton(
+                                text = stringResource(CommonStrings.action_signout),
+                                onClick = { state.eventSink(VerifySelfSessionViewEvents.SignOut) }
+                            )
+                        }
+                    }
+                )
+            },
+            header = {
+                HeaderContent(verificationFlowStep = verificationFlowStep)
+            },
+            footer = {
+                BottomMenu(
+                    screenState = state,
+                    goBack = ::resetFlow,
+                    onEnterRecoveryKey = onEnterRecoveryKey,
+                    onFinish = onFinish,
+                    onResetKey = onResetKey,
+                )
+            }
+        ) {
+            Content(flowState = verificationFlowStep)
        }
-    ) {
-        Content(flowState = verificationFlowStep)
    }

    when (state.signOutAction) {
@@ -157,6 +172,7 @@ fun VerifySelfSessionView(
@Composable
 private fun HeaderContent(verificationFlowStep: FlowStep) {
    val iconStyle = when (verificationFlowStep) {
+        VerifySelfSessionState.VerificationStep.Loading -> error("Should not happen")
        is FlowStep.Initial, FlowStep.AwaitingOtherDeviceResponse -> BigIcon.Style.Default(CompoundIcons.LockSolid())
        FlowStep.Canceled -> BigIcon.Style.AlertSolid
        FlowStep.Ready, is FlowStep.Verifying -> BigIcon.Style.Default(CompoundIcons.Reaction())
@@ -164,6 +180,7 @@ private fun HeaderContent(verificationFlowStep: FlowStep) {
        is FlowStep.Skipped -> return
    }
    val titleTextId = when (verificationFlowStep) {
+        VerifySelfSessionState.VerificationStep.Loading -> error("Should not happen")
        is FlowStep.Initial, FlowStep.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_title
        FlowStep.Canceled -> CommonStrings.common_verification_cancelled
        FlowStep.Ready -> R.string.screen_session_verification_compare_emojis_title
@@ -175,6 +192,7 @@ private fun HeaderContent(verificationFlowStep: FlowStep) {
        is FlowStep.Skipped -> return
    }
    val subtitleTextId = when (verificationFlowStep) {
+        VerifySelfSessionState.VerificationStep.Loading -> error("Should not happen")
        is FlowStep.Initial, FlowStep.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_subtitle
        FlowStep.Canceled -> R.string.screen_session_verification_cancelled_subtitle
        FlowStep.Ready -> R.string.screen_session_verification_ready_subtitle
@@ -268,6 +286,7 @@ private fun BottomMenu(
    val isVerifying = (verificationViewState as? FlowStep.Verifying)?.state is AsyncData.Loading<Unit>

    when (verificationViewState) {
+        VerifySelfSessionState.VerificationStep.Loading -> error("Should not happen")
        is FlowStep.Initial -> {
            BottomMenu {
                if (verificationViewState.isLastDevice) {
--- a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTest.kt
+++ b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionPresenterTest.kt
@@ -298,18 +298,39 @@ class VerifySelfSessionPresenterTest {
    }

    @Test
-    fun `present - When verification is not needed, the flow is completed`() = runTest {
+    fun `present - When verification is done using recovery key, the flow is completed`() = runTest {
        val service = FakeSessionVerificationService().apply {
            givenNeedsSessionVerification(false)
            givenVerifiedStatus(SessionVerifiedStatus.Verified)
            givenVerificationFlowState(VerificationFlowState.Finished)
        }
-        val presenter = createVerifySelfSessionPresenter(service)
+        val presenter = createVerifySelfSessionPresenter(
+            service = service,
+            showDeviceVerifiedScreen = true,
+        )
+        moleculeFlow(RecompositionMode.Immediate) {
+            presenter.present()
+        }.test {
+            assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Completed)
+        }
+    }
+
+    @Test
+    fun `present - When verification is not needed, the flow is skipped`() = runTest {
+        val service = FakeSessionVerificationService().apply {
+            givenNeedsSessionVerification(false)
+            givenVerifiedStatus(SessionVerifiedStatus.Verified)
+            givenVerificationFlowState(VerificationFlowState.Finished)
+        }
+        val presenter = createVerifySelfSessionPresenter(
+            service = service,
+            showDeviceVerifiedScreen = false,
+        )
        moleculeFlow(RecompositionMode.Immediate) {
            presenter.present()
        }.test {
            skipItems(1)
-            assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Completed)
+            assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Skipped)
        }
    }

@@ -374,8 +395,10 @@ class VerifySelfSessionPresenterTest {
        buildMeta: BuildMeta = aBuildMeta(),
        sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(),
        logoutUseCase: LogoutUseCase = FakeLogoutUseCase(),
+        showDeviceVerifiedScreen: Boolean = false,
    ): VerifySelfSessionPresenter {
        return VerifySelfSessionPresenter(
+            showDeviceVerifiedScreen = showDeviceVerifiedScreen,
            sessionVerificationService = service,
            encryptionService = encryptionService,
            stateMachine = VerifySelfSessionStateMachine(service, encryptionService),