Refactor alerts to be presented on their respective coordinator

mullvad · Sep 12, 2023 · 1ee0e89 · 1ee0e89
1 parent f928d2d
commit 1ee0e89
Show file tree

Hide file tree

Showing 30 changed files with 245 additions and 151 deletions.
diff --git a/ios/.swiftlint.yml b/ios/.swiftlint.yml
@@ -31,6 +31,8 @@ line_length:
   ignores_interpolated_strings: true
   warning: 120
   error: 300
+cyclomatic_complexity:
+  ignores_case_statements: true
 
 type_name:
   min_length: 4

diff --git a/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -0,0 +1,22 @@
+{
+  "pins" : [
+    {
+      "identity" : "swift-log",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/apple/swift-log.git",
+      "state" : {
+        "revision" : "173f567a2dfec11d74588eea82cecea555bdc0bc",
+        "version" : "1.4.0"
+      }
+    },
+    {
+      "identity" : "wireguard-apple",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/mullvad/wireguard-apple.git",
+      "state" : {
+        "revision" : "11a00c20dc03f2751db47e94f585c0778c7bde82"
+      }
+    }
+  ],
+  "version" : 2
+}
diff --git a/ios/MullvadVPN/Classes/AppRoutes.swift b/ios/MullvadVPN/Classes/AppRoutes.swift
@@ -13,7 +13,7 @@ import UIKit
  Enum type describing groups of routes. Each group is a modal layer with horizontal navigation
  inside with exception where primary navigation is a part of root controller on iPhone.
  */
