LOOP-1163: Unit tests for the alert facility (#67)

* DeviceAlertManager unit tests

* More PR feedback renaming: Handler -> Presenter, etc.

* In app modal unit tests

* Finished InAppModalDeviceAlertPresenterTests

* Adds UserNotificationDeviceAlertPresenterTests

* LOOP-1057: Unit tests for the alert facility

* comment out USE_NEW_ALERT_FACILITY code

* Some more renaming from PR review

* change the #if to be more obvious

* One more rename suggestion from @ps2

* Rename MockHandler->MockPresenter,

Author: rickpasetto

Loop.xcodeproj/project.pbxproj

@@ -22,9 +22,12 @@
 /* End PBXAggregateTarget section */
 
 /* Begin PBXBuildFile section */
-		1DA649A7244126CD00F61E75 /* UserNotificationDeviceAlertHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DA649A6244126CD00F61E75 /* UserNotificationDeviceAlertHandler.swift */; };
-		1DA649A9244126DA00F61E75 /* InAppModalDeviceAlertHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DA649A8244126DA00F61E75 /* InAppModalDeviceAlertHandler.swift */; };
+		1DA649A7244126CD00F61E75 /* UserNotificationDeviceAlertPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DA649A6244126CD00F61E75 /* UserNotificationDeviceAlertPresenter.swift */; };
+		1DA649A9244126DA00F61E75 /* InAppModalDeviceAlertPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DA649A8244126DA00F61E75 /* InAppModalDeviceAlertPresenter.swift */; };
+		1DA7A84224476EAD008257F0 /* DeviceAlertManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DA7A84124476EAD008257F0 /* DeviceAlertManagerTests.swift */; };
+		1DA7A84424477698008257F0 /* InAppModalDeviceAlertPresenterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DA7A84324477698008257F0 /* InAppModalDeviceAlertPresenterTests.swift */; };
 		1DB1065124467E18005542BD /* DeviceAlertManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DB1065024467E18005542BD /* DeviceAlertManager.swift */; };
+		1DFE9E172447B6270082C280 /* UserNotificationDeviceAlertPresenterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DFE9E162447B6270082C280 /* UserNotificationDeviceAlertPresenterTests.swift */; };
 		43027F0F1DFE0EC900C51989 /* HKUnit.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F526D5E1DF2459000A04910 /* HKUnit.swift */; };
 		4302F4E11D4E9C8900F0FCAF /* TextFieldTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4302F4E01D4E9C8900F0FCAF /* TextFieldTableViewController.swift */; };
 		4302F4E31D4EA54200F0FCAF /* InsulinDeliveryTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4302F4E21D4EA54200F0FCAF /* InsulinDeliveryTableViewController.swift */; };
@@ -593,9 +596,12 @@
 /* End PBXCopyFilesBuildPhase section */
 
 /* Begin PBXFileReference section */
-		1DA649A6244126CD00F61E75 /* UserNotificationDeviceAlertHandler.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UserNotificationDeviceAlertHandler.swift; sourceTree = "<group>"; };
-		1DA649A8244126DA00F61E75 /* InAppModalDeviceAlertHandler.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InAppModalDeviceAlertHandler.swift; sourceTree = "<group>"; };
+		1DA649A6244126CD00F61E75 /* UserNotificationDeviceAlertPresenter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UserNotificationDeviceAlertPresenter.swift; sourceTree = "<group>"; };
+		1DA649A8244126DA00F61E75 /* InAppModalDeviceAlertPresenter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InAppModalDeviceAlertPresenter.swift; sourceTree = "<group>"; };
+		1DA7A84124476EAD008257F0 /* DeviceAlertManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeviceAlertManagerTests.swift; sourceTree = "<group>"; };
+		1DA7A84324477698008257F0 /* InAppModalDeviceAlertPresenterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InAppModalDeviceAlertPresenterTests.swift; sourceTree = "<group>"; };
 		1DB1065024467E18005542BD /* DeviceAlertManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DeviceAlertManager.swift; sourceTree = "<group>"; };
+		1DFE9E162447B6270082C280 /* UserNotificationDeviceAlertPresenterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserNotificationDeviceAlertPresenterTests.swift; sourceTree = "<group>"; };
 		4302F4E01D4E9C8900F0FCAF /* TextFieldTableViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextFieldTableViewController.swift; sourceTree = "<group>"; };
 		4302F4E21D4EA54200F0FCAF /* InsulinDeliveryTableViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InsulinDeliveryTableViewController.swift; sourceTree = "<group>"; };
 		430B29892041F54A00BA9F93 /* NSUserDefaults.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSUserDefaults.swift; sourceTree = "<group>"; };
@@ -1213,8 +1219,26 @@
 			isa = PBXGroup;
 			children = (
 				1DB1065024467E18005542BD /* DeviceAlertManager.swift */,
