Skip to content

Commit

Permalink
PIA-884: Add proper error handling to login feature
Browse files Browse the repository at this point in the history
  • Loading branch information
@kp-said-rehouni
kp-said-rehouni committed Dec 13, 2023
1 parent 2bc4e0a commit eccb0f7
Show file tree
Hide file tree
Showing 5 changed files with 256 additions and 70 deletions.
7 changes: 6 additions & 1 deletion PIA VPN-tvOS/Login/CompositionRoot/LoginFactory.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class LoginFactory {
LoginViewModel(loginWithCredentialsUseCase: makeLoginWithCredentialsUseCase(),
checkLoginAvailability: CheckLoginAvailability(),
validateLoginCredentials: ValidateCredentialsFormat(),
errorMapper: LoginPresentableErrorMapper())
errorHandler: makeLoginViewModelErrorHandler())

}

private static func makeLoginWithCredentialsUseCase() -> LoginWithCredentialsUseCaseType {
Expand All @@ -30,4 +31,8 @@ class LoginFactory {
LoginProvider(accountProvider: Client.providers.accountProvider,
userAccountMapper: UserAccountMapper())
}

private static func makeLoginViewModelErrorHandler() -> LoginViewModelErrorHandlerType {
LoginViewModelErrorHandler(errorMapper: LoginPresentableErrorMapper())
}
}
61 changes: 33 additions & 28 deletions PIA VPN-tvOS/Login/Presentation/LoginViewModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,68 +8,73 @@

import Foundation

enum LoginStatus {
case none
case isLogging
case failed(error: LoginError)
case succeeded(userAccount: UserAccount)
}

