Selecting a server that doesn't support login now fails instead of letting you continue to a failure later. (#3342)

* Fail configuration of the authentication service if the homeserver doesn't support login. * Move the ServerSelectionCoordinator logic into the ViewModel. - Handle the new login alert. - Add more tests
2026-11-02 21:33:26 +00:00 · 2024-09-27 13:58:05 +01:00
parent 8a3994016d
commit 17dfe1359a
17 changed files with 263 additions and 139 deletions
@@ -1,32 +0,0 @@
-//
-// Copyright 2022-2024 New Vector Ltd.
-//
-// SPDX-License-Identifier: AGPL-3.0-only
-// Please see LICENSE in the repository root for full details.
-//
-
-import SwiftUI
-
-enum MockServerSelectionScreenState: CaseIterable {
-    case matrix
-    case emptyAddress
-    case invalidAddress
-    
-    /// Generate the view struct for the screen state.
-    @MainActor var viewModel: ServerSelectionScreenViewModel {
-        switch self {
-        case .matrix:
-            return ServerSelectionScreenViewModel(homeserverAddress: "https://matrix.org",
-                                                  slidingSyncLearnMoreURL: ServiceLocator.shared.settings.slidingSyncLearnMoreURL)
-                                                  
-        case .emptyAddress:
-            return ServerSelectionScreenViewModel(homeserverAddress: "",
-                                                  slidingSyncLearnMoreURL: ServiceLocator.shared.settings.slidingSyncLearnMoreURL)
-        case .invalidAddress:
-            let viewModel = ServerSelectionScreenViewModel(homeserverAddress: "thisisbad",
-                                                           slidingSyncLearnMoreURL: ServiceLocator.shared.settings.slidingSyncLearnMoreURL)
-            viewModel.displayError(.footerMessage(L10n.errorUnknown))
-            return viewModel
-        }
-    }
-}
@@ -37,8 +37,10 @@ final class ServerSelectionScreenCoordinator: CoordinatorProtocol {
    
    init(parameters: ServerSelectionScreenCoordinatorParameters) {
        self.parameters = parameters
-        viewModel = ServerSelectionScreenViewModel(homeserverAddress: parameters.authenticationService.homeserver.value.address,
-                                                   slidingSyncLearnMoreURL: parameters.slidingSyncLearnMoreURL)
+        viewModel = ServerSelectionScreenViewModel(authenticationService: parameters.authenticationService,
+                                                   authenticationFlow: parameters.authenticationFlow,
+                                                   slidingSyncLearnMoreURL: parameters.slidingSyncLearnMoreURL,
+                                                   userIndicatorController: parameters.userIndicatorController)
        userIndicatorController = parameters.userIndicatorController
    }
    
@@ -50,8 +52,8 @@ final class ServerSelectionScreenCoordinator: CoordinatorProtocol {
                guard let self else { return }
                
                switch action {
-                case .confirm(let homeserverAddress):
-                    self.useHomeserver(homeserverAddress)
+                case .updated:
+                    actionsSubject.send(.updated)
                case .dismiss:
                    actionsSubject.send(.dismiss)
                }
@@ -60,56 +62,10 @@ final class ServerSelectionScreenCoordinator: CoordinatorProtocol {
    }
    
    func stop() {
-        stopLoading()
+        parameters.userIndicatorController.retractAllIndicators()
    }
    
    func toPresentable() -> AnyView {
        AnyView(ServerSelectionScreen(context: viewModel.context))
    }
-    
-    // MARK: - Private
-    
-    private func startLoading(label: String = L10n.commonLoading) {
-        userIndicatorController.submitIndicator(UserIndicator(type: .modal,
-                                                              title: label,
-                                                              persistent: true))
-    }
-    
-    private func stopLoading() {
-        userIndicatorController.retractAllIndicators()
-    }
-    
-    /// Updates the login flow using the supplied homeserver address, or shows an error when this isn't possible.
-    private func useHomeserver(_ homeserverAddress: String) {
-        startLoading()
-        
-        Task {
-            switch await authenticationService.configure(for: homeserverAddress, flow: parameters.authenticationFlow) {
-            case .success:
-                MXLog.info("Selected homeserver: \(homeserverAddress)")
-                actionsSubject.send(.updated)
-                stopLoading()
-            case .failure(let error):
-                MXLog.info("Invalid homeserver: \(homeserverAddress)")
-                stopLoading()
-                handleError(error)
-            }
-        }
-    }
-    
-    /// Processes an error to either update the flow or display it to the user.
-    private func handleError(_ error: AuthenticationServiceError) {
-        switch error {
-        case .invalidServer, .invalidHomeserverAddress:
-            viewModel.displayError(.footerMessage(L10n.screenChangeServerErrorInvalidHomeserver))
-        case .invalidWellKnown(let error):
-            viewModel.displayError(.invalidWellKnownAlert(error))
-        case .slidingSyncNotAvailable:
-            viewModel.displayError(.slidingSyncAlert)
-        case .registrationNotSupported:
-            viewModel.displayError(.registrationAlert) // TODO: [DOUG] Test me!
-        default:
-            viewModel.displayError(.footerMessage(L10n.errorUnknown))
-        }
-    }
 }
@@ -8,8 +8,8 @@
 import Foundation

 enum ServerSelectionScreenViewModelAction {
-    /// The user would like to use the homeserver at the given address.
-    case confirm(homeserverAddress: String)
+    /// The homeserver selection has been updated.
+    case updated
    /// Dismiss the view without using the entered address.
    case dismiss
 }
@@ -74,6 +74,8 @@ enum ServerSelectionScreenErrorType: Hashable {
    case invalidWellKnownAlert(String)
    /// An alert that allows the user to learn about sliding sync.
    case slidingSyncAlert
+    /// An alert that informs the user that login isn't supported.
+    case loginAlert
    /// An alert that informs the user that registration isn't supported.
    case registrationAlert
 }
@@ -11,7 +11,10 @@ import SwiftUI
 typealias ServerSelectionScreenViewModelType = StateStoreViewModel<ServerSelectionScreenViewState, ServerSelectionScreenViewAction>

 class ServerSelectionScreenViewModel: ServerSelectionScreenViewModelType, ServerSelectionScreenViewModelProtocol {
+    private let authenticationService: AuthenticationServiceProtocol
+    private let authenticationFlow: AuthenticationFlow
    private let slidingSyncLearnMoreURL: URL
+    private let userIndicatorController: UserIndicatorControllerProtocol
    
    private var actionsSubject: PassthroughSubject<ServerSelectionScreenViewModelAction, Never> = .init()
    
@@ -19,18 +22,23 @@ class ServerSelectionScreenViewModel: ServerSelectionScreenViewModelType, Server
        actionsSubject.eraseToAnyPublisher()
    }

-    init(homeserverAddress: String, slidingSyncLearnMoreURL: URL) {
+    init(authenticationService: AuthenticationServiceProtocol,
+         authenticationFlow: AuthenticationFlow,
+         slidingSyncLearnMoreURL: URL,
+         userIndicatorController: UserIndicatorControllerProtocol) {
+        self.authenticationService = authenticationService
+        self.authenticationFlow = authenticationFlow
        self.slidingSyncLearnMoreURL = slidingSyncLearnMoreURL
-        let bindings = ServerSelectionScreenBindings(homeserverAddress: homeserverAddress)
+        self.userIndicatorController = userIndicatorController
        
-        super.init(initialViewState: ServerSelectionScreenViewState(slidingSyncLearnMoreURL: slidingSyncLearnMoreURL,
-                                                                    bindings: bindings))
+        let bindings = ServerSelectionScreenBindings(homeserverAddress: authenticationService.homeserver.value.address)
+        super.init(initialViewState: ServerSelectionScreenViewState(slidingSyncLearnMoreURL: slidingSyncLearnMoreURL, bindings: bindings))
    }
    
    override func process(viewAction: ServerSelectionScreenViewAction) {
        switch viewAction {
        case .confirm:
-            actionsSubject.send(.confirm(homeserverAddress: state.bindings.homeserverAddress))
+            configureHomeserver()
        case .dismiss:
            actionsSubject.send(.dismiss)
        case .clearFooterError:
@@ -38,31 +46,72 @@ class ServerSelectionScreenViewModel: ServerSelectionScreenViewModelType, Server
        }
    }
    
-    func displayError(_ type: ServerSelectionScreenErrorType) {
-        switch type {
-        case .footerMessage(let message):
-            withElementAnimation {
-                state.footerErrorMessage = message
+    // MARK: - Private
+    
+    /// Updates the login flow using the supplied homeserver address, or shows an error when this isn't possible.
+    private func configureHomeserver() {
+        let homeserverAddress = state.bindings.homeserverAddress
+        startLoading()
+        
+        Task {
+            switch await authenticationService.configure(for: homeserverAddress, flow: authenticationFlow) {
+            case .success:
+                MXLog.info("Selected homeserver: \(homeserverAddress)")
+                actionsSubject.send(.updated)
+                stopLoading()
+            case .failure(let error):
+                MXLog.info("Invalid homeserver: \(homeserverAddress)")
+                stopLoading()
+                handleError(error)
            }
-        case .invalidWellKnownAlert(let error):
+        }
+    }
+    
+    private func startLoading(label: String = L10n.commonLoading) {
+        userIndicatorController.submitIndicator(UserIndicator(type: .modal,
+                                                              title: label,
+                                                              persistent: true))
+    }
+    
+    private func stopLoading() {
+        userIndicatorController.retractAllIndicators()
+    }
+    
+    /// Processes an error to either update the flow or display it to the user.
+    private func handleError(_ error: AuthenticationServiceError) {
+        switch error {
+        case .invalidServer, .invalidHomeserverAddress:
+            showFooterMessage(L10n.screenChangeServerErrorInvalidHomeserver)
+        case .invalidWellKnown(let error):
            state.bindings.alertInfo = AlertInfo(id: .invalidWellKnownAlert(error),
                                                 title: L10n.commonServerNotSupported,
                                                 message: L10n.screenChangeServerErrorInvalidWellKnown(error))
-        case .slidingSyncAlert:
+        case .slidingSyncNotAvailable:
            let openURL = { UIApplication.shared.open(self.slidingSyncLearnMoreURL) }
            state.bindings.alertInfo = AlertInfo(id: .slidingSyncAlert,
                                                 title: L10n.commonServerNotSupported,
                                                 message: L10n.screenChangeServerErrorNoSlidingSyncMessage,
                                                 primaryButton: .init(title: L10n.actionLearnMore, role: .cancel, action: openURL),
                                                 secondaryButton: .init(title: L10n.actionCancel, action: nil))
-        case .registrationAlert:
+        case .loginNotSupported:
+            state.bindings.alertInfo = AlertInfo(id: .loginAlert,
+                                                 title: L10n.commonServerNotSupported,
+                                                 message: L10n.screenLoginErrorUnsupportedAuthentication)
+        case .registrationNotSupported:
            state.bindings.alertInfo = AlertInfo(id: .registrationAlert,
-                                                 title: L10n.errorUnknown,
+                                                 title: L10n.commonServerNotSupported,
                                                 message: L10n.errorAccountCreationNotPossible)
+        default:
+            showFooterMessage(L10n.errorUnknown)
        }
    }
    
-    // MARK: - Private
+    /// Set a new error message to be shown in the text field footer.
+    private func showFooterMessage(_ message: String) {
+        withElementAnimation {
+            state.footerErrorMessage = message
+        }
+    }
    
    /// Clear any errors shown in the text field footer.
    private func clearFooterError() {
@@ -11,7 +11,4 @@ import Combine
 protocol ServerSelectionScreenViewModelProtocol {
    var actions: AnyPublisher<ServerSelectionScreenViewModelAction, Never> { get }
    var context: ServerSelectionScreenViewModelType.Context { get }
-    
-    /// Displays an error to the user.
-    func displayError(_ type: ServerSelectionScreenErrorType)
 }
@@ -91,11 +91,38 @@ struct ServerSelectionScreen: View {
 // MARK: - Previews

 struct ServerSelection_Previews: PreviewProvider, TestablePreview {
+    static let matrixViewModel = makeViewModel(for: "https://matrix.org")
+    static let emptyViewModel = makeViewModel(for: "")
+    static let invalidViewModel = makeViewModel(for: "thisisbad")
+    
    static var previews: some View {
-        ForEach(MockServerSelectionScreenState.allCases, id: \.self) { state in
-            NavigationStack {
-                ServerSelectionScreen(context: state.viewModel.context)
-            }
+        NavigationStack {
+            ServerSelectionScreen(context: matrixViewModel.context)
        }
+        
+        NavigationStack {
+            ServerSelectionScreen(context: emptyViewModel.context)
+        }
+        
+        NavigationStack {
+            ServerSelectionScreen(context: invalidViewModel.context)
+        }
+        .snapshotPreferences(delay: 0.25)
+    }
+    
+    static func makeViewModel(for homeserverAddress: String) -> ServerSelectionScreenViewModel {
+        let authenticationService = AuthenticationService.mock
+        
+        let slidingSyncLearnMoreURL = ServiceLocator.shared.settings.slidingSyncLearnMoreURL
+        
+        let viewModel = ServerSelectionScreenViewModel(authenticationService: authenticationService,
+                                                       authenticationFlow: .login,
+                                                       slidingSyncLearnMoreURL: slidingSyncLearnMoreURL,
+                                                       userIndicatorController: UserIndicatorControllerMock())
+        viewModel.context.homeserverAddress = homeserverAddress
+        if homeserverAddress == "thisisbad" {
+            viewModel.context.send(viewAction: .confirm)
+        }
+        return viewModel
    }
 }