-				1DA649A8244126DA00F61E75 /* InAppModalDeviceAlertHandler.swift */,
-				1DA649A6244126CD00F61E75 /* UserNotificationDeviceAlertHandler.swift */,
+				1DA649A8244126DA00F61E75 /* InAppModalDeviceAlertPresenter.swift */,
+				1DA649A6244126CD00F61E75 /* UserNotificationDeviceAlertPresenter.swift */,
+			);
+			path = DeviceAlert;
+			sourceTree = "<group>";
+		};
+		1DA7A83F24476E8C008257F0 /* Managers */ = {
+			isa = PBXGroup;
+			children = (
+				1DA7A84024476E98008257F0 /* DeviceAlert */,
+			);
+			path = Managers;
+			sourceTree = "<group>";
+		};
+		1DA7A84024476E98008257F0 /* DeviceAlert */ = {
+			isa = PBXGroup;
+			children = (
+				1DA7A84124476EAD008257F0 /* DeviceAlertManagerTests.swift */,
+				1DA7A84324477698008257F0 /* InAppModalDeviceAlertPresenterTests.swift */,
+				1DFE9E162447B6270082C280 /* UserNotificationDeviceAlertPresenterTests.swift */,
 			);
 			path = DeviceAlert;
 			sourceTree = "<group>";
@@ -1640,6 +1664,7 @@
 		43F78D2C1C8FC58F002152D1 /* LoopTests */ = {
 			isa = PBXGroup;
 			children = (
+				1DA7A83F24476E8C008257F0 /* Managers */,
 				43E2D90F1D20C581004DA55F /* Info.plist */,
 				7D2366E421250E0A0028B67D /* InfoPlist.strings */,
 				A9DAE7CF2332D77F006AE942 /* LoopTests.swift */,
@@ -2667,13 +2692,13 @@
 				C1F8B243223E73FD00DD66CF /* BolusProgressTableViewCell.swift in Sources */,
 				89D6953E23B6DF8A002B3066 /* PotentialCarbEntryTableViewCell.swift in Sources */,
 				89CA2B30226C0161004D9350 /* DirectoryObserver.swift in Sources */,
-				1DA649A7244126CD00F61E75 /* UserNotificationDeviceAlertHandler.swift in Sources */,
+				1DA649A7244126CD00F61E75 /* UserNotificationDeviceAlertPresenter.swift in Sources */,
 				439A7942211F631C0041B75F /* RootNavigationController.swift in Sources */,
 				4F11D3C020DCBEEC006E072C /* GlucoseBackfillRequestUserInfo.swift in Sources */,
 				43F5C2DB1B92A5E1003EB13D /* SettingsTableViewController.swift in Sources */,
 				89E267FC2292456700A3F2AF /* FeatureFlags.swift in Sources */,
 				43A567691C94880B00334FAC /* LoopDataManager.swift in Sources */,
-				1DA649A9244126DA00F61E75 /* InAppModalDeviceAlertHandler.swift in Sources */,
+				1DA649A9244126DA00F61E75 /* InAppModalDeviceAlertPresenter.swift in Sources */,
 				43B260491ED248FB008CAA77 /* CarbEntryTableViewCell.swift in Sources */,
 				4302F4E11D4E9C8900F0FCAF /* TextFieldTableViewController.swift in Sources */,
 				43F64DD91D9C92C900D24DC6 /* TitleSubtitleTableViewCell.swift in Sources */,
@@ -2940,8 +2965,11 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				1DA7A84424477698008257F0 /* InAppModalDeviceAlertPresenterTests.swift in Sources */,
 				8968B114240C55F10074BB48 /* LoopSettingsTests.swift in Sources */,
+				1DA7A84224476EAD008257F0 /* DeviceAlertManagerTests.swift in Sources */,
 				A9DAE7D02332D77F006AE942 /* LoopTests.swift in Sources */,
+				1DFE9E172447B6270082C280 /* UserNotificationDeviceAlertPresenterTests.swift in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

Loop/AppDelegate.swift

@@ -137,12 +137,12 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
                 }
                 return
             }
-        case NotificationManager.Action.acknowledgeAlert.rawValue:
+        case NotificationManager.Action.acknowledgeDeviceAlert.rawValue:
             let userInfo = response.notification.request.content.userInfo
-            if let alertTypeIdentifier = userInfo[LoopNotificationUserInfoKey.alertTypeId.rawValue] as? DeviceAlert.TypeIdentifier,
+            if let alertIdentifier = userInfo[LoopNotificationUserInfoKey.alertTypeID.rawValue] as? DeviceAlert.AlertIdentifier,
                 let managerIdentifier = userInfo[LoopNotificationUserInfoKey.managerIDForAlert.rawValue] as? String {
                 deviceAlertManager.acknowledgeDeviceAlert(identifier:
-                    DeviceAlert.Identifier(managerIdentifier: managerIdentifier, typeIdentifier: alertTypeIdentifier))
+                    DeviceAlert.Identifier(managerIdentifier: managerIdentifier, alertIdentifier: alertIdentifier))
             }
         default:
             break

Loop/Managers/DeviceAlert/DeviceAlertManager.swift

@@ -20,13 +20,15 @@ protocol DeviceAlertManagerResponder: class {
 /// - etc.
 public final class DeviceAlertManager {
 
-    var handlers: [DeviceAlertHandler] = []
+    var handlers: [DeviceAlertPresenter] = []
     var responders: [String: Weak<DeviceAlertResponder>] = [:]
 
     public init(rootViewController: UIViewController,
-                isAppInBackgroundFunc: @escaping () -> Bool) {
-        handlers = [UserNotificationDeviceAlertHandler(isAppInBackgroundFunc: isAppInBackgroundFunc),
-                    InAppModalDeviceAlertHandler(rootViewController: rootViewController, deviceAlertManagerResponder: self)]
+                isAppInBackgroundFunc: @escaping () -> Bool,
+                handlers: [DeviceAlertPresenter]? = nil) {
+        self.handlers = handlers ??
+            [UserNotificationDeviceAlertPresenter(isAppInBackgroundFunc: isAppInBackgroundFunc),
+             InAppModalDeviceAlertPresenter(rootViewController: rootViewController, deviceAlertManagerResponder: self)]
     }
 
     public func addAlertResponder(key: String, alertResponder: DeviceAlertResponder) {
@@ -41,12 +43,12 @@ public final class DeviceAlertManager {
 extension DeviceAlertManager: DeviceAlertManagerResponder {
     func acknowledgeDeviceAlert(identifier: DeviceAlert.Identifier) {
         if let responder = responders[identifier.managerIdentifier]?.value {
-            responder.acknowledgeAlert(typeIdentifier: identifier.typeIdentifier)
+            responder.acknowledgeAlert(alertIdentifier: identifier.alertIdentifier)
         }
     }
 }
 
-extension DeviceAlertManager: DeviceAlertHandler {
+extension DeviceAlertManager: DeviceAlertPresenter {
 
     public func issueAlert(_ alert: DeviceAlert) {
         handlers.forEach { $0.issueAlert(alert) }

Loop/Managers/DeviceAlert/InAppModalDeviceAlertHandler.swift renamed to Loop/Managers/DeviceAlert/InAppModalDeviceAlertPresenter.swift

@@ -1,5 +1,5 @@
 //
-//  InAppUserAlertHandler.swift
+//  InAppModalDeviceAlertPresenter.swift
 //  LoopKit
 //
 //  Created by Rick Pasetto on 4/9/20.
@@ -9,17 +9,30 @@
 import Foundation
 import LoopKit
 
-public class InAppModalDeviceAlertHandler: DeviceAlertHandler {
-    
+public class InAppModalDeviceAlertPresenter: DeviceAlertPresenter {
+
     private weak var rootViewController: UIViewController?
     private weak var deviceAlertManagerResponder: DeviceAlertManagerResponder?
-    
+
     private var alertsShowing: [DeviceAlert.Identifier: (UIAlertController, DeviceAlert)] = [:]
     private var alertsPending: [DeviceAlert.Identifier: (Timer, DeviceAlert)] = [:]
+
+    typealias ActionFactoryFunction = (String?, UIAlertAction.Style, ((UIAlertAction) -> Void)?) -> UIAlertAction
+    private let newActionFunc: ActionFactoryFunction
 
-    init(rootViewController: UIViewController, deviceAlertManagerResponder: DeviceAlertManagerResponder) {
+    typealias TimerFactoryFunction = (TimeInterval, Bool, (() -> Void)?) -> Timer
+    private let newTimerFunc: TimerFactoryFunction
+
+    init(rootViewController: UIViewController,
+         deviceAlertManagerResponder: DeviceAlertManagerResponder,
+         newActionFunc: @escaping ActionFactoryFunction = UIAlertAction.init,
+         newTimerFunc: TimerFactoryFunction? = nil) {
         self.rootViewController = rootViewController
         self.deviceAlertManagerResponder = deviceAlertManagerResponder
+        self.newActionFunc = newActionFunc
+        self.newTimerFunc = newTimerFunc ?? { timeInterval, repeats, block in
+            return Timer.scheduledTimer(withTimeInterval: timeInterval, repeats: repeats) { _ in block?() }
+        }
     }
 
     public func issueAlert(_ alert: DeviceAlert) {
@@ -41,15 +54,20 @@ public class InAppModalDeviceAlertHandler: DeviceAlertHandler {
     }
 
     public func removeDeliveredAlert(identifier: DeviceAlert.Identifier) {
+        removeDeliveredAlert(identifier: identifier, completion: nil)
+    }
+    
+    // For tests only
+    func removeDeliveredAlert(identifier: DeviceAlert.Identifier, completion: (() -> Void)?) {
         DispatchQueue.main.async {
-            self.alertsShowing[identifier]?.0.dismiss(animated: true)
+            self.alertsShowing[identifier]?.0.dismiss(animated: true, completion: completion)
             self.clearDeliveredAlert(identifier: identifier)
         }
     }
 }
 
 /// Private functions
-extension InAppModalDeviceAlertHandler {
+extension InAppModalDeviceAlertPresenter {
 
     private func schedule(alert: DeviceAlert, interval: TimeInterval, repeats: Bool) {
         guard alert.foregroundContent != nil else {
@@ -59,7 +77,7 @@ extension InAppModalDeviceAlertHandler {
             if self.isAlertPending(identifier: alert.identifier) {
                 return
             }
-            let timer = Timer.scheduledTimer(withTimeInterval: interval, repeats: repeats) { [weak self] timer in
+            let timer = self.newTimerFunc(interval, repeats) { [weak self] in
                 self?.show(alert: alert)
                 if !repeats {
                     self?.clearPendingAlert(identifier: alert.identifier)
@@ -119,7 +137,7 @@ extension InAppModalDeviceAlertHandler {
         dispatchPrecondition(condition: .onQueue(.main))
         // For now, this is a simple alert with an "OK" button
         let alertController = UIAlertController(title: title, message: message, preferredStyle: .alert)
-        alertController.addAction(UIAlertAction(title: action, style: .default, handler: { _ in completion() }))
+        alertController.addAction(newActionFunc(action, .default, { _ in completion() }))
         topViewController(controller: rootViewController)?.present(alertController, animated: true)
         return alertController
     }

Loop/Managers/DeviceAlert/UserNotificationDeviceAlertHandler.swift renamed to Loop/Managers/DeviceAlert/UserNotificationDeviceAlertPresenter.swift

@@ -1,5 +1,5 @@
 //
-//  UserNotificationAlertHandler.swift
+//  UserNotificationDeviceAlertPresenter.swift
 //  LoopKit
 //
 //  Created by Rick Pasetto on 4/9/20.
@@ -9,34 +9,51 @@
 import LoopKit
 import UserNotifications
 
-class UserNotificationDeviceAlertHandler: DeviceAlertHandler {
+protocol UserNotificationCenter {
+    func add(_ request: UNNotificationRequest, withCompletionHandler: ((Error?) -> Void)?)
+    func removePendingNotificationRequests(withIdentifiers: [String])
+    func removeDeliveredNotifications(withIdentifiers: [String])
+}
+
+extension UNUserNotificationCenter: UserNotificationCenter {}
+
+class UserNotificationDeviceAlertPresenter: DeviceAlertPresenter {
 
-    let alertInBackgroundOnly: Bool
+    let alertInBackgroundOnly = true
     let isAppInBackgroundFunc: () -> Bool
-    let userNotificationCenter: UNUserNotificationCenter = UNUserNotificationCenter.current()
+    let userNotificationCenter: UserNotificationCenter
 
-    init(alertInBackgroundOnly: Bool = true, isAppInBackgroundFunc: @escaping () -> Bool) {
-        self.alertInBackgroundOnly = alertInBackgroundOnly
+    init(isAppInBackgroundFunc: @escaping () -> Bool,
+         userNotificationCenter: UserNotificationCenter = UNUserNotificationCenter.current()) {
         self.isAppInBackgroundFunc = isAppInBackgroundFunc
+        self.userNotificationCenter = userNotificationCenter
     }
 
     func issueAlert(_ alert: DeviceAlert) {
         DispatchQueue.main.async {
             if self.alertInBackgroundOnly && self.isAppInBackgroundFunc() || !self.alertInBackgroundOnly {
                 if let request = alert.asUserNotificationRequest() {
-                    self.userNotificationCenter.add(request)
+                    self.userNotificationCenter.add(request) { error in
+                        if let error = error {
+                            print("Something went wrong posting the user notification: \(error)")
+                        }
+                    }
                     // For now, UserNotifications do not not acknowledge...not yet at least
                 }
             }
         }
     }
 
     func removePendingAlert(identifier: DeviceAlert.Identifier) {
-        userNotificationCenter.removePendingNotificationRequests(withIdentifiers: [identifier.value])
+        DispatchQueue.main.async {
+            self.userNotificationCenter.removePendingNotificationRequests(withIdentifiers: [identifier.value])
+        }
     }
 
     func removeDeliveredAlert(identifier: DeviceAlert.Identifier) {
-        userNotificationCenter.removeDeliveredNotifications(withIdentifiers: [identifier.value])
+        DispatchQueue.main.async {
+            self.userNotificationCenter.removeDeliveredNotifications(withIdentifiers: [identifier.value])
+        }
     }
 }
 
@@ -64,7 +81,7 @@ public extension DeviceAlert {
         userNotificationContent.threadIdentifier = identifier.value // Used to match categoryIdentifier, but I /think/ we want multiple threads for multiple alert types, no?
         userNotificationContent.userInfo = [
             LoopNotificationUserInfoKey.managerIDForAlert.rawValue: identifier.managerIdentifier,
-            LoopNotificationUserInfoKey.alertTypeId.rawValue: identifier.typeIdentifier
+            LoopNotificationUserInfoKey.alertTypeID.rawValue: identifier.alertIdentifier
         ]
         return userNotificationContent
     }

Loop/Managers/DeviceDataManager.swift

@@ -369,7 +369,7 @@ extension DeviceDataManager {
 
 // MARK: - DeviceManagerDelegate
 extension DeviceDataManager: DeviceManagerDelegate {
-    #if !USE_NEW_ALERT_FACILITY
+//    #if TO BE REMOVED
     func scheduleNotification(for manager: DeviceManager,
                               identifier: String,
                               content: UNNotificationContent,
@@ -390,15 +390,15 @@ extension DeviceDataManager: DeviceManagerDelegate {
     func removeNotificationRequests(for manager: DeviceManager, identifiers: [String]) {
         UNUserNotificationCenter.current().removePendingNotificationRequests(withIdentifiers: identifiers)
     }
-    #endif
+//    #endif
 
     func deviceManager(_ manager: DeviceManager, logEventForDeviceIdentifier deviceIdentifier: String?, type: DeviceLogEntryType, message: String, completion: ((Error?) -> Void)?) {
         deviceLog.log(managerIdentifier: Swift.type(of: manager).managerIdentifier, deviceIdentifier: deviceIdentifier, type: type, message: message, completion: completion)
     }
 }
 
 // MARK: - UserAlertHandler
-extension DeviceDataManager: DeviceAlertHandler {
+extension DeviceDataManager: DeviceAlertPresenter {
 
     func issueAlert(_ alert: DeviceAlert) {
         deviceAlertManager?.issueAlert(alert)

Loop/Managers/NotificationManager.swift

@@ -14,7 +14,7 @@ struct NotificationManager {
 
     enum Action: String {
         case retryBolus
-        case acknowledgeAlert
+        case acknowledgeDeviceAlert
     }
 
     private static var notificationCategories: Set<UNNotificationCategory> {
@@ -33,17 +33,17 @@ struct NotificationManager {
             options: []
         ))
 
-        let acknowledgeAlertAction = UNNotificationAction(
-            identifier: Action.acknowledgeAlert.rawValue,
+        let acknowledgeDeviceAlertAction = UNNotificationAction(
+            identifier: Action.acknowledgeDeviceAlert.rawValue,
             title: NSLocalizedString("OK", comment: "The title of the notification action to acknowledge a device alert"),
-            options: [.foreground]
+            options: .foreground
         )
 
         categories.append(UNNotificationCategory(
             identifier: LoopNotificationCategory.alert.rawValue,
-            actions: [acknowledgeAlertAction],
+            actions: [acknowledgeDeviceAlertAction],
             intentIdentifiers: [],
-            options: [.customDismissAction]
+            options: .customDismissAction
         ))
 
         return Set(categories)