Merge pull request #6888 from vector-im/feature/adm/change-server-on-error

FTUE - Allow changing server on onboarding start error

changelog.d/6718.bugfix

@@ -0,0 +1 @@
+Fixes onboarding requiring matrix.org to be accessible on the first step, the server can now be manually changed

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt

@@ -89,6 +89,8 @@ fun Throwable.isInvalidUIAAuth() = this is Failure.ServerError &&
 fun Throwable.isHomeserverUnavailable() = this is Failure.NetworkConnection &&
         this.ioException is UnknownHostException
 
+fun Throwable.isHomeserverConnectionError() = this is Failure.NetworkConnection
+
 fun Throwable.isMissingEmailVerification() = this is Failure.ServerError &&
         error.code == MatrixError.M_UNAUTHORIZED &&
         error.message == "Unable to get validated threepid"

vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt

@@ -29,7 +29,6 @@ import org.matrix.android.sdk.api.failure.Failure as SdkFailure
 sealed class OnboardingViewEvents : VectorViewEvents {
     data class Loading(val message: CharSequence? = null) : OnboardingViewEvents()
     data class Failure(val throwable: Throwable) : OnboardingViewEvents()
-    data class DeeplinkAuthenticationFailure(val retryAction: OnboardingAction) : OnboardingViewEvents()
     data class UnrecognisedCertificateFailure(val retryAction: OnboardingAction, val cause: SdkFailure.UnrecognizedCertificateFailure) : OnboardingViewEvents()
 
     object DisplayRegistrationFallback : OnboardingViewEvents()

vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt

@@ -61,6 +61,7 @@ import org.matrix.android.sdk.api.auth.login.LoginWizard
 import org.matrix.android.sdk.api.auth.registration.RegistrationAvailability
 import org.matrix.android.sdk.api.auth.registration.RegistrationWizard
 import org.matrix.android.sdk.api.failure.Failure
+import org.matrix.android.sdk.api.failure.isHomeserverConnectionError
 import org.matrix.android.sdk.api.failure.isHomeserverUnavailable
 import org.matrix.android.sdk.api.failure.isUnrecognisedCertificate
 import org.matrix.android.sdk.api.network.ssl.Fingerprint
@@ -701,14 +702,15 @@ class OnboardingViewModel @AssistedInject constructor(
 
     private fun onAuthenticationStartError(error: Throwable, trigger: OnboardingAction.HomeServerChange) {
         when {
-            error.isHomeserverUnavailable() && applicationContext.inferNoConnectivity(sdkIntProvider) -> _viewEvents.post(
-                    OnboardingViewEvents.Failure(error)
-            )
-            deeplinkUrlIsUnavailable(error, trigger) -> _viewEvents.post(
-                    OnboardingViewEvents.DeeplinkAuthenticationFailure(
-                            retryAction = (trigger as OnboardingAction.HomeServerChange.SelectHomeServer).resetToDefaultUrl()
-                    )
-            )
+            error.isHomeserverUnavailable() && applicationContext.inferNoConnectivity(sdkIntProvider) -> _viewEvents.post(OnboardingViewEvents.Failure(error))
+            isUnableToSelectServer(error, trigger) -> {
+                withState { state ->
+                    when {
+                        canEditServerSelectionError(state) -> handle(OnboardingAction.PostViewEvent(OnboardingViewEvents.EditServerSelection))
+                        else -> _viewEvents.post(OnboardingViewEvents.Failure(error))
+                    }
+                }
+            }
             error.isUnrecognisedCertificate() -> {
                 _viewEvents.post(OnboardingViewEvents.UnrecognisedCertificateFailure(trigger, error as Failure.UnrecognizedCertificateFailure))
             }
@@ -716,11 +718,12 @@ class OnboardingViewModel @AssistedInject constructor(
         }
     }
 
-    private fun deeplinkUrlIsUnavailable(error: Throwable, trigger: OnboardingAction.HomeServerChange) = error.isHomeserverUnavailable() &&
-            loginConfig != null &&
-            trigger is OnboardingAction.HomeServerChange.SelectHomeServer
+    private fun canEditServerSelectionError(state: OnboardingViewState) =
+            (state.onboardingFlow == OnboardingFlow.SignIn && vectorFeatures.isOnboardingCombinedLoginEnabled()) ||
+                    (state.onboardingFlow == OnboardingFlow.SignUp && vectorFeatures.isOnboardingCombinedRegisterEnabled())
 
-    private fun OnboardingAction.HomeServerChange.SelectHomeServer.resetToDefaultUrl() = copy(homeServerUrl = defaultHomeserverUrl)
+    private fun isUnableToSelectServer(error: Throwable, trigger: OnboardingAction.HomeServerChange) =
+            trigger is OnboardingAction.HomeServerChange.SelectHomeServer && error.isHomeserverConnectionError()
 
     private suspend fun onAuthenticationStartedSuccess(
             trigger: OnboardingAction.HomeServerChange,
@@ -807,6 +810,8 @@ class OnboardingViewModel @AssistedInject constructor(
         return loginConfig?.homeServerUrl
     }
 
+    fun getDefaultHomeserverUrl() = defaultHomeserverUrl
+
     fun fetchSsoUrl(redirectUrl: String, deviceId: String?, provider: SsoIdentityProvider?): String? {
         setState {
             val authDescription = AuthenticationDescription.Register(provider.toAuthenticationType())

vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedServerSelectionFragment.kt

@@ -86,7 +86,7 @@ class FtueAuthCombinedServerSelectionFragment @Inject constructor() : AbstractFt
         )
 
         if (views.chooseServerInput.content().isEmpty()) {
-            val userUrlInput = state.selectedHomeserver.userFacingUrl?.toReducedUrlKeepingSchemaIfInsecure()
+            val userUrlInput = state.selectedHomeserver.userFacingUrl?.toReducedUrlKeepingSchemaIfInsecure() ?: viewModel.getDefaultHomeserverUrl()
             views.chooseServerInput.editText().setText(userUrlInput)
         }
     }

vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt

@@ -46,6 +46,7 @@ import im.vector.app.features.login.SignMode
 import im.vector.app.features.login.TextInputFormFragmentMode
 import im.vector.app.features.onboarding.OnboardingAction
 import im.vector.app.features.onboarding.OnboardingActivity
+import im.vector.app.features.onboarding.OnboardingFlow
 import im.vector.app.features.onboarding.OnboardingVariant
 import im.vector.app.features.onboarding.OnboardingViewEvents
 import im.vector.app.features.onboarding.OnboardingViewModel
@@ -213,7 +214,7 @@ class FtueAuthVariant(
                         option = commonOption
                 )
             }
-            OnboardingViewEvents.OpenCombinedRegister -> openStartCombinedRegister()
+            OnboardingViewEvents.OpenCombinedRegister -> onStartCombinedRegister()
             is OnboardingViewEvents.OnAccountCreated -> onAccountCreated()
             OnboardingViewEvents.OnAccountSignedIn -> onAccountSignedIn()
             OnboardingViewEvents.OnChooseDisplayName -> onChooseDisplayName()
@@ -229,42 +230,47 @@ class FtueAuthVariant(
                         tag = FRAGMENT_EDIT_HOMESERVER_TAG
                 )
             }
-            OnboardingViewEvents.OnHomeserverEdited -> supportFragmentManager.popBackStack(
-                    FRAGMENT_EDIT_HOMESERVER_TAG,
-                    FragmentManager.POP_BACK_STACK_INCLUSIVE
-            )
+            OnboardingViewEvents.OnHomeserverEdited -> {
+                supportFragmentManager.popBackStack(
+                        FRAGMENT_EDIT_HOMESERVER_TAG,
+                        FragmentManager.POP_BACK_STACK_INCLUSIVE
+                )
+                ensureEditServerBackstack()
+            }
             OnboardingViewEvents.OpenCombinedLogin -> onStartCombinedLogin()
-            is OnboardingViewEvents.DeeplinkAuthenticationFailure -> onDeeplinkedHomeserverUnavailable(viewEvents)
             OnboardingViewEvents.DisplayRegistrationFallback -> displayFallbackWebDialog()
             is OnboardingViewEvents.DisplayRegistrationStage -> doStage(viewEvents.stage)
             OnboardingViewEvents.DisplayStartRegistration -> when {
-                vectorFeatures.isOnboardingCombinedRegisterEnabled() -> openStartCombinedRegister()
+                vectorFeatures.isOnboardingCombinedRegisterEnabled() -> onStartCombinedRegister()
                 else -> openAuthLoginFragmentWithTag(FRAGMENT_REGISTRATION_STAGE_TAG)
             }
         }
     }
 
