From b3bbb0329ec44df6f64113610edbc460f15d158c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 5 May 2022 15:22:16 +0100 Subject: [PATCH] directing to the combined login after homeserver check - also removes the subtitle view which is not needed for login --- .../features/onboarding/DirectLoginUseCase.kt | 15 ++-- .../features/onboarding/OnboardingAction.kt | 3 +- .../onboarding/OnboardingViewModel.kt | 26 ++++--- .../ftueauth/FtueAuthLoginFragment.kt | 72 +++++++++++-------- .../layout/fragment_ftue_combined_login.xml | 18 +---- .../onboarding/DirectLoginUseCaseTest.kt | 2 +- .../onboarding/OnboardingViewModelTest.kt | 10 +-- .../app/test/fakes/FakeDirectLoginUseCase.kt | 4 +- 8 files changed, 71 insertions(+), 79 deletions(-) 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 3014b199b4..579d77be8d 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 @@ -19,7 +19,6 @@ package im.vector.app.features.onboarding import im.vector.app.R import im.vector.app.core.extensions.andThen import im.vector.app.core.resources.StringProvider -import im.vector.app.features.onboarding.OnboardingAction.LoginOrRegister import org.matrix.android.sdk.api.MatrixPatterns.getServerName import org.matrix.android.sdk.api.auth.AuthenticationService import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig @@ -33,7 +32,7 @@ class DirectLoginUseCase @Inject constructor( private val uriFactory: UriFactory ) { - suspend fun execute(action: LoginOrRegister, homeServerConnectionConfig: HomeServerConnectionConfig?): Result { + suspend fun execute(action: OnboardingAction.LoginDirect, homeServerConnectionConfig: HomeServerConnectionConfig?): Result { return fetchWellKnown(action.username, homeServerConnectionConfig) .andThen { wellKnown -> createSessionFor(wellKnown, action, homeServerConnectionConfig) } } @@ -42,13 +41,13 @@ class DirectLoginUseCase @Inject constructor( authenticationService.getWellKnownData(matrixId, config) } - private suspend fun createSessionFor(data: WellknownResult, action: LoginOrRegister, config: HomeServerConnectionConfig?) = when (data) { - is WellknownResult.Prompt -> loginDirect(action, data, config) + private suspend fun createSessionFor(data: WellknownResult, action: OnboardingAction.LoginDirect, config: HomeServerConnectionConfig?) = when (data) { + is WellknownResult.Prompt -> loginDirect(action, data, config) is WellknownResult.FailPrompt -> handleFailPrompt(data, action, config) - else -> onWellKnownError() + else -> onWellKnownError() } - private suspend fun handleFailPrompt(data: WellknownResult.FailPrompt, action: LoginOrRegister, config: HomeServerConnectionConfig?): Result { + private suspend fun handleFailPrompt(data: WellknownResult.FailPrompt, action: OnboardingAction.LoginDirect, config: HomeServerConnectionConfig?): Result { // Relax on IS discovery if homeserver is valid val isMissingInformationToLogin = data.homeServerUrl == null || data.wellKnown == null return when { @@ -57,7 +56,7 @@ class DirectLoginUseCase @Inject constructor( } } - private suspend fun loginDirect(action: LoginOrRegister, wellKnownPrompt: WellknownResult.Prompt, config: HomeServerConnectionConfig?): Result { + private suspend fun loginDirect(action: OnboardingAction.LoginDirect, wellKnownPrompt: WellknownResult.Prompt, config: HomeServerConnectionConfig?): Result { val alteredHomeServerConnectionConfig = config?.updateWith(wellKnownPrompt) ?: fallbackConfig(action, wellKnownPrompt) return runCatching { authenticationService.directAuthentication( @@ -74,7 +73,7 @@ class DirectLoginUseCase @Inject constructor( identityServerUri = wellKnownPrompt.identityServerUrl?.let { uriFactory.parse(it) } ) - private fun fallbackConfig(action: LoginOrRegister, wellKnownPrompt: WellknownResult.Prompt) = HomeServerConnectionConfig( + private fun fallbackConfig(action: OnboardingAction.LoginDirect, wellKnownPrompt: WellknownResult.Prompt) = HomeServerConnectionConfig( homeServerUri = uriFactory.parse("https://${action.username.getServerName()}"), homeServerUriBase = uriFactory.parse(wellKnownPrompt.homeServerUrl), identityServerUri = wellKnownPrompt.identityServerUrl?.let { uriFactory.parse(it) } 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 d5b12a8071..1725ec2ca5 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 @@ -46,10 +46,9 @@ sealed interface OnboardingAction : VectorViewModelAction { data class ResetPassword(val email: String, val newPassword: String) : OnboardingAction object ResetPasswordMailConfirmed : OnboardingAction - // Login or Register, depending on the signMode - data class LoginOrRegister(val username: String, val password: String, val initialDeviceName: String) : OnboardingAction 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 object StopEmailValidationCheck : OnboardingAction data class PostRegisterAction(val registerAction: RegisterAction) : OnboardingAction diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 1ca2556c71..46bfd5e899 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -139,9 +139,9 @@ class OnboardingViewModel @AssistedInject constructor( is OnboardingAction.UpdateSignMode -> handleUpdateSignMode(action) is OnboardingAction.InitWith -> handleInitWith(action) is OnboardingAction.HomeServerChange -> withAction(action) { handleHomeserverChange(action) } - is OnboardingAction.LoginOrRegister -> handleLoginOrRegister(action).also { lastAction = action } is OnboardingAction.Register -> handleRegisterWith(action).also { lastAction = action } is OnboardingAction.Login -> handleLogin(action).also { lastAction = action } + is OnboardingAction.LoginDirect -> handleDirectLogin(action, homeServerConnectionConfig = null).also { lastAction = action } is OnboardingAction.LoginWithToken -> handleLoginWithToken(action) is OnboardingAction.WebLoginSuccess -> handleWebLoginSuccess(action) is OnboardingAction.ResetPassword -> handleResetPassword(action) @@ -215,7 +215,7 @@ class OnboardingViewModel @AssistedInject constructor( ?.let { it.copy(allowedFingerprints = it.allowedFingerprints + action.fingerprint) } ?.let { startAuthenticationFlow(finalLastAction, it) } } - is OnboardingAction.LoginOrRegister -> + is OnboardingAction.LoginDirect -> handleDirectLogin( finalLastAction, HomeServerConnectionConfig.Builder() @@ -488,16 +488,7 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun handleLoginOrRegister(action: OnboardingAction.LoginOrRegister) = withState { state -> - when (state.signMode) { - SignMode.Unknown -> error("Developer error, invalid sign mode") - SignMode.SignIn -> handleLogin(OnboardingAction.Login(action.username, action.password, action.initialDeviceName)) - SignMode.SignUp -> handleRegisterWith(OnboardingAction.Register(action.username, action.password, action.initialDeviceName)) - SignMode.SignInWithMatrixId -> handleDirectLogin(action, null) - } - } - - private fun handleDirectLogin(action: OnboardingAction.LoginOrRegister, homeServerConnectionConfig: HomeServerConnectionConfig?) { + private fun handleDirectLogin(action: OnboardingAction.LoginDirect, homeServerConnectionConfig: HomeServerConnectionConfig?) { setState { copy(isLoading = true) } currentJob = viewModelScope.launch { directLoginUseCase.execute(action, homeServerConnectionConfig).fold( @@ -654,7 +645,11 @@ class OnboardingViewModel @AssistedInject constructor( when (trigger) { is OnboardingAction.HomeServerChange.EditHomeServer -> { when (awaitState().onboardingFlow) { - OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration) { _ -> + OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration) { + updateServerSelection(config, serverTypeOverride, authResult) + _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) + } + OnboardingFlow.SignIn -> { updateServerSelection(config, serverTypeOverride, authResult) _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) } @@ -667,7 +662,10 @@ class OnboardingViewModel @AssistedInject constructor( when (awaitState().onboardingFlow) { OnboardingFlow.SignIn -> { updateSignMode(SignMode.SignIn) - _viewEvents.post(OnboardingViewEvents.OnSignModeSelected(SignMode.SignIn)) + when (vectorFeatures.isOnboardingCombinedLoginEnabled()) { + true -> _viewEvents.post(OnboardingViewEvents.OpenCombinedLogin) + false -> _viewEvents.post(OnboardingViewEvents.OnSignModeSelected(SignMode.SignIn)) + } } OnboardingFlow.SignUp -> { updateSignMode(SignMode.SignUp) 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 2308280400..ecda60311a 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 @@ -26,6 +26,7 @@ import androidx.autofill.HintConstants import androidx.core.text.isDigitsOnly import androidx.core.view.isVisible import androidx.lifecycle.lifecycleScope +import com.airbnb.mvrx.withState import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R import im.vector.app.core.extensions.hideKeyboard @@ -119,40 +120,49 @@ class FtueAuthLoginFragment @Inject constructor() : AbstractSSOFtueAuthFragment< } private fun submit() { - cleanupUi() + withState(viewModel) { state -> + cleanupUi() - val login = views.loginField.text.toString() - val password = views.passwordField.text.toString() + val login = views.loginField.text.toString() + val password = views.passwordField.text.toString() - // This can be called by the IME action, so deal with empty cases - var error = 0 - if (login.isEmpty()) { - views.loginFieldTil.error = getString( - if (isSignupMode) { - R.string.error_empty_field_choose_user_name - } else { - R.string.error_empty_field_enter_user_name - } - ) - error++ - } - if (isSignupMode && isNumericOnlyUserIdForbidden && login.isDigitsOnly()) { - views.loginFieldTil.error = getString(R.string.error_forbidden_digits_only_username) - error++ - } - if (password.isEmpty()) { - views.passwordFieldTil.error = getString( - if (isSignupMode) { - R.string.error_empty_field_choose_password - } else { - R.string.error_empty_field_your_password - } - ) - error++ - } + // This can be called by the IME action, so deal with empty cases + var error = 0 + if (login.isEmpty()) { + views.loginFieldTil.error = getString( + if (isSignupMode) { + R.string.error_empty_field_choose_user_name + } else { + R.string.error_empty_field_enter_user_name + } + ) + error++ + } + if (isSignupMode && isNumericOnlyUserIdForbidden && login.isDigitsOnly()) { + views.loginFieldTil.error = getString(R.string.error_forbidden_digits_only_username) + error++ + } + if (password.isEmpty()) { + views.passwordFieldTil.error = getString( + if (isSignupMode) { + R.string.error_empty_field_choose_password + } else { + R.string.error_empty_field_your_password + } + ) + error++ + } - if (error == 0) { - viewModel.handle(OnboardingAction.LoginOrRegister(login, password, getString(R.string.login_default_session_public_name))) + if (error == 0) { + 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) + } + viewModel.handle(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 684d6cf671..ff437ab9aa 100644 --- a/vector/src/main/res/layout/fragment_ftue_combined_login.xml +++ b/vector/src/main/res/layout/fragment_ftue_combined_login.xml @@ -46,24 +46,10 @@ android:gravity="center" android:text="@string/ftue_auth_welcome_back_title" android:textColor="?vctr_content_primary" - app:layout_constraintBottom_toTopOf="@id/createAccountHeaderSubtitle" - app:layout_constraintEnd_toEndOf="@id/createAccountGutterEnd" - app:layout_constraintStart_toStartOf="@id/createAccountGutterStart" - app:layout_constraintTop_toBottomOf="@id/headerSpacing" /> - - + app:layout_constraintTop_toBottomOf="@id/headerSpacing" /> + app:layout_constraintTop_toBottomOf="@id/createAccountHeaderTitle" /> () - fun givenSuccessResult(action: OnboardingAction.LoginOrRegister, config: HomeServerConnectionConfig?, result: FakeSession) { + fun givenSuccessResult(action: OnboardingAction.LoginDirect, config: HomeServerConnectionConfig?, result: FakeSession) { coEvery { instance.execute(action, config) } returns Result.success(result) } - fun givenFailureResult(action: OnboardingAction.LoginOrRegister, config: HomeServerConnectionConfig?, cause: Throwable) { + fun givenFailureResult(action: OnboardingAction.LoginDirect, config: HomeServerConnectionConfig?, cause: Throwable) { coEvery { instance.execute(action, config) } returns Result.failure(cause) } }