class LoginViewModel: ObservableObject {
private let loginWithCredentialsUseCase: LoginWithCredentialsUseCaseType
private let checkLoginAvailability: CheckLoginAvailabilityType
private let validateLoginCredentials: ValidateCredentialsFormatType
private let errorMapper: LoginPresentableErrorMapper
private let errorHandler: LoginViewModelErrorHandlerType

var loginStatus: LoginStatus = .none
@Published var isAccountExpired = false
@Published var didLoginSuccessfully = false
@Published var shouldShowErrorMessage = false
@Published var loginStatus: LoginStatus = .none

init(loginWithCredentialsUseCase: LoginWithCredentialsUseCaseType, checkLoginAvailability: CheckLoginAvailabilityType, validateLoginCredentials: ValidateCredentialsFormatType, errorMapper: LoginPresentableErrorMapper) {
init(loginWithCredentialsUseCase: LoginWithCredentialsUseCaseType, checkLoginAvailability: CheckLoginAvailabilityType, validateLoginCredentials: ValidateCredentialsFormatType, errorHandler: LoginViewModelErrorHandlerType) {
self.loginWithCredentialsUseCase = loginWithCredentialsUseCase
self.checkLoginAvailability = checkLoginAvailability
self.validateLoginCredentials = validateLoginCredentials
self.errorMapper = errorMapper
self.errorHandler = errorHandler
}

func login(username: String, password: String) async {
func login(username: String, password: String) {
if case .isLogging = loginStatus {
return
}

if case .failure(let error) = checkLoginAvailability() {
handleError(error: error)
handleError(error)
return
}

if case .failure(let error) = validateLoginCredentials(username: username, password: password) {
// Handle credentials wrong format
loginStatus = .failed(error: error)
handleError(error)
return
}

loginStatus = .isLogging

await withCheckedContinuation { continuation in
loginWithCredentialsUseCase.execute(username: username, password: password) { [weak self] result in
guard let self = self else { return }
switch result {
case .success(let userAccount):
loginWithCredentialsUseCase.execute(username: username, password: password) { [weak self] result in
guard let self = self else { return }

switch result {
case .success(let userAccount):
Task { @MainActor in
self.loginStatus = .succeeded(userAccount: userAccount)
self.didLoginSuccessfully = true
}

case .failure(let error):
self.handleError(error: error)
}
continuation.resume()
case .failure(let error):
handleError(error)
}
}
}

private func handleError(error: LoginError) {
private func handleError(_ error: LoginError) {
guard error != .expired else {
Task { @MainActor in
loginStatus = .failed(errorMessage: nil, field: .none)
isAccountExpired = true
}
return
}

if case .throttled(let delay) = error {
checkLoginAvailability.disableLoginFor(delay)
}

let errorMessage = errorMapper.map(error: error)
loginStatus = .failed(error: error)
Task { @MainActor in
loginStatus = errorHandler(error: error)
shouldShowErrorMessage = true
}
}
}
34 changes: 23 additions & 11 deletions PIA VPN-tvOS/Login/UI/LoginView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,29 @@ struct LoginView: View {

var body: some View {
VStack {
Text("Sign in to your account")
.font(.system(size: 36))
TextField("Username(p1234567)", text: $userName)
TextField("Password", text: $password)
Button(action: {
Task {
await viewModel.login(username: userName, password: password)
ZStack {
if viewModel.loginStatus == .isLogging {
ProgressView().progressViewStyle(.circular)
}
}, label: {
Text("LOGIN")
})
}

VStack {
Text("Sign in to your account")
.font(.system(size: 36))
TextField("Username(p1234567)", text: $userName)
SecureField("Password", text: $password)
Button(action: {
viewModel.login(username: userName, password: password)
}, label: {
Text("LOGIN")
})
}
}
}.alert("Error", isPresented: $viewModel.shouldShowErrorMessage, actions: {
Button("OK") {}
}, message: {
if case .failed(let errorMessage, _) = viewModel.loginStatus, let errorMessage = errorMessage {
Text(errorMessage)
}
})
}
}
56 changes: 46 additions & 10 deletions PIA VPN-tvOSTests/Login/LoginIntegrationTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@

import XCTest
import PIALibrary
import Combine
@testable import PIA_VPN_tvOS

final class LoginIntegrationTests: XCTestCase {

func test_login_succeeds() async throws {
func test_login_succeeds() throws {
// GIVEN
let userAccount = PIALibrary.UserAccount.makeStub()
let accountProviderMock = AccountProviderMock(userResult: userAccount,
Expand All @@ -27,15 +28,34 @@ final class LoginIntegrationTests: XCTestCase {
let sut = LoginViewModel(loginWithCredentialsUseCase: loginWithCredentialsUseCase,
checkLoginAvailability: CheckLoginAvailability(),
validateLoginCredentials: ValidateCredentialsFormat(),
errorMapper: LoginPresentableErrorMapper())
errorHandler: LoginViewModelErrorHandler(errorMapper: LoginPresentableErrorMapper()))

var cancellables = Set<AnyCancellable>()
let expectation = expectation(description: "Waiting for didLoginSuccessfully property to be updated")
XCTAssertEqual(sut.loginStatus, .none)

var capturedLoginStatuses = [LoginStatus]()

sut.$loginStatus.dropFirst().sink(receiveValue: { status in
capturedLoginStatuses.append(status)
}).store(in: &cancellables)

sut.$didLoginSuccessfully.dropFirst().sink(receiveValue: { status in
XCTAssertTrue(status)
expectation.fulfill()
}).store(in: &cancellables)

// WHEN
await sut.login(username: "username", password: "password")
sut.login(username: "username", password: "password")

// THEN
guard case .succeeded(let capturedUserResult) = sut.loginStatus else {
wait(for: [expectation], timeout: 1)
XCTAssertFalse(sut.shouldShowErrorMessage)
XCTAssertFalse(sut.isAccountExpired)
XCTAssertEqual(capturedLoginStatuses.count, 2)
XCTAssertEqual(capturedLoginStatuses[0], LoginStatus.isLogging)

guard case .succeeded(let capturedUserResult) = capturedLoginStatuses[1] else {
XCTFail("Expected success, got failure")
return
}
Expand All @@ -60,11 +80,9 @@ final class LoginIntegrationTests: XCTestCase {
default:
XCTFail("Expected the same plan, got \(capturedPlan) and \(userPlan)")
}


}

func test_login_fails() async throws {
func test_login_fails() {
// GIVEN
let accountProviderMock = AccountProviderMock(userResult: nil,
errorResult: ClientError.expired)
Expand All @@ -78,14 +96,32 @@ final class LoginIntegrationTests: XCTestCase {
let sut = LoginViewModel(loginWithCredentialsUseCase: loginWithCredentialsUseCase,
checkLoginAvailability: CheckLoginAvailability(),
validateLoginCredentials: ValidateCredentialsFormat(),
errorMapper: LoginPresentableErrorMapper())
errorHandler: LoginViewModelErrorHandler(errorMapper: LoginPresentableErrorMapper()))

var cancellables = Set<AnyCancellable>()
let expectation = expectation(description: "Waiting for isAccountExpired property to be updated")
XCTAssertEqual(sut.loginStatus, .none)

var capturedLoginStatuses = [LoginStatus]()

sut.$loginStatus.dropFirst().sink(receiveValue: { status in
capturedLoginStatuses.append(status)
}).store(in: &cancellables)

sut.$isAccountExpired.dropFirst().sink(receiveValue: { status in
XCTAssertTrue(status)
expectation.fulfill()
}).store(in: &cancellables)

// WHEN
await sut.login(username: "username", password: "password")
sut.login(username: "username", password: "password")

// THEN
XCTAssertEqual(sut.loginStatus, .failed(error: .expired))
wait(for: [expectation], timeout: 1)
XCTAssertFalse(sut.shouldShowErrorMessage)
XCTAssertFalse(sut.didLoginSuccessfully)
XCTAssertEqual(capturedLoginStatuses.count, 2)
XCTAssertEqual(capturedLoginStatuses[0], LoginStatus.isLogging)
XCTAssertEqual(capturedLoginStatuses[1], LoginStatus.failed(errorMessage: nil, field: .none))
}
}
Loading

0 comments on commit eccb0f7

Please sign in to comment.