Always display 'lost recovery key?' option (#2745)

* Always display 'lost recovery key?' option

* Use `isLastDevice` it to display only 'enter recovery key' option for verification.

* Update strings. This should fix the wrong term 'passcode' being used in the recovery key screen title.

* Disable 'lost your recovery key?' button while the screen is in a loading state

* Update screenshots

---------

Co-authored-by: ElementBot <benoitm+elementbot@element.io>
This commit is contained in:
Jorge Martin Espinosa
2024-04-25 15:28:24 +02:00
committed by GitHub
parent 5579119f2e
commit fe9b3f7cdb
30 changed files with 181 additions and 83 deletions

1
changelog.d/2740.bugfix Normal file
View File

@@ -0,0 +1 @@
Instead of displaying 'create new recovery key' on the session verification screen when there is no other session active, display it always under the 'enter recovery key' screen.

View File

@@ -58,9 +58,6 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
@Parcelize
data object EnterRecoveryKey : NavTarget
@Parcelize
data object CreateNewRecoveryKey : NavTarget
}
interface Callback : Plugin {
@@ -68,10 +65,6 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
}
private val secureBackupEntryPointCallback = object : SecureBackupEntryPoint.Callback {
override fun onCreateNewRecoveryKey() {
backstack.push(NavTarget.CreateNewRecoveryKey)
}
override fun onDone() {
lifecycleScope.launch {
// Move to the completed state view in the verification flow
@@ -89,10 +82,6 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
backstack.push(NavTarget.EnterRecoveryKey)
}
override fun onCreateNewRecoveryKey() {
backstack.push(NavTarget.CreateNewRecoveryKey)
}
override fun onDone() {
plugins<Callback>().forEach { it.onDone() }
}
@@ -105,12 +94,6 @@ class FtueSessionVerificationFlowNode @AssistedInject constructor(
.callback(secureBackupEntryPointCallback)
.build()
}
is NavTarget.CreateNewRecoveryKey -> {
secureBackupEntryPoint.nodeBuilder(this, buildContext)
.params(SecureBackupEntryPoint.Params(SecureBackupEntryPoint.InitialTarget.CreateNewRecoveryKey))
.callback(secureBackupEntryPointCallback)
.build()
}
}
}

View File

@@ -9,8 +9,8 @@
<string name="screen_qr_code_login_connection_note_secure_state_list_item_2">"If you encounter the same problem, try a different wifi network or use your mobile data instead of wifi"</string>
<string name="screen_qr_code_login_connection_note_secure_state_list_item_3">"If that doesnt work, sign in manually"</string>
<string name="screen_qr_code_login_connection_note_secure_state_title">"Connection not secure"</string>
<string name="screen_qr_code_login_device_code_subtitle">"Youll be asked to enter the two digits shown below."</string>
<string name="screen_qr_code_login_device_code_title">"Enter number on your device"</string>
<string name="screen_qr_code_login_device_code_subtitle">"Youll be asked to enter the two digits shown on this device."</string>
<string name="screen_qr_code_login_device_code_title">"Enter the number below on your other device"</string>
<string name="screen_qr_code_login_initial_state_item_1">"Open %1$s on a desktop device"</string>
<string name="screen_qr_code_login_initial_state_item_2">"Click on your avatar"</string>
<string name="screen_qr_code_login_initial_state_item_3">"Select %1$s"</string>

View File

@@ -41,7 +41,6 @@ interface SecureBackupEntryPoint : FeatureEntryPoint {
fun nodeBuilder(parentNode: Node, buildContext: BuildContext): NodeBuilder
interface Callback : Plugin {
fun onCreateNewRecoveryKey()
fun onDone()
}

View File

@@ -23,6 +23,12 @@ plugins {
android {
namespace = "io.element.android.features.securebackup.impl"
testOptions {
unitTests {
isIncludeAndroidResources = true
}
}
}
anvil {
@@ -51,8 +57,11 @@ dependencies {
testImplementation(libs.molecule.runtime)
testImplementation(libs.test.truth)
testImplementation(libs.test.turbine)
testImplementation(libs.test.robolectric)
testImplementation(libs.androidx.compose.ui.test.junit)
testImplementation(projects.libraries.matrix.test)
testImplementation(projects.tests.testutils)
testReleaseImplementation(libs.androidx.compose.ui.test.manifest)
ksp(libs.showkase.processor)
}

View File

@@ -136,6 +136,10 @@ class SecureBackupFlowNode @AssistedInject constructor(
backstack.pop()
}
}
override fun onCreateNewRecoveryKey() {
backstack.push(NavTarget.CreateNewRecoveryKey)
}
}
createNode<SecureBackupEnterRecoveryKeyNode>(buildContext, plugins = listOf(callback))
}