-enum AppRouteGroup: String, AppRouteGroupProtocol {
+enum AppRouteGroup: AppRouteGroupProtocol {
     /**
      Primary horizontal navigation group.
      */
@@ -42,7 +42,7 @@ enum AppRouteGroup: String, AppRouteGroupProtocol {
     /**
      Alert group.
      */
-    case alert
+    case alert(String)
 
     var isModal: Bool {
         switch self {
@@ -92,9 +92,10 @@ enum AppRoute: AppRouteProtocol {
     case changelog
 
     /**
-     Alert route.
+     Alert route. Alert id must be a unique string in order to produce a unique route
+     that distinguishes between different kinds of alerts.
      */
-    case alert(AlertPresentation)
+    case alert(_ alertId: String)
 
     /**
      Routes that are part of primary horizontal navigation group.
@@ -130,8 +131,8 @@ enum AppRoute: AppRouteProtocol {
             return .account
         case .settings:
             return .settings
-        case .alert:
-            return .alert
+        case .alert(let id):
+            return .alert(id)
         }
     }
 }
diff --git a/ios/MullvadVPN/Coordinators/AccountCoordinator.swift b/ios/MullvadVPN/Coordinators/AccountCoordinator.swift
@@ -29,10 +29,6 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting {
         navigationController
     }
 
-    var presentationContext: UIViewController {
-        navigationController
-    }
-
     var didFinish: ((AccountCoordinator, AccountDismissReason) -> Void)?
 
     init(
@@ -48,7 +44,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting {
 
         let accountController = AccountViewController(
             interactor: interactor,
-            errorPresenter: PaymentAlertPresenter(coordinator: self)
+            errorPresenter: PaymentAlertPresenter(alertContext: self)
         )
 
         accountController.actionHandler = handleViewControllerAction
@@ -133,18 +129,25 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting {
     // MARK: - Alerts
 
     private func logOut() {
-        let presentation = AlertPresentation(icon: .spinner, message: nil, buttons: [])
+        let presentation = AlertPresentation(
+            id: "account-logout-alert",
+            icon: .spinner,
+            message: nil,
+            buttons: []
+        )
+
+        let alertPresenter = AlertPresenter(context: self)
 
         interactor.logout {
             DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(1)) { [weak self] in
                 guard let self else { return }
 
-                applicationRouter?.dismiss(.alert(presentation), animated: true)
+                alertPresenter.dismissAlert(presentation: presentation, animated: true)
                 self.didFinish?(self, .userLoggedOut)
             }
         }
 
-        applicationRouter?.present(.alert(presentation))
+        alertPresenter.showAlert(presentation: presentation, animated: true)
     }
 
     private func showAccountDeviceInfo() {
@@ -164,6 +167,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting {
         )
 
         let presentation = AlertPresentation(
+            id: "account-device-info-alert",
             message: message,
             buttons: [AlertAction(
                 title: NSLocalizedString(
@@ -176,6 +180,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting {
             )]
         )
 
-        applicationRouter?.present(.alert(presentation), animated: true)
+        let presenter = AlertPresenter(context: self)
+        presenter.showAlert(presentation: presentation, animated: true)
     }
 }
diff --git a/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift b/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift
@@ -16,8 +16,7 @@ import UIKit
  Application coordinator managing split view and two navigation contexts.
  */
 final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewControllerDelegate,
-    UISplitViewControllerDelegate, ApplicationRouterDelegate,
-    NotificationManagerDelegate {
+    UISplitViewControllerDelegate, ApplicationRouterDelegate, NotificationManagerDelegate {
     typealias RouteType = AppRoute
 
     /**
@@ -120,11 +119,11 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
 
     func applicationRouter(
         _ router: ApplicationRouter<RouteType>,
-        route: AppRoute,
+        presentWithContext context: RoutePresentationContext<RouteType>,
         animated: Bool,
         completion: @escaping (Coordinator) -> Void
     ) {
-        switch route {
+        switch context.route {
         case .account:
             presentAccount(animated: animated, completion: completion)
 
@@ -155,8 +154,8 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
         case .welcome:
             presentWelcome(animated: animated, completion: completion)
 
-        case let .alert(presentation):
-            presentAlert(presentation: presentation, animated: animated, completion: completion)
+        case .alert:
+            presentAlert(animated: animated, context: context, completion: completion)
         }
     }
 
@@ -564,11 +563,10 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
         )
 
         coordinator.didFinishPayment = { [weak self] _ in
-            guard let self else { return }
+            guard let self = self else { return }
 
             if shouldDismissOutOfTime() {
                 router.dismiss(.outOfTime, animated: true)
-
                 continueFlow(animated: true)
             }
         }
@@ -589,7 +587,7 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
         )
 
         coordinator.didFinish = { [weak self] _ in
-            guard let self else { return }
+            guard let self = self else { return }
             appPreferences.isShownOnboarding = true
             router.dismiss(.welcome, animated: false)
             continueFlow(animated: false)
@@ -607,10 +605,7 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
         !(tunnelManager.deviceState.accountData?.isExpired ?? false)
     }
 
-    private func presentSelectLocation(
-        animated: Bool,
-        completion: @escaping (Coordinator) -> Void
-    ) {
+    private func presentSelectLocation(animated: Bool, completion: @escaping (Coordinator) -> Void) {
         let coordinator = makeSelectLocationCoordinator(forModalPresentation: true)
         coordinator.start()
 
@@ -642,19 +637,24 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
     }
 
     private func presentAlert(
-        presentation: AlertPresentation,
         animated: Bool,
+        context: RoutePresentationContext<RouteType>,
         completion: @escaping (Coordinator) -> Void
     ) {
-        let coordinator = AlertCoordinator(presentation: presentation)
+        guard let metadata = context.metadata as? AlertMetadata else {
+            assertionFailure("Could not get AlertMetadata from RoutePresentationContext.")
+            return
+        }
+
+        let coordinator = AlertCoordinator(presentation: metadata.presentation)
 
         coordinator.didFinish = { [weak self] in
-            self?.router.dismiss(.alert(presentation))
+            self?.router.dismiss(context.route)
         }
 
         coordinator.start()
 
-        presentChild(coordinator, animated: animated) {
+        metadata.context.presentChild(coordinator, animated: animated) {
             completion(coordinator)
         }
     }
@@ -989,13 +989,13 @@ fileprivate extension AppPreferencesDataSource {
     mutating func markChangeLogSeen() {
         self.lastSeenChangeLogVersion = Bundle.main.shortVersion
     }
-
-    // swiftlint:disable:next file_length
 }
 
 private protocol Poppable: Presentable {
     func popFromNavigationStack(
         animated: Bool,
         completion: () -> Void
     )
+
+    // swiftlint:disable:next file_length
 }
diff --git a/ios/MullvadVPN/Coordinators/InAppPurchaseCoordinator.swift b/ios/MullvadVPN/Coordinators/InAppPurchaseCoordinator.swift
@@ -11,7 +11,7 @@ import Routing
 import StoreKit
 import UIKit
 
-class InAppPurchaseCoordinator: Coordinator, Presentable {
+class InAppPurchaseCoordinator: Coordinator, Presenting, Presentable {
     private let navigationController: RootContainerViewController
     private let interactor: InAppPurchaseInteractor
 
@@ -50,6 +50,7 @@ class InAppPurchaseCoordinator: Coordinator, Presentable {
 
             case let .failure(failure):
                 let presentation = AlertPresentation(
+                    id: "in-app-purchase-error-alert",
                     icon: .alert,
                     message: failure.error.localizedDescription,
                     buttons: [
@@ -69,7 +70,8 @@ class InAppPurchaseCoordinator: Coordinator, Presentable {
                     ]
                 )
 
-                applicationRouter?.present(.alert(presentation), animated: true)
+                let presenter = AlertPresenter(context: self)
+                presenter.showAlert(presentation: presentation, animated: true)
             }
         }
     }

diff --git a/ios/MullvadVPN/Coordinators/LoginCoordinator.swift b/ios/MullvadVPN/Coordinators/LoginCoordinator.swift
@@ -12,7 +12,7 @@ import Operations
 import Routing
 import UIKit
 
-final class LoginCoordinator: Coordinator, DeviceManagementViewControllerDelegate {
+final class LoginCoordinator: Coordinator, Presenting, DeviceManagementViewControllerDelegate {
     private let tunnelManager: TunnelManager
     private let devicesProxy: REST.DevicesProxy
 
@@ -22,6 +22,10 @@ final class LoginCoordinator: Coordinator, DeviceManagementViewControllerDelegat
     var didFinish: ((LoginCoordinator) -> Void)?
     var didCreateAccount: (() -> Void)?
 
+    var presentationContext: UIViewController {
+        navigationController
+    }
+
     let navigationController: RootContainerViewController
 
     init(
@@ -107,11 +111,12 @@ final class LoginCoordinator: Coordinator, DeviceManagementViewControllerDelegat
         )
         let controller = DeviceManagementViewController(
             interactor: interactor,
-            alertPresenter: AlertPresenter(coordinator: self)
+            alertPresenter: AlertPresenter(context: self)
         )
         controller.delegate = self
+
         controller.fetchDevices(animateUpdates: false) { [weak self] result in
-            guard let self else { return }
+            guard let self = self else { return }
 
             switch result {
             case .success:

diff --git a/ios/MullvadVPN/Coordinators/OutOfTimeCoordinator.swift b/ios/MullvadVPN/Coordinators/OutOfTimeCoordinator.swift
@@ -9,13 +9,17 @@
 import Routing
 import UIKit
 
-class OutOfTimeCoordinator: Coordinator, OutOfTimeViewControllerDelegate {
+class OutOfTimeCoordinator: Coordinator, Presenting, OutOfTimeViewControllerDelegate {
     let navigationController: RootContainerViewController
     let storePaymentManager: StorePaymentManager
     let tunnelManager: TunnelManager
 
     var didFinishPayment: ((OutOfTimeCoordinator) -> Void)?
 
+    var presentationContext: UIViewController {
+        navigationController
+    }
+
     private(set) var isMakingPayment = false
     private var viewController: OutOfTimeViewController?
 
@@ -42,7 +46,7 @@ class OutOfTimeCoordinator: Coordinator, OutOfTimeViewControllerDelegate {
 
         let controller = OutOfTimeViewController(
             interactor: interactor,
-            errorPresenter: PaymentAlertPresenter(coordinator: self)
+            errorPresenter: PaymentAlertPresenter(alertContext: self)
         )
 
         controller.delegate = self

diff --git a/ios/MullvadVPN/Coordinators/SettingsCoordinator.swift b/ios/MullvadVPN/Coordinators/SettingsCoordinator.swift
@@ -32,10 +32,6 @@ final class SettingsCoordinator: Coordinator, Presentable, Presenting, SettingsV
         navigationController
     }
 
-    var presentationContext: UIViewController {
-        navigationController
-    }
-
     var willNavigate: ((
         _ coordinator: SettingsCoordinator,
         _ from: SettingsNavigationRoute?,
@@ -159,13 +155,13 @@ final class SettingsCoordinator: Coordinator, Presentable, Presenting, SettingsV
         case .preferences:
             return PreferencesViewController(
                 interactor: interactorFactory.makePreferencesInteractor(),
-                alertPresenter: AlertPresenter(coordinator: self)
+                alertPresenter: AlertPresenter(context: self)
             )
 
         case .problemReport:
             return ProblemReportViewController(
                 interactor: interactorFactory.makeProblemReportInteractor(),
-                alertPresenter: AlertPresenter(coordinator: self)
+                alertPresenter: AlertPresenter(context: self)
             )
 
         case .faq: