From c3ce887e3354a3e8b9393dd1559ebbf0214ea62a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 9 May 2022 13:34:40 +0100 Subject: [PATCH] minor refactors - extracting login fields validation - renaming xml fields to login - renaming direct login property to matrixId --- .../features/onboarding/DirectLoginUseCase.kt | 4 +- .../features/onboarding/OnboardingAction.kt | 2 +- .../ftueauth/FtueAuthCombinedLoginFragment.kt | 69 ++++++-------- .../ftueauth/FtueAuthLoginFragment.kt | 6 +- .../ftueauth/LoginFieldsValidation.kt | 58 ++++++++++++ .../layout/fragment_ftue_combined_login.xml | 90 +++++++++---------- .../onboarding/DirectLoginUseCaseTest.kt | 12 +-- 7 files changed, 141 insertions(+), 100 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginFieldsValidation.kt diff --git a/vector/src/main/java/im/vector/app/features/onboarding/DirectLoginUseCase.kt b/vector/src/main/java/im/vector/app/features/onboarding/DirectLoginUseCase.kt index 579d77be8d..4553f6b9d0 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/DirectLoginUseCase.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/DirectLoginUseCase.kt @@ -33,7 +33,7 @@ class DirectLoginUseCase @Inject constructor( ) { suspend fun execute(action: OnboardingAction.LoginDirect, homeServerConnectionConfig: HomeServerConnectionConfig?): Result { - return fetchWellKnown(action.username, homeServerConnectionConfig) + return fetchWellKnown(action.matrixId, homeServerConnectionConfig) .andThen { wellKnown -> createSessionFor(wellKnown, action, homeServerConnectionConfig) } } @@ -61,7 +61,7 @@ class DirectLoginUseCase @Inject constructor( return runCatching { authenticationService.directAuthentication( alteredHomeServerConnectionConfig, - action.username, + action.matrixId, action.password, action.initialDeviceName ) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt index 1725ec2ca5..9226bc0cfa 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt @@ -48,7 +48,7 @@ sealed interface OnboardingAction : VectorViewModelAction { data class Register(val username: String, val password: String, val initialDeviceName: String) : OnboardingAction data class Login(val username: String, val password: String, val initialDeviceName: String) : OnboardingAction - data class LoginDirect(val username: String, val password: String, val initialDeviceName: String) : OnboardingAction + data class LoginDirect(val matrixId: String, val password: String, val initialDeviceName: String) : OnboardingAction object StopEmailValidationCheck : OnboardingAction data class PostRegisterAction(val registerAction: RegisterAction) : OnboardingAction diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt index 66c7199acb..24f6c19908 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt @@ -23,7 +23,6 @@ import android.view.View import android.view.ViewGroup import android.view.inputmethod.EditorInfo import androidx.autofill.HintConstants -import androidx.core.text.isDigitsOnly import androidx.core.view.isVisible import androidx.lifecycle.lifecycleScope import com.airbnb.mvrx.withState @@ -52,7 +51,9 @@ import org.matrix.android.sdk.api.failure.isInvalidUsername import org.matrix.android.sdk.api.failure.isLoginEmailUnknown import javax.inject.Inject -class FtueAuthCombinedLoginFragment @Inject constructor() : AbstractSSOFtueAuthFragment() { +class FtueAuthCombinedLoginFragment @Inject constructor( + private val loginFieldsValidation: LoginFieldsValidation +) : AbstractSSOFtueAuthFragment() { override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): FragmentFtueCombinedLoginBinding { return FragmentFtueCombinedLoginBinding.inflate(inflater, container, false) @@ -61,12 +62,12 @@ class FtueAuthCombinedLoginFragment @Inject constructor() : AbstractSSOFtueAuthF override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) setupSubmitButton() - views.createAccountRoot.realignPercentagesToParent() + views.loginRoot.realignPercentagesToParent() views.editServerButton.debouncedClicks { viewModel.handle(OnboardingAction.PostViewEvent(OnboardingViewEvents.EditServerSelection)) } - views.createAccountPasswordInput.editText().setOnEditorActionListener { _, actionId, _ -> + views.loginPasswordInput.editText().setOnEditorActionListener { _, actionId, _ -> if (actionId == EditorInfo.IME_ACTION_DONE) { submit() return@setOnEditorActionListener true @@ -76,54 +77,38 @@ class FtueAuthCombinedLoginFragment @Inject constructor() : AbstractSSOFtueAuthF } private fun setupSubmitButton() { - views.createAccountSubmit.setOnClickListener { submit() } + views.loginSubmit.setOnClickListener { submit() } observeInputFields() .onEach { - views.createAccountPasswordInput.error = null - views.createAccountInput.error = null - views.createAccountSubmit.isEnabled = it + views.loginPasswordInput.error = null + views.loginInput.error = null + views.loginSubmit.isEnabled = it } .launchIn(viewLifecycleOwner.lifecycleScope) } private fun observeInputFields() = combine( - views.createAccountInput.hasContentFlow { it.trim() }, - views.createAccountPasswordInput.hasContentFlow(), + views.loginInput.hasContentFlow { it.trim() }, + views.loginPasswordInput.hasContentFlow(), transform = { isLoginNotEmpty, isPasswordNotEmpty -> isLoginNotEmpty && isPasswordNotEmpty } ) private fun submit() { withState(viewModel) { state -> cleanupUi() - - val login = views.createAccountInput.content() - val password = views.createAccountPasswordInput.content() - - // This can be called by the IME action, so deal with empty cases - var error = 0 - if (login.isEmpty()) { - views.createAccountInput.error = getString(R.string.error_empty_field_choose_user_name) - error++ - } - if (state.isNumericOnlyUserIdForbidden() && login.isDigitsOnly()) { - views.createAccountInput.error = getString(R.string.error_forbidden_digits_only_username) - error++ - } - if (password.isEmpty()) { - views.createAccountPasswordInput.error = getString(R.string.error_empty_field_choose_password) - error++ - } - - if (error == 0) { + val login = views.loginInput.content() + val password = views.loginPasswordInput.content() + val isMatrixOrg = state.selectedHomeserver.userFacingUrl == getString(R.string.matrix_org_server_url) + loginFieldsValidation.validate(login, password, isMatrixOrg).onValid { viewModel.handle(OnboardingAction.Login(login, password, getString(R.string.login_default_session_public_name))) } } } private fun cleanupUi() { - views.createAccountSubmit.hideKeyboard() - views.createAccountInput.error = null - views.createAccountPasswordInput.error = null + views.loginSubmit.hideKeyboard() + views.loginInput.error = null + views.loginPasswordInput.error = null } override fun resetViewModel() { @@ -132,16 +117,16 @@ class FtueAuthCombinedLoginFragment @Inject constructor() : AbstractSSOFtueAuthF override fun onError(throwable: Throwable) { // Trick to display the error without text. - views.createAccountInput.error = " " + views.loginInput.error = " " when { throwable.isInvalidUsername() -> { - views.createAccountInput.error = errorFormatter.toHumanReadable(throwable) + views.loginInput.error = errorFormatter.toHumanReadable(throwable) } throwable.isLoginEmailUnknown() -> { - views.createAccountInput.error = getString(R.string.login_login_with_email_error) + views.loginInput.error = getString(R.string.login_login_with_email_error) } - throwable.isInvalidPassword() && views.createAccountPasswordInput.hasSurroundingSpaces() -> { - views.createAccountPasswordInput.error = getString(R.string.auth_invalid_login_param_space_in_password) + throwable.isInvalidPassword() && views.loginPasswordInput.hasSurroundingSpaces() -> { + views.loginPasswordInput.error = getString(R.string.auth_invalid_login_param_space_in_password) } else -> { super.onError(throwable) @@ -158,7 +143,7 @@ class FtueAuthCombinedLoginFragment @Inject constructor() : AbstractSSOFtueAuthF if (state.isLoading) { // Ensure password is hidden - views.createAccountPasswordInput.editText().hidePassword() + views.loginPasswordInput.editText().hidePassword() } } @@ -189,10 +174,8 @@ class FtueAuthCombinedLoginFragment @Inject constructor() : AbstractSSOFtueAuthF private fun setupAutoFill() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - views.createAccountInput.setAutofillHints(HintConstants.AUTOFILL_HINT_NEW_USERNAME) - views.createAccountPasswordInput.setAutofillHints(HintConstants.AUTOFILL_HINT_NEW_PASSWORD) + views.loginInput.setAutofillHints(HintConstants.AUTOFILL_HINT_NEW_USERNAME) + views.loginPasswordInput.setAutofillHints(HintConstants.AUTOFILL_HINT_NEW_PASSWORD) } } - - private fun OnboardingViewState.isNumericOnlyUserIdForbidden() = selectedHomeserver.userFacingUrl == getString(R.string.matrix_org_server_url) } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthLoginFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthLoginFragment.kt index ecda60311a..f899ca9c52 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthLoginFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthLoginFragment.kt @@ -157,9 +157,9 @@ class FtueAuthLoginFragment @Inject constructor() : AbstractSSOFtueAuthFragment< val initialDeviceName = getString(R.string.login_default_session_public_name) val action = when (state.signMode) { SignMode.Unknown -> error("developer error") - SignMode.SignUp -> OnboardingAction.Register(login, password, initialDeviceName) - SignMode.SignIn -> OnboardingAction.Login(login, password, initialDeviceName) - SignMode.SignInWithMatrixId -> OnboardingAction.LoginDirect(login, password, initialDeviceName) + SignMode.SignUp -> OnboardingAction.Register(username = login, password, initialDeviceName) + SignMode.SignIn -> OnboardingAction.Login(username = login, password, initialDeviceName) + SignMode.SignInWithMatrixId -> OnboardingAction.LoginDirect(matrixId = login, password, initialDeviceName) } viewModel.handle(action) } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginFieldsValidation.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginFieldsValidation.kt new file mode 100644 index 0000000000..476c82a20c --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginFieldsValidation.kt @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2022 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 im.vector.app.features.onboarding.ftueauth + +import androidx.core.text.isDigitsOnly +import im.vector.app.R +import im.vector.app.core.resources.StringProvider +import javax.inject.Inject + +typealias LoginValidationResult = Pair + +class LoginFieldsValidation @Inject constructor( + private val stringProvider: StringProvider +) { + + fun validate(usernameOrId: String, password: String, isMatrixOrg: Boolean): Pair { + return validateUsernameOrId(usernameOrId, isMatrixOrg) to validatePassword(password) + } + + private fun validateUsernameOrId(usernameOrId: String, isMatrixOrg: Boolean): String? { + val accountError = when { + usernameOrId.isEmpty() -> stringProvider.getString(R.string.error_empty_field_choose_user_name) + isNumericOnlyUserIdForbidden(isMatrixOrg) && usernameOrId.isDigitsOnly() -> stringProvider.getString(R.string.error_forbidden_digits_only_username) + else -> null + } + return accountError + } + + private fun isNumericOnlyUserIdForbidden(isMatrixOrg: Boolean) = isMatrixOrg + + private fun validatePassword(password: String): String? { + val passwordError = when { + password.isEmpty() -> stringProvider.getString(R.string.error_empty_field_choose_password) + else -> null + } + return passwordError + } +} + +fun LoginValidationResult.onValid(action: () -> Unit) { + when { + first != null && second != null -> action() + } +} diff --git a/vector/src/main/res/layout/fragment_ftue_combined_login.xml b/vector/src/main/res/layout/fragment_ftue_combined_login.xml index ff437ab9aa..2ca288b715 100644 --- a/vector/src/main/res/layout/fragment_ftue_combined_login.xml +++ b/vector/src/main/res/layout/fragment_ftue_combined_login.xml @@ -10,19 +10,19 @@ android:paddingBottom="0dp"> + app:layout_constraintTop_toBottomOf="@id/loginHeaderTitle" />