Skip to content

Commit

Permalink
Self review for PR.
Browse files Browse the repository at this point in the history
  • Loading branch information
pixlwave committed Jul 1, 2022
1 parent 7116d47 commit a816988
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,14 @@ final class LoginCoordinator: Coordinator, Presentable {
}

/// Processes an error to either update the flow or display it to the user.
private func handleError(_ error: Error) {
private func handleError(_ error: AuthenticationServiceError) {
switch error {
case AuthenticationServiceError.invalidCredentials:
case .invalidCredentials:
loginViewModel.displayError(.alert(ElementL10n.authInvalidLoginParam))
case AuthenticationServiceError.accountDeactivated:
case .accountDeactivated:
loginViewModel.displayError(.alert(ElementL10n.authInvalidLoginDeactivatedAccount))
default:
loginViewModel.displayError(.alert(error.localizedDescription))
loginViewModel.displayError(.alert(ElementL10n.unknownError))
}
}

Expand All @@ -146,6 +146,7 @@ final class LoginCoordinator: Coordinator, Presentable {
switch await authenticationService.login(username: username, password: password) {
case .success(let userSession):
callback?(.signedIn(userSession))
stopLoading()
case .failure(let error):
stopLoading()
handleError(error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ final class ServerSelectionCoordinator: Coordinator, Presentable {
}

/// Processes an error to either update the flow or display it to the user.
private func handleError(_ error: Error) {
private func handleError(_ error: AuthenticationServiceError) {
switch error {
case AuthenticationServiceError.invalidServer:
case .invalidServer:
serverSelectionViewModel.displayError(.footerMessage(ElementL10n.loginErrorHomeserverNotFound))
default:
serverSelectionViewModel.displayError(.footerMessage(ElementL10n.unknownError))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ class AuthenticationService: AuthenticationServiceProtocol {

switch await loginTask.result {
case .success(let client):
Benchmark.endTrackingForIdentifier("Login", message: "Finished login")
return await userSession(for: client)
case .failure(let error):
Benchmark.endTrackingForIdentifier("Login", message: "Login failed")

MXLog.error("Failed logging in with error: \(error)")
guard let error = error as? ClientError else { return .failure(.failedLoggingIn) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class MockAuthenticationService: AuthenticationServiceProtocol {
}

func startLogin(for homeserverAddress: String) async -> Result<Void, AuthenticationServiceError> {
// Map the address to the mock homeservers
if LoginHomeserver.mockMatrixDotOrg.address.contains(homeserverAddress) {
homeserver = .mockMatrixDotOrg
return .success(())
Expand All @@ -34,10 +35,12 @@ class MockAuthenticationService: AuthenticationServiceProtocol {
return .success(())
}

// Otherwise fail with an invalid server.
return .failure(.invalidServer)
}

func login(username: String, password: String) async -> Result<UserSessionProtocol, AuthenticationServiceError> {
// Login only succeeds if the username and password match the valid credentials property
guard username == validCredentials.username, password == validCredentials.password else {
return .failure(.invalidCredentials)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ class UserSessionStore: UserSessionStoreProtocol {
}

private func setupProxyForClient(_ client: Client) async -> Result<ClientProxyProtocol, UserSessionStoreError> {
Benchmark.endTrackingForIdentifier("Login", message: "Finished login")

do {
let accessToken = try client.restoreToken()
let userId = try client.userId()
Expand Down
1 change: 1 addition & 0 deletions changelog.d/pr-126.change
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add AuthenticationService and missing UI tests on the flow.

0 comments on commit a816988

Please sign in to comment.