From 1d42b457ccf19d34abe07158b4e12b7a432ce524 Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Thu, 5 Sep 2024 15:44:25 +0200 Subject: [PATCH] Upgrade the UX in Daita scenarios --- ios/MullvadSettings/DAITASettings.swift | 5 + .../TunnelManager/TunnelManager.swift | 20 ++-- .../VPNSettings/VPNSettingsDataSource.swift | 53 ++++++--- .../VPNSettingsDataSourceDelegate.swift | 3 +- .../VPNSettingsInfoButtonItem.swift | 3 +- .../VPNSettings/VPNSettingsInteractor.swift | 16 +++ .../VPNSettingsViewController.swift | 111 +++++++++--------- 7 files changed, 122 insertions(+), 89 deletions(-) diff --git a/ios/MullvadSettings/DAITASettings.swift b/ios/MullvadSettings/DAITASettings.swift index fe87cbb95e0e..ca946b68911b 100644 --- a/ios/MullvadSettings/DAITASettings.swift +++ b/ios/MullvadSettings/DAITASettings.swift @@ -18,6 +18,11 @@ public enum DAITAState: Codable { } } +/// Selected relay is incompatible with Daita, either through singlehop or multihop. +public enum DAITASettingsCompatibilityError { + case singlehop, multihop +} + public struct DAITASettings: Codable, Equatable { public let state: DAITAState diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift index 8850cb477f46..9f5cc98b6221 100644 --- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift +++ b/ios/MullvadVPN/TunnelManager/TunnelManager.swift @@ -535,6 +535,15 @@ final class TunnelManager: StorePaymentObserver { try relayCacheTracker.refreshCachedRelays() } + func selectRelays(tunnelSettings: LatestTunnelSettings) throws -> SelectedRelays { + let retryAttempts = tunnelStatus.observedState.connectionState?.connectionAttemptCount ?? 0 + + return try relaySelector.selectRelays( + tunnelSettings: tunnelSettings, + connectionAttemptCount: retryAttempts + ) + } + // MARK: - Tunnel observeration /// Add tunnel observer. @@ -782,15 +791,6 @@ final class TunnelManager: StorePaymentObserver { updateTunnelStatus(tunnel?.status ?? .disconnected) } - fileprivate func selectRelays() throws -> SelectedRelays { - let retryAttempts = tunnelStatus.observedState.connectionState?.connectionAttemptCount ?? 0 - - return try relaySelector.selectRelays( - tunnelSettings: settings, - connectionAttemptCount: retryAttempts - ) - } - fileprivate func prepareForVPNConfigurationDeletion() { nslock.lock() defer { nslock.unlock() } @@ -1265,7 +1265,7 @@ private struct TunnelInteractorProxy: TunnelInteractor { } func selectRelays() throws -> SelectedRelays { - try tunnelManager.selectRelays() + try tunnelManager.selectRelays(tunnelSettings: tunnelManager.settings) } func handleRestError(_ error: Error) { diff --git a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSource.swift b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSource.swift index 41433770b0e6..cfaa8220034d 100644 --- a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSource.swift +++ b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSource.swift @@ -261,6 +261,12 @@ final class VPNSettingsDataSource: UITableViewDiffableDataSource< deselectAllRowsInSectionExceptRowAt(indexPath) + let obfuscationSettingsUpdate = TunnelSettingsUpdate.obfuscation(WireGuardObfuscationSettings( + state: viewModel.obfuscationState, + port: viewModel.obfuscationPort + )) + let quantumResistanceUpdate = TunnelSettingsUpdate.quantumResistance(viewModel.quantumResistance) + switch item { case .dnsSettings: tableView.deselectRow(at: indexPath, animated: false) @@ -285,26 +291,25 @@ final class VPNSettingsDataSource: UITableViewDiffableDataSource< case .wireGuardObfuscationAutomatic: selectObfuscationState(.automatic) - delegate?.didChangeViewModel(viewModel) + delegate?.didUpdateTunnelSettings(obfuscationSettingsUpdate) case .wireGuardObfuscationOn: selectObfuscationState(.on) - delegate?.didChangeViewModel(viewModel) + delegate?.didUpdateTunnelSettings(obfuscationSettingsUpdate) case .wireGuardObfuscationOff: selectObfuscationState(.off) - delegate?.didChangeViewModel(viewModel) + delegate?.didUpdateTunnelSettings(obfuscationSettingsUpdate) case let .wireGuardObfuscationPort(port): selectObfuscationPort(port) - delegate?.didChangeViewModel(viewModel) - + delegate?.didUpdateTunnelSettings(obfuscationSettingsUpdate) case .quantumResistanceAutomatic: selectQuantumResistance(.automatic) - delegate?.didChangeViewModel(viewModel) + delegate?.didUpdateTunnelSettings(quantumResistanceUpdate) case .quantumResistanceOn: selectQuantumResistance(.on) - delegate?.didChangeViewModel(viewModel) + delegate?.didUpdateTunnelSettings(quantumResistanceUpdate) case .quantumResistanceOff: selectQuantumResistance(.off) - delegate?.didChangeViewModel(viewModel) + delegate?.didUpdateTunnelSettings(quantumResistanceUpdate) default: break } @@ -642,22 +647,32 @@ extension VPNSettingsDataSource: VPNSettingsCellEventHandler { func switchMultihop(_ state: MultihopState) { viewModel.setMultihop(state) - delegate?.didChangeViewModel(viewModel) + delegate?.didUpdateTunnelSettings(.multihop(viewModel.multihopState)) } func switchDaitaState(_ settings: DAITASettings) { - if settings.state.isEnabled { - delegate?.showPrompt(for: .daita) { [weak self] in - guard let self else { return } - viewModel.setDAITASettings(settings) - delegate?.didChangeViewModel(viewModel) - } onDiscard: { [weak self] in - guard let self else { return } - tableView?.reloadData() + let updateSettings = { [weak self] in + self?.viewModel.setDAITASettings(settings) + self?.delegate?.didUpdateTunnelSettings(.daita(settings)) + } + + if let error = delegate?.didAttemptToChangeDaitaSettings(settings) { + switch error { + case .singlehop: + delegate?.showPrompt(for: .daitaSettingIncompatibleWithSinglehop) { + updateSettings() + } onDiscard: { [weak self] in + self?.tableView?.reloadData() + } + case .multihop: + delegate?.showPrompt(for: .daitaSettingIncompatibleWithMultihop) { + updateSettings() + } onDiscard: { [weak self] in + self?.tableView?.reloadData() + } } } else { - viewModel.setDAITASettings(settings) - delegate?.didChangeViewModel(viewModel) + updateSettings() } } } diff --git a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSourceDelegate.swift b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSourceDelegate.swift index 91e2108fc20b..f6482e80ef37 100644 --- a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSourceDelegate.swift +++ b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsDataSourceDelegate.swift @@ -15,7 +15,8 @@ protocol DNSSettingsDataSourceDelegate: AnyObject { } protocol VPNSettingsDataSourceDelegate: AnyObject { - func didChangeViewModel(_ viewModel: VPNSettingsViewModel) + func didUpdateTunnelSettings(_ update: TunnelSettingsUpdate) + func didAttemptToChangeDaitaSettings(_ settings: DAITASettings) -> DAITASettingsCompatibilityError? func showInfo(for: VPNSettingsInfoButtonItem) func showDNSSettings() func showIPOverrides() diff --git a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInfoButtonItem.swift b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInfoButtonItem.swift index 978ccdce8696..f8cb79bb417d 100644 --- a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInfoButtonItem.swift +++ b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInfoButtonItem.swift @@ -18,5 +18,6 @@ enum VPNSettingsInfoButtonItem { } enum VPNSettingsPromptAlertItem { - case daita + case daitaSettingIncompatibleWithSinglehop + case daitaSettingIncompatibleWithMultihop } diff --git a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInteractor.swift b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInteractor.swift index e304a8e4b6b2..c7a5749ce24f 100644 --- a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInteractor.swift +++ b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsInteractor.swift @@ -51,6 +51,22 @@ final class VPNSettingsInteractor { tunnelManager.updateSettings([.relayConstraints(relayConstraints)], completionHandler: completion) } + + func evaluateDaitaSettingsCompatibility(_ settings: DAITASettings) -> DAITASettingsCompatibilityError? { + guard settings.state.isEnabled else { return nil } + + var tunnelSettings = tunnelSettings + tunnelSettings.daita = settings + + let selectedRelays = try? tunnelManager.selectRelays(tunnelSettings: tunnelSettings) + let multihopEnabled = tunnelSettings.tunnelMultihopState.isEnabled + + return if multihopEnabled { + selectedRelays?.entry == nil ? .multihop : nil + } else { + selectedRelays?.exit == nil ? .singlehop : nil + } + } } extension VPNSettingsInteractor: RelayCacheTrackerObserver { diff --git a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift index 8558924c1b94..73c89c5dfb8b 100644 --- a/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift +++ b/ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift @@ -106,18 +106,8 @@ class VPNSettingsViewController: UITableViewController { } extension VPNSettingsViewController: VPNSettingsDataSourceDelegate { - func didChangeViewModel(_ viewModel: VPNSettingsViewModel) { - interactor.updateSettings( - [ - .obfuscation(WireGuardObfuscationSettings( - state: viewModel.obfuscationState, - port: viewModel.obfuscationPort - )), - .quantumResistance(viewModel.quantumResistance), - .multihop(viewModel.multihopState), - .daita(viewModel.daitaSettings), - ] - ) + func didUpdateTunnelSettings(_ update: TunnelSettingsUpdate) { + interactor.updateSettings([update]) } // swiftlint:disable:next function_body_length @@ -217,58 +207,63 @@ extension VPNSettingsViewController: VPNSettingsDataSourceDelegate { interactor.setPort(port) } + func didAttemptToChangeDaitaSettings(_ settings: DAITASettings) -> DAITASettingsCompatibilityError? { + interactor.evaluateDaitaSettingsCompatibility(settings) + } + func showPrompt( for item: VPNSettingsPromptAlertItem, onSave: @escaping () -> Void, onDiscard: @escaping () -> Void ) { - switch item { - case .daita: - let presentation = AlertPresentation( - id: "vpn-settings-content-blockers-alert", - accessibilityIdentifier: .daitaPromptAlert, - icon: .info, - message: NSLocalizedString( - "DAITA_INFORMATION_TEXT", - tableName: "DAITA", - value: """ - This feature isn't available on all servers. \ - You might need to change location after enabling. - Attention: Since this increases your total network traffic,\ - be cautious if you have a limited data plan. It can also \ - negatively impact your network speed. Please consider \ - this if you want to enable DAITA. - """, - comment: "" - ), - buttons: [ - AlertAction( - title: NSLocalizedString( - "VPN_SETTINGS_VPN_DAITA_OK_ACTION", - tableName: "DAITA", - value: "Enable anyway", - comment: "" - ), - style: .default, - accessibilityId: .daitaConfirmAlertEnableButton, - handler: { - onSave() - } + let messageString = switch item { + case .daitaSettingIncompatibleWithSinglehop: + """ + DAITA isn’t available on the current server. After enabling, please go to \ + the Switch location view and select a location that supports DAITA. + """ + case .daitaSettingIncompatibleWithMultihop: + """ + DAITA isn’t available on the current entry server. After enabling, please go to \ + the Switch location view and select an entry location that supports DAITA. + """ + } + + let presentation = AlertPresentation( + id: "vpn-settings-content-blockers-alert", + accessibilityIdentifier: .daitaPromptAlert, + icon: .info, + message: NSLocalizedString( + "DAITA_INFORMATION_TEXT", + tableName: "DAITA", + value: messageString, + comment: "" + ), + buttons: [ + AlertAction( + title: NSLocalizedString( + "VPN_SETTINGS_VPN_DAITA_OK_ACTION", + tableName: "DAITA", + value: "Enable anyway", + comment: "" ), - AlertAction( - title: NSLocalizedString( - "VPN_SETTINGS_VPN_DAITA_CANCEL_ACTION", - tableName: "DAITA", - value: "Back", - comment: "" - ), - style: .default, handler: { - onDiscard() - } + style: .default, + accessibilityId: .daitaConfirmAlertEnableButton, + handler: { onSave() } + ), + AlertAction( + title: NSLocalizedString( + "VPN_SETTINGS_VPN_DAITA_CANCEL_ACTION", + tableName: "DAITA", + value: "Back", + comment: "" ), - ] - ) - alertPresenter.showAlert(presentation: presentation, animated: true) - } + style: .default, + handler: { onDiscard() } + ), + ] + ) + + alertPresenter.showAlert(presentation: presentation, animated: true) } }