From e3175afc1350d16888f1f4b614951edb040df57a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montwe=CC=81?= Date: Thu, 1 Jun 2023 16:51:12 +0200 Subject: [PATCH 1/3] Add validation use case definition and concrete implementation for server, port, username and password --- .../setup/domain/usecase/ValidatePassword.kt | 21 ++++++++ .../setup/domain/usecase/ValidatePort.kt | 27 ++++++++++ .../setup/domain/usecase/ValidateServer.kt | 20 +++++++ .../setup/domain/usecase/ValidateUsername.kt | 20 +++++++ .../domain/usecase/ValidatePasswordTest.kt | 41 ++++++++++++++ .../setup/domain/usecase/ValidatePortTest.kt | 53 +++++++++++++++++++ .../domain/usecase/ValidateServerTest.kt | 41 ++++++++++++++ .../domain/usecase/ValidateUsernameTest.kt | 41 ++++++++++++++ 8 files changed, 264 insertions(+) create mode 100644 feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePassword.kt create mode 100644 feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt create mode 100644 feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServer.kt create mode 100644 feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsername.kt create mode 100644 feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePasswordTest.kt create mode 100644 feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt create mode 100644 feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServerTest.kt create mode 100644 feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsernameTest.kt diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePassword.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePassword.kt new file mode 100644 index 000000000..277cd2b6d --- /dev/null +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePassword.kt @@ -0,0 +1,21 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationError +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import app.k9mail.core.common.domain.usecase.validation.ValidationUseCase + +class ValidatePassword : ValidationUseCase { + + // TODO change behavior to allow empty password when no password is required based on auth type + override fun execute(input: String): ValidationResult { + return when { + input.isBlank() -> ValidationResult.Failure(ValidatePasswordError.EmptyPassword) + + else -> ValidationResult.Success + } + } + + sealed interface ValidatePasswordError : ValidationError { + object EmptyPassword : ValidatePasswordError + } +} diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt new file mode 100644 index 000000000..b8f061836 --- /dev/null +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt @@ -0,0 +1,27 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationError +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import app.k9mail.core.common.domain.usecase.validation.ValidationUseCase + +class ValidatePort : ValidationUseCase { + + override fun execute(input: Long?): ValidationResult { + return when { + input == null -> ValidationResult.Failure(ValidatePortError.EmptyPort) + input < MIN_PORT_NUMBER -> ValidationResult.Failure(ValidatePortError.InvalidPort) + input > MAX_PORT_NUMBER -> ValidationResult.Failure(ValidatePortError.InvalidPort) + else -> ValidationResult.Success + } + } + + sealed interface ValidatePortError : ValidationError { + object EmptyPort : ValidatePortError + object InvalidPort : ValidatePortError + } + + companion object { + const val MAX_PORT_NUMBER = 65535 + const val MIN_PORT_NUMBER = 0 + } +} diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServer.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServer.kt new file mode 100644 index 000000000..8a6a17935 --- /dev/null +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServer.kt @@ -0,0 +1,20 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationError +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import app.k9mail.core.common.domain.usecase.validation.ValidationUseCase + +class ValidateServer : ValidationUseCase { + + // TODO validate domain, ip4 or ip6 + override fun execute(input: String): ValidationResult { + return when { + input.isBlank() -> ValidationResult.Failure(ValidateServerError.EmptyServer) + else -> ValidationResult.Success + } + } + + sealed interface ValidateServerError : ValidationError { + object EmptyServer : ValidateServerError + } +} diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsername.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsername.kt new file mode 100644 index 000000000..cc23efe93 --- /dev/null +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsername.kt @@ -0,0 +1,20 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationError +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import app.k9mail.core.common.domain.usecase.validation.ValidationUseCase + +class ValidateUsername : ValidationUseCase { + + override fun execute(input: String): ValidationResult { + return when { + input.isBlank() -> ValidationResult.Failure(ValidateUsernameError.EmptyUsername) + + else -> ValidationResult.Success + } + } + + sealed interface ValidateUsernameError : ValidationError { + object EmptyUsername : ValidateUsernameError + } +} diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePasswordTest.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePasswordTest.kt new file mode 100644 index 000000000..977eea300 --- /dev/null +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePasswordTest.kt @@ -0,0 +1,41 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import assertk.assertThat +import assertk.assertions.isInstanceOf +import assertk.assertions.prop +import org.junit.Test + +class ValidatePasswordTest { + + @Test + fun `should succeed when password is set`() { + val useCase = ValidatePassword() + + val result = useCase.execute("password") + + assertThat(result).isInstanceOf(ValidationResult.Success::class) + } + + @Test + fun `should fail when password is empty`() { + val useCase = ValidatePassword() + + val result = useCase.execute("") + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidatePassword.ValidatePasswordError.EmptyPassword::class) + } + + @Test + fun `should fail when password is blank`() { + val useCase = ValidatePassword() + + val result = useCase.execute(" ") + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidatePassword.ValidatePasswordError.EmptyPassword::class) + } +} diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt new file mode 100644 index 000000000..1f399330c --- /dev/null +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt @@ -0,0 +1,53 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import app.k9mail.feature.account.setup.domain.usecase.ValidatePort.ValidatePortError +import assertk.assertThat +import assertk.assertions.isInstanceOf +import assertk.assertions.prop +import org.junit.Test + +class ValidatePortTest { + + @Test + fun `should succeed when port is set`() { + val useCase = ValidatePort() + + val result = useCase.execute(123L) + + assertThat(result).isInstanceOf(ValidationResult.Success::class) + } + + @Test + fun `should fail when port is negative`() { + val useCase = ValidatePort() + + val result = useCase.execute(-1L) + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidatePortError.InvalidPort::class) + } + + @Test + fun `should fail when port exceeds maximum`() { + val useCase = ValidatePort() + + val result = useCase.execute(65536L) + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidatePortError.InvalidPort::class) + } + + @Test + fun `should fail when port is null`() { + val useCase = ValidatePort() + + val result = useCase.execute(null) + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidatePortError.EmptyPort::class) + } +} diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServerTest.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServerTest.kt new file mode 100644 index 000000000..3857152a5 --- /dev/null +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateServerTest.kt @@ -0,0 +1,41 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import assertk.assertThat +import assertk.assertions.isInstanceOf +import assertk.assertions.prop +import org.junit.Test + +class ValidateServerTest { + + @Test + fun `should succeed when server is set`() { + val useCase = ValidateServer() + + val result = useCase.execute("server") + + assertThat(result).isInstanceOf(ValidationResult.Success::class) + } + + @Test + fun `should fail when server is empty`() { + val useCase = ValidateServer() + + val result = useCase.execute("") + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidateServer.ValidateServerError.EmptyServer::class) + } + + @Test + fun `should fail when server is blank`() { + val useCase = ValidateServer() + + val result = useCase.execute(" ") + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidateServer.ValidateServerError.EmptyServer::class) + } +} diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsernameTest.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsernameTest.kt new file mode 100644 index 000000000..ffbf13330 --- /dev/null +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidateUsernameTest.kt @@ -0,0 +1,41 @@ +package app.k9mail.feature.account.setup.domain.usecase + +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import assertk.assertThat +import assertk.assertions.isInstanceOf +import assertk.assertions.prop +import org.junit.Test + +class ValidateUsernameTest { + + @Test + fun `should succeed when username is set`() { + val useCase = ValidateUsername() + + val result = useCase.execute("username") + + assertThat(result).isInstanceOf(ValidationResult.Success::class) + } + + @Test + fun `should fail when username is empty`() { + val useCase = ValidateUsername() + + val result = useCase.execute("") + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidateUsername.ValidateUsernameError.EmptyUsername::class) + } + + @Test + fun `should fail when username is blank`() { + val useCase = ValidateUsername() + + val result = useCase.execute(" ") + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidateUsername.ValidateUsernameError.EmptyUsername::class) + } +} From 3d311b554fa84c5f0174c4ffdfc0bea3974be20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montwe=CC=81?= Date: Fri, 2 Jun 2023 16:29:15 +0200 Subject: [PATCH 2/3] Add AccountOutgoingConfig validation --- .../account/setup/AccountSetupModule.kt | 9 +- .../outgoing/AccountOutgoingConfigContent.kt | 8 ++ .../outgoing/AccountOutgoingConfigContract.kt | 9 ++ .../outgoing/AccountOutgoingConfigScreen.kt | 10 +- .../AccountOutgoingConfigValidator.kt | 30 ++++++ .../AccountOutgoingConfigViewModel.kt | 26 ++++- .../outgoing/AccountOutgoingStringMapper.kt | 51 ++++++++++ .../setup/src/main/res/values/strings.xml | 7 +- .../AccountOutgoingConfigViewModelTest.kt | 97 ++++++++++++++----- .../FakeAccountOutgoingConfigValidator.kt | 15 +++ 10 files changed, 233 insertions(+), 29 deletions(-) create mode 100644 feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigValidator.kt create mode 100644 feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/FakeAccountOutgoingConfigValidator.kt diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/AccountSetupModule.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/AccountSetupModule.kt index 2a4fba3bf..e92f1d53c 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/AccountSetupModule.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/AccountSetupModule.kt @@ -4,16 +4,23 @@ import app.k9mail.feature.account.setup.ui.AccountSetupViewModel import app.k9mail.feature.account.setup.ui.options.AccountOptionsContract import app.k9mail.feature.account.setup.ui.options.AccountOptionsValidator import app.k9mail.feature.account.setup.ui.options.AccountOptionsViewModel +import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract +import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigValidator import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigViewModel import org.koin.androidx.viewmodel.dsl.viewModel import org.koin.core.module.Module import org.koin.dsl.module val featureAccountSetupModule: Module = module { + factory { AccountOutgoingConfigValidator() } factory { AccountOptionsValidator() } viewModel { AccountSetupViewModel() } - viewModel { AccountOutgoingConfigViewModel() } + viewModel { + AccountOutgoingConfigViewModel( + validator = get(), + ) + } viewModel { AccountOptionsViewModel( validator = get(), diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContent.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContent.kt index 8b47f03df..90047b622 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContent.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContent.kt @@ -63,8 +63,10 @@ internal fun AccountOutgoingConfigContent( item { TextInput( text = state.server.value, + errorMessage = state.server.error?.toResourceString(resources), onTextChange = { onEvent(Event.ServerChanged(it)) }, label = stringResource(id = R.string.account_setup_outgoing_config_server_label), + isRequired = true, contentPadding = defaultItemPadding(), ) } @@ -83,8 +85,10 @@ internal fun AccountOutgoingConfigContent( item { NumberInput( value = state.port.value, + errorMessage = state.port.error?.toResourceString(resources), onValueChange = { onEvent(Event.PortChanged(it)) }, label = stringResource(id = R.string.account_setup_outgoing_config_port_label), + isRequired = true, contentPadding = defaultItemPadding(), ) } @@ -92,8 +96,10 @@ internal fun AccountOutgoingConfigContent( item { TextInput( text = state.username.value, + errorMessage = state.username.error?.toResourceString(resources), onTextChange = { onEvent(Event.UsernameChanged(it)) }, label = stringResource(id = R.string.account_setup_outgoing_config_username_label), + isRequired = true, contentPadding = defaultItemPadding(), ) } @@ -101,7 +107,9 @@ internal fun AccountOutgoingConfigContent( item { PasswordInput( password = state.password.value, + errorMessage = state.password.error?.toResourceString(resources), onPasswordChange = { onEvent(Event.PasswordChanged(it)) }, + isRequired = true, contentPadding = defaultItemPadding(), ) } diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContract.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContract.kt index 9ff88faa0..9568f65e0 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContract.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigContract.kt @@ -1,5 +1,6 @@ package app.k9mail.feature.account.setup.ui.outgoing +import app.k9mail.core.common.domain.usecase.validation.ValidationResult import app.k9mail.core.ui.compose.common.mvi.UnidirectionalViewModel import app.k9mail.feature.account.setup.domain.entity.ConnectionSecurity import app.k9mail.feature.account.setup.domain.entity.toSmtpDefaultPort @@ -40,4 +41,12 @@ interface AccountOutgoingConfigContract { object NavigateNext : Effect() object NavigateBack : Effect() } + + interface Validator { + fun validateServer(server: String): ValidationResult + + fun validatePort(port: Long?): ValidationResult + fun validateUsername(username: String): ValidationResult + fun validatePassword(password: String): ValidationResult + } } diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigScreen.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigScreen.kt index 034f1d90c..19d171492 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigScreen.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigScreen.kt @@ -27,7 +27,7 @@ fun AccountOutgoingConfigScreen( Effect.NavigateBack -> onBack() Effect.NavigateNext -> onNext() } - } + } Scaffold( topBar = { @@ -60,7 +60,9 @@ internal fun AccountOutgoingConfigScreenK9Preview() { AccountOutgoingConfigScreen( onNext = {}, onBack = {}, - viewModel = AccountOutgoingConfigViewModel(), + viewModel = AccountOutgoingConfigViewModel( + validator = AccountOutgoingConfigValidator(), + ), ) } } @@ -72,7 +74,9 @@ internal fun AccountOutgoingConfigScreenThunderbirdPreview() { AccountOutgoingConfigScreen( onNext = {}, onBack = {}, - viewModel = AccountOutgoingConfigViewModel(), + viewModel = AccountOutgoingConfigViewModel( + validator = AccountOutgoingConfigValidator(), + ), ) } } diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigValidator.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigValidator.kt new file mode 100644 index 000000000..2322d488c --- /dev/null +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigValidator.kt @@ -0,0 +1,30 @@ +package app.k9mail.feature.account.setup.ui.outgoing + +import app.k9mail.core.common.domain.usecase.validation.ValidationResult +import app.k9mail.feature.account.setup.domain.usecase.ValidatePassword +import app.k9mail.feature.account.setup.domain.usecase.ValidatePort +import app.k9mail.feature.account.setup.domain.usecase.ValidateServer +import app.k9mail.feature.account.setup.domain.usecase.ValidateUsername + +class AccountOutgoingConfigValidator( + private val serverValidator: ValidateServer = ValidateServer(), + private val portValidator: ValidatePort = ValidatePort(), + private val usernameValidator: ValidateUsername = ValidateUsername(), + private val passwordValidator: ValidatePassword = ValidatePassword(), +) : AccountOutgoingConfigContract.Validator { + override fun validateServer(server: String): ValidationResult { + return serverValidator.execute(server) + } + + override fun validatePort(port: Long?): ValidationResult { + return portValidator.execute(port) + } + + override fun validateUsername(username: String): ValidationResult { + return usernameValidator.execute(username) + } + + override fun validatePassword(password: String): ValidationResult { + return passwordValidator.execute(password) + } +} diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModel.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModel.kt index 3efc5e575..e4ffe39f0 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModel.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModel.kt @@ -1,5 +1,6 @@ package app.k9mail.feature.account.setup.ui.outgoing +import app.k9mail.core.common.domain.usecase.validation.ValidationResult import app.k9mail.core.ui.compose.common.mvi.BaseViewModel import app.k9mail.feature.account.setup.domain.entity.ConnectionSecurity import app.k9mail.feature.account.setup.domain.entity.toSmtpDefaultPort @@ -16,10 +17,12 @@ import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContrac import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.Event.UseCompressionChanged import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.Event.UsernameChanged import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.State +import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.Validator import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.ViewModel class AccountOutgoingConfigViewModel( initialState: State = State(), + private val validator: Validator, ) : BaseViewModel(initialState), ViewModel { override fun initState(state: State) { @@ -56,8 +59,27 @@ class AccountOutgoingConfigViewModel( } } - private fun submit() { - navigateNext() + private fun submit() = with(state.value) { + val serverResult = validator.validateServer(server.value) + val portResult = validator.validatePort(port.value) + val usernameResult = validator.validateUsername(username.value) + val passwordResult = validator.validatePassword(password.value) + + val hasError = listOf(serverResult, portResult, usernameResult, passwordResult) + .any { it is ValidationResult.Failure } + + updateState { + it.copy( + server = it.server.updateFromValidationResult(serverResult), + port = it.port.updateFromValidationResult(portResult), + username = it.username.updateFromValidationResult(usernameResult), + password = it.password.updateFromValidationResult(passwordResult), + ) + } + + if (!hasError) { + navigateNext() + } } private fun navigateBack() = emitEffect(Effect.NavigateBack) diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingStringMapper.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingStringMapper.kt index 23ed5d1aa..258bfd68b 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingStringMapper.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingStringMapper.kt @@ -1,8 +1,13 @@ package app.k9mail.feature.account.setup.ui.outgoing import android.content.res.Resources +import app.k9mail.core.common.domain.usecase.validation.ValidationError import app.k9mail.feature.account.setup.R import app.k9mail.feature.account.setup.domain.entity.ConnectionSecurity +import app.k9mail.feature.account.setup.domain.usecase.ValidatePassword.ValidatePasswordError +import app.k9mail.feature.account.setup.domain.usecase.ValidatePort.ValidatePortError +import app.k9mail.feature.account.setup.domain.usecase.ValidateServer.ValidateServerError +import app.k9mail.feature.account.setup.domain.usecase.ValidateUsername.ValidateUsernameError internal fun ConnectionSecurity.toResourceString(resources: Resources): String { return when (this) { @@ -11,3 +16,49 @@ internal fun ConnectionSecurity.toResourceString(resources: Resources): String { ConnectionSecurity.TLS -> resources.getString(R.string.account_setup_outgoing_config_security_ssl) } } + +internal fun ValidationError.toResourceString(resources: Resources): String { + return when (this) { + is ValidateServerError -> toServerErrorString(resources) + is ValidatePortError -> toPortErrorString(resources) + is ValidateUsernameError -> toUsernameErrorString(resources) + is ValidatePasswordError -> toPasswordErrorString(resources) + else -> throw IllegalArgumentException("Unknown error: $this") + } +} + +private fun ValidateServerError.toServerErrorString(resources: Resources): String { + return when (this) { + is ValidateServerError.EmptyServer -> resources.getString( + R.string.account_setup_outgoing_config_server_error_required, + ) + } +} + +private fun ValidatePortError.toPortErrorString(resources: Resources): String { + return when (this) { + is ValidatePortError.EmptyPort -> resources.getString( + R.string.account_setup_outgoing_config_port_error_required, + ) + + is ValidatePortError.InvalidPort -> resources.getString( + R.string.account_setup_outgoing_config_port_error_invalid, + ) + } +} + +private fun ValidateUsernameError.toUsernameErrorString(resources: Resources): String { + return when (this) { + ValidateUsernameError.EmptyUsername -> resources.getString( + R.string.account_setup_outgoing_config_username_error_required, + ) + } +} + +private fun ValidatePasswordError.toPasswordErrorString(resources: Resources): String { + return when (this) { + ValidatePasswordError.EmptyPassword -> resources.getString( + R.string.account_setup_outgoing_config_password_error_required, + ) + } +} diff --git a/feature/account/setup/src/main/res/values/strings.xml b/feature/account/setup/src/main/res/values/strings.xml index 871d63dd5..a08ff2dc9 100644 --- a/feature/account/setup/src/main/res/values/strings.xml +++ b/feature/account/setup/src/main/res/values/strings.xml @@ -11,18 +11,23 @@ Outgoing server settings Server + Server name is required. Security None SSL/TLS StartTLS Port + Port is required. + Port is invalid (0–65535). Username + Username is required. + Password is required. Client certificate None available Auto-detect IMAP namespace Use compression - Account options + Account options Display options Account name Account name can\'t be blank. diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModelTest.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModelTest.kt index 9aaa76df2..861e5bb7d 100644 --- a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModelTest.kt +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/AccountOutgoingConfigViewModelTest.kt @@ -1,6 +1,8 @@ package app.k9mail.feature.account.setup.ui.outgoing import app.cash.turbine.testIn +import app.k9mail.core.common.domain.usecase.validation.ValidationError +import app.k9mail.core.common.domain.usecase.validation.ValidationResult import app.k9mail.core.ui.compose.testing.MainDispatcherRule import app.k9mail.feature.account.setup.domain.entity.ConnectionSecurity import app.k9mail.feature.account.setup.domain.entity.toSmtpDefaultPort @@ -10,6 +12,7 @@ import app.k9mail.feature.account.setup.testing.eventStateTest import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.Effect import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.Event import app.k9mail.feature.account.setup.ui.outgoing.AccountOutgoingConfigContract.State +import assertk.assertThat import assertk.assertions.assertThatAndTurbinesConsumed import assertk.assertions.isEqualTo import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -23,7 +26,9 @@ class AccountOutgoingConfigViewModelTest { @get:Rule val mainDispatcherRule = MainDispatcherRule() - private val testSubject = AccountOutgoingConfigViewModel() + private val testSubject = AccountOutgoingConfigViewModel( + validator = FakeAccountOutgoingConfigValidator(), + ) @Test fun `should change state when ServerChanged event is received`() = runTest { @@ -117,31 +122,77 @@ class AccountOutgoingConfigViewModelTest { } @Test - fun `should emit NavigateNext effect when OnNextClicked event is received`() = runTest { - val viewModel = testSubject - val stateTurbine = viewModel.state.testIn(backgroundScope) - val effectTurbine = viewModel.effect.testIn(backgroundScope) - val turbines = listOf(stateTurbine, effectTurbine) + fun `should change state and emit NavigateNext effect when OnNextClicked event is received and input is valid`() = + runTest { + val viewModel = testSubject + val stateTurbine = viewModel.state.testIn(backgroundScope) + val effectTurbine = viewModel.effect.testIn(backgroundScope) + val turbines = listOf(stateTurbine, effectTurbine) - assertThatAndTurbinesConsumed( - actual = stateTurbine.awaitItem(), - turbines = turbines, - ) { - isEqualTo(State()) + assertThatAndTurbinesConsumed( + actual = stateTurbine.awaitItem(), + turbines = turbines, + ) { + isEqualTo(State()) + } + + viewModel.event(Event.OnNextClicked) + + assertThat(stateTurbine.awaitItem()).isEqualTo( + State( + server = StringInputField(value = "", isValid = true), + port = NumberInputField(value = 465L, isValid = true), + username = StringInputField(value = "", isValid = true), + password = StringInputField(value = "", isValid = true), + ), + ) + + assertThatAndTurbinesConsumed( + actual = effectTurbine.awaitItem(), + turbines = turbines, + ) { + isEqualTo(Effect.NavigateNext) + } } - viewModel.event(Event.OnNextClicked) - - assertThatAndTurbinesConsumed( - actual = effectTurbine.awaitItem(), - turbines = turbines, - ) { - isEqualTo(Effect.NavigateNext) - } - } - @Test - fun `should emit NavigateBack effect when OnBackClicked event is received`() = runTest { + fun `should change state and not emit NavigateNext effect when OnNextClicked event received and input invalid`() = + runTest { + val viewModel = AccountOutgoingConfigViewModel( + validator = FakeAccountOutgoingConfigValidator( + serverAnswer = ValidationResult.Failure(TestError), + ), + ) + val stateTurbine = viewModel.state.testIn(backgroundScope) + val effectTurbine = viewModel.effect.testIn(backgroundScope) + val turbines = listOf(stateTurbine, effectTurbine) + + assertThatAndTurbinesConsumed( + actual = stateTurbine.awaitItem(), + turbines = turbines, + ) { + isEqualTo(State()) + } + + viewModel.event(Event.OnNextClicked) + + assertThatAndTurbinesConsumed( + actual = stateTurbine.awaitItem(), + turbines = turbines, + ) { + isEqualTo( + State( + server = StringInputField(value = "", error = TestError, isValid = false), + port = NumberInputField(value = 465L, isValid = true), + username = StringInputField(value = "", isValid = true), + password = StringInputField(value = "", isValid = true), + ), + ) + } + } + + @Test + fun `should emit NavigateBack effect when OnBackClicked event received`() = runTest { val viewModel = testSubject val stateTurbine = viewModel.state.testIn(backgroundScope) val effectTurbine = viewModel.effect.testIn(backgroundScope) @@ -163,4 +214,6 @@ class AccountOutgoingConfigViewModelTest { isEqualTo(Effect.NavigateBack) } } + + private object TestError : ValidationError } diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/FakeAccountOutgoingConfigValidator.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/FakeAccountOutgoingConfigValidator.kt new file mode 100644 index 000000000..5c342e995 --- /dev/null +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/ui/outgoing/FakeAccountOutgoingConfigValidator.kt @@ -0,0 +1,15 @@ +package app.k9mail.feature.account.setup.ui.outgoing + +import app.k9mail.core.common.domain.usecase.validation.ValidationResult + +class FakeAccountOutgoingConfigValidator( + private val serverAnswer: ValidationResult = ValidationResult.Success, + private val portAnswer: ValidationResult = ValidationResult.Success, + private val usernameAnswer: ValidationResult = ValidationResult.Success, + private val passwordAnswer: ValidationResult = ValidationResult.Success, +) : AccountOutgoingConfigContract.Validator { + override fun validateServer(server: String): ValidationResult = serverAnswer + override fun validatePort(port: Long?): ValidationResult = portAnswer + override fun validateUsername(username: String): ValidationResult = usernameAnswer + override fun validatePassword(password: String): ValidationResult = passwordAnswer +} From d4c949e35008b8c856c9fad587b50dde5652cdff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montwe=CC=81?= Date: Wed, 7 Jun 2023 15:12:25 +0200 Subject: [PATCH 3/3] Fix validate port allowing 0 --- .../account/setup/domain/usecase/ValidatePort.kt | 11 +++++------ feature/account/setup/src/main/res/values/strings.xml | 2 +- .../account/setup/domain/usecase/ValidatePortTest.kt | 11 +++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt index b8f061836..6629d8a7d 100644 --- a/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt +++ b/feature/account/setup/src/main/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePort.kt @@ -7,11 +7,10 @@ import app.k9mail.core.common.domain.usecase.validation.ValidationUseCase class ValidatePort : ValidationUseCase { override fun execute(input: Long?): ValidationResult { - return when { - input == null -> ValidationResult.Failure(ValidatePortError.EmptyPort) - input < MIN_PORT_NUMBER -> ValidationResult.Failure(ValidatePortError.InvalidPort) - input > MAX_PORT_NUMBER -> ValidationResult.Failure(ValidatePortError.InvalidPort) - else -> ValidationResult.Success + return when (input) { + null -> ValidationResult.Failure(ValidatePortError.EmptyPort) + in MIN_PORT_NUMBER..MAX_PORT_NUMBER -> ValidationResult.Success + else -> ValidationResult.Failure(ValidatePortError.InvalidPort) } } @@ -22,6 +21,6 @@ class ValidatePort : ValidationUseCase { companion object { const val MAX_PORT_NUMBER = 65535 - const val MIN_PORT_NUMBER = 0 + const val MIN_PORT_NUMBER = 1 } } diff --git a/feature/account/setup/src/main/res/values/strings.xml b/feature/account/setup/src/main/res/values/strings.xml index a08ff2dc9..b91b971b1 100644 --- a/feature/account/setup/src/main/res/values/strings.xml +++ b/feature/account/setup/src/main/res/values/strings.xml @@ -18,7 +18,7 @@ StartTLS Port Port is required. - Port is invalid (0–65535). + Port is invalid (must be 1–65535). Username Username is required. Password is required. diff --git a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt index 1f399330c..c92c8c5a2 100644 --- a/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt +++ b/feature/account/setup/src/test/kotlin/app/k9mail/feature/account/setup/domain/usecase/ValidatePortTest.kt @@ -29,6 +29,17 @@ class ValidatePortTest { .isInstanceOf(ValidatePortError.InvalidPort::class) } + @Test + fun `should fail when port is zero`() { + val useCase = ValidatePort() + + val result = useCase.execute(0) + + assertThat(result).isInstanceOf(ValidationResult.Failure::class) + .prop(ValidationResult.Failure::error) + .isInstanceOf(ValidatePortError.InvalidPort::class) + } + @Test fun `should fail when port exceeds maximum`() { val useCase = ValidatePort()