From 96bd25eae5dce025fc966e36583f25ed8d6d5401 Mon Sep 17 00:00:00 2001 From: David Perez Date: Wed, 20 Nov 2024 16:00:08 -0600 Subject: [PATCH] PM-12733: Add error dialog to be displayed if TOTP code is blank (#4345) --- .../components/dialog/BitwardenBasicDialog.kt | 89 ++++++++++++------- .../manualcodeentry/ManualCodeEntryScreen.kt | 26 ++++++ .../ManualCodeEntryViewModel.kt | 48 +++++++++- .../ManualCodeEntryScreenTests.kt | 64 ++++++++++++- .../ManualCodeEntryViewModelTests.kt | 59 +++++++++--- 5 files changed, 237 insertions(+), 49 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenBasicDialog.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenBasicDialog.kt index d0c5de307..5fc4ff499 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenBasicDialog.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenBasicDialog.kt @@ -19,6 +19,58 @@ import com.x8bit.bitwarden.ui.platform.components.button.BitwardenTextButton import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme import kotlinx.parcelize.Parcelize +/** + * Represents a Bitwarden-styled dialog. + * + * @param title The optional title to be displayed by the dialog. + * @param message The message to be displayed under the [title] by the dialog. + * @param onDismissRequest A lambda that is invoked when the user has requested to dismiss the + * dialog, whether by tapping "OK", tapping outside the dialog, or pressing the back button. + */ +@OptIn(ExperimentalComposeUiApi::class) +@Composable +fun BitwardenBasicDialog( + title: String?, + message: String, + onDismissRequest: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismissRequest, + confirmButton = { + BitwardenTextButton( + label = stringResource(id = R.string.ok), + onClick = onDismissRequest, + modifier = Modifier.testTag(tag = "AcceptAlertButton"), + ) + }, + title = title?.let { + { + Text( + text = it, + style = BitwardenTheme.typography.headlineSmall, + modifier = Modifier.testTag(tag = "AlertTitleText"), + ) + } + }, + text = { + Text( + text = message, + style = BitwardenTheme.typography.bodyMedium, + modifier = Modifier.testTag(tag = "AlertContentText"), + ) + }, + shape = BitwardenTheme.shapes.dialog, + containerColor = BitwardenTheme.colorScheme.background.primary, + iconContentColor = BitwardenTheme.colorScheme.icon.secondary, + titleContentColor = BitwardenTheme.colorScheme.text.primary, + textContentColor = BitwardenTheme.colorScheme.text.primary, + modifier = Modifier.semantics { + testTagsAsResourceId = true + testTag = "AlertPopup" + }, + ) +} + /** * Represents a Bitwarden-styled dialog that is hidden or shown based on [visibilityState]. * @@ -26,7 +78,6 @@ import kotlinx.parcelize.Parcelize * @param onDismissRequest called when the user has requested to dismiss the dialog, whether by * tapping "OK", tapping outside the dialog, or pressing the back button. */ -@OptIn(ExperimentalComposeUiApi::class) @Composable fun BitwardenBasicDialog( visibilityState: BasicDialogState, @@ -34,40 +85,10 @@ fun BitwardenBasicDialog( ): Unit = when (visibilityState) { BasicDialogState.Hidden -> Unit is BasicDialogState.Shown -> { - AlertDialog( + BitwardenBasicDialog( + title = visibilityState.title?.invoke(), + message = visibilityState.message(), onDismissRequest = onDismissRequest, - confirmButton = { - BitwardenTextButton( - label = stringResource(id = R.string.ok), - onClick = onDismissRequest, - modifier = Modifier.testTag("AcceptAlertButton"), - ) - }, - title = visibilityState.title?.let { - { - Text( - text = it(), - style = BitwardenTheme.typography.headlineSmall, - modifier = Modifier.testTag("AlertTitleText"), - ) - } - }, - text = { - Text( - text = visibilityState.message(), - style = BitwardenTheme.typography.bodyMedium, - modifier = Modifier.testTag("AlertContentText"), - ) - }, - shape = BitwardenTheme.shapes.dialog, - containerColor = BitwardenTheme.colorScheme.background.primary, - iconContentColor = BitwardenTheme.colorScheme.icon.secondary, - titleContentColor = BitwardenTheme.colorScheme.text.primary, - textContentColor = BitwardenTheme.colorScheme.text.primary, - modifier = Modifier.semantics { - testTagsAsResourceId = true - testTag = "AlertPopup" - }, ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreen.kt index 4ac5a552d..e86abd68e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreen.kt @@ -32,6 +32,7 @@ import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenTopAppBar import com.x8bit.bitwarden.ui.platform.components.button.BitwardenOutlinedButton +import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenBasicDialog import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog import com.x8bit.bitwarden.ui.platform.components.field.BitwardenTextField import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold @@ -108,6 +109,13 @@ fun ManualCodeEntryScreen( ) } + ManualCodeEntryDialogs( + state = state.dialog, + onDismissRequest = remember(viewModel) { + { viewModel.trySendAction(ManualCodeEntryAction.DialogDismiss) } + }, + ) + BitwardenScaffold( modifier = Modifier.fillMaxSize(), topBar = { @@ -200,3 +208,21 @@ fun ManualCodeEntryScreen( } } } + +@Composable +private fun ManualCodeEntryDialogs( + state: ManualCodeEntryState.DialogState?, + onDismissRequest: () -> Unit, +) { + when (state) { + is ManualCodeEntryState.DialogState.Error -> { + BitwardenBasicDialog( + title = state.title?.invoke(), + message = state.message(), + onDismissRequest = onDismissRequest, + ) + } + + null -> Unit + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModel.kt index fccbcbbc9..dba7826b9 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModel.kt @@ -2,10 +2,12 @@ package com.x8bit.bitwarden.ui.vault.feature.manualcodeentry import android.os.Parcelable import androidx.lifecycle.SavedStateHandle +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModel import com.x8bit.bitwarden.ui.platform.base.util.Text +import com.x8bit.bitwarden.ui.platform.base.util.asText import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.update import kotlinx.parcelize.Parcelize @@ -23,13 +25,17 @@ class ManualCodeEntryViewModel @Inject constructor( private val vaultRepository: VaultRepository, ) : BaseViewModel( initialState = savedStateHandle[KEY_STATE] - ?: ManualCodeEntryState(code = ""), + ?: ManualCodeEntryState( + code = "", + dialog = null, + ), ) { override fun handleAction(action: ManualCodeEntryAction) { when (action) { is ManualCodeEntryAction.CloseClick -> handleCloseClick() is ManualCodeEntryAction.CodeTextChange -> handleCodeTextChange(action) is ManualCodeEntryAction.CodeSubmit -> handleCodeSubmit() + ManualCodeEntryAction.DialogDismiss -> handleDialogDismiss() is ManualCodeEntryAction.ScanQrCodeTextClick -> handleScanQrCodeTextClick() is ManualCodeEntryAction.SettingsClick -> handleSettingsClick() } @@ -46,10 +52,26 @@ class ManualCodeEntryViewModel @Inject constructor( } private fun handleCodeSubmit() { - vaultRepository.emitTotpCodeResult(TotpCodeResult.Success(state.code.trim())) + val code = state.code.trim() + if (code.isEmpty()) { + mutableStateFlow.update { + it.copy( + dialog = ManualCodeEntryState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.authenticator_key_read_error.asText(), + ), + ) + } + return + } + vaultRepository.emitTotpCodeResult(TotpCodeResult.Success(code)) sendEvent(ManualCodeEntryEvent.NavigateBack) } + private fun handleDialogDismiss() { + mutableStateFlow.update { it.copy(dialog = null) } + } + private fun handleScanQrCodeTextClick() { sendEvent(ManualCodeEntryEvent.NavigateToQrCodeScreen) } @@ -65,7 +87,22 @@ class ManualCodeEntryViewModel @Inject constructor( @Parcelize data class ManualCodeEntryState( val code: String, -) : Parcelable + val dialog: DialogState?, +) : Parcelable { + /** + * Represents the current state of any dialogs on the screen. + */ + sealed class DialogState : Parcelable { + /** + * Represents a dismissible dialog with the given error [message]. + */ + @Parcelize + data class Error( + val title: Text?, + val message: Text, + ) : DialogState() + } +} /** * Models events for the [ManualCodeEntryScreen]. @@ -113,6 +150,11 @@ sealed class ManualCodeEntryAction { */ data class CodeTextChange(val code: String) : ManualCodeEntryAction() + /** + * User dismissed the dialog. + */ + data object DialogDismiss : ManualCodeEntryAction() + /** * The text to switch to QR code scanning is clicked. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreenTests.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreenTests.kt index fed86456b..c8901b47d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreenTests.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryScreenTests.kt @@ -15,10 +15,13 @@ import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextInput import androidx.test.core.app.ApplicationProvider +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest +import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager import com.x8bit.bitwarden.ui.platform.manager.permissions.FakePermissionManager +import com.x8bit.bitwarden.ui.util.assertNoDialogExists import io.mockk.every import io.mockk.mockk import io.mockk.slot @@ -36,8 +39,7 @@ class ManualCodeEntryScreenTests : BaseComposeTest() { private var onNavigateToScanQrCodeCalled = false private val mutableEventFlow = bufferedMutableSharedFlow() - private val mutableStateFlow = - MutableStateFlow(ManualCodeEntryState("")) + private val mutableStateFlow = MutableStateFlow(DEFAULT_STATE) private val fakePermissionManager: FakePermissionManager = FakePermissionManager() private val intentManager = mockk(relaxed = true) @@ -131,6 +133,59 @@ class ManualCodeEntryScreenTests : BaseComposeTest() { .assertIsNotDisplayed() } + @Test + fun `error dialog should be updated according to state`() { + composeTestRule.assertNoDialogExists() + + mutableStateFlow.update { + it.copy( + dialog = ManualCodeEntryState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.authenticator_key_read_error.asText(), + ), + ) + } + + composeTestRule + .onAllNodesWithText(text = "An error has occurred.") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText(text = "Cannot read authenticator key.") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + + mutableStateFlow.update { + it.copy(dialog = null) + } + + composeTestRule.assertNoDialogExists() + } + + @Test + fun `error dialog Ok click should emit DialogDismiss`() { + composeTestRule.assertNoDialogExists() + + mutableStateFlow.update { + it.copy( + dialog = ManualCodeEntryState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.authenticator_key_read_error.asText(), + ), + ) + } + + composeTestRule + .onAllNodesWithText(text = "Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + .performClick() + + verify(exactly = 1) { + viewModel.trySendAction(ManualCodeEntryAction.DialogDismiss) + } + } + @Test fun `settings dialog should call SettingsClick action on confirm click`() { fakePermissionManager.checkPermissionResult = false @@ -202,3 +257,8 @@ class ManualCodeEntryScreenTests : BaseComposeTest() { } } } + +private val DEFAULT_STATE: ManualCodeEntryState = ManualCodeEntryState( + code = "", + dialog = null, +) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModelTests.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModelTests.kt index d7f76d3aa..a0c22b854 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModelTests.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/manualcodeentry/ManualCodeEntryViewModelTests.kt @@ -2,10 +2,12 @@ package com.x8bit.bitwarden.ui.vault.feature.manualcodeentry import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import com.x8bit.bitwarden.ui.platform.base.util.asText import io.mockk.every import io.mockk.just import io.mockk.mockk @@ -26,7 +28,7 @@ class ManualCodeEntryViewModelTests : BaseViewModelTest() { @Test fun `CloseClick should emit NavigateBack`() = runTest { - val viewModel = createViewModel(initialState = ManualCodeEntryState("")) + val viewModel = createViewModel() viewModel.eventFlow.test { viewModel.trySendAction(ManualCodeEntryAction.CloseClick) @@ -34,9 +36,30 @@ class ManualCodeEntryViewModelTests : BaseViewModelTest() { } } + @Test + fun `CodeSubmit wihh blank code should display error dialog`() { + val initialState = DEFAULT_STATE.copy(code = " ") + val viewModel = createViewModel(initialState = initialState) + + viewModel.trySendAction(ManualCodeEntryAction.CodeSubmit) + + verify(exactly = 0) { + vaultRepository.emitTotpCodeResult(TotpCodeResult.Success("TestCode")) + } + assertEquals( + initialState.copy( + dialog = ManualCodeEntryState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.authenticator_key_read_error.asText(), + ), + ), + viewModel.stateFlow.value, + ) + } + @Test fun `CodeSubmit should emit new code and NavigateBack`() = runTest { - val viewModel = createViewModel(initialState = ManualCodeEntryState(" TestCode ")) + val viewModel = createViewModel(initialState = DEFAULT_STATE.copy(code = " TestCode ")) viewModel.eventFlow.test { viewModel.trySendAction(ManualCodeEntryAction.CodeSubmit) @@ -50,20 +73,29 @@ class ManualCodeEntryViewModelTests : BaseViewModelTest() { @Test fun `CodeTextChange should update state with new value`() = runTest { - val viewModel = - createViewModel(initialState = ManualCodeEntryState("TestCode")) + val viewModel = createViewModel(initialState = DEFAULT_STATE.copy(code = "TestCode")) - val expectedState = ManualCodeEntryState("NewCode") + val expectedState = DEFAULT_STATE.copy(code = "NewCode") viewModel.trySendAction(ManualCodeEntryAction.CodeTextChange("NewCode")) assertEquals(expectedState, viewModel.stateFlow.value) } @Test - fun `SettingsClick should emit NavigateToAppSettings and update state`() = runTest { - val viewModel = createViewModel(initialState = ManualCodeEntryState("")) + fun `DialogDismiss should clear the dialog state`() = runTest { + val viewModel = createViewModel() - val expectedState = ManualCodeEntryState("") + viewModel.eventFlow.test { + viewModel.trySendAction(ManualCodeEntryAction.CloseClick) + assertEquals(ManualCodeEntryEvent.NavigateBack, awaitItem()) + } + } + + @Test + fun `SettingsClick should emit NavigateToAppSettings and update state`() = runTest { + val viewModel = createViewModel() + + val expectedState = DEFAULT_STATE viewModel.eventFlow.test { viewModel.trySendAction(ManualCodeEntryAction.SettingsClick) @@ -75,7 +107,7 @@ class ManualCodeEntryViewModelTests : BaseViewModelTest() { @Test fun `ScanQrTextCodeClick should emit NavigateToQrCodeScreen`() = runTest { - val viewModel = createViewModel(initialState = ManualCodeEntryState("")) + val viewModel = createViewModel() viewModel.eventFlow.test { viewModel.trySendAction(ManualCodeEntryAction.ScanQrCodeTextClick) @@ -84,7 +116,9 @@ class ManualCodeEntryViewModelTests : BaseViewModelTest() { } } - private fun createViewModel(initialState: ManualCodeEntryState): ManualCodeEntryViewModel = + private fun createViewModel( + initialState: ManualCodeEntryState? = null, + ): ManualCodeEntryViewModel = ManualCodeEntryViewModel( vaultRepository = vaultRepository, savedStateHandle = SavedStateHandle( @@ -92,3 +126,8 @@ class ManualCodeEntryViewModelTests : BaseViewModelTest() { ), ) } + +private val DEFAULT_STATE: ManualCodeEntryState = ManualCodeEntryState( + code = "", + dialog = null, +)