From a95a07f313ad4facbf49ea08f108777b27511c5f Mon Sep 17 00:00:00 2001 From: Andrew Haisting <142518658+ahaisting-livefront@users.noreply.github.com> Date: Thu, 5 Oct 2023 17:23:04 -0500 Subject: [PATCH] BIT-193 Implement password length validation on create account (#96) --- .../createaccount/CreateAccountScreen.kt | 10 ++- .../createaccount/CreateAccountViewModel.kt | 34 +++++++-- .../components/BitwardenBasicDialog.kt | 72 +++++++++++++++++++ .../createaccount/CreateAccountScreenTest.kt | 56 ++++++++++++++- .../CreateAccountViewModelTest.kt | 31 ++++++-- 5 files changed, 189 insertions(+), 14 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreen.kt index 0329f0de9..32870f8fc 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreen.kt @@ -21,9 +21,11 @@ import androidx.hilt.navigation.compose.hiltViewModel import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.ConfirmPasswordInputChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.EmailInputChange +import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.ErrorDialogDismiss import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.PasswordHintChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.PasswordInputChange import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect +import com.x8bit.bitwarden.ui.platform.components.BitwardenBasicDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenFilledButton import com.x8bit.bitwarden.ui.platform.components.BitwardenPasswordField import com.x8bit.bitwarden.ui.platform.components.BitwardenTextButtonTopAppBar @@ -48,6 +50,10 @@ fun CreateAccountScreen( } } } + BitwardenBasicDialog( + visibilityState = state.errorDialogState, + onDismissRequest = remember(viewModel) { { viewModel.trySendAction(ErrorDialogDismiss) } }, + ) Column( modifier = Modifier @@ -65,7 +71,7 @@ fun CreateAccountScreen( onButtonClick = remember(viewModel) { { viewModel.trySendAction(CreateAccountAction.SubmitClick) } }, - isButtonEnabled = state.isSubmitEnabled, + isButtonEnabled = true, ) Column( modifier = Modifier.padding(horizontal = 16.dp), @@ -104,7 +110,7 @@ fun CreateAccountScreen( BitwardenFilledButton( label = stringResource(id = R.string.submit), onClick = remember { { viewModel.trySendAction(CreateAccountAction.SubmitClick) } }, - isEnabled = state.isSubmitEnabled, + isEnabled = true, modifier = Modifier.fillMaxWidth(), ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModel.kt index 28b270c3e..eec864546 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModel.kt @@ -3,12 +3,15 @@ package com.x8bit.bitwarden.ui.auth.feature.createaccount import android.os.Parcelable import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.ConfirmPasswordInputChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.EmailInputChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.PasswordHintChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.PasswordInputChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.SubmitClick import com.x8bit.bitwarden.ui.platform.base.BaseViewModel +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.components.BasicDialogState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -17,6 +20,7 @@ import kotlinx.parcelize.Parcelize import javax.inject.Inject private const val KEY_STATE = "state" +private const val MIN_PASSWORD_LENGTH = 12 /** * Models logic for the create account screen. @@ -31,7 +35,7 @@ class CreateAccountViewModel @Inject constructor( passwordInput = "", confirmPasswordInput = "", passwordHintInput = "", - isSubmitEnabled = false, + errorDialogState = BasicDialogState.Hidden, ), ) { @@ -50,6 +54,13 @@ class CreateAccountViewModel @Inject constructor( is PasswordHintChange -> handlePasswordHintChanged(action) is PasswordInputChange -> handlePasswordInputChanged(action) is CreateAccountAction.CloseClick -> handleCloseClick() + is CreateAccountAction.ErrorDialogDismiss -> handleDialogDismiss() + } + } + + private fun handleDialogDismiss() { + mutableStateFlow.update { + it.copy(errorDialogState = BasicDialogState.Hidden) } } @@ -73,8 +84,18 @@ class CreateAccountViewModel @Inject constructor( mutableStateFlow.update { it.copy(confirmPasswordInput = action.input) } } - private fun handleSubmitClick() { - sendEvent(CreateAccountEvent.ShowToast("TODO: Handle Submit Click")) + private fun handleSubmitClick() = when { + mutableStateFlow.value.passwordInput.length < MIN_PASSWORD_LENGTH -> { + val dialog = BasicDialogState.Shown( + title = R.string.an_error_has_occurred.asText(), + message = R.string.master_password_length_val_message_x.asText(MIN_PASSWORD_LENGTH), + ) + mutableStateFlow.update { it.copy(errorDialogState = dialog) } + } + + else -> { + sendEvent(CreateAccountEvent.ShowToast("TODO: Handle Submit Click")) + } } } @@ -87,7 +108,7 @@ data class CreateAccountState( val passwordInput: String, val confirmPasswordInput: String, val passwordHintInput: String, - val isSubmitEnabled: Boolean, + val errorDialogState: BasicDialogState, ) : Parcelable /** @@ -139,4 +160,9 @@ sealed class CreateAccountAction { * Password hint input changed. */ data class PasswordHintChange(val input: String) : CreateAccountAction() + + /** + * User dismissed the error dialog. + */ + data object ErrorDialogDismiss : CreateAccountAction() } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt new file mode 100644 index 000000000..c6637a703 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenBasicDialog.kt @@ -0,0 +1,72 @@ +package com.x8bit.bitwarden.ui.platform.components + +import android.os.Parcelable +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.res.stringResource +import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.ui.platform.base.util.Text +import kotlinx.parcelize.Parcelize + +/** + * Represents a Bitwarden-styled dialog that is hidden or shown based on [visibilityState]. + * + * @param visibilityState the [BasicDialogState] used to populate the dialog. + * @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. + */ +@Composable +fun BitwardenBasicDialog( + visibilityState: BasicDialogState, + onDismissRequest: () -> Unit, +): Unit = when (visibilityState) { + BasicDialogState.Hidden -> Unit + is BasicDialogState.Shown -> { + AlertDialog( + onDismissRequest = onDismissRequest, + confirmButton = { + BitwardenTextButton( + label = stringResource(id = R.string.ok), + onClick = onDismissRequest, + ) + }, + title = { + Text( + text = visibilityState.title(), + color = MaterialTheme.colorScheme.onSurface, + style = MaterialTheme.typography.headlineSmall, + ) + }, + text = { + Text( + text = visibilityState.message(), + color = MaterialTheme.colorScheme.onSurfaceVariant, + style = MaterialTheme.typography.bodyMedium, + ) + }, + ) + } +} + +/** + * Models display of a [BitwardenBasicDialog]. + */ +sealed class BasicDialogState : Parcelable { + + /** + * Hide the dialog. + */ + @Parcelize + data object Hidden : BasicDialogState() + + /** + * Show the dialog with the given values. + */ + @Parcelize + data class Shown( + val title: Text, + val message: Text, + ) : BasicDialogState() +} diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreenTest.kt index 7be21e04e..bed96beac 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountScreenTest.kt @@ -1,5 +1,9 @@ package com.x8bit.bitwarden.ui.auth.feature.createaccount +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.filterToOne +import androidx.compose.ui.test.hasAnyAncestor +import androidx.compose.ui.test.isDialog import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText @@ -12,6 +16,8 @@ import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.Pas import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.PasswordInputChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.SubmitClick import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.components.BasicDialogState import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -25,7 +31,7 @@ class CreateAccountScreenTest : BaseComposeTest() { @Test fun `app bar submit click should send SubmitClick action`() { val viewModel = mockk(relaxed = true) { - every { stateFlow } returns MutableStateFlow(DEFAULT_STATE.copy(isSubmitEnabled = true)) + every { stateFlow } returns MutableStateFlow(DEFAULT_STATE) every { eventFlow } returns emptyFlow() every { trySendAction(SubmitClick) } returns Unit } @@ -39,7 +45,7 @@ class CreateAccountScreenTest : BaseComposeTest() { @Test fun `bottom button submit click should send SubmitClick action`() { val viewModel = mockk(relaxed = true) { - every { stateFlow } returns MutableStateFlow(DEFAULT_STATE.copy(isSubmitEnabled = true)) + every { stateFlow } returns MutableStateFlow(DEFAULT_STATE) every { eventFlow } returns emptyFlow() every { trySendAction(SubmitClick) } returns Unit } @@ -136,6 +142,50 @@ class CreateAccountScreenTest : BaseComposeTest() { verify { viewModel.trySendAction(PasswordHintChange(TEST_INPUT)) } } + @Test + fun `clicking OK on the error dialog should send ErrorDialogDismiss action`() { + val viewModel = mockk(relaxed = true) { + every { stateFlow } returns MutableStateFlow( + DEFAULT_STATE.copy( + errorDialogState = BasicDialogState.Shown( + title = "title".asText(), + message = "message".asText(), + ), + ), + ) + every { eventFlow } returns emptyFlow() + every { trySendAction(CreateAccountAction.ErrorDialogDismiss) } returns Unit + } + composeTestRule.setContent { + CreateAccountScreen(onNavigateBack = {}, viewModel = viewModel) + } + composeTestRule + .onAllNodesWithText("Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + verify { viewModel.trySendAction(CreateAccountAction.ErrorDialogDismiss) } + } + + @Test + fun `when BasicDialogState is Shown should show dialog`() { + val viewModel = mockk(relaxed = true) { + every { stateFlow } returns MutableStateFlow( + DEFAULT_STATE.copy( + errorDialogState = BasicDialogState.Shown( + title = "title".asText(), + message = "message".asText(), + ), + ), + ) + every { eventFlow } returns emptyFlow() + every { trySendAction(CreateAccountAction.ErrorDialogDismiss) } returns Unit + } + composeTestRule.setContent { + CreateAccountScreen(onNavigateBack = {}, viewModel = viewModel) + } + composeTestRule.onNode(isDialog()).assertIsDisplayed() + } + companion object { private const val TEST_INPUT = "input" private val DEFAULT_STATE = CreateAccountState( @@ -143,7 +193,7 @@ class CreateAccountScreenTest : BaseComposeTest() { passwordInput = "", confirmPasswordInput = "", passwordHintInput = "", - isSubmitEnabled = false, + errorDialogState = BasicDialogState.Hidden, ) } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModelTest.kt index 601c34a9b..04c1de9e8 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/createaccount/CreateAccountViewModelTest.kt @@ -2,13 +2,15 @@ package com.x8bit.bitwarden.ui.auth.feature.createaccount import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.CloseClick import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.ConfirmPasswordInputChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.EmailInputChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.PasswordHintChange import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.PasswordInputChange -import com.x8bit.bitwarden.ui.auth.feature.createaccount.CreateAccountAction.SubmitClick import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.components.BasicDialogState import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test @@ -28,7 +30,7 @@ class CreateAccountViewModelTest : BaseViewModelTest() { passwordInput = "password", confirmPasswordInput = "confirmPassword", passwordHintInput = "hint", - isSubmitEnabled = false, + errorDialogState = BasicDialogState.Hidden, ) val handle = SavedStateHandle(mapOf("state" to savedState)) val viewModel = CreateAccountViewModel(handle) @@ -36,10 +38,29 @@ class CreateAccountViewModelTest : BaseViewModelTest() { } @Test - fun `SubmitClick should emit ShowToast`() = runTest { + fun `SubmitClick with password below 12 chars should show password length dialog`() = runTest { val viewModel = CreateAccountViewModel(SavedStateHandle()) + val input = "abcdefghikl" + viewModel.trySendAction(PasswordInputChange("abcdefghikl")) + val expectedState = DEFAULT_STATE.copy( + passwordInput = input, + errorDialogState = BasicDialogState.Shown( + title = R.string.an_error_has_occurred.asText(), + message = R.string.master_password_length_val_message_x.asText(12), + ), + ) + viewModel.actionChannel.trySend(CreateAccountAction.SubmitClick) + viewModel.stateFlow.test { + assertEquals(expectedState, awaitItem()) + } + } + + @Test + fun `SubmitClick with long enough password emit ShowToast`() = runTest { + val viewModel = CreateAccountViewModel(SavedStateHandle()) + viewModel.trySendAction(PasswordInputChange("longenoughpassword")) viewModel.eventFlow.test { - viewModel.actionChannel.trySend(SubmitClick) + viewModel.actionChannel.trySend(CreateAccountAction.SubmitClick) assert(awaitItem() is CreateAccountEvent.ShowToast) } } @@ -95,7 +116,7 @@ class CreateAccountViewModelTest : BaseViewModelTest() { emailInput = "", confirmPasswordInput = "", passwordHintInput = "", - isSubmitEnabled = false, + errorDialogState = BasicDialogState.Hidden, ) } }