View File

@@ -35,6 +35,7 @@ class SecureBackupEnterRecoveryKeyNode @AssistedInject constructor(
) : Node(buildContext, plugins = plugins) {
interface Callback : Plugin {
fun onEnterRecoveryKeySuccess()
fun onCreateNewRecoveryKey()
}
private val callback = plugins<Callback>().first()
@@ -47,6 +48,7 @@ class SecureBackupEnterRecoveryKeyNode @AssistedInject constructor(
modifier = modifier,
onDone = callback::onEnterRecoveryKeySuccess,
onBackClicked = ::navigateUp,
onCreateNewRecoveryKey = callback::onCreateNewRecoveryKey
)
}
}

View File

@@ -36,6 +36,7 @@ fun aSecureBackupEnterRecoveryKeyState(
recoveryKey: String = aFormattedRecoveryKey(),
isSubmitEnabled: Boolean = recoveryKey.isNotEmpty(),
submitAction: AsyncAction<Unit> = AsyncAction.Uninitialized,
eventSink: (SecureBackupEnterRecoveryKeyEvents) -> Unit = {},
) = SecureBackupEnterRecoveryKeyState(
recoveryKeyViewState = RecoveryKeyViewState(
recoveryKeyUserStory = RecoveryKeyUserStory.Enter,
@@ -44,5 +45,5 @@ fun aSecureBackupEnterRecoveryKeyState(
),
isSubmitEnabled = isSubmitEnabled,
submitAction = submitAction,
eventSink = {}
eventSink = eventSink,
)

View File

@@ -32,6 +32,7 @@ import io.element.android.libraries.designsystem.components.async.AsyncActionVie
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.TextButton
import io.element.android.libraries.ui.strings.CommonStrings
@Composable
@@ -39,6 +40,7 @@ fun SecureBackupEnterRecoveryKeyView(
state: SecureBackupEnterRecoveryKeyState,
onDone: () -> Unit,
onBackClicked: () -> Unit,
onCreateNewRecoveryKey: () -> Unit,
modifier: Modifier = Modifier,
) {
AsyncActionView(
@@ -57,7 +59,7 @@ fun SecureBackupEnterRecoveryKeyView(
title = stringResource(id = R.string.screen_recovery_key_confirm_title),
subTitle = stringResource(id = R.string.screen_recovery_key_confirm_description),
content = { Content(state = state) },
buttons = { Buttons(state = state) }
buttons = { Buttons(state = state, onCreateRecoveryKey = onCreateNewRecoveryKey) }
)
}
@@ -81,6 +83,7 @@ private fun Content(
@Composable
private fun ColumnScope.Buttons(
state: SecureBackupEnterRecoveryKeyState,
onCreateRecoveryKey: () -> Unit,
) {
Button(
text = stringResource(id = CommonStrings.action_continue),
@@ -91,6 +94,12 @@ private fun ColumnScope.Buttons(
state.eventSink.invoke(SecureBackupEnterRecoveryKeyEvents.Submit)
}
)
TextButton(
text = stringResource(id = R.string.screen_recovery_key_confirm_lost_recovery_key),
enabled = !state.submitAction.isLoading(),
modifier = Modifier.fillMaxWidth(),
onClick = onCreateRecoveryKey,
)
}
@PreviewsDayNight
@@ -102,5 +111,6 @@ internal fun SecureBackupEnterRecoveryKeyViewPreview(
state = state,
onDone = {},
onBackClicked = {},
onCreateNewRecoveryKey = {},
)
}

View File

@@ -35,8 +35,9 @@
<string name="screen_recovery_key_confirm_key_description">"If you have a security key or security phrase, this will work too."</string>
<string name="screen_recovery_key_confirm_key_label">"Recovery key or passcode"</string>
<string name="screen_recovery_key_confirm_key_placeholder">"Enter…"</string>
<string name="screen_recovery_key_confirm_lost_recovery_key">"Lost your recovery key?"</string>
<string name="screen_recovery_key_confirm_success">"Recovery key confirmed"</string>
<string name="screen_recovery_key_confirm_title">"Enter your recovery key or passcode"</string>
<string name="screen_recovery_key_confirm_title">"Enter your recovery key"</string>
<string name="screen_recovery_key_copied_to_clipboard">"Copied recovery key"</string>
<string name="screen_recovery_key_generating_key">"Generating…"</string>
<string name="screen_recovery_key_save_action">"Save recovery key"</string>

View File

@@ -0,0 +1,110 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.features.securebackup.impl.enter
import androidx.activity.ComponentActivity
import androidx.compose.ui.test.junit4.AndroidComposeTestRule
import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.element.android.features.securebackup.impl.R
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.ui.strings.CommonStrings
import io.element.android.tests.testutils.EnsureNeverCalled
import io.element.android.tests.testutils.EventsRecorder
import io.element.android.tests.testutils.clickOn
import io.element.android.tests.testutils.ensureCalledOnce
import io.element.android.tests.testutils.pressBack
import io.element.android.tests.testutils.pressBackKey
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TestRule
import org.junit.runner.RunWith
@RunWith(AndroidJUnit4::class)
class SecureBackupEnterRecoveryKeyViewTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
@Test
fun `back key pressed - calls onBackClicked`() {
ensureCalledOnce { callback ->
rule.setSecureBackupEnterRecoveryKeyView(
aSecureBackupEnterRecoveryKeyState(),
onBackClicked = callback,
)
rule.pressBackKey()
}
}
@Test
fun `back button clicked - calls onBackClicked`() {
ensureCalledOnce { callback ->
rule.setSecureBackupEnterRecoveryKeyView(
aSecureBackupEnterRecoveryKeyState(),
onBackClicked = callback,
)
rule.pressBack()
}
}
@Test
fun `tapping on Continue when key is valid - calls expected action`() {
val recorder = EventsRecorder<SecureBackupEnterRecoveryKeyEvents>()
rule.setSecureBackupEnterRecoveryKeyView(
aSecureBackupEnterRecoveryKeyState(isSubmitEnabled = true, eventSink = recorder),
)
rule.clickOn(CommonStrings.action_continue)
recorder.assertSingle(SecureBackupEnterRecoveryKeyEvents.Submit)
}
@Test
fun `tapping on Lost your recovery key - calls onCreateNewRecoveryKey`() {
ensureCalledOnce { callback ->
rule.setSecureBackupEnterRecoveryKeyView(
aSecureBackupEnterRecoveryKeyState(),
onCreateNewRecoveryKey = callback,
)
rule.clickOn(R.string.screen_recovery_key_confirm_lost_recovery_key)
}
}
@Test
fun `when submit action succeeds - calls onDone`() {
ensureCalledOnce { callback ->
rule.setSecureBackupEnterRecoveryKeyView(
aSecureBackupEnterRecoveryKeyState(submitAction = AsyncAction.Success(Unit)),
onDone = callback,
)
}
}
private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setSecureBackupEnterRecoveryKeyView(
state: SecureBackupEnterRecoveryKeyState,
onDone: () -> Unit = EnsureNeverCalled(),
onBackClicked: () -> Unit = EnsureNeverCalled(),
onCreateNewRecoveryKey: () -> Unit = EnsureNeverCalled(),
) {
rule.setContent {
SecureBackupEnterRecoveryKeyView(
state = state,
onDone = onDone,
onBackClicked = onBackClicked,
onCreateNewRecoveryKey = onCreateNewRecoveryKey
)
}
}
}

View File

@@ -31,7 +31,6 @@ interface VerifySessionEntryPoint : FeatureEntryPoint {
interface Callback : Plugin {
fun onEnterRecoveryKey()
fun onCreateNewRecoveryKey()
fun onDone()
}
}

View File

@@ -43,7 +43,6 @@ class VerifySelfSessionNode @AssistedInject constructor(
state = state,
modifier = modifier,
onEnterRecoveryKey = callback::onEnterRecoveryKey,
onCreateNewRecoveryKey = callback::onCreateNewRecoveryKey,
onFinished = callback::onDone,
)
}

