From 36ad59dc0f863d2c90c7bcd965a84e31d2b5f488 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Apr 2022 16:52:12 +0100 Subject: [PATCH 01/14] handling server urls as texturis to avoid auto spacing and applying errors to the input field error section --- .../FtueAuthCombinedServerSelectionFragment.kt | 18 ++++++++++++++++++ ...fragment_ftue_server_selection_combined.xml | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt index d1560d7be0..97e2db2123 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt @@ -21,6 +21,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.view.inputmethod.EditorInfo +import androidx.lifecycle.lifecycleScope import im.vector.app.R import im.vector.app.core.extensions.content import im.vector.app.core.extensions.editText @@ -33,6 +34,11 @@ import im.vector.app.databinding.FragmentFtueServerSelectionCombinedBinding import im.vector.app.features.onboarding.OnboardingAction import im.vector.app.features.onboarding.OnboardingViewEvents import im.vector.app.features.onboarding.OnboardingViewState +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import org.matrix.android.sdk.api.failure.Failure +import reactivecircus.flowbinding.android.widget.textChanges +import java.net.UnknownHostException import javax.inject.Inject class FtueAuthCombinedServerSelectionFragment @Inject constructor() : AbstractFtueAuthFragment() { @@ -61,6 +67,9 @@ class FtueAuthCombinedServerSelectionFragment @Inject constructor() : AbstractFt } views.chooseServerGetInTouch.debouncedClicks { openUrlInExternalBrowser(requireContext(), getString(R.string.ftue_ems_url)) } views.chooseServerSubmit.debouncedClicks { updateServerUrl() } + views.chooseServerInput.editText().textChanges() + .onEach { views.chooseServerInput.error = null } + .launchIn(lifecycleScope) } private fun updateServerUrl() { @@ -78,5 +87,14 @@ class FtueAuthCombinedServerSelectionFragment @Inject constructor() : AbstractFt } } + override fun onError(throwable: Throwable) { + views.chooseServerInput.error = if (throwable is Failure.NetworkConnection && + throwable.ioException is UnknownHostException) { + getString(R.string.login_error_homeserver_not_found) + } else { + errorFormatter.toHumanReadable(throwable) + } + } + private fun String.toReducedUrlKeepingSchemaIfInsecure() = toReducedUrl(keepSchema = this.startsWith("http://")) } diff --git a/vector/src/main/res/layout/fragment_ftue_server_selection_combined.xml b/vector/src/main/res/layout/fragment_ftue_server_selection_combined.xml index 8f4902a577..5a60632e86 100644 --- a/vector/src/main/res/layout/fragment_ftue_server_selection_combined.xml +++ b/vector/src/main/res/layout/fragment_ftue_server_selection_combined.xml @@ -106,7 +106,7 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:imeOptions="actionDone" - android:inputType="text" + android:inputType="textUri" android:maxLines="1" /> From c7065fc123d956fdde4782681e4fdd56e50e301e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Apr 2022 16:54:16 +0100 Subject: [PATCH 02/14] splitting the success action from the handleRegisterAction, allowing the homeserver editing to start the registation flow --- .../onboarding/OnboardingViewModel.kt | 119 ++++++++++-------- 1 file changed, 67 insertions(+), 52 deletions(-) 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 54aea0185c..e5bbf39127 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 @@ -147,7 +147,7 @@ class OnboardingViewModel @AssistedInject constructor( is OnboardingAction.WebLoginSuccess -> handleWebLoginSuccess(action) is OnboardingAction.ResetPassword -> handleResetPassword(action) is OnboardingAction.ResetPasswordMailConfirmed -> handleResetPasswordMailConfirmed() - is OnboardingAction.PostRegisterAction -> handleRegisterAction(action.registerAction) + is OnboardingAction.PostRegisterAction -> handleRegisterAction(action.registerAction, ::emitFlowResultViewEvent) is OnboardingAction.ResetAction -> handleResetAction(action) is OnboardingAction.UserAcceptCertificate -> handleUserAcceptCertificate(action) OnboardingAction.ClearHomeServerHistory -> handleClearHomeServerHistory() @@ -211,7 +211,7 @@ class OnboardingViewModel @AssistedInject constructor( ?.let { it.copy(allowedFingerprints = it.allowedFingerprints + action.fingerprint) } ?.let { startAuthenticationFlow(it) } } - is OnboardingAction.LoginOrRegister -> + is OnboardingAction.LoginOrRegister -> handleDirectLogin( finalLastAction, HomeServerConnectionConfig.Builder() @@ -220,7 +220,7 @@ class OnboardingViewModel @AssistedInject constructor( .withAllowedFingerPrints(listOf(action.fingerprint)) .build() ) - else -> Unit + else -> Unit } } @@ -255,11 +255,12 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun handleRegisterAction(action: RegisterAction) { + private fun handleRegisterAction(action: RegisterAction, onNextRegistrationStepAction: (FlowResult) -> Unit) { currentJob = viewModelScope.launch { if (action.hasLoadingState()) { setState { copy(isLoading = true) } } + runCatching { registrationActionHandler.handleRegisterAction(registrationWizard, action) } .fold( onSuccess = { @@ -269,7 +270,7 @@ class OnboardingViewModel @AssistedInject constructor( } else -> when (it) { is RegistrationResult.Success -> onSessionCreated(it.session, isAccountCreated = true) - is RegistrationResult.FlowResponse -> onFlowResponse(it.flowResult) + is RegistrationResult.FlowResponse -> onFlowResponse(it.flowResult, onNextRegistrationStepAction) } } }, @@ -283,13 +284,20 @@ class OnboardingViewModel @AssistedInject constructor( } } + private fun emitFlowResultViewEvent(flowResult: FlowResult) { + _viewEvents.post(OnboardingViewEvents.RegistrationFlowResult(flowResult, isRegistrationStarted)) + } + private fun handleRegisterWith(action: OnboardingAction.Register) { reAuthHelper.data = action.password - handleRegisterAction(RegisterAction.CreateAccount( - action.username, - action.password, - action.initialDeviceName - )) + handleRegisterAction( + RegisterAction.CreateAccount( + action.username, + action.password, + action.initialDeviceName + ), + ::emitFlowResultViewEvent + ) } private fun handleResetAction(action: OnboardingAction.ResetAction) { @@ -344,7 +352,7 @@ class OnboardingViewModel @AssistedInject constructor( } when (action.signMode) { - SignMode.SignUp -> handleRegisterAction(RegisterAction.StartRegistration) + SignMode.SignUp -> handleRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) SignMode.SignIn -> startAuthenticationFlow() SignMode.SignInWithMatrixId -> _viewEvents.post(OnboardingViewEvents.OnSignModeSelected(SignMode.SignInWithMatrixId)) SignMode.Unknown -> Unit @@ -509,18 +517,17 @@ class OnboardingViewModel @AssistedInject constructor( _viewEvents.post(OnboardingViewEvents.OnSignModeSelected(SignMode.SignIn)) } - private fun onFlowResponse(flowResult: FlowResult) { + private fun onFlowResponse(flowResult: FlowResult, onNextRegistrationStepAction: (FlowResult) -> Unit) { // If dummy stage is mandatory, and password is already sent, do the dummy stage now if (isRegistrationStarted && flowResult.missingStages.any { it is Stage.Dummy && it.mandatory }) { - handleRegisterDummy() + handleRegisterDummy(onNextRegistrationStepAction) } else { - // Notify the user - _viewEvents.post(OnboardingViewEvents.RegistrationFlowResult(flowResult, isRegistrationStarted)) + onNextRegistrationStepAction(flowResult) } } - private fun handleRegisterDummy() { - handleRegisterAction(RegisterAction.RegisterDummy) + private fun handleRegisterDummy(onNextRegistrationStepAction: (FlowResult) -> Unit) { + handleRegisterAction(RegisterAction.RegisterDummy, onNextRegistrationStepAction) } private suspend fun onSessionCreated(session: Session, isAccountCreated: Boolean) { @@ -599,19 +606,7 @@ class OnboardingViewModel @AssistedInject constructor( runCatching { startAuthenticationFlowUseCase.execute(homeServerConnectionConfig) }.fold( onSuccess = { - rememberHomeServer(homeServerConnectionConfig.homeServerUri.toString()) - if (it.isHomeserverOutdated) { - _viewEvents.post(OnboardingViewEvents.OutdatedHomeserver) - } - - setState { - copy( - serverType = alignServerTypeAfterSubmission(homeServerConnectionConfig, serverTypeOverride), - selectedHomeserver = it.selectedHomeserver, - isLoading = false, - ) - } - onAuthenticationStartedSuccess() + onAuthenticationStartedSuccess(homeServerConnectionConfig, it, serverTypeOverride) }, onFailure = { setState { copy(isLoading = false) } @@ -621,6 +616,48 @@ class OnboardingViewModel @AssistedInject constructor( } } + private fun onAuthenticationStartedSuccess(config: HomeServerConnectionConfig, authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult, serverTypeOverride: ServerType?) { + rememberHomeServer(config.homeServerUri.toString()) + if (authResult.isHomeserverOutdated) { + _viewEvents.post(OnboardingViewEvents.OutdatedHomeserver) + } + + setState { + copy( + serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), + selectedHomeserver = authResult.selectedHomeserver, + isLoading = false + ) + } + withState { + when (lastAction) { + is OnboardingAction.HomeServerChange.EditHomeServer -> { + when (it.onboardingFlow) { + OnboardingFlow.SignUp -> handleRegisterAction(RegisterAction.StartRegistration) { _ -> + _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) + } + else -> throw IllegalArgumentException("developer error") + } + } + is OnboardingAction.HomeServerChange.SelectHomeServer -> { + if (it.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { + when (it.onboardingFlow) { + OnboardingFlow.SignIn -> handleUpdateSignMode(OnboardingAction.UpdateSignMode(SignMode.SignIn)) + OnboardingFlow.SignUp -> handleUpdateSignMode(OnboardingAction.UpdateSignMode(SignMode.SignUp)) + OnboardingFlow.SignInSignUp, + null -> { + _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) + } + } + } else { + _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) + } + } + else -> _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) + } + } + } + /** * If user has entered https://matrix.org, ensure that server type is ServerType.MatrixOrg * It is also useful to set the value again in the case of a certificate error on matrix.org @@ -633,28 +670,6 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun onAuthenticationStartedSuccess() { - withState { - when (lastAction) { - is OnboardingAction.HomeServerChange.EditHomeServer -> _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) - is OnboardingAction.HomeServerChange.SelectHomeServer -> { - if (it.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { - when (it.onboardingFlow) { - OnboardingFlow.SignIn -> handleUpdateSignMode(OnboardingAction.UpdateSignMode(SignMode.SignIn)) - OnboardingFlow.SignUp -> handleUpdateSignMode(OnboardingAction.UpdateSignMode(SignMode.SignUp)) - OnboardingFlow.SignInSignUp, - null -> { - _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) - } - } - } else { - _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) - } - } - else -> _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) - } - } - } fun getInitialHomeServerUrl(): String? { return loginConfig?.homeServerUrl From 10be5920943e894d200156bdb7d7634351fdfd05 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Apr 2022 17:03:09 +0100 Subject: [PATCH 03/14] flattening nested scope.launch --- .../onboarding/OnboardingViewModel.kt | 120 +++++++++--------- 1 file changed, 62 insertions(+), 58 deletions(-) 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 e5bbf39127..617896ee0c 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 @@ -257,31 +257,35 @@ class OnboardingViewModel @AssistedInject constructor( private fun handleRegisterAction(action: RegisterAction, onNextRegistrationStepAction: (FlowResult) -> Unit) { currentJob = viewModelScope.launch { - if (action.hasLoadingState()) { - setState { copy(isLoading = true) } - } + internalRegisterAction(action, onNextRegistrationStepAction) + } + } - runCatching { registrationActionHandler.handleRegisterAction(registrationWizard, action) } - .fold( - onSuccess = { - when { - action.ignoresResult() -> { - // do nothing - } - else -> when (it) { - is RegistrationResult.Success -> onSessionCreated(it.session, isAccountCreated = true) - is RegistrationResult.FlowResponse -> onFlowResponse(it.flowResult, onNextRegistrationStepAction) - } + private suspend fun internalRegisterAction(action: RegisterAction, onNextRegistrationStepAction: (FlowResult) -> Unit) { + if (action.hasLoadingState()) { + setState { copy(isLoading = true) } + } + + runCatching { registrationActionHandler.handleRegisterAction(registrationWizard, action) } + .fold( + onSuccess = { + when { + action.ignoresResult() -> { + // do nothing } - }, - onFailure = { - if (it !is CancellationException) { - _viewEvents.post(OnboardingViewEvents.Failure(it)) + else -> when (it) { + is RegistrationResult.Success -> onSessionCreated(it.session, isAccountCreated = true) + is RegistrationResult.FlowResponse -> onFlowResponse(it.flowResult, onNextRegistrationStepAction) } } - ) - setState { copy(isLoading = false) } - } + }, + onFailure = { + if (it !is CancellationException) { + _viewEvents.post(OnboardingViewEvents.Failure(it)) + } + } + ) + setState { copy(isLoading = false) } } private fun emitFlowResultViewEvent(flowResult: FlowResult) { @@ -289,15 +293,17 @@ class OnboardingViewModel @AssistedInject constructor( } private fun handleRegisterWith(action: OnboardingAction.Register) { - reAuthHelper.data = action.password - handleRegisterAction( - RegisterAction.CreateAccount( - action.username, - action.password, - action.initialDeviceName - ), - ::emitFlowResultViewEvent - ) + currentJob = viewModelScope.launch { + reAuthHelper.data = action.password + internalRegisterAction( + RegisterAction.CreateAccount( + action.username, + action.password, + action.initialDeviceName + ), + ::emitFlowResultViewEvent + ) + } } private fun handleResetAction(action: OnboardingAction.ResetAction) { @@ -517,7 +523,7 @@ class OnboardingViewModel @AssistedInject constructor( _viewEvents.post(OnboardingViewEvents.OnSignModeSelected(SignMode.SignIn)) } - private fun onFlowResponse(flowResult: FlowResult, onNextRegistrationStepAction: (FlowResult) -> Unit) { + private suspend fun onFlowResponse(flowResult: FlowResult, onNextRegistrationStepAction: (FlowResult) -> Unit) { // If dummy stage is mandatory, and password is already sent, do the dummy stage now if (isRegistrationStarted && flowResult.missingStages.any { it is Stage.Dummy && it.mandatory }) { handleRegisterDummy(onNextRegistrationStepAction) @@ -526,8 +532,8 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun handleRegisterDummy(onNextRegistrationStepAction: (FlowResult) -> Unit) { - handleRegisterAction(RegisterAction.RegisterDummy, onNextRegistrationStepAction) + private suspend fun handleRegisterDummy(onNextRegistrationStepAction: (FlowResult) -> Unit) { + internalRegisterAction(RegisterAction.RegisterDummy, onNextRegistrationStepAction) } private suspend fun onSessionCreated(session: Session, isAccountCreated: Boolean) { @@ -616,7 +622,7 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun onAuthenticationStartedSuccess(config: HomeServerConnectionConfig, authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult, serverTypeOverride: ServerType?) { + private suspend fun onAuthenticationStartedSuccess(config: HomeServerConnectionConfig, authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult, serverTypeOverride: ServerType?) { rememberHomeServer(config.homeServerUri.toString()) if (authResult.isHomeserverOutdated) { _viewEvents.post(OnboardingViewEvents.OutdatedHomeserver) @@ -629,32 +635,31 @@ class OnboardingViewModel @AssistedInject constructor( isLoading = false ) } - withState { - when (lastAction) { - is OnboardingAction.HomeServerChange.EditHomeServer -> { - when (it.onboardingFlow) { - OnboardingFlow.SignUp -> handleRegisterAction(RegisterAction.StartRegistration) { _ -> - _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) - } - else -> throw IllegalArgumentException("developer error") + val state = awaitState() + when (lastAction) { + is OnboardingAction.HomeServerChange.EditHomeServer -> { + when (state.onboardingFlow) { + OnboardingFlow.SignUp -> handleRegisterAction(RegisterAction.StartRegistration) { _ -> + _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) } + else -> throw IllegalArgumentException("developer error") } - is OnboardingAction.HomeServerChange.SelectHomeServer -> { - if (it.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { - when (it.onboardingFlow) { - OnboardingFlow.SignIn -> handleUpdateSignMode(OnboardingAction.UpdateSignMode(SignMode.SignIn)) - OnboardingFlow.SignUp -> handleUpdateSignMode(OnboardingAction.UpdateSignMode(SignMode.SignUp)) - OnboardingFlow.SignInSignUp, - null -> { - _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) - } - } - } else { - _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) - } - } - else -> _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) } + is OnboardingAction.HomeServerChange.SelectHomeServer -> { + if (state.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { + when (state.onboardingFlow) { + OnboardingFlow.SignIn -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) + OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) + OnboardingFlow.SignInSignUp, + null -> { + _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) + } + } + } else { + _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) + } + } + else -> _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) } } @@ -670,7 +675,6 @@ class OnboardingViewModel @AssistedInject constructor( } } - fun getInitialHomeServerUrl(): String? { return loginConfig?.homeServerUrl } From 86b87e12d7de3678a15cc85d64dcc007f6a1f43d Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Apr 2022 17:27:04 +0100 Subject: [PATCH 04/14] flattening nested loading state changes to avoid flashing --- .../im/vector/app/features/onboarding/OnboardingViewModel.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 617896ee0c..7ce25190cd 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 @@ -615,10 +615,10 @@ class OnboardingViewModel @AssistedInject constructor( onAuthenticationStartedSuccess(homeServerConnectionConfig, it, serverTypeOverride) }, onFailure = { - setState { copy(isLoading = false) } _viewEvents.post(OnboardingViewEvents.Failure(it)) } ) + setState { copy(isLoading = false) } } } @@ -632,7 +632,6 @@ class OnboardingViewModel @AssistedInject constructor( copy( serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), selectedHomeserver = authResult.selectedHomeserver, - isLoading = false ) } val state = awaitState() From 197df340973f8d0490791e6c23e6d077ffbc5406 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 10:03:45 +0100 Subject: [PATCH 05/14] only setting selected homeserver state after a successful start registration when editing --- .../onboarding/OnboardingViewModel.kt | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) 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 7ce25190cd..448fd514ce 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 @@ -628,24 +628,33 @@ class OnboardingViewModel @AssistedInject constructor( _viewEvents.post(OnboardingViewEvents.OutdatedHomeserver) } - setState { - copy( - serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), - selectedHomeserver = authResult.selectedHomeserver, - ) - } val state = awaitState() + when (lastAction) { is OnboardingAction.HomeServerChange.EditHomeServer -> { when (state.onboardingFlow) { - OnboardingFlow.SignUp -> handleRegisterAction(RegisterAction.StartRegistration) { _ -> + OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration) { _ -> + setState { + copy( + serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), + selectedHomeserver = authResult.selectedHomeserver, + ) + } _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) } else -> throw IllegalArgumentException("developer error") } } is OnboardingAction.HomeServerChange.SelectHomeServer -> { - if (state.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { + setState { + copy( + serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), + selectedHomeserver = authResult.selectedHomeserver, + ) + } + + + if (authResult.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { when (state.onboardingFlow) { OnboardingFlow.SignIn -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) From c9e0868917f9e64f70c06ad942e1d21ec3070af3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 11:04:34 +0100 Subject: [PATCH 06/14] passing the authenication start trigger instead of relying on the mutable last action state --- .../onboarding/OnboardingViewModel.kt | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) 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 448fd514ce..b11d6ff54e 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 @@ -140,7 +140,7 @@ class OnboardingViewModel @AssistedInject constructor( is OnboardingAction.UpdateServerType -> handleUpdateServerType(action) is OnboardingAction.UpdateSignMode -> handleUpdateSignMode(action) is OnboardingAction.InitWith -> handleInitWith(action) - is OnboardingAction.HomeServerChange -> withAction(action) { handleHomeserverChange(action.homeServerUrl) } + 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.LoginWithToken -> handleLoginWithToken(action) @@ -175,7 +175,7 @@ class OnboardingViewModel @AssistedInject constructor( return when (val config = loginConfig.toHomeserverConfig()) { null -> continueToPageAfterSplash(onboardingFlow) - else -> startAuthenticationFlow(config, ServerType.Other) + else -> startAuthenticationFlow(trigger = null, config, ServerType.Other) } } @@ -209,7 +209,7 @@ class OnboardingViewModel @AssistedInject constructor( is OnboardingAction.HomeServerChange.SelectHomeServer -> { currentHomeServerConnectionConfig ?.let { it.copy(allowedFingerprints = it.allowedFingerprints + action.fingerprint) } - ?.let { startAuthenticationFlow(it) } + ?.let { startAuthenticationFlow(finalLastAction, it) } } is OnboardingAction.LoginOrRegister -> handleDirectLogin( @@ -594,35 +594,35 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun handleHomeserverChange(homeserverUrl: String) { - val homeServerConnectionConfig = homeServerConnectionConfigFactory.create(homeserverUrl) + private fun handleHomeserverChange(action: OnboardingAction.HomeServerChange) { + val homeServerConnectionConfig = homeServerConnectionConfigFactory.create(action.homeServerUrl) if (homeServerConnectionConfig == null) { // This is invalid _viewEvents.post(OnboardingViewEvents.Failure(Throwable("Unable to create a HomeServerConnectionConfig"))) } else { - startAuthenticationFlow(homeServerConnectionConfig) + startAuthenticationFlow(action, homeServerConnectionConfig) } } - private fun startAuthenticationFlow(homeServerConnectionConfig: HomeServerConnectionConfig, serverTypeOverride: ServerType? = null) { + private fun startAuthenticationFlow(trigger: OnboardingAction?, homeServerConnectionConfig: HomeServerConnectionConfig, serverTypeOverride: ServerType? = null) { currentHomeServerConnectionConfig = homeServerConnectionConfig currentJob = viewModelScope.launch { setState { copy(isLoading = true) } - runCatching { startAuthenticationFlowUseCase.execute(homeServerConnectionConfig) }.fold( - onSuccess = { - onAuthenticationStartedSuccess(homeServerConnectionConfig, it, serverTypeOverride) - }, - onFailure = { - _viewEvents.post(OnboardingViewEvents.Failure(it)) - } + onSuccess = { onAuthenticationStartedSuccess(trigger, homeServerConnectionConfig, it, serverTypeOverride) }, + onFailure = { _viewEvents.post(OnboardingViewEvents.Failure(it)) } ) setState { copy(isLoading = false) } } } - private suspend fun onAuthenticationStartedSuccess(config: HomeServerConnectionConfig, authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult, serverTypeOverride: ServerType?) { + private suspend fun onAuthenticationStartedSuccess( + trigger: OnboardingAction?, + config: HomeServerConnectionConfig, + authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult, + serverTypeOverride: ServerType? + ) { rememberHomeServer(config.homeServerUri.toString()) if (authResult.isHomeserverOutdated) { _viewEvents.post(OnboardingViewEvents.OutdatedHomeserver) @@ -630,7 +630,7 @@ class OnboardingViewModel @AssistedInject constructor( val state = awaitState() - when (lastAction) { + when (trigger) { is OnboardingAction.HomeServerChange.EditHomeServer -> { when (state.onboardingFlow) { OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration) { _ -> @@ -652,8 +652,6 @@ class OnboardingViewModel @AssistedInject constructor( selectedHomeserver = authResult.selectedHomeserver, ) } - - if (authResult.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { when (state.onboardingFlow) { OnboardingFlow.SignIn -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) From 5001be9f211466bb7913e67a1ccc9a7e5046c029 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 11:05:02 +0100 Subject: [PATCH 07/14] adding test around editing error flow and reducing initial test state setup boilerplate --- .../onboarding/OnboardingViewModelTest.kt | 86 ++++++++++++------- .../test/fakes/FakeRegisterActionHandler.kt | 4 + 2 files changed, 60 insertions(+), 30 deletions(-) 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 77b3f495f0..62fc9548b2 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 @@ -73,7 +73,6 @@ class OnboardingViewModelTest { private val fakeUri = FakeUri() private val fakeContext = FakeContext() - private val initialState = OnboardingViewState() private val fakeSession = FakeSession() private val fakeUriFilenameResolver = FakeUriFilenameResolver() private val fakeActiveSessionHolder = FakeActiveSessionHolder(fakeSession) @@ -85,11 +84,12 @@ class OnboardingViewModelTest { private val fakeStartAuthenticationFlowUseCase = FakeStartAuthenticationFlowUseCase() private val fakeHomeServerHistoryService = FakeHomeServerHistoryService() - lateinit var viewModel: OnboardingViewModel + private var initialState = OnboardingViewState() + private lateinit var viewModel: OnboardingViewModel @Before fun setUp() { - viewModel = createViewModel() + viewModelWith(initialState) } @Test @@ -105,8 +105,7 @@ class OnboardingViewModelTest { @Test fun `given supports changing display name, when handling PersonalizeProfile, then emits contents choose display name`() = runTest { - val initialState = initialState.copy(personalizationState = PersonalizationState(supportsChangingDisplayName = true, supportsChangingProfilePicture = false)) - viewModel = createViewModel(initialState) + viewModelWith(initialState.copy(personalizationState = PersonalizationState(supportsChangingDisplayName = true, supportsChangingProfilePicture = false))) val test = viewModel.test() viewModel.handle(OnboardingAction.PersonalizeProfile) @@ -118,8 +117,7 @@ class OnboardingViewModelTest { @Test fun `given only supports changing profile picture, when handling PersonalizeProfile, then emits contents choose profile picture`() = runTest { - val initialState = initialState.copy(personalizationState = PersonalizationState(supportsChangingDisplayName = false, supportsChangingProfilePicture = true)) - viewModel = createViewModel(initialState) + viewModelWith(initialState.copy(personalizationState = PersonalizationState(supportsChangingDisplayName = false, supportsChangingProfilePicture = true))) val test = viewModel.test() viewModel.handle(OnboardingAction.PersonalizeProfile) @@ -131,8 +129,7 @@ class OnboardingViewModelTest { @Test fun `given has sign in with matrix id sign mode, when handling login or register action, then logs in directly`() = runTest { - val initialState = initialState.copy(signMode = SignMode.SignInWithMatrixId) - viewModel = createViewModel(initialState) + viewModelWith(initialState.copy(signMode = SignMode.SignInWithMatrixId)) fakeDirectLoginUseCase.givenSuccessResult(A_LOGIN_OR_REGISTER_ACTION, config = null, result = fakeSession) givenInitialisesSession(fakeSession) val test = viewModel.test() @@ -151,8 +148,7 @@ class OnboardingViewModelTest { @Test fun `given has sign in with matrix id sign mode, when handling login or register action fails, then emits error`() = runTest { - val initialState = initialState.copy(signMode = SignMode.SignInWithMatrixId) - viewModel = createViewModel(initialState) + viewModelWith(initialState.copy(signMode = SignMode.SignInWithMatrixId)) fakeDirectLoginUseCase.givenFailureResult(A_LOGIN_OR_REGISTER_ACTION, config = null, cause = AN_ERROR) givenInitialisesSession(fakeSession) val test = viewModel.test() @@ -235,11 +231,13 @@ class OnboardingViewModelTest { } @Test - fun `given when editing homeserver, then updates selected homeserver state and emits edited event`() = runTest { - val test = viewModel.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(false, SELECTED_HOMESERVER_STATE)) + fakeStartAuthenticationFlowUseCase.givenResult(A_HOMESERVER_CONFIG, StartAuthenticationResult(isHomeserverOutdated = false, SELECTED_HOMESERVER_STATE)) + givenRegistrationResultFor(RegisterAction.StartRegistration, RegistrationResult.FlowResponse(AN_IGNORED_FLOW_RESULT)) fakeHomeServerHistoryService.expectUrlToBeAdded(A_HOMESERVER_CONFIG.homeServerUri.toString()) + val test = viewModel.test() viewModel.handle(OnboardingAction.HomeServerChange.EditHomeServer(A_HOMESERVER_URL)) @@ -247,12 +245,35 @@ class OnboardingViewModelTest { .assertStatesChanges( initialState, { copy(isLoading = true) }, - { copy(isLoading = false, selectedHomeserver = SELECTED_HOMESERVER_STATE) }, + { copy(selectedHomeserver = SELECTED_HOMESERVER_STATE) }, + { copy(isLoading = false) } + ) .assertEvents(OnboardingViewEvents.OnHomeserverEdited) .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()) + val test = viewModel.test() + + viewModel.handle(OnboardingAction.HomeServerChange.EditHomeServer(A_HOMESERVER_URL)) + + test + .assertStatesChanges( + initialState, + { copy(isLoading = true) }, + { copy(isLoading = false) } + ) + .assertEvents(OnboardingViewEvents.Failure(AN_ERROR)) + .finish() + } + @Test fun `given personalisation enabled, when registering account, then updates state and emits account created event`() = runTest { fakeVectorFeatures.givenPersonalisationEnabled() @@ -292,14 +313,13 @@ class OnboardingViewModelTest { @Test fun `given changing profile picture is supported, when updating display name, then updates upstream user display name and moves to choose profile picture`() = runTest { - val personalisedInitialState = initialState.copy(personalizationState = PersonalizationState(supportsChangingProfilePicture = true)) - viewModel = createViewModel(personalisedInitialState) + viewModelWith(initialState.copy(personalizationState = PersonalizationState(supportsChangingProfilePicture = true))) val test = viewModel.test() viewModel.handle(OnboardingAction.UpdateDisplayName(A_DISPLAY_NAME)) test - .assertStatesChanges(personalisedInitialState, expectedSuccessfulDisplayNameUpdateStates()) + .assertStatesChanges(initialState, expectedSuccessfulDisplayNameUpdateStates()) .assertEvents(OnboardingViewEvents.OnChooseProfilePicture) .finish() fakeSession.fakeProfileService.verifyUpdatedName(fakeSession.myUserId, A_DISPLAY_NAME) @@ -307,14 +327,13 @@ class OnboardingViewModelTest { @Test fun `given changing profile picture is not supported, when updating display name, then updates upstream user display name and completes personalization`() = runTest { - val personalisedInitialState = initialState.copy(personalizationState = PersonalizationState(supportsChangingProfilePicture = false)) - viewModel = createViewModel(personalisedInitialState) + viewModelWith(initialState.copy(personalizationState = PersonalizationState(supportsChangingProfilePicture = false))) val test = viewModel.test() viewModel.handle(OnboardingAction.UpdateDisplayName(A_DISPLAY_NAME)) test - .assertStatesChanges(personalisedInitialState, expectedSuccessfulDisplayNameUpdateStates()) + .assertStatesChanges(initialState, expectedSuccessfulDisplayNameUpdateStates()) .assertEvents(OnboardingViewEvents.OnPersonalizationComplete) .finish() fakeSession.fakeProfileService.verifyUpdatedName(fakeSession.myUserId, A_DISPLAY_NAME) @@ -354,14 +373,13 @@ class OnboardingViewModelTest { @Test fun `given a selected picture, when handling save selected profile picture, then updates upstream avatar and completes personalization`() = runTest { - val initialStateWithPicture = givenPictureSelected(fakeUri.instance, A_PICTURE_FILENAME) - viewModel = createViewModel(initialStateWithPicture) + viewModelWith(givenPictureSelected(fakeUri.instance, A_PICTURE_FILENAME)) val test = viewModel.test() viewModel.handle(OnboardingAction.SaveSelectedProfilePicture) test - .assertStates(expectedProfilePictureSuccessStates(initialStateWithPicture)) + .assertStates(expectedProfilePictureSuccessStates(initialState)) .assertEvents(OnboardingViewEvents.OnPersonalizationComplete) .finish() fakeSession.fakeProfileService.verifyAvatarUpdated(fakeSession.myUserId, fakeUri.instance, A_PICTURE_FILENAME) @@ -370,14 +388,13 @@ class OnboardingViewModelTest { @Test fun `given upstream update avatar fails, when saving selected profile picture, then emits failure event`() = runTest { fakeSession.fakeProfileService.givenUpdateAvatarErrors(AN_ERROR) - val initialStateWithPicture = givenPictureSelected(fakeUri.instance, A_PICTURE_FILENAME) - viewModel = createViewModel(initialStateWithPicture) + viewModelWith(givenPictureSelected(fakeUri.instance, A_PICTURE_FILENAME)) val test = viewModel.test() viewModel.handle(OnboardingAction.SaveSelectedProfilePicture) test - .assertStates(expectedProfilePictureFailureStates(initialStateWithPicture)) + .assertStates(expectedProfilePictureFailureStates(initialState)) .assertEvents(OnboardingViewEvents.Failure(AN_ERROR)) .finish() } @@ -406,8 +423,8 @@ class OnboardingViewModelTest { .finish() } - private fun createViewModel(state: OnboardingViewState = initialState): OnboardingViewModel { - return OnboardingViewModel( + private fun viewModelWith(state: OnboardingViewState) { + OnboardingViewModel( state, fakeContext.instance, fakeAuthenticationService, @@ -423,7 +440,10 @@ class OnboardingViewModelTest { fakeDirectLoginUseCase.instance, fakeStartAuthenticationFlowUseCase.instance, FakeVectorOverrides() - ) + ).also { + viewModel = it + initialState = state + } } private fun givenPictureSelected(fileUri: Uri, filename: String): OnboardingViewState { @@ -481,6 +501,12 @@ class OnboardingViewModelTest { fakeAuthenticationService.givenRegistrationWizard(registrationWizard) fakeRegisterActionHandler.givenResultsFor(registrationWizard, results) } + + private fun givenRegistrationActionErrors(action: RegisterAction, cause: Throwable) { + val registrationWizard = FakeRegistrationWizard() + fakeAuthenticationService.givenRegistrationWizard(registrationWizard) + fakeRegisterActionHandler.givenThrowsFor(registrationWizard, action, cause) + } } private fun HomeServerCapabilities.toPersonalisationState() = PersonalizationState( diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeRegisterActionHandler.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeRegisterActionHandler.kt index 8d595d91e9..61d0e438ab 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeRegisterActionHandler.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeRegisterActionHandler.kt @@ -33,4 +33,8 @@ class FakeRegisterActionHandler { result.first { it.first == actionArg }.second } } + + fun givenThrowsFor(wizard: RegistrationWizard, action: RegisterAction, cause: Throwable) { + coEvery { instance.handleRegisterAction(wizard, action) } throws cause + } } From ee693b5ad48d618a69f330099c4b2974ae19e32c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 11:27:13 +0100 Subject: [PATCH 08/14] flattening loading state to the handle entry points, reducing duplication --- .../onboarding/OnboardingViewModel.kt | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) 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 b11d6ff54e..00b36ba60e 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 @@ -257,15 +257,15 @@ class OnboardingViewModel @AssistedInject constructor( private fun handleRegisterAction(action: RegisterAction, onNextRegistrationStepAction: (FlowResult) -> Unit) { currentJob = viewModelScope.launch { + if (action.hasLoadingState()) { + setState { copy(isLoading = true) } + } internalRegisterAction(action, onNextRegistrationStepAction) + setState { copy(isLoading = false) } } } private suspend fun internalRegisterAction(action: RegisterAction, onNextRegistrationStepAction: (FlowResult) -> Unit) { - if (action.hasLoadingState()) { - setState { copy(isLoading = true) } - } - runCatching { registrationActionHandler.handleRegisterAction(registrationWizard, action) } .fold( onSuccess = { @@ -285,7 +285,6 @@ class OnboardingViewModel @AssistedInject constructor( } } ) - setState { copy(isLoading = false) } } private fun emitFlowResultViewEvent(flowResult: FlowResult) { @@ -293,17 +292,15 @@ class OnboardingViewModel @AssistedInject constructor( } private fun handleRegisterWith(action: OnboardingAction.Register) { - currentJob = viewModelScope.launch { - reAuthHelper.data = action.password - internalRegisterAction( - RegisterAction.CreateAccount( - action.username, - action.password, - action.initialDeviceName - ), - ::emitFlowResultViewEvent - ) - } + reAuthHelper.data = action.password + handleRegisterAction( + RegisterAction.CreateAccount( + action.username, + action.password, + action.initialDeviceName + ), + ::emitFlowResultViewEvent + ) } private fun handleResetAction(action: OnboardingAction.ResetAction) { From a34b424b7b11dc4d78531f8f1b1ff0950e3f2daf Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 11:46:34 +0100 Subject: [PATCH 09/14] updating the server selection on non Edit/Select events - such as deeplinks - extracts a common function --- .../onboarding/OnboardingViewModel.kt | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) 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 00b36ba60e..8c71441d46 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 @@ -625,32 +625,20 @@ class OnboardingViewModel @AssistedInject constructor( _viewEvents.post(OnboardingViewEvents.OutdatedHomeserver) } - val state = awaitState() - when (trigger) { is OnboardingAction.HomeServerChange.EditHomeServer -> { - when (state.onboardingFlow) { + when (awaitState().onboardingFlow) { OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration) { _ -> - setState { - copy( - serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), - selectedHomeserver = authResult.selectedHomeserver, - ) - } + updateServerSelection(config, serverTypeOverride, authResult) _viewEvents.post(OnboardingViewEvents.OnHomeserverEdited) } else -> throw IllegalArgumentException("developer error") } } is OnboardingAction.HomeServerChange.SelectHomeServer -> { - setState { - copy( - serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), - selectedHomeserver = authResult.selectedHomeserver, - ) - } + updateServerSelection(config, serverTypeOverride, authResult) if (authResult.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { - when (state.onboardingFlow) { + when (awaitState().onboardingFlow) { OnboardingFlow.SignIn -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) OnboardingFlow.SignInSignUp, @@ -662,7 +650,19 @@ class OnboardingViewModel @AssistedInject constructor( _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) } } - else -> _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) + else -> { + updateServerSelection(config, serverTypeOverride, authResult) + _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) + } + } + } + + private fun updateServerSelection(config: HomeServerConnectionConfig, serverTypeOverride: ServerType?, authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult) { + setState { + copy( + serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), + selectedHomeserver = authResult.selectedHomeserver, + ) } } From 1b33c03d9179d6aff7f17f3efff1ec2acb8033bc Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 12:01:23 +0100 Subject: [PATCH 10/14] lifting unavailable homeserver condition to the other error types --- .../org/matrix/android/sdk/api/failure/Extensions.kt | 6 ++++++ .../FtueAuthCombinedServerSelectionFragment.kt | 11 ++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt index 99fc0ba8b7..462c0cd638 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt @@ -22,6 +22,7 @@ import org.matrix.android.sdk.api.session.contentscanner.ContentScannerError import org.matrix.android.sdk.api.session.contentscanner.ScanFailure import org.matrix.android.sdk.internal.di.MoshiProvider import java.io.IOException +import java.net.UnknownHostException import javax.net.ssl.HttpsURLConnection fun Throwable.is401() = @@ -99,6 +100,11 @@ fun Throwable.isInvalidUIAAuth(): Boolean { error.flows != null } +fun Throwable.isHomeserverUnavailable(): Boolean { + return this is Failure.NetworkConnection && + this.ioException is UnknownHostException +} + /** * Try to convert to a RegistrationFlowResponse. Return null in the cases it's not possible */ diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt index 97e2db2123..2e6057288a 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt @@ -36,9 +36,8 @@ import im.vector.app.features.onboarding.OnboardingViewEvents import im.vector.app.features.onboarding.OnboardingViewState import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import org.matrix.android.sdk.api.failure.Failure +import org.matrix.android.sdk.api.failure.isHomeserverUnavailable import reactivecircus.flowbinding.android.widget.textChanges -import java.net.UnknownHostException import javax.inject.Inject class FtueAuthCombinedServerSelectionFragment @Inject constructor() : AbstractFtueAuthFragment() { @@ -88,11 +87,9 @@ class FtueAuthCombinedServerSelectionFragment @Inject constructor() : AbstractFt } override fun onError(throwable: Throwable) { - views.chooseServerInput.error = if (throwable is Failure.NetworkConnection && - throwable.ioException is UnknownHostException) { - getString(R.string.login_error_homeserver_not_found) - } else { - errorFormatter.toHumanReadable(throwable) + views.chooseServerInput.error = when { + throwable.isHomeserverUnavailable() -> getString(R.string.login_error_homeserver_not_found) + else -> errorFormatter.toHumanReadable(throwable) } } From be22be53df7c9c864e2c73617c5097643eeed462 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 12:22:49 +0100 Subject: [PATCH 11/14] fixing line length --- .../app/features/onboarding/OnboardingViewModel.kt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 8c71441d46..0faa66024c 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 @@ -41,6 +41,7 @@ import im.vector.app.features.login.LoginMode import im.vector.app.features.login.ReAuthHelper import im.vector.app.features.login.ServerType import im.vector.app.features.login.SignMode +import im.vector.app.features.onboarding.StartAuthenticationFlowUseCase.StartAuthenticationResult import kotlinx.coroutines.Job import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.launch @@ -601,7 +602,11 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun startAuthenticationFlow(trigger: OnboardingAction?, homeServerConnectionConfig: HomeServerConnectionConfig, serverTypeOverride: ServerType? = null) { + private fun startAuthenticationFlow( + trigger: OnboardingAction?, + homeServerConnectionConfig: HomeServerConnectionConfig, + serverTypeOverride: ServerType? = null + ) { currentHomeServerConnectionConfig = homeServerConnectionConfig currentJob = viewModelScope.launch { @@ -617,7 +622,7 @@ class OnboardingViewModel @AssistedInject constructor( private suspend fun onAuthenticationStartedSuccess( trigger: OnboardingAction?, config: HomeServerConnectionConfig, - authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult, + authResult: StartAuthenticationResult, serverTypeOverride: ServerType? ) { rememberHomeServer(config.homeServerUri.toString()) @@ -657,7 +662,7 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun updateServerSelection(config: HomeServerConnectionConfig, serverTypeOverride: ServerType?, authResult: StartAuthenticationFlowUseCase.StartAuthenticationResult) { + private fun updateServerSelection(config: HomeServerConnectionConfig, serverTypeOverride: ServerType?, authResult: StartAuthenticationResult) { setState { copy( serverType = alignServerTypeAfterSubmission(config, serverTypeOverride), From 55c981f18b03da10c8f58fe84dbcb696c7825fd4 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 12:37:46 +0100 Subject: [PATCH 12/14] adding back sign mode setting to fix crash when using legacy other flow --- .../onboarding/OnboardingViewModel.kt | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) 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 0faa66024c..2b286e6d93 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 @@ -349,12 +349,7 @@ class OnboardingViewModel @AssistedInject constructor( } private fun handleUpdateSignMode(action: OnboardingAction.UpdateSignMode) { - setState { - copy( - signMode = action.signMode - ) - } - + updateSignMode(action.signMode) when (action.signMode) { SignMode.SignUp -> handleRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) SignMode.SignIn -> startAuthenticationFlow() @@ -363,6 +358,10 @@ class OnboardingViewModel @AssistedInject constructor( } } + private fun updateSignMode(signMode: SignMode) { + setState { copy(signMode = signMode) } + } + private fun handleUpdateUseCase(action: OnboardingAction.UpdateUseCase) { setState { copy(useCase = action.useCase) } when (vectorFeatures.isOnboardingCombinedRegisterEnabled()) { @@ -644,8 +643,14 @@ class OnboardingViewModel @AssistedInject constructor( updateServerSelection(config, serverTypeOverride, authResult) if (authResult.selectedHomeserver.preferredLoginMode.supportsSignModeScreen()) { when (awaitState().onboardingFlow) { - OnboardingFlow.SignIn -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) - OnboardingFlow.SignUp -> internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) + OnboardingFlow.SignIn -> { + updateSignMode(SignMode.SignIn) + internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) + } + OnboardingFlow.SignUp -> { + updateSignMode(SignMode.SignUp) + internalRegisterAction(RegisterAction.StartRegistration, ::emitFlowResultViewEvent) + } OnboardingFlow.SignInSignUp, null -> { _viewEvents.post(OnboardingViewEvents.OnLoginFlowRetrieved) From 11ef4a7fec79a3ae4b3d0b1864a8a79c5b9b5224 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 13:34:54 +0100 Subject: [PATCH 13/14] adding changelog entry --- changelog.d/5749.wip | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5749.wip diff --git a/changelog.d/5749.wip b/changelog.d/5749.wip new file mode 100644 index 0000000000..a933f55cf5 --- /dev/null +++ b/changelog.d/5749.wip @@ -0,0 +1 @@ +Adds error handling within the new FTUE server selection screen \ No newline at end of file From eda1d9142ca2fac44bb2d6a5635221f09f516f58 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Apr 2022 16:10:22 +0100 Subject: [PATCH 14/14] using expression bodies for boolean checks - moves first expression line onto the declaration line --- .../android/sdk/api/failure/Extensions.kt | 103 +++++++----------- 1 file changed, 41 insertions(+), 62 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt index 462c0cd638..362ebcec26 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt @@ -25,32 +25,26 @@ import java.io.IOException import java.net.UnknownHostException import javax.net.ssl.HttpsURLConnection -fun Throwable.is401() = - this is Failure.ServerError && - httpCode == HttpsURLConnection.HTTP_UNAUTHORIZED && /* 401 */ - error.code == MatrixError.M_UNAUTHORIZED +fun Throwable.is401() = this is Failure.ServerError && + httpCode == HttpsURLConnection.HTTP_UNAUTHORIZED && /* 401 */ + error.code == MatrixError.M_UNAUTHORIZED -fun Throwable.is404() = - this is Failure.ServerError && - httpCode == HttpsURLConnection.HTTP_NOT_FOUND && /* 404 */ - error.code == MatrixError.M_NOT_FOUND +fun Throwable.is404() = this is Failure.ServerError && + httpCode == HttpsURLConnection.HTTP_NOT_FOUND && /* 404 */ + error.code == MatrixError.M_NOT_FOUND -fun Throwable.isTokenError() = - this is Failure.ServerError && - (error.code == MatrixError.M_UNKNOWN_TOKEN || - error.code == MatrixError.M_MISSING_TOKEN || - error.code == MatrixError.ORG_MATRIX_EXPIRED_ACCOUNT) +fun Throwable.isTokenError() = this is Failure.ServerError && + (error.code == MatrixError.M_UNKNOWN_TOKEN || + error.code == MatrixError.M_MISSING_TOKEN || + error.code == MatrixError.ORG_MATRIX_EXPIRED_ACCOUNT) -fun Throwable.isLimitExceededError() = - this is Failure.ServerError && - httpCode == 429 && - error.code == MatrixError.M_LIMIT_EXCEEDED +fun Throwable.isLimitExceededError() = this is Failure.ServerError && + httpCode == 429 && + error.code == MatrixError.M_LIMIT_EXCEEDED -fun Throwable.shouldBeRetried(): Boolean { - return this is Failure.NetworkConnection || - this is IOException || - this.isLimitExceededError() -} +fun Throwable.shouldBeRetried() = this is Failure.NetworkConnection || + this is IOException || + isLimitExceededError() /** * Get the retry delay in case of rate limit exceeded error, adding 100 ms, of defaultValue otherwise @@ -64,46 +58,33 @@ fun Throwable.getRetryDelay(defaultValue: Long): Long { ?: defaultValue } -fun Throwable.isUsernameInUse(): Boolean { - return this is Failure.ServerError && error.code == MatrixError.M_USER_IN_USE -} +fun Throwable.isUsernameInUse() = this is Failure.ServerError && + error.code == MatrixError.M_USER_IN_USE -fun Throwable.isInvalidUsername(): Boolean { - return this is Failure.ServerError && - error.code == MatrixError.M_INVALID_USERNAME -} +fun Throwable.isInvalidUsername() = this is Failure.ServerError && + error.code == MatrixError.M_INVALID_USERNAME -fun Throwable.isInvalidPassword(): Boolean { - return this is Failure.ServerError && - error.code == MatrixError.M_FORBIDDEN && - error.message == "Invalid password" -} +fun Throwable.isInvalidPassword() = this is Failure.ServerError && + error.code == MatrixError.M_FORBIDDEN && + error.message == "Invalid password" -fun Throwable.isRegistrationDisabled(): Boolean { - return this is Failure.ServerError && error.code == MatrixError.M_FORBIDDEN && - httpCode == HttpsURLConnection.HTTP_FORBIDDEN -} +fun Throwable.isRegistrationDisabled() = this is Failure.ServerError && + error.code == MatrixError.M_FORBIDDEN && + httpCode == HttpsURLConnection.HTTP_FORBIDDEN -fun Throwable.isWeakPassword(): Boolean { - return this is Failure.ServerError && error.code == MatrixError.M_WEAK_PASSWORD -} +fun Throwable.isWeakPassword() = this is Failure.ServerError && + error.code == MatrixError.M_WEAK_PASSWORD -fun Throwable.isLoginEmailUnknown(): Boolean { - return this is Failure.ServerError && - error.code == MatrixError.M_FORBIDDEN && - error.message.isEmpty() -} +fun Throwable.isLoginEmailUnknown() = this is Failure.ServerError && + error.code == MatrixError.M_FORBIDDEN && + error.message.isEmpty() -fun Throwable.isInvalidUIAAuth(): Boolean { - return this is Failure.ServerError && - error.code == MatrixError.M_FORBIDDEN && - error.flows != null -} +fun Throwable.isInvalidUIAAuth() = this is Failure.ServerError && + error.code == MatrixError.M_FORBIDDEN && + error.flows != null -fun Throwable.isHomeserverUnavailable(): Boolean { - return this is Failure.NetworkConnection && - this.ioException is UnknownHostException -} +fun Throwable.isHomeserverUnavailable() = this is Failure.NetworkConnection && + this.ioException is UnknownHostException /** * Try to convert to a RegistrationFlowResponse. Return null in the cases it's not possible @@ -135,13 +116,11 @@ fun Throwable.toRegistrationFlowResponse(): RegistrationFlowResponse? { } } -fun Throwable.isRegistrationAvailabilityError(): Boolean { - return this is Failure.ServerError && - httpCode == HttpsURLConnection.HTTP_BAD_REQUEST && /* 400 */ - (error.code == MatrixError.M_USER_IN_USE || - error.code == MatrixError.M_INVALID_USERNAME || - error.code == MatrixError.M_EXCLUSIVE) -} +fun Throwable.isRegistrationAvailabilityError() = this is Failure.ServerError && + httpCode == HttpsURLConnection.HTTP_BAD_REQUEST && /* 400 */ + (error.code == MatrixError.M_USER_IN_USE || + error.code == MatrixError.M_INVALID_USERNAME || + error.code == MatrixError.M_EXCLUSIVE) /** * Try to convert to a ScanFailure. Return null in the cases it's not possible