Make sure we display errors when we create a recovery key and it fails (#5079)

* Make sure we display errors when we create a recovery key and it fails

* Add another preview for the error state

* Update screenshots

---------

Co-authored-by: ElementBot <android@element.io>
This commit is contained in:
Jorge Martin Espinosa 2025-07-25 13:36:43 +02:00 committed by GitHub
parent 7958bb4692
commit 2750835a36
11 changed files with 86 additions and 11 deletions

View file

@ -60,6 +60,7 @@ class SecureBackupSetupPresenter @AssistedInject constructor(
stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.UserSavedKey) stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.UserSavedKey)
SecureBackupSetupEvents.DismissDialog -> { SecureBackupSetupEvents.DismissDialog -> {
showSaveConfirmationDialog = false showSaveConfirmationDialog = false
stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.ClearError)
} }
SecureBackupSetupEvents.Done -> { SecureBackupSetupEvents.Done -> {
showSaveConfirmationDialog = true showSaveConfirmationDialog = true
@ -89,6 +90,7 @@ class SecureBackupSetupPresenter @AssistedInject constructor(
SecureBackupSetupStateMachine.State.CreatingKey -> SetupState.Creating SecureBackupSetupStateMachine.State.CreatingKey -> SetupState.Creating
is SecureBackupSetupStateMachine.State.KeyCreated -> SetupState.Created(formattedRecoveryKey = key) is SecureBackupSetupStateMachine.State.KeyCreated -> SetupState.Created(formattedRecoveryKey = key)
is SecureBackupSetupStateMachine.State.KeyCreatedAndSaved -> SetupState.CreatedAndSaved(formattedRecoveryKey = key) is SecureBackupSetupStateMachine.State.KeyCreatedAndSaved -> SetupState.CreatedAndSaved(formattedRecoveryKey = key)
is SecureBackupSetupStateMachine.State.Error -> SetupState.Error(exception)
} }
} }
@ -103,13 +105,20 @@ class SecureBackupSetupPresenter @AssistedInject constructor(
stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.SdkHasCreatedKey(it)) stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.SdkHasCreatedKey(it))
}, },
onFailure = { onFailure = {
stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.SdkError(it)) if (it is Exception) {
stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.SdkError(it))
}
} }
) )
} else { } else {
observeEncryptionService(stateAndDispatch) observeEncryptionService(stateAndDispatch)
Timber.tag(loggerTagSetup.value).d("Calling encryptionService.enableRecovery()") Timber.tag(loggerTagSetup.value).d("Calling encryptionService.enableRecovery()")
encryptionService.enableRecovery(waitForBackupsToUpload = false) encryptionService.enableRecovery(waitForBackupsToUpload = false).onFailure {
Timber.tag(loggerTagSetup.value).e(it, "Failed to enable recovery")
if (it is Exception) {
stateAndDispatch.dispatchAction(SecureBackupSetupStateMachine.Event.SdkError(it))
}
}
} }
} }

View file

@ -23,6 +23,7 @@ sealed interface SetupState {
data object Creating : SetupState data object Creating : SetupState
data class Created(val formattedRecoveryKey: String) : SetupState data class Created(val formattedRecoveryKey: String) : SetupState
data class CreatedAndSaved(val formattedRecoveryKey: String) : SetupState data class CreatedAndSaved(val formattedRecoveryKey: String) : SetupState
data class Error(val exception: Exception) : SetupState
} }
fun SetupState.recoveryKey(): String? = when (this) { fun SetupState.recoveryKey(): String? = when (this) {

View file

@ -26,8 +26,8 @@ class SecureBackupSetupStateMachine @Inject constructor() : FlowReduxStateMachin
} }
} }
inState<State.CreatingKey> { inState<State.CreatingKey> {
on { _: Event.SdkError, state: MachineState<State.CreatingKey> -> on { event: Event.SdkError, state: MachineState<State.CreatingKey> ->
state.override { State.Initial } state.override { State.Error(event.exception) }
} }
on { event: Event.SdkHasCreatedKey, state: MachineState<State.CreatingKey> -> on { event: Event.SdkHasCreatedKey, state: MachineState<State.CreatingKey> ->
state.override { State.KeyCreated(event.key) } state.override { State.KeyCreated(event.key) }
@ -38,6 +38,11 @@ class SecureBackupSetupStateMachine @Inject constructor() : FlowReduxStateMachin
state.override { State.KeyCreatedAndSaved(state.snapshot.key) } state.override { State.KeyCreatedAndSaved(state.snapshot.key) }
} }
} }
inState<State.Error> {
on { _: Event.ClearError, state: MachineState<State.Error> ->
state.override { State.Initial }
}
}
inState<State.KeyCreatedAndSaved> { inState<State.KeyCreatedAndSaved> {
} }
} }
@ -48,12 +53,14 @@ class SecureBackupSetupStateMachine @Inject constructor() : FlowReduxStateMachin
data object CreatingKey : State data object CreatingKey : State
data class KeyCreated(val key: String) : State data class KeyCreated(val key: String) : State
data class KeyCreatedAndSaved(val key: String) : State data class KeyCreatedAndSaved(val key: String) : State
data class Error(val exception: Exception) : State
} }
sealed interface Event { sealed interface Event {
data object UserCreatesKey : Event data object UserCreatesKey : Event
data class SdkHasCreatedKey(val key: String) : Event data class SdkHasCreatedKey(val key: String) : Event
data class SdkError(val throwable: Throwable) : Event data class SdkError(val exception: Exception) : Event
data object UserSavedKey : Event data object UserSavedKey : Event
data object ClearError : Event
} }
} }

