Merge pull request #432 from woocommerce/issue/19-mark-as-read-for-multiple-notes

Notifications: Mark All as Read

Author: jleandroperez

Networking/Networking/Remote/NotificationsRemote.swift

@@ -43,16 +43,26 @@ public class NotificationsRemote: Remote {
     ///     - read: The new Read Status to be set.
     ///     - completion: Closure to be executed on completion, indicating whether the OP was successful or not.
     ///
-    public func updateReadStatus(_ notificationID: String, read: Bool, completion: @escaping (Error?) -> Void) {
+    public func updateReadStatus(noteIds: [Int64], read: Bool, completion: @escaping (Error?) -> Void) {
         // Note: Isn't the API wonderful?
+        //
         let booleanFromPlanetMars = read ? Constants.readAsInteger : Constants.unreadAsInteger
-        let payload = [notificationID: booleanFromPlanetMars]
 
-        var parameters = [String: String]()
-        if let encoded = payload.toJSONEncoded() {
-            parameters[ParameterKeys.counts] = encoded
+        // Payload: [NoteID: ReadStatus]
+        //
+        var payload = [String: Int]()
+
+        for noteId in noteIds {
+            let noteIdAsString = String(noteId)
+            payload[noteIdAsString] = booleanFromPlanetMars
         }
 
+        // Parameters: [.counts: [Payload]]
+        //
+        let parameters: [String: Any] = [
+            ParameterKeys.counts: payload
+        ]
+
         let request = DotcomRequest(wordpressApiVersion: .mark1_1, method: .post, path: Paths.read, parameters: parameters)
         let mapper = SuccessResultMapper()
 

Networking/Networking/Requests/DotcomRequest.swift

@@ -24,7 +24,7 @@ struct DotcomRequest: URLRequestConvertible {
 
     /// Parameters
     ///
-    let parameters: [String: String]?
+    let parameters: [String: Any]?
 
 
     /// Designated Initializer.
@@ -35,7 +35,7 @@ struct DotcomRequest: URLRequestConvertible {
     ///     - path: RPC that should be executed.
     ///     - parameters: Collection of String parameters to be passed over to our target RPC.
     ///
-    init(wordpressApiVersion: WordPressAPIVersion, method: HTTPMethod, path: String, parameters: [String: String]? = nil) {
+    init(wordpressApiVersion: WordPressAPIVersion, method: HTTPMethod, path: String, parameters: [String: Any]? = nil) {
         self.wordpressApiVersion = wordpressApiVersion
         self.method = method
         self.path = path

Networking/NetworkingTests/Remote/NotificationsRemoteTests.swift

@@ -114,7 +114,7 @@ class NotificationsRemoteTests: XCTestCase {
 
         network.simulateResponse(requestUrlSuffix: "notifications/read", filename: "generic_error")
 
-        remote.updateReadStatus("", read: true) { error in
+        remote.updateReadStatus(noteIds: [], read: true) { error in
             let error = error as? DotcomError
             XCTAssertEqual(error?.code, "unknown_token")
             expectation.fulfill()
@@ -131,7 +131,7 @@ class NotificationsRemoteTests: XCTestCase {
 
         network.simulateResponse(requestUrlSuffix: "notifications/read", filename: "generic_success")
 
-        remote.updateReadStatus("", read: true) { error in
+        remote.updateReadStatus(noteIds: [], read: true) { error in
             XCTAssertNil(error)
             expectation.fulfill()
         }

WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift

@@ -3,6 +3,7 @@ import Gridicons
 import Yosemite
 import WordPressUI
 import SafariServices
+import Gridicons
 
 
 // MARK: - NotificationsViewController
@@ -13,6 +14,10 @@ class NotificationsViewController: UIViewController {
     ///
     @IBOutlet private var tableView: UITableView!
 
+    /// Haptic Feedback!
+    ///
+    private let hapticGenerator = UINotificationFeedbackGenerator()
+
     /// ResultsController: Surrounds us. Binds the galaxy together. And also, keeps the UITableView <> (Stored) Notes in sync.
     ///
     private lazy var resultsController: ResultsController<StorageNote> = {
@@ -80,6 +85,7 @@ class NotificationsViewController: UIViewController {
         view.backgroundColor = StyleManager.tableViewBackgroundColor
 
         configureNavigationItem()
+        configureNavigationBarButtons()
         configureTableView()
         configureTableViewCells()
         configureResultsController()
@@ -111,6 +117,20 @@ private extension NotificationsViewController {
         navigationItem.backBarButtonItem = UIBarButtonItem(title: String(), style: .plain, target: nil, action: nil)
     }
 
+    /// Setup: NavigationBar Buttons
+    ///
+    func configureNavigationBarButtons() {
+        let rightBarButton = UIBarButtonItem(image: Gridicon.iconOfType(.menus),
+                                             style: .plain,
+                                             target: self,
+                                             action: #selector(markAllAsRead))
+        rightBarButton.tintColor = .white
+        rightBarButton.accessibilityTraits = .button
+        rightBarButton.accessibilityLabel = NSLocalizedString("Mark All as Read", comment: "Accessibility label for the Mark All Notifications as Read Button")
+        rightBarButton.accessibilityHint = NSLocalizedString("Marks Every Notification as Read", comment: "VoiceOver accessibility hint for the Mark All Notifications as Read Action")
+        navigationItem.rightBarButtonItem = rightBarButton
+    }
+
     /// Setup: TableView
     ///
     func configureTableView() {
@@ -149,13 +169,37 @@ private extension NotificationsViewController {
             sender.endRefreshing()
         }
     }
+
+    @IBAction func markAllAsRead() {
+        let unread = resultsController.fetchedObjects.filter { $0.read == false }
+        if unread.isEmpty {
+            DDLogVerbose("# Every single notification is already marked as Read!")
+            return
+        }
+
+        markAsRead(notes: unread)
+        hapticGenerator.notificationOccurred(.success)
+    }
 }
 
 
-// MARK: - Sync'ing Helpers
+// MARK: - Yosemite Wrappers
 //
 private extension NotificationsViewController {
 
+    /// Marks the specified collection of Notifications as Read.
+    ///
+    func markAsRead(notes: [Note]) {
+        let identifiers = notes.map { $0.noteId }
+        let action = NotificationAction.updateMultipleReadStatus(noteIds: identifiers, read: true) { error in
+            if let error = error {
+                DDLogError("⛔️ Error marking notifications as read: \(error)")
+            }
+        }
+
+        StoresManager.shared.dispatch(action)
+    }
+
     /// Synchronizes the Notifications associated to the active WordPress.com account.
     ///
     func synchronizeNotifications(onCompletion: (() -> Void)? = nil) {

Yosemite/Yosemite/Actions/NotificationAction.swift

@@ -9,5 +9,6 @@ public enum NotificationAction: Action {
     case synchronizeNotifications(onCompletion: (Error?) -> Void)
     case synchronizeNotification(noteId: Int64, onCompletion: (Error?) -> Void)
     case updateLastSeen(timestamp: String, onCompletion: (Error?) -> Void)
-    case updateReadStatus(noteID: Int64, read: Bool, onCompletion: (Error?) -> Void)
+    case updateReadStatus(noteId: Int64, read: Bool, onCompletion: (Error?) -> Void)
+    case updateMultipleReadStatus(noteIds: [Int64], read: Bool, onCompletion: (Error?) -> Void)
 }

Yosemite/Yosemite/Stores/NotificationStore.swift

@@ -36,8 +36,10 @@ public class NotificationStore: Store {
             synchronizeNotification(with: noteId, onCompletion: onCompletion)
         case .updateLastSeen(let timestamp, let onCompletion):
             updateLastSeen(timestamp: timestamp, onCompletion: onCompletion)
-        case .updateReadStatus(let noteID, let read, let onCompletion):
-            updateReadStatus(noteID: noteID, read: read, onCompletion: onCompletion)
+        case .updateReadStatus(let noteId, let read, let onCompletion):
+            updateReadStatus(for: [noteId], read: read, onCompletion: onCompletion)
+        case .updateMultipleReadStatus(let noteIds, let read, let onCompletion):
+            updateReadStatus(for: noteIds, read: read, onCompletion: onCompletion)
         }
     }
 }
@@ -116,18 +118,34 @@ private extension NotificationStore {
     }
 
 
-    /// Updates the read status for the given notification ID
+    /// Updates the read status for the given notification ID(s)
     ///
-    func updateReadStatus(noteID: Int64, read: Bool, onCompletion: @escaping (Error?) -> Void) {
+    func updateReadStatus(for noteIds: [Int64], read: Bool, onCompletion: @escaping (Error?) -> Void) {
+        /// Optimistically Update
+        ///
+        updateLocalNoteReadStatus(for: noteIds, read: read)
+
+        /// On error we'll just mark the Note for Refresh
+        ///
         let remote = NotificationsRemote(network: network)
-        remote.updateReadStatus(String(noteID), read: read) { [weak self] (error) in
-            guard let `self` = self, error == nil else {
-                onCompletion(error)
+        remote.updateReadStatus(noteIds: noteIds, read: read) { error in
+            guard let error = error else {
+
+                /// What is this about:
+                /// Notice that there are few conditions in which the Network Request's callback may run *before*
+                /// the Optimisitc Update, such as in Unit Tests.
+                /// This may cause the Callback to run before the Read Flag has been toggled, which isn't cool.
+                /// *FORGIVE ME*, this is a workaround: the onCompletion closure must run after a No-OP in the derived
+                /// storage.
+                ///
+                self.performSharedDerivedStorageNoOp {
+                    onCompletion(nil)
+                }
                 return
             }
 
-            self.updateLocalNoteReadStatus(for: noteID, read: read) {
-                onCompletion(nil)
+            self.invalidateCache(for: noteIds) {
+                onCompletion(error)
             }
         }
     }
@@ -169,7 +187,7 @@ extension NotificationStore {
     ///     - remoteNotes: Collection of Notes
     ///     - completion: Callback to be executed on completion
     ///
-    func updateLocalNotes(with remoteNotes: [Note], completion: (() -> Void)? = nil) {
+    func updateLocalNotes(with remoteNotes: [Note], onCompletion: (() -> Void)? = nil) {
         let derivedStorage = type(of: self).sharedDerivedStorage(with: storageManager)
 
         derivedStorage.perform {
@@ -180,26 +198,33 @@ extension NotificationStore {
         }
 
         storageManager.saveDerivedType(derivedStorage: derivedStorage) {
-            DispatchQueue.main.async {
-                completion?()
+            guard let onCompletion = onCompletion else {
+                return
             }
+
+            DispatchQueue.main.async(execute: onCompletion)
         }
     }
 
-    /// Updates the read status for the given noteID in the Storage layer. If the local note to update cannot be found,
-    /// nothing is updated/created in storage.
+    /// Updates the read status for the specified Notifications. The callback happens on the Main Thread.
     ///
-    func updateLocalNoteReadStatus(for noteID: Int64, read: Bool, completion: (() -> Void)? = nil) {
-        assert(Thread.isMainThread)
-        let storage = storageManager.viewStorage
-        guard let storageNote = storage.loadNotification(noteID: noteID) else {
-            completion?()
-            return
+    func updateLocalNoteReadStatus(for noteIDs: [Int64], read: Bool, onCompletion: (() -> Void)? = nil) {
+        let derivedStorage = type(of: self).sharedDerivedStorage(with: storageManager)
+
+        derivedStorage.perform {
+            let notifications = noteIDs.compactMap { derivedStorage.loadNotification(noteID: $0) }
+            for note in notifications {
+                note.read = read
+            }
         }
 
-        storageNote.read = read
-        storage.saveIfNeeded()
-        completion?()
+        storageManager.saveDerivedType(derivedStorage: derivedStorage) {
+            guard let onCompletion = onCompletion else {
+                return
+            }
+
+            DispatchQueue.main.async(execute: onCompletion)
+        }
     }
 
     /// Given a collection of NoteHash Entities, this method will determine the `.noteID`'s of those entities that
@@ -229,6 +254,37 @@ extension NotificationStore {
             }
         }
     }
+
+    /// Invalidates the Hash for the specified Notifications.
+    ///
+    func invalidateCache(for noteIds: [Int64], onCompletion: (() -> Void)? = nil) {
+        let derivedStorage = type(of: self).sharedDerivedStorage(with: storageManager)
+
+        derivedStorage.perform {
+            let notifications = noteIds.compactMap { derivedStorage.loadNotification(noteID: $0) }
+            for note in notifications {
+                note.noteHash = Int64.min
+            }
+        }
+
+        storageManager.saveDerivedType(derivedStorage: derivedStorage) {
+            guard let onCompletion = onCompletion else {
+                return
+            }
+
+            DispatchQueue.main.async(execute: onCompletion)
+        }
+    }
+
+    /// Runs a No-OP in the Shared Derived Storage. On completion, the callback will be executed on the main thread.
+    ///
+    func performSharedDerivedStorageNoOp(onCompletion: @escaping () -> Void) {
+        let derivedStorage = type(of: self).sharedDerivedStorage(with: storageManager)
+
+        derivedStorage.perform {
+            DispatchQueue.main.async(execute: onCompletion)
+        }
+    }
 }