-    private fun onDeeplinkedHomeserverUnavailable(viewEvents: OnboardingViewEvents.DeeplinkAuthenticationFailure) {
-        showHomeserverUnavailableDialog(onboardingViewModel.getInitialHomeServerUrl().orEmpty()) {
-            onboardingViewModel.handle(OnboardingAction.ResetDeeplinkConfig)
-            onboardingViewModel.handle(viewEvents.retryAction)
+    private fun ensureEditServerBackstack() {
+        when (activity.supportFragmentManager.findFragmentById(views.loginFragmentContainer.id)) {
+            is FtueAuthCombinedLoginFragment,
+            is FtueAuthCombinedRegisterFragment -> {
+                // do nothing
+            }
+            else -> {
+                withState(onboardingViewModel) { state ->
+                    when (state.onboardingFlow) {
+                        OnboardingFlow.SignIn -> onStartCombinedLogin()
+                        OnboardingFlow.SignUp -> onStartCombinedRegister()
+                        OnboardingFlow.SignInSignUp,
+                        null -> error("${state.onboardingFlow} does not support editing server url")
+                    }
+                }
+            }
         }
     }
 
-    private fun showHomeserverUnavailableDialog(url: String, action: () -> Unit) {
-        MaterialAlertDialogBuilder(activity)
-                .setTitle(R.string.dialog_title_error)
-                .setMessage(activity.getString(R.string.login_error_homeserver_from_url_not_found, url))
-                .setPositiveButton(R.string.login_error_homeserver_from_url_not_found_enter_manual) { _, _ -> action() }
-                .setNegativeButton(R.string.action_cancel, null)
-                .show()
-    }
-
     private fun onStartCombinedLogin() {
         addRegistrationStageFragmentToBackstack(FtueAuthCombinedLoginFragment::class.java, allowStateLoss = true)
     }
 
-    private fun openStartCombinedRegister() {
+    private fun onStartCombinedRegister() {
         addRegistrationStageFragmentToBackstack(FtueAuthCombinedRegisterFragment::class.java, allowStateLoss = true)
     }
 

vector/src/test/java/im/vector/app/features/onboarding/OnboardingViewModelTest.kt

@@ -412,23 +412,59 @@ class OnboardingViewModelTest {
     }
 
     @Test
-    fun `given unavailable deeplink, when selecting homeserver, then emits failure with default homeserver as retry action`() = runTest {
-        fakeContext.givenHasConnection()
-        fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, fingerprint = null, A_HOMESERVER_CONFIG)
-        fakeStartAuthenticationFlowUseCase.givenHomeserverUnavailable(A_HOMESERVER_CONFIG)
+    fun `given in sign in flow, when selecting homeserver fails with network error, then emits Failure`() = runTest {
+        viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignIn))
+        fakeVectorFeatures.givenCombinedLoginEnabled()
+        givenHomeserverSelectionFailsWith(AN_ERROR)
         val test = viewModel.test()
 
-        viewModel.handle(OnboardingAction.InitWith(LoginConfig(A_HOMESERVER_URL, null)))
         viewModel.handle(OnboardingAction.HomeServerChange.SelectHomeServer(A_HOMESERVER_URL))
 
-        val expectedRetryAction = OnboardingAction.HomeServerChange.SelectHomeServer("${R.string.matrix_org_server_url.toTestString()}/")
         test
                 .assertStatesChanges(
                         initialState,
                         { copy(isLoading = true) },
                         { copy(isLoading = false) }
                 )
-                .assertEvents(OnboardingViewEvents.DeeplinkAuthenticationFailure(expectedRetryAction))
+                .assertEvents(OnboardingViewEvents.Failure(AN_ERROR))
+                .finish()
+    }
+
+    @Test
+    fun `given in sign in flow, when selecting homeserver fails with network error, then emits EditServerSelection`() = runTest {
+        viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignIn))
+        fakeVectorFeatures.givenCombinedLoginEnabled()
+        givenHomeserverSelectionFailsWithNetworkError()
+        val test = viewModel.test()
+
+        viewModel.handle(OnboardingAction.HomeServerChange.SelectHomeServer(A_HOMESERVER_URL))
+
+        test
+                .assertStatesChanges(
+                        initialState,
+                        { copy(isLoading = true) },
+                        { copy(isLoading = false) }
+                )
+                .assertEvents(OnboardingViewEvents.EditServerSelection)
+                .finish()
+    }
+
+    @Test
+    fun `given in sign up flow, when selecting homeserver fails with network error, then emits EditServerSelection`() = runTest {
+        viewModelWith(initialState.copy(onboardingFlow = OnboardingFlow.SignUp))
+        fakeVectorFeatures.givenCombinedRegisterEnabled()
+        givenHomeserverSelectionFailsWithNetworkError()
+        val test = viewModel.test()
+
+        viewModel.handle(OnboardingAction.HomeServerChange.SelectHomeServer(A_HOMESERVER_URL))
+
+        test
+                .assertStatesChanges(
+                        initialState,
+                        { copy(isLoading = true) },
+                        { copy(isLoading = false) }
+                )
+                .assertEvents(OnboardingViewEvents.EditServerSelection)
                 .finish()
     }
 
@@ -1142,6 +1178,18 @@ class OnboardingViewModelTest {
     private fun initialRegistrationState(homeServerUrl: String) = initialState.copy(
             onboardingFlow = OnboardingFlow.SignUp, selectedHomeserver = SelectedHomeserverState(userFacingUrl = homeServerUrl)
     )
+
+    private fun givenHomeserverSelectionFailsWithNetworkError() {
+        fakeContext.givenHasConnection()
+        fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, fingerprint = null, A_HOMESERVER_CONFIG)
+        fakeStartAuthenticationFlowUseCase.givenHomeserverUnavailable(A_HOMESERVER_CONFIG)
+    }
+
+    private fun givenHomeserverSelectionFailsWith(cause: Throwable) {
+        fakeContext.givenHasConnection()
+        fakeHomeServerConnectionConfigFactory.givenConfigFor(A_HOMESERVER_URL, fingerprint = null, A_HOMESERVER_CONFIG)
+        fakeStartAuthenticationFlowUseCase.givenErrors(A_HOMESERVER_CONFIG, cause)
+    }
 }
 
 private fun HomeServerCapabilities.toPersonalisationState(displayName: String? = null) = PersonalizationState(