View File

@@ -103,7 +103,10 @@ class VerifySelfSessionPresenter @Inject constructor(
): VerifySelfSessionState.VerificationStep =
when (val machineState = this) {
StateMachineState.Initial, null -> {
VerifySelfSessionState.VerificationStep.Initial(canEnterRecoveryKey = canEnterRecoveryKey, isLastDevice = encryptionService.isLastDevice.value)
VerifySelfSessionState.VerificationStep.Initial(
canEnterRecoveryKey = canEnterRecoveryKey,
isLastDevice = encryptionService.isLastDevice.value
)
}
StateMachineState.RequestingVerification,
StateMachineState.StartingSasVerification,

View File

@@ -29,7 +29,7 @@ data class VerifySelfSessionState(
) {
@Stable
sealed interface VerificationStep {
data class Initial(val canEnterRecoveryKey: Boolean, val isLastDevice: Boolean) : VerificationStep
data class Initial(val canEnterRecoveryKey: Boolean, val isLastDevice: Boolean = false) : VerificationStep
data object Canceled : VerificationStep
data object AwaitingOtherDeviceResponse : VerificationStep
data object Ready : VerificationStep

View File

@@ -45,7 +45,7 @@ open class VerifySelfSessionStateProvider : PreviewParameterProvider<VerifySelfS
verificationFlowStep = VerificationStep.Verifying(aDecimalsSessionVerificationData(), AsyncData.Uninitialized)
),
aVerifySelfSessionState(
verificationFlowStep = VerificationStep.Initial(canEnterRecoveryKey = true, isLastDevice = false)
verificationFlowStep = VerificationStep.Initial(canEnterRecoveryKey = true)
),
aVerifySelfSessionState(
verificationFlowStep = VerificationStep.Initial(canEnterRecoveryKey = true, isLastDevice = true)
@@ -71,7 +71,7 @@ private fun aDecimalsSessionVerificationData(
}
internal fun aVerifySelfSessionState(
verificationFlowStep: VerificationStep = VerificationStep.Initial(canEnterRecoveryKey = false, isLastDevice = false),
verificationFlowStep: VerificationStep = VerificationStep.Initial(canEnterRecoveryKey = false),
displaySkipButton: Boolean = false,
eventSink: (VerifySelfSessionViewEvents) -> Unit = {},
) = VerifySelfSessionState(

View File

@@ -66,7 +66,6 @@ import io.element.android.features.verifysession.impl.VerifySelfSessionState.Ver
fun VerifySelfSessionView(
state: VerifySelfSessionState,
onEnterRecoveryKey: () -> Unit,
onCreateNewRecoveryKey: () -> Unit,
onFinished: () -> Unit,
modifier: Modifier = Modifier,
) {
@@ -115,7 +114,6 @@ fun VerifySelfSessionView(
screenState = state,
goBack = ::resetFlow,
onEnterRecoveryKey = onEnterRecoveryKey,
onCreateNewRecoveryKey = onCreateNewRecoveryKey,
onFinished = onFinished,
)
}
@@ -228,7 +226,6 @@ private fun EmojiItemView(emoji: VerificationEmoji, modifier: Modifier = Modifie
private fun BottomMenu(
screenState: VerifySelfSessionState,
onEnterRecoveryKey: () -> Unit,
onCreateNewRecoveryKey: () -> Unit,
goBack: () -> Unit,
onFinished: () -> Unit,
) {
@@ -243,8 +240,6 @@ private fun BottomMenu(
BottomMenu(
positiveButtonTitle = stringResource(R.string.screen_session_verification_enter_recovery_key),
onPositiveButtonClicked = onEnterRecoveryKey,
negativeButtonTitle = stringResource(R.string.screen_identity_confirmation_create_new_recovery_key),
onNegativeButtonClicked = onCreateNewRecoveryKey,
)
} else {
BottomMenu(
@@ -346,7 +341,6 @@ internal fun VerifySelfSessionViewPreview(@PreviewParameter(VerifySelfSessionSta
VerifySelfSessionView(
state = state,
onEnterRecoveryKey = {},
onCreateNewRecoveryKey = {},
onFinished = {},
)
}

View File

@@ -53,7 +53,7 @@ class VerifySelfSessionPresenterTests {
presenter.present()
}.test {
awaitItem().run {
assertThat(verificationFlowStep).isEqualTo(VerificationStep.Initial(false, false))
assertThat(verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
assertThat(displaySkipButton).isTrue()
}
}
@@ -80,7 +80,7 @@ class VerifySelfSessionPresenterTests {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(true, false))
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(true))
}
}
@@ -95,7 +95,7 @@ class VerifySelfSessionPresenterTests {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(true, true))
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(canEnterRecoveryKey = true, isLastDevice = true))
}
}
@@ -118,7 +118,7 @@ class VerifySelfSessionPresenterTests {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.verificationFlowStep).isEqualTo(VerificationStep.Initial(false, false))
assertThat(initialState.verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
val eventSink = initialState.eventSink
eventSink(VerifySelfSessionViewEvents.StartSasVerification)
// Await for other device response:
@@ -137,7 +137,7 @@ class VerifySelfSessionPresenterTests {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.verificationFlowStep).isEqualTo(VerificationStep.Initial(false, false))
assertThat(initialState.verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
val eventSink = initialState.eventSink
eventSink(VerifySelfSessionViewEvents.Cancel)
expectNoEvents()
@@ -172,7 +172,7 @@ class VerifySelfSessionPresenterTests {
awaitItem().eventSink(VerifySelfSessionViewEvents.RequestVerification)
service.shouldFail = false
assertThat(awaitItem().verificationFlowStep).isInstanceOf(VerificationStep.AwaitingOtherDeviceResponse::class.java)
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(false, false))
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
}
}
@@ -231,7 +231,7 @@ class VerifySelfSessionPresenterTests {
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Canceled)
state.eventSink(VerifySelfSessionViewEvents.Reset)
// Went back to initial state
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(false, false))
assertThat(awaitItem().verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
cancelAndIgnoreRemainingEvents()
}
}
@@ -315,7 +315,7 @@ class VerifySelfSessionPresenterTests {
sessionVerificationData: SessionVerificationData = SessionVerificationData.Emojis(emptyList()),
): VerifySelfSessionState {
var state = awaitItem()
assertThat(state.verificationFlowStep).isEqualTo(VerificationStep.Initial(false, false))
assertThat(state.verificationFlowStep).isEqualTo(VerificationStep.Initial(false))
state.eventSink(VerifySelfSessionViewEvents.RequestVerification)
// Await for other device response:
state = awaitItem()

View File

@@ -144,7 +144,7 @@ class VerifySelfSessionViewTest {
ensureCalledOnce { callback ->
rule.setVerifySelfSessionView(
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Initial(true, false),
verificationFlowStep = VerifySelfSessionState.VerificationStep.Initial(true),
eventSink = eventsRecorder
),
onEnterRecoveryKey = callback,
@@ -153,22 +153,6 @@ class VerifySelfSessionViewTest {
}
}
@Config(qualifiers = "h1024dp")
@Test
fun `clicking on create new recovery key calls the expected callback`() {
val eventsRecorder = EventsRecorder<VerifySelfSessionViewEvents>(expectEvents = false)
ensureCalledOnce { callback ->
rule.setVerifySelfSessionView(
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Initial(true, true),
eventSink = eventsRecorder
),
onCreateNewRecoveryKey = callback,
)
rule.clickOn(R.string.screen_identity_confirmation_create_new_recovery_key)
}
}
@Test
fun `clicking on they match emits the expected event`() {
val eventsRecorder = EventsRecorder<VerifySelfSessionViewEvents>()
@@ -206,7 +190,7 @@ class VerifySelfSessionViewTest {
val eventsRecorder = EventsRecorder<VerifySelfSessionViewEvents>()
rule.setVerifySelfSessionView(
aVerifySelfSessionState(
verificationFlowStep = VerifySelfSessionState.VerificationStep.Initial(canEnterRecoveryKey = true, isLastDevice = false),
verificationFlowStep = VerifySelfSessionState.VerificationStep.Initial(canEnterRecoveryKey = true),
displaySkipButton = true,
eventSink = eventsRecorder
),
@@ -232,14 +216,12 @@ class VerifySelfSessionViewTest {
private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setVerifySelfSessionView(
state: VerifySelfSessionState,
onEnterRecoveryKey: () -> Unit = EnsureNeverCalled(),
onCreateNewRecoveryKey: () -> Unit = EnsureNeverCalled(),
onFinished: () -> Unit = EnsureNeverCalled(),
) {
rule.setContent {
VerifySelfSessionView(
state = state,
onEnterRecoveryKey = onEnterRecoveryKey,
onCreateNewRecoveryKey = onCreateNewRecoveryKey,
onFinished = onFinished,
)
}

View File

@@ -215,6 +215,7 @@
<string name="common_topic">"Topic"</string>
<string name="common_topic_placeholder">"What is this room about?"</string>
<string name="common_unable_to_decrypt">"Unable to decrypt"</string>
<string name="common_unable_to_decrypt_no_access">"You don\'t have access to this message"</string>
<string name="common_unable_to_invite_message">"Invites couldn\'t be sent to one or more users."</string>
<string name="common_unable_to_invite_title">"Unable to send invite(s)"</string>
<string name="common_unlock">"Unlock"</string>