View file

@ -23,6 +23,7 @@ open class SecureBackupSetupStateProvider : PreviewParameterProvider<SecureBacku
setupState = SetupState.CreatedAndSaved(aFormattedRecoveryKey()), setupState = SetupState.CreatedAndSaved(aFormattedRecoveryKey()),
showSaveConfirmationDialog = true, showSaveConfirmationDialog = true,
), ),
aSecureBackupSetupState(setupState = SetupState.Error(Exception("Test error"))),
// Add other states here // Add other states here
) )
} }

View file

@ -24,6 +24,7 @@ import io.element.android.libraries.androidutils.system.startSharePlainTextInten
import io.element.android.libraries.designsystem.atomic.pages.FlowStepPage import io.element.android.libraries.designsystem.atomic.pages.FlowStepPage
import io.element.android.libraries.designsystem.components.BigIcon import io.element.android.libraries.designsystem.components.BigIcon
import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight 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.Button
@ -49,6 +50,16 @@ fun SecureBackupSetupView(
Content(state = state) Content(state = state)
} }
if (state.setupState is SetupState.Error) {
ErrorDialog(
title = stringResource(id = CommonStrings.common_something_went_wrong),
content = stringResource(id = CommonStrings.common_something_went_wrong_message),
onSubmit = {
state.eventSink.invoke(SecureBackupSetupEvents.DismissDialog)
},
)
}
if (state.showSaveConfirmationDialog) { if (state.showSaveConfirmationDialog) {
ConfirmationDialog( ConfirmationDialog(
title = stringResource(id = R.string.screen_recovery_key_setup_confirmation_title), title = stringResource(id = R.string.screen_recovery_key_setup_confirmation_title),
@ -70,7 +81,8 @@ private fun SecureBackupSetupState.canGoBack(): Boolean {
private fun title(state: SecureBackupSetupState): String { private fun title(state: SecureBackupSetupState): String {
return when (state.setupState) { return when (state.setupState) {
SetupState.Init, SetupState.Init,
SetupState.Creating -> if (state.isChangeRecoveryKeyUserStory) { SetupState.Creating,
is SetupState.Error -> if (state.isChangeRecoveryKeyUserStory) {
stringResource(id = R.string.screen_recovery_key_change_title) stringResource(id = R.string.screen_recovery_key_change_title)
} else { } else {
stringResource(id = R.string.screen_recovery_key_setup_title) stringResource(id = R.string.screen_recovery_key_setup_title)
@ -85,7 +97,8 @@ private fun title(state: SecureBackupSetupState): String {
private fun subtitle(state: SecureBackupSetupState): String { private fun subtitle(state: SecureBackupSetupState): String {
return when (state.setupState) { return when (state.setupState) {
SetupState.Init, SetupState.Init,
SetupState.Creating -> if (state.isChangeRecoveryKeyUserStory) { SetupState.Creating,
is SetupState.Error -> if (state.isChangeRecoveryKeyUserStory) {
stringResource(id = R.string.screen_recovery_key_change_description) stringResource(id = R.string.screen_recovery_key_change_description)
} else { } else {
stringResource(id = R.string.screen_recovery_key_setup_description) stringResource(id = R.string.screen_recovery_key_setup_description)
@ -137,7 +150,8 @@ private fun ColumnScope.Buttons(
val chooserTitle = stringResource(id = R.string.screen_recovery_key_save_action) val chooserTitle = stringResource(id = R.string.screen_recovery_key_save_action)
when (state.setupState) { when (state.setupState) {
SetupState.Init, SetupState.Init,
SetupState.Creating -> { SetupState.Creating,
is SetupState.Error -> {
Button( Button(
text = stringResource(id = CommonStrings.action_done), text = stringResource(id = CommonStrings.action_done),
enabled = false, enabled = false,

View file

@ -109,6 +109,34 @@ class SecureBackupSetupPresenterTest {
} }
} }
@Test
fun `present - handle errors`() = runTest {
val encryptionService = FakeEncryptionService(
enableRecoveryLambda = { Result.failure(IllegalStateException("Test error")) }
)
val presenter = createSecureBackupSetupPresenter(
isChangeRecoveryKeyUserStory = false,
encryptionService = encryptionService
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
assertThat(initialState.isChangeRecoveryKeyUserStory).isFalse()
assertThat(initialState.setupState).isEqualTo(SetupState.Init)
initialState.eventSink(SecureBackupSetupEvents.CreateRecoveryKey)
val creatingState = awaitItem()
assertThat(creatingState.setupState).isEqualTo(SetupState.Creating)
val failedState = awaitItem()
assertThat(failedState.setupState).isInstanceOf(SetupState.Error::class.java)
failedState.eventSink(SecureBackupSetupEvents.DismissDialog)
val finalState = awaitItem()
assertThat(finalState.setupState).isEqualTo(SetupState.Init)
}
}
@Test @Test
fun `present - change recovery key and save it`() = runTest { fun `present - change recovery key and save it`() = runTest {
val encryptionService = FakeEncryptionService() val encryptionService = FakeEncryptionService()
@ -153,7 +181,9 @@ class SecureBackupSetupPresenterTest {
private fun createSecureBackupSetupPresenter( private fun createSecureBackupSetupPresenter(
isChangeRecoveryKeyUserStory: Boolean = false, isChangeRecoveryKeyUserStory: Boolean = false,
encryptionService: EncryptionService = FakeEncryptionService(), encryptionService: EncryptionService = FakeEncryptionService(
enableRecoveryLambda = { Result.success(Unit) },
),
): SecureBackupSetupPresenter { ): SecureBackupSetupPresenter {
return SecureBackupSetupPresenter( return SecureBackupSetupPresenter(
isChangeRecoveryKeyUserStory = isChangeRecoveryKeyUserStory, isChangeRecoveryKeyUserStory = isChangeRecoveryKeyUserStory,

View file

@ -26,7 +26,8 @@ class FakeEncryptionService(
private val pinUserIdentityResult: (UserId) -> Result<Unit> = { lambdaError() }, private val pinUserIdentityResult: (UserId) -> Result<Unit> = { lambdaError() },
private val isUserVerifiedResult: (UserId) -> Result<Boolean> = { lambdaError() }, private val isUserVerifiedResult: (UserId) -> Result<Boolean> = { lambdaError() },
private val withdrawVerificationResult: (UserId) -> Result<Unit> = { lambdaError() }, private val withdrawVerificationResult: (UserId) -> Result<Unit> = { lambdaError() },
private val getUserIdentityResult: (UserId) -> Result<IdentityState?> = { lambdaError() } private val getUserIdentityResult: (UserId) -> Result<IdentityState?> = { lambdaError() },
private val enableRecoveryLambda: (Boolean) -> Result<Unit> = { lambdaError() },
) : EncryptionService { ) : EncryptionService {
private var disableRecoveryFailure: Exception? = null private var disableRecoveryFailure: Exception? = null
override val backupStateStateFlow: MutableStateFlow<BackupState> = MutableStateFlow(BackupState.UNKNOWN) override val backupStateStateFlow: MutableStateFlow<BackupState> = MutableStateFlow(BackupState.UNKNOWN)
@ -87,7 +88,7 @@ class FakeEncryptionService(
} }
override suspend fun enableRecovery(waitForBackupsToUpload: Boolean): Result<Unit> = simulateLongTask { override suspend fun enableRecovery(waitForBackupsToUpload: Boolean): Result<Unit> = simulateLongTask {
return Result.success(Unit) return enableRecoveryLambda(waitForBackupsToUpload)
} }
fun givenWaitForBackupUploadSteadyStateFlow(flow: Flow<BackupUploadState>) { fun givenWaitForBackupUploadSteadyStateFlow(flow: Flow<BackupUploadState>) {

View file

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:d55e9236e761726e03742c6292ec3ced7af48b385da5a7be9eccc444a78ec312
size 35886

View file

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:0bc9364efa8aa639f8989d74a5172665ee0fa8f4c9c354b584cfdabfd98d723c
size 32964

View file

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:34272a001ad54175280dc251375cee45e2978cba78c7d4b08b10d59ae7093d93
size 34340

View file

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:86e9cbfe60551d6d7f052ad0a7f75eda029569d9d51cf63339e223df5332ec63
size 31674