From b085c1b3d4560ac53bd779658d757b23e223362d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Nov 2024 12:16:05 +0100 Subject: [PATCH 1/8] Session verification: add new screen to get ready on the other session. --- .../outgoing/VerifySelfSessionPresenter.kt | 4 ++++ .../impl/outgoing/VerifySelfSessionState.kt | 1 + .../outgoing/VerifySelfSessionStateMachine.kt | 15 +++++++++++--- .../VerifySelfSessionStateProvider.kt | 3 +++ .../impl/outgoing/VerifySelfSessionView.kt | 20 +++++++++++++++++-- .../outgoing/VerifySelfSessionViewEvents.kt | 1 + .../impl/src/main/res/values/localazy.xml | 2 ++ 7 files changed, 41 insertions(+), 5 deletions(-) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenter.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenter.kt index 360bf64875..97778b322b 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenter.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenter.kt @@ -104,6 +104,7 @@ class VerifySelfSessionPresenter @AssistedInject constructor( fun handleEvents(event: VerifySelfSessionViewEvents) { Timber.d("Verification user action: ${event::class.simpleName}") when (event) { + VerifySelfSessionViewEvents.UseAnotherDevice -> stateAndDispatch.dispatchAction(StateMachineEvent.UseAnotherDevice) VerifySelfSessionViewEvents.RequestVerification -> stateAndDispatch.dispatchAction(StateMachineEvent.RequestVerification) VerifySelfSessionViewEvents.StartSasVerification -> stateAndDispatch.dispatchAction(StateMachineEvent.StartSasVerification) VerifySelfSessionViewEvents.ConfirmVerification -> stateAndDispatch.dispatchAction(StateMachineEvent.AcceptChallenge) @@ -134,6 +135,9 @@ class VerifySelfSessionPresenter @AssistedInject constructor( isLastDevice = encryptionService.isLastDevice.value ) } + VerifySelfSessionStateMachine.State.UseAnotherDevice -> { + VerifySelfSessionState.Step.UseAnotherDevice + } StateMachineState.RequestingVerification, StateMachineState.StartingSasVerification, StateMachineState.SasVerificationStarted, diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionState.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionState.kt index e763305caa..32664b347f 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionState.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionState.kt @@ -26,6 +26,7 @@ data class VerifySelfSessionState( // FIXME canEnterRecoveryKey value is never read. data class Initial(val canEnterRecoveryKey: Boolean, val isLastDevice: Boolean = false) : Step + data object UseAnotherDevice : Step data object Canceled : Step data object AwaitingOtherDeviceResponse : Step data object Ready : Step diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt index f423b14aae..09f18538d0 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt @@ -38,12 +38,14 @@ class VerifySelfSessionStateMachine @Inject constructor( init { spec { inState { + on { _: Event.UseAnotherDevice, state -> + state.override { State.UseAnotherDevice.andLogStateChange() } + } + } + inState { on { _: Event.RequestVerification, state -> state.override { State.RequestingVerification.andLogStateChange() } } - on { _: Event.StartSasVerification, state -> - state.override { State.StartingSasVerification.andLogStateChange() } - } } inState { onEnterEffect { @@ -119,6 +121,7 @@ class VerifySelfSessionStateMachine @Inject constructor( on { _: Event.Cancel, state: MachineState -> when (state.snapshot) { State.Initial, State.Completed, State.Canceled -> state.noChange() + State.UseAnotherDevice -> state.override { State.Initial.andLogStateChange() } // For some reason `cancelVerification` is not calling its delegate `didCancel` method so we don't pass from // `Canceling` state to `Canceled` automatically anymore else -> { @@ -144,6 +147,9 @@ class VerifySelfSessionStateMachine @Inject constructor( /** The initial state, before verification started. */ data object Initial : State + /** Let the user know that they need to get ready on their other session. */ + data object UseAnotherDevice : State + /** Waiting for verification acceptance. */ data object RequestingVerification : State @@ -175,6 +181,9 @@ class VerifySelfSessionStateMachine @Inject constructor( } sealed interface Event { + /** User wants to use another session. */ + data object UseAnotherDevice : Event + /** Request verification. */ data object RequestVerification : Event diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateProvider.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateProvider.kt index 8cb60a822f..b9de96aae4 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateProvider.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateProvider.kt @@ -56,6 +56,9 @@ open class VerifySelfSessionStateProvider : PreviewParameterProvider state.eventSink(VerifySelfSessionViewEvents.Reset) - is Step.AwaitingOtherDeviceResponse, Step.Ready -> state.eventSink(VerifySelfSessionViewEvents.Cancel) + is Step.AwaitingOtherDeviceResponse, + Step.UseAnotherDevice, + Step.Ready -> state.eventSink(VerifySelfSessionViewEvents.Cancel) is Step.Verifying -> { if (!step.state.isLoading()) { state.eventSink(VerifySelfSessionViewEvents.DeclineVerification) @@ -159,6 +161,7 @@ fun VerifySelfSessionView( private fun VerifySelfSessionHeader(step: Step) { val iconStyle = when (step) { Step.Loading -> error("Should not happen") + Step.UseAnotherDevice -> BigIcon.Style.Default(CompoundIcons.Devices()) is Step.Initial, Step.AwaitingOtherDeviceResponse -> BigIcon.Style.Default(CompoundIcons.LockSolid()) Step.Canceled -> BigIcon.Style.AlertSolid Step.Ready, is Step.Verifying -> BigIcon.Style.Default(CompoundIcons.Reaction()) @@ -167,6 +170,7 @@ private fun VerifySelfSessionHeader(step: Step) { } val titleTextId = when (step) { Step.Loading -> error("Should not happen") + Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_title is Step.Initial, Step.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_title Step.Canceled -> CommonStrings.common_verification_cancelled Step.Ready -> R.string.screen_session_verification_compare_emojis_title @@ -179,6 +183,7 @@ private fun VerifySelfSessionHeader(step: Step) { } val subtitleTextId = when (step) { Step.Loading -> error("Should not happen") + Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_subtitle is Step.Initial, Step.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_subtitle Step.Canceled -> R.string.screen_session_verification_cancelled_subtitle Step.Ready -> R.string.screen_session_verification_ready_subtitle @@ -252,7 +257,7 @@ private fun VerifySelfSessionBottomMenu( Button( modifier = Modifier.fillMaxWidth(), text = stringResource(R.string.screen_identity_use_another_device), - onClick = { eventSink(VerifySelfSessionViewEvents.RequestVerification) }, + onClick = { eventSink(VerifySelfSessionViewEvents.UseAnotherDevice) }, ) } Button( @@ -267,6 +272,17 @@ private fun VerifySelfSessionBottomMenu( ) } } + is Step.UseAnotherDevice -> { + VerificationBottomMenu { + Button( + modifier = Modifier.fillMaxWidth(), + text = stringResource(CommonStrings.action_start_verification), + onClick = { eventSink(VerifySelfSessionViewEvents.RequestVerification) }, + ) + // Placeholder so the 1st button keeps its vertical position + Spacer(modifier = Modifier.height(40.dp)) + } + } is Step.Canceled -> { VerificationBottomMenu { Button( diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionViewEvents.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionViewEvents.kt index 869bdc7051..b4af38f780 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionViewEvents.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionViewEvents.kt @@ -8,6 +8,7 @@ package io.element.android.features.verifysession.impl.outgoing sealed interface VerifySelfSessionViewEvents { + data object UseAnotherDevice : VerifySelfSessionViewEvents data object RequestVerification : VerifySelfSessionViewEvents data object StartSasVerification : VerifySelfSessionViewEvents data object ConfirmVerification : VerifySelfSessionViewEvents diff --git a/features/verifysession/impl/src/main/res/values/localazy.xml b/features/verifysession/impl/src/main/res/values/localazy.xml index f67a2024b9..85fa3b8246 100644 --- a/features/verifysession/impl/src/main/res/values/localazy.xml +++ b/features/verifysession/impl/src/main/res/values/localazy.xml @@ -35,6 +35,8 @@ "Verification requested" "They don’t match" "They match" + "Make sure you have the app open in the other device before starting verification from here." + "Open the app on another verified device" "Accept the request to start the verification process in your other session to continue." "Waiting to accept request" "Signing out…" From d0a64a16df1f11005116763f08057218445e04fc Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Nov 2024 12:21:38 +0100 Subject: [PATCH 2/8] Session verification: Iterate on the waiting for other device screen. --- .../impl/outgoing/VerifySelfSessionView.kt | 10 +++++++--- .../impl/src/main/res/values/localazy.xml | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt index cbc79a219f..3013dc7613 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt @@ -161,8 +161,9 @@ fun VerifySelfSessionView( private fun VerifySelfSessionHeader(step: Step) { val iconStyle = when (step) { Step.Loading -> error("Should not happen") + is Step.Initial -> BigIcon.Style.Default(CompoundIcons.LockSolid()) Step.UseAnotherDevice -> BigIcon.Style.Default(CompoundIcons.Devices()) - is Step.Initial, Step.AwaitingOtherDeviceResponse -> BigIcon.Style.Default(CompoundIcons.LockSolid()) + Step.AwaitingOtherDeviceResponse -> BigIcon.Style.Default(CompoundIcons.Devices()) Step.Canceled -> BigIcon.Style.AlertSolid Step.Ready, is Step.Verifying -> BigIcon.Style.Default(CompoundIcons.Reaction()) Step.Completed -> BigIcon.Style.SuccessSolid @@ -170,8 +171,9 @@ private fun VerifySelfSessionHeader(step: Step) { } val titleTextId = when (step) { Step.Loading -> error("Should not happen") + is Step.Initial -> R.string.screen_identity_confirmation_title Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_title - is Step.Initial, Step.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_title + Step.AwaitingOtherDeviceResponse -> R.string.screen_session_verification_waiting_another_device_title Step.Canceled -> CommonStrings.common_verification_cancelled Step.Ready -> R.string.screen_session_verification_compare_emojis_title Step.Completed -> R.string.screen_identity_confirmed_title @@ -183,8 +185,9 @@ private fun VerifySelfSessionHeader(step: Step) { } val subtitleTextId = when (step) { Step.Loading -> error("Should not happen") + is Step.Initial -> R.string.screen_identity_confirmation_subtitle Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_subtitle - is Step.Initial, Step.AwaitingOtherDeviceResponse -> R.string.screen_identity_confirmation_subtitle + Step.AwaitingOtherDeviceResponse -> R.string.screen_session_verification_waiting_another_device_subtitle Step.Canceled -> R.string.screen_session_verification_cancelled_subtitle Step.Ready -> R.string.screen_session_verification_ready_subtitle Step.Completed -> R.string.screen_identity_confirmed_subtitle @@ -318,6 +321,7 @@ private fun VerifySelfSessionBottomMenu( text = stringResource(R.string.screen_identity_waiting_on_other_device), onClick = {}, showProgress = true, + enabled = false, ) // Placeholder so the 1st button keeps its vertical position Spacer(modifier = Modifier.height(40.dp)) diff --git a/features/verifysession/impl/src/main/res/values/localazy.xml b/features/verifysession/impl/src/main/res/values/localazy.xml index 85fa3b8246..db18590cbe 100644 --- a/features/verifysession/impl/src/main/res/values/localazy.xml +++ b/features/verifysession/impl/src/main/res/values/localazy.xml @@ -37,6 +37,8 @@ "They match" "Make sure you have the app open in the other device before starting verification from here." "Open the app on another verified device" + "You should see a popup on the other device. Start the verification from there now." + "Start verification on the other device" "Accept the request to start the verification process in your other session to continue." "Waiting to accept request" "Signing out…" From 2b061085741a63ecc3ef2e06e3bf320b60a5ce5f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Nov 2024 12:43:02 +0100 Subject: [PATCH 3/8] Update wording for verification cancelled. --- .../verifysession/impl/outgoing/VerifySelfSessionView.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt index 3013dc7613..4351bb0bf2 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt @@ -174,7 +174,7 @@ private fun VerifySelfSessionHeader(step: Step) { is Step.Initial -> R.string.screen_identity_confirmation_title Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_title Step.AwaitingOtherDeviceResponse -> R.string.screen_session_verification_waiting_another_device_title - Step.Canceled -> CommonStrings.common_verification_cancelled + Step.Canceled -> CommonStrings.common_verification_failed Step.Ready -> R.string.screen_session_verification_compare_emojis_title Step.Completed -> R.string.screen_identity_confirmed_title is Step.Verifying -> when (step.data) { @@ -188,7 +188,7 @@ private fun VerifySelfSessionHeader(step: Step) { is Step.Initial -> R.string.screen_identity_confirmation_subtitle Step.UseAnotherDevice -> R.string.screen_session_verification_use_another_device_subtitle Step.AwaitingOtherDeviceResponse -> R.string.screen_session_verification_waiting_another_device_subtitle - Step.Canceled -> R.string.screen_session_verification_cancelled_subtitle + Step.Canceled -> R.string.screen_session_verification_failed_subtitle Step.Ready -> R.string.screen_session_verification_ready_subtitle Step.Completed -> R.string.screen_identity_confirmed_subtitle is Step.Verifying -> when (step.data) { From 683b694eec9e3c26526aaac42ec7fcc2d199a37b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Nov 2024 12:46:13 +0100 Subject: [PATCH 4/8] Incoming verification: do not distinguish UI between cancelled and failed state. --- .../verifysession/impl/incoming/IncomingVerificationView.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/incoming/IncomingVerificationView.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/incoming/IncomingVerificationView.kt index cc2c9fbd2d..e8f098f848 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/incoming/IncomingVerificationView.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/incoming/IncomingVerificationView.kt @@ -80,14 +80,14 @@ fun IncomingVerificationView( @Composable private fun IncomingVerificationHeader(step: Step) { val iconStyle = when (step) { - Step.Canceled, + Step.Canceled -> BigIcon.Style.AlertSolid is Step.Initial -> BigIcon.Style.Default(CompoundIcons.LockSolid()) is Step.Verifying -> BigIcon.Style.Default(CompoundIcons.Reaction()) Step.Completed -> BigIcon.Style.SuccessSolid Step.Failure -> BigIcon.Style.AlertSolid } val titleTextId = when (step) { - Step.Canceled -> CommonStrings.common_verification_cancelled + Step.Canceled -> R.string.screen_session_verification_request_failure_title is Step.Initial -> R.string.screen_session_verification_request_title is Step.Verifying -> when (step.data) { is SessionVerificationData.Decimals -> R.string.screen_session_verification_compare_numbers_title @@ -97,7 +97,7 @@ private fun IncomingVerificationHeader(step: Step) { Step.Failure -> R.string.screen_session_verification_request_failure_title } val subtitleTextId = when (step) { - Step.Canceled -> R.string.screen_session_verification_cancelled_subtitle + Step.Canceled -> R.string.screen_session_verification_request_failure_subtitle is Step.Initial -> R.string.screen_session_verification_request_subtitle is Step.Verifying -> when (step.data) { is SessionVerificationData.Decimals -> R.string.screen_session_verification_compare_numbers_subtitle From 153aa6eceaeb2062f490ce4ad478e89359b38104 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Nov 2024 13:34:18 +0100 Subject: [PATCH 5/8] Update test. --- .../VerifySelfSessionPresenterTest.kt | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt index bfdf27ee5d..a7c47182f8 100644 --- a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt +++ b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt @@ -121,28 +121,6 @@ class VerifySelfSessionPresenterTest { } } - @Test - fun `present - Handles startSasVerification`() = runTest { - val service = unverifiedSessionService( - startVerificationLambda = { }, - ) - val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { - val initialState = awaitItem() - assertThat(initialState.step).isEqualTo(Step.Initial(false)) - initialState.eventSink(VerifySelfSessionViewEvents.StartSasVerification) - // Await for other device response: - assertThat(awaitItem().step).isEqualTo(Step.AwaitingOtherDeviceResponse) - service.emitVerificationFlowState(VerificationFlowState.DidStartSasVerification) - // ChallengeReceived: - service.emitVerificationFlowState(VerificationFlowState.DidReceiveVerificationData(SessionVerificationData.Emojis(emptyList()))) - val verifyingState = awaitItem() - assertThat(verifyingState.step).isInstanceOf(Step.Verifying::class.java) - } - } - @Test fun `present - Cancellation on initial state does nothing`() = runTest { val presenter = createVerifySelfSessionPresenter( @@ -189,6 +167,7 @@ class VerifySelfSessionPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + awaitItem().eventSink(VerifySelfSessionViewEvents.UseAnotherDevice) awaitItem().eventSink(VerifySelfSessionViewEvents.RequestVerification) service.emitVerificationFlowState(VerificationFlowState.DidFail) assertThat(awaitItem().step).isInstanceOf(Step.AwaitingOtherDeviceResponse::class.java) @@ -414,6 +393,9 @@ class VerifySelfSessionPresenterTest { ): VerifySelfSessionState { var state = awaitItem() assertThat(state.step).isEqualTo(Step.Initial(false)) + state.eventSink(VerifySelfSessionViewEvents.UseAnotherDevice) + state = awaitItem() + assertThat(state.step).isEqualTo(Step.UseAnotherDevice) state.eventSink(VerifySelfSessionViewEvents.RequestVerification) // Await for other device response: fakeService.emitVerificationFlowState(VerificationFlowState.DidAcceptVerificationRequest) From fff7c04421d9ad2f3c350870357aecde233ac495 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Nov 2024 13:36:36 +0100 Subject: [PATCH 6/8] Use `test` extension --- .../VerifySelfSessionPresenterTest.kt | 76 +++++-------------- 1 file changed, 19 insertions(+), 57 deletions(-) diff --git a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt index a7c47182f8..806a1dbdf1 100644 --- a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt +++ b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt @@ -7,10 +7,7 @@ package io.element.android.features.verifysession.impl.outgoing -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow import app.cash.turbine.ReceiveTurbine -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.logout.api.LogoutUseCase import io.element.android.features.logout.test.FakeLogoutUseCase @@ -33,6 +30,7 @@ import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value +import io.element.android.tests.testutils.test import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Rule @@ -48,9 +46,7 @@ class VerifySelfSessionPresenterTest { val presenter = createVerifySelfSessionPresenter( service = unverifiedSessionService(), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { awaitItem().run { assertThat(step).isEqualTo(Step.Initial(false)) assertThat(displaySkipButton).isTrue() @@ -65,9 +61,7 @@ class VerifySelfSessionPresenterTest { service = unverifiedSessionService(), buildMeta = buildMeta, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { assertThat(awaitItem().displaySkipButton).isFalse() } } @@ -83,9 +77,7 @@ class VerifySelfSessionPresenterTest { emitRecoveryState(RecoveryState.INCOMPLETE) } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { assertThat(awaitItem().step).isEqualTo(Step.Initial(true)) resetLambda.assertions().isCalledOnce().with(value(true)) } @@ -100,9 +92,7 @@ class VerifySelfSessionPresenterTest { emitRecoveryState(RecoveryState.INCOMPLETE) } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { assertThat(awaitItem().step).isEqualTo(Step.Initial(canEnterRecoveryKey = true, isLastDevice = true)) } } @@ -114,9 +104,7 @@ class VerifySelfSessionPresenterTest { startVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { requestVerificationAndAwaitVerifyingState(service) } } @@ -126,9 +114,7 @@ class VerifySelfSessionPresenterTest { val presenter = createVerifySelfSessionPresenter( service = unverifiedSessionService(), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.step).isEqualTo(Step.Initial(false)) val eventSink = initialState.eventSink @@ -145,9 +131,7 @@ class VerifySelfSessionPresenterTest { approveVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val state = requestVerificationAndAwaitVerifyingState(service) state.eventSink(VerifySelfSessionViewEvents.ConfirmVerification) // Cancelling @@ -164,9 +148,7 @@ class VerifySelfSessionPresenterTest { requestVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { awaitItem().eventSink(VerifySelfSessionViewEvents.UseAnotherDevice) awaitItem().eventSink(VerifySelfSessionViewEvents.RequestVerification) service.emitVerificationFlowState(VerificationFlowState.DidFail) @@ -183,9 +165,7 @@ class VerifySelfSessionPresenterTest { cancelVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val state = requestVerificationAndAwaitVerifyingState(service) state.eventSink(VerifySelfSessionViewEvents.Cancel) assertThat(awaitItem().step).isEqualTo(Step.Canceled) @@ -199,9 +179,7 @@ class VerifySelfSessionPresenterTest { startVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { requestVerificationAndAwaitVerifyingState(service) service.emitVerificationFlowState(VerificationFlowState.DidReceiveVerificationData(SessionVerificationData.Emojis(emptyList()))) ensureAllEventsConsumed() @@ -215,9 +193,7 @@ class VerifySelfSessionPresenterTest { startVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val state = requestVerificationAndAwaitVerifyingState(service) service.emitVerificationFlowState(VerificationFlowState.DidCancel) assertThat(awaitItem().step).isEqualTo(Step.Canceled) @@ -235,9 +211,7 @@ class VerifySelfSessionPresenterTest { startVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val state = requestVerificationAndAwaitVerifyingState(service) service.emitVerificationFlowState(VerificationFlowState.DidCancel) assertThat(awaitItem().step).isEqualTo(Step.Canceled) @@ -259,9 +233,7 @@ class VerifySelfSessionPresenterTest { approveVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val state = requestVerificationAndAwaitVerifyingState( service, SessionVerificationData.Emojis(emojis) @@ -286,9 +258,7 @@ class VerifySelfSessionPresenterTest { declineVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val state = requestVerificationAndAwaitVerifyingState(service) state.eventSink(VerifySelfSessionViewEvents.DeclineVerification) assertThat(awaitItem().step).isEqualTo( @@ -309,9 +279,7 @@ class VerifySelfSessionPresenterTest { startVerificationLambda = { }, ) val presenter = createVerifySelfSessionPresenter(service) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val state = requestVerificationAndAwaitVerifyingState(service) state.eventSink(VerifySelfSessionViewEvents.SkipVerification) assertThat(awaitItem().step).isEqualTo(Step.Skipped) @@ -331,9 +299,7 @@ class VerifySelfSessionPresenterTest { service = service, showDeviceVerifiedScreen = true, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { assertThat(awaitItem().step).isEqualTo(Step.Completed) } } @@ -351,9 +317,7 @@ class VerifySelfSessionPresenterTest { service = service, showDeviceVerifiedScreen = false, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { skipItems(1) assertThat(awaitItem().step).isEqualTo(Step.Skipped) } @@ -373,9 +337,7 @@ class VerifySelfSessionPresenterTest { service, logoutUseCase = FakeLogoutUseCase(signOutLambda) ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { skipItems(1) val initialItem = awaitItem() initialItem.eventSink(VerifySelfSessionViewEvents.SignOut) From 5311cf28c9fbc6ff5446fed157e42f45cf11d0fb Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 7 Nov 2024 15:12:10 +0100 Subject: [PATCH 7/8] Change action to only "Done" when there is a verification failure. --- .../outgoing/VerifySelfSessionStateMachine.kt | 3 --- .../impl/outgoing/VerifySelfSessionView.kt | 9 +++------ .../outgoing/VerifySelfSessionPresenterTest.kt | 18 ------------------ 3 files changed, 3 insertions(+), 27 deletions(-) diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt index 09f18538d0..a67cc4d331 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionStateMachine.kt @@ -66,9 +66,6 @@ class VerifySelfSessionStateMachine @Inject constructor( } } inState { - on { _: Event.RequestVerification, state -> - state.override { State.RequestingVerification.andLogStateChange() } - } on { _: Event.Reset, state -> state.override { State.Initial.andLogStateChange() } } diff --git a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt index 4351bb0bf2..afebffea47 100644 --- a/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt +++ b/features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionView.kt @@ -290,14 +290,11 @@ private fun VerifySelfSessionBottomMenu( VerificationBottomMenu { Button( modifier = Modifier.fillMaxWidth(), - text = stringResource(R.string.screen_session_verification_positive_button_canceled), - onClick = { eventSink(VerifySelfSessionViewEvents.RequestVerification) }, - ) - TextButton( - modifier = Modifier.fillMaxWidth(), - text = stringResource(CommonStrings.action_cancel), + text = stringResource(CommonStrings.action_done), onClick = onCancelClick, ) + // Placeholder so the 1st button keeps its vertical position + Spacer(modifier = Modifier.height(40.dp)) } } is Step.Ready -> { diff --git a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt index 806a1dbdf1..d8fe2f671c 100644 --- a/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt +++ b/features/verifysession/impl/src/test/kotlin/io/element/android/features/verifysession/impl/outgoing/VerifySelfSessionPresenterTest.kt @@ -186,24 +186,6 @@ class VerifySelfSessionPresenterTest { } } - @Test - fun `present - Restart after cancellation returns to requesting verification`() = runTest { - val service = unverifiedSessionService( - requestVerificationLambda = { }, - startVerificationLambda = { }, - ) - val presenter = createVerifySelfSessionPresenter(service) - presenter.test { - val state = requestVerificationAndAwaitVerifyingState(service) - service.emitVerificationFlowState(VerificationFlowState.DidCancel) - assertThat(awaitItem().step).isEqualTo(Step.Canceled) - state.eventSink(VerifySelfSessionViewEvents.RequestVerification) - // Went back to requesting verification - assertThat(awaitItem().step).isEqualTo(Step.AwaitingOtherDeviceResponse) - cancelAndIgnoreRemainingEvents() - } - } - @Test fun `present - Go back after cancellation returns to initial state`() = runTest { val service = unverifiedSessionService( From 88d04985cda184255da822f719c2293cc4c52b40 Mon Sep 17 00:00:00 2001 From: ElementBot Date: Thu, 7 Nov 2024 14:28:54 +0000 Subject: [PATCH 8/8] Update screenshots --- ...ession.impl.incoming_IncomingVerificationView_Day_7_en.png | 4 ++-- ...sion.impl.incoming_IncomingVerificationView_Night_7_en.png | 4 ++-- ...ysession.impl.outgoing_VerifySelfSessionView_Day_13_en.png | 3 +++ ...fysession.impl.outgoing_VerifySelfSessionView_Day_1_en.png | 4 ++-- ...fysession.impl.outgoing_VerifySelfSessionView_Day_4_en.png | 4 ++-- ...ession.impl.outgoing_VerifySelfSessionView_Night_13_en.png | 3 +++ ...session.impl.outgoing_VerifySelfSessionView_Night_1_en.png | 4 ++-- ...session.impl.outgoing_VerifySelfSessionView_Night_4_en.png | 4 ++-- 8 files changed, 18 insertions(+), 12 deletions(-) create mode 100644 tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_13_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_13_en.png diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Day_7_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Day_7_en.png index 0aa47d9aa9..1c316959ed 100644 --- a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Day_7_en.png +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Day_7_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bee1454db757b0897ae2113d3a11ed9adef0eb6bc52f3775edac56ed8533a88f -size 24076 +oid sha256:d3340a2d29e6c1d86f5ec5664179cae9501fcc95381311174d7d6b45b15af326 +size 24123 diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Night_7_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Night_7_en.png index 32fa8a3383..1c46f8eb53 100644 --- a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Night_7_en.png +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.incoming_IncomingVerificationView_Night_7_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:59a23ec5e3086349d3382f006cf1bcb4121183c83ea8c749aa95e3160561aef1 -size 23524 +oid sha256:c6c7e8cdf40bdf018931635565ac649fed518140a047a71cf70cf63e9edf54d3 +size 23932 diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_13_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_13_en.png new file mode 100644 index 0000000000..567462d90f --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_13_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:18dadaebe7a32aacde31afa0352a343913955b099ca4a07851e3ffe75e88b4d6 +size 31012 diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_1_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_1_en.png index 130cc1d84a..29667b296d 100644 --- a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:2cc647747c41f4c2cf96a3425e8b279b39fd4e09e1441e5b5afcd260f79afaa1 -size 22714 +oid sha256:71b9f32b26b391ff3bf231ca9f364a157f535c1e6ff52ee3e3ead3630bb1b239 +size 30007 diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_4_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_4_en.png index 4e84bcb0c2..1faa04af50 100644 --- a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_4_en.png +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Day_4_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a7fe45575eb8423161d355d9b99ecff4acc0f708e5b106d9d38aa2657942d736 -size 28219 +oid sha256:000374157cb5fbf6670b4af041fc385538df78bf4aecaf83ea24d39dfec84f23 +size 24278 diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_13_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_13_en.png new file mode 100644 index 0000000000..bebfa94f6d --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_13_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4dd274f8c2ade6213a13a47400ec3a571844eafcc82b292b1c813bd9aa098236 +size 30241 diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_1_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_1_en.png index 7e3a113610..af8d7eadbe 100644 --- a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:786a63d0d824657270b44428a1a251d0d521e6a97a4239516ae5f2eff06fdc3b -size 22125 +oid sha256:994b39ba25e011cce97504a1f3d4ed0c420b7bce87daafae77ba481cc629cdae +size 29051 diff --git a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_4_en.png b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_4_en.png index 76173e1b2d..d9b8a3a0de 100644 --- a/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_4_en.png +++ b/tests/uitests/src/test/snapshots/images/features.verifysession.impl.outgoing_VerifySelfSessionView_Night_4_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c29d3f5bbff739d23800345feb42e73fe9374c5cb5db27690278fb22c5546fe3 -size 27754 +oid sha256:3a3f08002e805fe5f7a4c96aa4b73c2fcd6e8b79e6a9e82bcc7bd3df50d8c22d +size 24015