Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions ElementX.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,6 @@
B6DA66EFC13A90846B625836 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 91DE43B8815918E590912DDA /* InfoPlist.strings */; };
B6DF6B6FA8734B70F9BF261E /* BlurHashDecode.swift in Sources */ = {isa = PBXBuildFile; fileRef = E5272BC4A60B6AD7553BACA1 /* BlurHashDecode.swift */; };
B6EC2148FA5443C9289BEEBA /* MediaProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = F17EFA1D3D09FC2F9C5E1CB2 /* MediaProvider.swift */; };
B721125D17A0BA86794F29FB /* MockServerSelectionScreenState.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8E057FB1F07A5C201C89061 /* MockServerSelectionScreenState.swift */; };
B773ACD8881DB18E876D950C /* WaveformSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94028A227645FA880B966211 /* WaveformSource.swift */; };
B7888FC1E1DEF816D175C8D6 /* SecureBackupKeyBackupScreenModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = AD72A9B720D75DBE60AC299F /* SecureBackupKeyBackupScreenModels.swift */; };
B796A25F282C0A340D1B9C12 /* ImageRoomTimelineItemContent.swift in Sources */ = {isa = PBXBuildFile; fileRef = B2B5EDCD05D50BA9B815C66C /* ImageRoomTimelineItemContent.swift */; };
Expand Down Expand Up @@ -2141,7 +2140,6 @@
D79BB714D28C9F588DD69353 /* SecureBackupScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SecureBackupScreenViewModelProtocol.swift; sourceTree = "<group>"; };
D7BB243B26D54EF1A0C422C0 /* NotificationContentBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationContentBuilder.swift; sourceTree = "<group>"; };
D7BEB970F500BFB248443FA1 /* BloomView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BloomView.swift; sourceTree = "<group>"; };
D8E057FB1F07A5C201C89061 /* MockServerSelectionScreenState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockServerSelectionScreenState.swift; sourceTree = "<group>"; };
D8E60332509665C00179ACF6 /* MessageForwardingScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageForwardingScreenViewModel.swift; sourceTree = "<group>"; };
D8F5F9E02B1AB5350B1815E7 /* TimelineStartRoomTimelineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineStartRoomTimelineItem.swift; sourceTree = "<group>"; };
D8FC33C3F6BF597E095CE9FA /* HomeScreenInviteCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeScreenInviteCell.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2815,7 +2813,6 @@
2D0D49B0533C4C2EB889BF3A /* ServerSelectionScreen */ = {
isa = PBXGroup;
children = (
D8E057FB1F07A5C201C89061 /* MockServerSelectionScreenState.swift */,
BB8BC4C791D0E88CFCF4E5DF /* ServerSelectionScreenCoordinator.swift */,
9501D11B4258DFA33BA3B40F /* ServerSelectionScreenModels.swift */,
E3059CFA00C67D8787273B20 /* ServerSelectionScreenViewModel.swift */,
Expand Down Expand Up @@ -6593,7 +6590,6 @@
C97325EFDCCEE457432A9E82 /* MessageText.swift in Sources */,
B659E3A49889E749E3239EA7 /* MockMediaProvider.swift in Sources */,
09C83DDDB07C28364F325209 /* MockRoomTimelineController.swift in Sources */,
B721125D17A0BA86794F29FB /* MockServerSelectionScreenState.swift in Sources */,
AF2ABA2794E376B64104C964 /* MockSoftLogoutScreenState.swift in Sources */,
F9842667B68DC6FA1F9ECCBB /* NSItemProvider.swift in Sources */,
EA01A06EEDFEF4AE7652E5F3 /* NSRegularExpresion.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ enum ServerConfirmationScreenAlert: Hashable {
case invalidWellKnown(String)
/// An alert that allows the user to learn about sliding sync.
case slidingSync
/// An alert that informs the user that login isn't supported.
case login
/// An alert that informs the user that registration isn't supported.
case registration
/// An unknown error has occurred.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
displayError(.invalidWellKnown(error))
case .slidingSyncNotAvailable:
displayError(.slidingSync)
case .loginNotSupported:
displayError(.login)
case .registrationNotSupported:
displayError(.registration)
default:
Expand Down Expand Up @@ -117,9 +119,13 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
message: L10n.screenChangeServerErrorNoSlidingSyncMessage,
primaryButton: .init(title: L10n.actionLearnMore, role: .cancel, action: openURL),
secondaryButton: .init(title: L10n.actionCancel, action: nil))
case .login:
state.bindings.alertInfo = AlertInfo(id: .login,
title: L10n.commonServerNotSupported,
message: L10n.screenLoginErrorUnsupportedAuthentication)
case .registration:
state.bindings.alertInfo = AlertInfo(id: .registration,
title: L10n.errorUnknown,
title: L10n.commonServerNotSupported,
message: L10n.errorAccountCreationNotPossible)
case .unknownError:
state.bindings.alertInfo = AlertInfo(id: .unknownError)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
}
Expand All @@ -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))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,58 +11,107 @@ 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()

var actions: AnyPublisher<ServerSelectionScreenViewModelAction, Never> {
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:
clearFooterError()
}
}

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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ class AuthenticationService: AuthenticationServiceProtocol {
case .failure: nil
}

if flow == .login, homeserver.loginMode == .unsupported {
return .failure(.loginNotSupported)
}
if flow == .register, !homeserver.supportsRegistration {
return .failure(.registrationNotSupported)
}
Expand Down
Loading
Loading