From 825f14d919309981bc1938bf258edd1f4ff461ed Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 14:06:49 +0100 Subject: [PATCH 01/11] ignoring text suggestions on username inputs, to avoid the spell checker introducing word breaks --- vector/src/main/res/layout/fragment_ftue_combined_login.xml | 2 +- vector/src/main/res/layout/fragment_ftue_combined_register.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 1b65056e9f..8037f207fc 100644 --- a/vector/src/main/res/layout/fragment_ftue_combined_login.xml +++ b/vector/src/main/res/layout/fragment_ftue_combined_login.xml @@ -150,7 +150,7 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:imeOptions="actionNext" - android:inputType="text" + android:inputType="textNoSuggestions" android:maxLines="1" android:nextFocusForward="@id/loginPasswordInput" /> diff --git a/vector/src/main/res/layout/fragment_ftue_combined_register.xml b/vector/src/main/res/layout/fragment_ftue_combined_register.xml index 9d61780ad0..304e5b475f 100644 --- a/vector/src/main/res/layout/fragment_ftue_combined_register.xml +++ b/vector/src/main/res/layout/fragment_ftue_combined_register.xml @@ -174,7 +174,7 @@ android:layout_height="match_parent" android:imeOptions="actionNext" android:nextFocusForward="@id/createAccountPasswordInput" - android:inputType="text" + android:inputType="textNoSuggestions" android:maxLines="1" /> From 17f8009ce0a92374296a169a8131146bde53729a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 14:22:12 +0100 Subject: [PATCH 02/11] only removing the edit server fragment when homeserver edits are complete --- .../app/features/onboarding/ftueauth/FtueAuthVariant.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index bae90f1960..fa37e2edce 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -59,6 +59,7 @@ import org.matrix.android.sdk.api.extensions.tryOrNull private const val FRAGMENT_REGISTRATION_STAGE_TAG = "FRAGMENT_REGISTRATION_STAGE_TAG" private const val FRAGMENT_LOGIN_TAG = "FRAGMENT_LOGIN_TAG" +private const val FRAGMENT_EDIT_HOMESERVER_TAG = "FRAGMENT_EDIT_HOMESERVER" class FtueAuthVariant( private val views: ActivityLoginBinding, @@ -220,10 +221,14 @@ class FtueAuthVariant( activity.addFragmentToBackstack( views.loginFragmentContainer, FtueAuthCombinedServerSelectionFragment::class.java, - option = commonOption + option = commonOption, + tag = FRAGMENT_EDIT_HOMESERVER_TAG ) } - OnboardingViewEvents.OnHomeserverEdited -> activity.popBackstack() + OnboardingViewEvents.OnHomeserverEdited -> supportFragmentManager.popBackStack( + FRAGMENT_EDIT_HOMESERVER_TAG, + FragmentManager.POP_BACK_STACK_INCLUSIVE + ) OnboardingViewEvents.OpenCombinedLogin -> onStartCombinedLogin() is OnboardingViewEvents.DeeplinkAuthenticationFailure -> onDeeplinkedHomeserverUnavailable(viewEvents) OnboardingViewEvents.DisplayRegistrationFallback -> displayFallbackWebDialog() From 3a97cfcc36d452a1882b9da4d20301d8bff49e00 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 15:28:31 +0100 Subject: [PATCH 03/11] updating the selected homeserver when we detect a full matrix id within the username field in the login/register input fields --- .../app/core/extensions/TextInputLayout.kt | 11 ++++ .../features/onboarding/OnboardingAction.kt | 1 + .../onboarding/OnboardingViewModel.kt | 13 ++++ .../ftueauth/FtueAuthCombinedLoginFragment.kt | 2 + .../FtueAuthCombinedRegisterFragment.kt | 4 ++ .../onboarding/OnboardingViewModelTest.kt | 59 +++++++++++++++---- 6 files changed, 80 insertions(+), 10 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/extensions/TextInputLayout.kt b/vector/src/main/java/im/vector/app/core/extensions/TextInputLayout.kt index c5009bd072..41016365c0 100644 --- a/vector/src/main/java/im/vector/app/core/extensions/TextInputLayout.kt +++ b/vector/src/main/java/im/vector/app/core/extensions/TextInputLayout.kt @@ -57,3 +57,14 @@ fun TextInputLayout.setOnImeDoneListener(action: () -> Unit) { } } } + +fun TextInputLayout.setOnFocusLostListener(action: () -> Unit) { + editText().setOnFocusChangeListener { _, hasFocus -> + when (hasFocus) { + false -> action() + else -> { + // do nothing + } + } + } +} 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 bd2ff1a26a..b6a7550a58 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 @@ -50,6 +50,7 @@ sealed interface OnboardingAction : VectorViewModelAction { data class ResetPassword(val email: String, val newPassword: String) : OnboardingAction object ResetPasswordMailConfirmed : OnboardingAction + data class MaybeUpdateHomeserverFromMatrixId(val userId: String) : OnboardingAction sealed interface AuthenticateAction : OnboardingAction { data class Register(val username: String, val password: String, val initialDeviceName: String) : AuthenticateAction data class Login(val username: String, val password: String, val initialDeviceName: String) : AuthenticateAction 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 c41c9717f5..50e68dd324 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 @@ -50,6 +50,8 @@ import im.vector.app.features.onboarding.StartAuthenticationFlowUseCase.StartAut import kotlinx.coroutines.Job import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.launch +import org.matrix.android.sdk.api.MatrixPatterns +import org.matrix.android.sdk.api.MatrixPatterns.getServerName import org.matrix.android.sdk.api.auth.AuthenticationService import org.matrix.android.sdk.api.auth.HomeServerHistoryService import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig @@ -142,6 +144,7 @@ class OnboardingViewModel @AssistedInject constructor( is OnboardingAction.UpdateSignMode -> handleUpdateSignMode(action) is OnboardingAction.InitWith -> handleInitWith(action) is OnboardingAction.HomeServerChange -> withAction(action) { handleHomeserverChange(action) } + is OnboardingAction.MaybeUpdateHomeserverFromMatrixId -> handleMaybeUpdateHomeserver(action) is AuthenticateAction -> withAction(action) { handleAuthenticateAction(action) } is OnboardingAction.LoginWithToken -> handleLoginWithToken(action) is OnboardingAction.WebLoginSuccess -> handleWebLoginSuccess(action) @@ -162,6 +165,16 @@ class OnboardingViewModel @AssistedInject constructor( } } + private fun handleMaybeUpdateHomeserver(action: OnboardingAction.MaybeUpdateHomeserverFromMatrixId) { + val isFullMatrixId = MatrixPatterns.isUserId(action.userId) + if (isFullMatrixId) { + val domain = action.userId.getServerName().substringBeforeLast(":").ensureProtocol() + handleHomeserverChange(OnboardingAction.HomeServerChange.EditHomeServer(domain)) + } else { + // ignore the action + } + } + private fun withAction(action: OnboardingAction, block: (OnboardingAction) -> Unit) { lastAction = action block(action) 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 10b9cf4683..205a604aab 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 @@ -30,6 +30,7 @@ import im.vector.app.core.extensions.editText import im.vector.app.core.extensions.hideKeyboard import im.vector.app.core.extensions.hidePassword import im.vector.app.core.extensions.realignPercentagesToParent +import im.vector.app.core.extensions.setOnFocusLostListener import im.vector.app.core.extensions.setOnImeDoneListener import im.vector.app.core.extensions.toReducedUrl import im.vector.app.databinding.FragmentFtueCombinedLoginBinding @@ -59,6 +60,7 @@ class FtueAuthCombinedLoginFragment @Inject constructor( views.loginRoot.realignPercentagesToParent() views.editServerButton.debouncedClicks { viewModel.handle(OnboardingAction.PostViewEvent(OnboardingViewEvents.EditServerSelection)) } views.loginPasswordInput.setOnImeDoneListener { submit() } + views.loginInput.setOnFocusLostListener { viewModel.handle(OnboardingAction.MaybeUpdateHomeserverFromMatrixId(views.loginInput.content())) } } private fun setupSubmitButton() { diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt index e19f7837c3..6918a3682f 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt @@ -34,6 +34,7 @@ import im.vector.app.core.extensions.hasSurroundingSpaces import im.vector.app.core.extensions.hideKeyboard import im.vector.app.core.extensions.hidePassword import im.vector.app.core.extensions.realignPercentagesToParent +import im.vector.app.core.extensions.setOnFocusLostListener import im.vector.app.core.extensions.setOnImeDoneListener import im.vector.app.core.extensions.toReducedUrl import im.vector.app.databinding.FragmentFtueCombinedRegisterBinding @@ -67,6 +68,9 @@ class FtueAuthCombinedRegisterFragment @Inject constructor() : AbstractSSOFtueAu views.createAccountRoot.realignPercentagesToParent() views.editServerButton.debouncedClicks { viewModel.handle(OnboardingAction.PostViewEvent(OnboardingViewEvents.EditServerSelection)) } views.createAccountPasswordInput.setOnImeDoneListener { submit() } + views.createAccountInput.setOnFocusLostListener { + viewModel.handle(OnboardingAction.MaybeUpdateHomeserverFromMatrixId(views.createAccountInput.content())) + } } private fun setupSubmitButton() { diff --git a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt index 658e14d411..c5d24c0ec3 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt @@ -271,10 +271,7 @@ class OnboardingViewModelTest { @Test fun `given in the sign up flow, when editing homeserver, then updates selected homeserver state and emits edited event`() = runTest { viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignUp)) - fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, A_HOMESERVER_CONFIG) - fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, SELECTED_HOMESERVER_STATE)) - givenRegistrationResultFor(RegisterAction.StartRegistration, ANY_CONTINUING_REGISTRATION_RESULT) - fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) + givenCanSuccessfullyUpdateHomeserver(A_HOMESERVER_URL, SELECTED_HOMESERVER_STATE) val test = viewModel.test() viewModel.handle(OnboardingAction.HomeServerChange.EditHomeServer(A_HOMESERVER_URL)) @@ -291,13 +288,45 @@ class OnboardingViewModelTest { .finish() } + @Test + fun `given a full matrix id, when maybe updating homeserver, then updates selected homeserver state and emits edited event`() = runTest { + viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignUp)) + givenCanSuccessfullyUpdateHomeserver(A_HOMESERVER_URL, SELECTED_HOMESERVER_STATE) + val test = viewModel.test() + val fullMatrixId = "@a-user:${A_HOMESERVER_URL.removePrefix("https://")}" + + viewModel.handle(OnboardingAction.MaybeUpdateHomeserverFromMatrixId(fullMatrixId)) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(selectedHomeserver = SELECTED_HOMESERVER_STATE) }, + { copy(isLoading = false) } + + ) + .assertEvents(OnboardingViewEvents.OnHomeserverEdited) + .finish() + } + + @Test + fun `given a username, when maybe updating homeserver, then does nothing`() = runTest { + viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignUp)) + val test = viewModel.test() + val onlyUsername = "a-username" + + viewModel.handle(OnboardingAction.MaybeUpdateHomeserverFromMatrixId(onlyUsername)) + + test + .assertStates(initialState) + .assertNoEvents() + .finish() + } + @Test fun `given in the sign up flow, when editing homeserver errors, then does not update the selected homeserver state and emits error`() = runTest { viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignUp)) - fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, A_HOMESERVER_CONFIG) - fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, SELECTED_HOMESERVER_STATE)) - givenRegistrationActionErrors(RegisterAction.StartRegistration, AN_ERROR) - fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) + givenUpdatingHomeserverErrors(A_HOMESERVER_URL, SELECTED_HOMESERVER_STATE, AN_ERROR) val test = viewModel.test() viewModel.handle(OnboardingAction.HomeServerChange.EditHomeServer(A_HOMESERVER_URL)) @@ -552,8 +581,18 @@ class OnboardingViewModelTest { fakeRegistrationActionHandler.givenResultsFor(results) } - private fun givenRegistrationActionErrors(action: RegisterAction, cause: Throwable) { - fakeRegistrationActionHandler.givenThrows(action, cause) + private fun givenCanSuccessfullyUpdateHomeserver(homeserverUrl: String, resultingState: SelectedHomeserverState) { + fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, A_HOMESERVER_CONFIG) + fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) + givenRegistrationResultFor(RegisterAction.StartRegistration, ANY_CONTINUING_REGISTRATION_RESULT) + fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) + } + + private fun givenUpdatingHomeserverErrors(homeserverUrl: String, resultingState: SelectedHomeserverState, error: Throwable) { + fakeHomeServerConnectionConfigFactory.givenConfigFor(homeserverUrl, A_HOMESERVER_CONFIG) + fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, resultingState)) + givenRegistrationResultFor(RegisterAction.StartRegistration, RegistrationActionHandler.Result.Error(error)) + fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) } } From f89b9305e89a1d96d00c9b61bd34022e04ea0be9 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 15:33:46 +0100 Subject: [PATCH 04/11] handling the unavailable homeserver error case in the error formatting as this is now possible via full matrix id handling# --- .../app/features/onboarding/ftueauth/LoginErrorParser.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParser.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParser.kt index 271c1ced14..ac79419312 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParser.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParser.kt @@ -20,6 +20,7 @@ import im.vector.app.R import im.vector.app.core.error.ErrorFormatter import im.vector.app.core.resources.StringProvider import im.vector.app.features.onboarding.ftueauth.LoginErrorParser.LoginErrorResult +import org.matrix.android.sdk.api.failure.isHomeserverUnavailable import org.matrix.android.sdk.api.failure.isInvalidPassword import org.matrix.android.sdk.api.failure.isInvalidUsername import org.matrix.android.sdk.api.failure.isLoginEmailUnknown @@ -40,6 +41,9 @@ class LoginErrorParser @Inject constructor( throwable.isInvalidPassword() && password.hasSurroundingSpaces() -> { LoginErrorResult(throwable, passwordError = stringProvider.getString(R.string.auth_invalid_login_param_space_in_password)) } + throwable.isHomeserverUnavailable() -> { + LoginErrorResult(throwable, usernameOrIdError = stringProvider.getString(R.string.login_error_homeserver_not_found)) + } else -> { LoginErrorResult(throwable) } From b25fd4a5406c93fbf7c70abaddfe3d7c1049c6b8 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 16:08:17 +0100 Subject: [PATCH 05/11] adding tests around the login error parsing --- .../ftueauth/LoginErrorParserTest.kt | 102 ++++++++++++++++++ .../app/test/fakes/FakeErrorFormatter.kt | 27 +++++ .../app/test/fixtures/FailureFixture.kt | 12 +++ 3 files changed, 141 insertions(+) create mode 100644 vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeErrorFormatter.kt diff --git a/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt new file mode 100644 index 0000000000..9c7859685d --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt @@ -0,0 +1,102 @@ +/* + * 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 im.vector.app.R +import im.vector.app.test.fakes.FakeErrorFormatter +import im.vector.app.test.fakes.FakeStringProvider +import im.vector.app.test.fakes.toTestString +import im.vector.app.test.fixtures.aHomeserverUnavailableError +import im.vector.app.test.fixtures.aLoginEmailUnknownError +import im.vector.app.test.fixtures.anInvalidPasswordError +import im.vector.app.test.fixtures.anInvalidUserNameError +import org.amshove.kluent.shouldBeEqualTo +import org.junit.Test + +private const val A_VALID_PASSWORD = "11111111" +private const val A_FORMATTED_ERROR_MESSAGE = "error message" + +class LoginErrorParserTest { + + private val fakeErrorFormatter = FakeErrorFormatter() + private val fakeStringProvider = FakeStringProvider() + + private val loginErrorParser = LoginErrorParser(fakeErrorFormatter, fakeStringProvider.instance) + + @Test + fun `given a generic error, when parsing, then has null username and password errors`() { + val cause = RuntimeException() + + val result = loginErrorParser.parse(throwable = cause, password = A_VALID_PASSWORD) + + result shouldBeEqualTo LoginErrorParser.LoginErrorResult(cause, usernameOrIdError = null, passwordError = null) + } + + @Test + fun `given an invalid username error, when parsing, then has username error`() { + val cause = anInvalidUserNameError() + fakeErrorFormatter.given(cause, formatsTo = A_FORMATTED_ERROR_MESSAGE) + + val result = loginErrorParser.parse(throwable = cause, password = A_VALID_PASSWORD) + + result shouldBeEqualTo LoginErrorParser.LoginErrorResult( + cause, + usernameOrIdError = A_FORMATTED_ERROR_MESSAGE, + passwordError = null + ) + } + + @Test + fun `given a homeserver unavailable error, when parsing, then has username error`() { + val cause = aHomeserverUnavailableError() + + val result = loginErrorParser.parse(throwable = cause, password = A_VALID_PASSWORD) + + result shouldBeEqualTo LoginErrorParser.LoginErrorResult( + cause, + usernameOrIdError = R.string.login_error_homeserver_not_found.toTestString(), + passwordError = null + ) + } + + @Test + fun `given a login email unknown error, when parsing, then has username error`() { + val cause = aLoginEmailUnknownError() + + val result = loginErrorParser.parse(throwable = cause, password = A_VALID_PASSWORD) + + result shouldBeEqualTo LoginErrorParser.LoginErrorResult( + cause, + usernameOrIdError = R.string.login_login_with_email_error.toTestString(), + passwordError = null + ) + } + + @Test + fun `given a password with surrounding spaces and an invalid password error, when parsing, then has password error`() { + val cause = anInvalidPasswordError() + + val result = loginErrorParser.parse(throwable = cause, password = " $A_VALID_PASSWORD ") + + result shouldBeEqualTo LoginErrorParser.LoginErrorResult( + cause, + usernameOrIdError = null, + passwordError = R.string.auth_invalid_login_param_space_in_password.toTestString() + ) + } +} + diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeErrorFormatter.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeErrorFormatter.kt new file mode 100644 index 0000000000..98c554b5af --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeErrorFormatter.kt @@ -0,0 +1,27 @@ +/* + * 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.test.fakes + +import im.vector.app.core.error.ErrorFormatter +import io.mockk.every +import io.mockk.mockk + +class FakeErrorFormatter : ErrorFormatter by mockk() { + fun given(cause: Throwable, formatsTo: String) { + every { toHumanReadable(cause) } returns formatsTo + } +} diff --git a/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt b/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt index 9ac851ef5e..0f44976ab3 100644 --- a/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt +++ b/vector/src/test/java/im/vector/app/test/fixtures/FailureFixture.kt @@ -25,4 +25,16 @@ fun a401ServerError() = Failure.ServerError( MatrixError(MatrixError.M_UNAUTHORIZED, ""), HttpsURLConnection.HTTP_UNAUTHORIZED ) +fun anInvalidUserNameError() = Failure.ServerError( + MatrixError(MatrixError.M_INVALID_USERNAME, ""), HttpsURLConnection.HTTP_BAD_REQUEST +) + +fun anInvalidPasswordError() = Failure.ServerError( + MatrixError(MatrixError.M_FORBIDDEN, "Invalid password"), HttpsURLConnection.HTTP_FORBIDDEN +) + +fun aLoginEmailUnknownError() = Failure.ServerError( + MatrixError(MatrixError.M_FORBIDDEN, ""), HttpsURLConnection.HTTP_FORBIDDEN +) + fun aHomeserverUnavailableError() = Failure.NetworkConnection(UnknownHostException()) From d71d37c1ce414164f40ec8f4ef20ef39c672fa3a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 16:33:17 +0100 Subject: [PATCH 06/11] adding tests around the result _on_ helper callbacks --- .../ftueauth/LoginErrorParserTest.kt | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt index 9c7859685d..f36600e75f 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/LoginErrorParserTest.kt @@ -29,6 +29,8 @@ import org.junit.Test private const val A_VALID_PASSWORD = "11111111" private const val A_FORMATTED_ERROR_MESSAGE = "error message" +private const val ANOTHER_FORMATTED_ERROR_MESSAGE = "error message 2" +private val AN_ERROR = RuntimeException() class LoginErrorParserTest { @@ -98,5 +100,75 @@ class LoginErrorParserTest { passwordError = R.string.auth_invalid_login_param_space_in_password.toTestString() ) } + + @Test + fun `given an error result with no known errors, then is unknown`() { + val errorResult = LoginErrorParser.LoginErrorResult(AN_ERROR, usernameOrIdError = null, passwordError = null) + val captures = Captures(expectUnknownError = true) + + errorResult.callOnMethods(captures) + + captures.unknownResult shouldBeEqualTo AN_ERROR + } + + @Test + fun `given an error result with only username error, then is username or id error`() { + val errorResult = LoginErrorParser.LoginErrorResult(AN_ERROR, usernameOrIdError = A_FORMATTED_ERROR_MESSAGE, passwordError = null) + val captures = Captures(expectUsernameOrIdError = true) + + errorResult.callOnMethods(captures) + + captures.usernameOrIdError shouldBeEqualTo A_FORMATTED_ERROR_MESSAGE + } + + @Test + fun `given an error result with only password error, then is password error`() { + val errorResult = LoginErrorParser.LoginErrorResult(AN_ERROR, usernameOrIdError = null, passwordError = A_FORMATTED_ERROR_MESSAGE) + val captures = Captures(expectPasswordError = true) + + errorResult.callOnMethods(captures) + + captures.passwordError shouldBeEqualTo A_FORMATTED_ERROR_MESSAGE + } + + @Test + fun `given an error result with username and password error, then triggers both username and password error`() { + val errorResult = LoginErrorParser.LoginErrorResult( + AN_ERROR, + usernameOrIdError = A_FORMATTED_ERROR_MESSAGE, + passwordError = ANOTHER_FORMATTED_ERROR_MESSAGE + ) + val captures = Captures(expectPasswordError = true, expectUsernameOrIdError = true) + + errorResult.callOnMethods(captures) + + captures.usernameOrIdError shouldBeEqualTo A_FORMATTED_ERROR_MESSAGE + captures.passwordError shouldBeEqualTo ANOTHER_FORMATTED_ERROR_MESSAGE + } } +private fun LoginErrorParser.LoginErrorResult.callOnMethods(captures: Captures) { + onUnknown(captures.onUnknown) + onUsernameOrIdError(captures.onUsernameOrIdError) + onPasswordError(captures.onPasswordError) +} + +private class Captures( + val expectUnknownError: Boolean = false, + val expectUsernameOrIdError: Boolean = false, + val expectPasswordError: Boolean = false, +) { + var unknownResult: Throwable? = null + var usernameOrIdError: String? = null + var passwordError: String? = null + + val onUnknown: (Throwable) -> Unit = { + if (expectUnknownError) unknownResult = it else throw IllegalStateException("Not expected to be called") + } + val onUsernameOrIdError: (String) -> Unit = { + if (expectUsernameOrIdError) usernameOrIdError = it else throw IllegalStateException("Not expected to be called") + } + val onPasswordError: (String) -> Unit = { + if (expectPasswordError) passwordError = it else throw IllegalStateException("Not expected to be called") + } +} From c6bae6812d61601d65f228fd0f740ab4b212cf99 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 16:48:15 +0100 Subject: [PATCH 07/11] adding unavailable homeserver error messaging in the registration page --- .../onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt index 6918a3682f..7df1940970 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedRegisterFragment.kt @@ -48,6 +48,7 @@ import im.vector.app.features.onboarding.OnboardingViewEvents import im.vector.app.features.onboarding.OnboardingViewState import kotlinx.coroutines.flow.launchIn import org.matrix.android.sdk.api.auth.data.SsoIdentityProvider +import org.matrix.android.sdk.api.failure.isHomeserverUnavailable import org.matrix.android.sdk.api.failure.isInvalidPassword import org.matrix.android.sdk.api.failure.isInvalidUsername import org.matrix.android.sdk.api.failure.isLoginEmailUnknown @@ -133,6 +134,9 @@ class FtueAuthCombinedRegisterFragment @Inject constructor() : AbstractSSOFtueAu throwable.isWeakPassword() || throwable.isInvalidPassword() -> { views.createAccountPasswordInput.error = errorFormatter.toHumanReadable(throwable) } + throwable.isHomeserverUnavailable() -> { + views.createAccountInput.error = getString(R.string.login_error_homeserver_not_found) + } throwable.isRegistrationDisabled() -> { MaterialAlertDialogBuilder(requireActivity()) .setTitle(R.string.dialog_title_error) From 19de43dd65ae21c3d1a68532a722eb3dd667f09b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 9 Jun 2022 16:48:21 +0100 Subject: [PATCH 08/11] adding changelog entry --- changelog.d/6162.wip | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6162.wip diff --git a/changelog.d/6162.wip b/changelog.d/6162.wip new file mode 100644 index 0000000000..8b32a72571 --- /dev/null +++ b/changelog.d/6162.wip @@ -0,0 +1 @@ +FTUE - Adds automatic homeserver selection when typing a full matrix id during registration or login From 30f5e2bb6c74de7a40bcbebbb7df2f7467baadd3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 16 Jun 2022 10:48:29 +0100 Subject: [PATCH 09/11] adding test around matrix user id check --- .../android/sdk/api/MatrixPatternsTest.kt | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt new file mode 100644 index 0000000000..f9920e39c7 --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt @@ -0,0 +1,42 @@ +/* + * Copyright 2022 The Matrix.org Foundation C.I.C. + * + * 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 org.matrix.android.sdk.api + +import org.amshove.kluent.shouldBeEqualTo +import org.junit.Test + +class MatrixPatternsTest { + + @Test + fun `given user id cases, when checking isUserId, then returns expected`() { + val cases = listOf( + UserIdCase("foobar", isUserId = false), + UserIdCase("@foobar", isUserId = false), + UserIdCase("foobar@matrix.org", isUserId = false), + UserIdCase("@foobar: matrix.org", isUserId = false), + UserIdCase("@foobar:matrix.org", isUserId = true), + ) + + cases.forEach { (input, expected) -> + MatrixPatterns.isUserId(input) shouldBeEqualTo expected + } + } + +} + +private data class UserIdCase(val input: String, val isUserId: Boolean) From 6a66125286090522ca5609b20a8abb8d216009fe Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 16 Jun 2022 11:19:14 +0100 Subject: [PATCH 10/11] formatting --- .../android/sdk/api/MatrixPatternsTest.kt | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt index f9920e39c7..eff1b14106 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt @@ -24,19 +24,18 @@ class MatrixPatternsTest { @Test fun `given user id cases, when checking isUserId, then returns expected`() { - val cases = listOf( - UserIdCase("foobar", isUserId = false), - UserIdCase("@foobar", isUserId = false), - UserIdCase("foobar@matrix.org", isUserId = false), - UserIdCase("@foobar: matrix.org", isUserId = false), - UserIdCase("@foobar:matrix.org", isUserId = true), - ) + val cases = listOf( + UserIdCase("foobar", isUserId = false), + UserIdCase("@foobar", isUserId = false), + UserIdCase("foobar@matrix.org", isUserId = false), + UserIdCase("@foobar: matrix.org", isUserId = false), + UserIdCase("@foobar:matrix.org", isUserId = true), + ) - cases.forEach { (input, expected) -> - MatrixPatterns.isUserId(input) shouldBeEqualTo expected - } + cases.forEach { (input, expected) -> + MatrixPatterns.isUserId(input) shouldBeEqualTo expected + } } - } private data class UserIdCase(val input: String, val isUserId: Boolean) From 7558d71ec2e25b9e82637f719712857ee1907f75 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 16 Jun 2022 12:47:40 +0100 Subject: [PATCH 11/11] removing extra blank line --- .../test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt index eff1b14106..0d0450adc2 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/MatrixPatternsTest.kt @@ -14,7 +14,6 @@ * limitations under the License. */ - package org.matrix.android.sdk.api import org.amshove.kluent.shouldBeEqualTo