From 683965f1c57f7913f7a24068a8d044b84d99ad18 Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:01:56 -0300 Subject: [PATCH 1/8] NotificationAction: Synchronize Notification Action --- .../Yosemite/Actions/NotificationAction.swift | 1 + .../Yosemite/Stores/NotificationStore.swift | 26 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Yosemite/Yosemite/Actions/NotificationAction.swift b/Yosemite/Yosemite/Actions/NotificationAction.swift index ef306a21d4b..166f6afc632 100644 --- a/Yosemite/Yosemite/Actions/NotificationAction.swift +++ b/Yosemite/Yosemite/Actions/NotificationAction.swift @@ -7,6 +7,7 @@ import Networking // 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) } diff --git a/Yosemite/Yosemite/Stores/NotificationStore.swift b/Yosemite/Yosemite/Stores/NotificationStore.swift index 3e99205ceff..48e077ded1c 100644 --- a/Yosemite/Yosemite/Stores/NotificationStore.swift +++ b/Yosemite/Yosemite/Stores/NotificationStore.swift @@ -32,6 +32,8 @@ public class NotificationStore: Store { switch action { case .synchronizeNotifications(let onCompletion): synchronizeNotifications(onCompletion: onCompletion) + case .synchronizeNotification(let noteId, let onCompletion): + synchronizeNotification(with: noteId, onCompletion: onCompletion) case .updateLastSeen(let timestamp, let onCompletion): updateLastSeen(timestamp: timestamp, onCompletion: onCompletion) case .updateReadStatus(let noteID, let read, let onCompletion): @@ -81,7 +83,28 @@ private extension NotificationStore { } } - + + /// Synchronizes the Notification matching the specified ID, and updates the local entity. + /// + /// - Parameters: + /// - noteId: Notification ID of the note to be downloaded. + /// - onCompletion: Closure to be executed on completion. + /// + func synchronizeNotification(with noteId: Int64, onCompletion: @escaping (Error?) -> Void) { + let remote = NotificationsRemote(network: network) + + remote.loadNotes(noteIds: [noteId]) { notes, error in + guard let notes = notes else { + onCompletion(error) + return + } + + self.updateLocalNotes(with: notes) { + onCompletion(nil) + } + } + } + /// Updates the last seen notification /// @@ -92,6 +115,7 @@ private extension NotificationStore { } } + /// Updates the read status for the given notification ID /// func updateReadStatus(noteID: Int64, read: Bool, onCompletion: @escaping (Error?) -> Void) { From 2dfb17e7438907678ee3ab585abdde62e6f4c16d Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:02:04 -0300 Subject: [PATCH 2/8] NotificationStoreTests: New Test --- .../Stores/NotificationStoreTests.swift | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/Yosemite/YosemiteTests/Stores/NotificationStoreTests.swift b/Yosemite/YosemiteTests/Stores/NotificationStoreTests.swift index 02741dbab71..99e498ddcc0 100644 --- a/Yosemite/YosemiteTests/Stores/NotificationStoreTests.swift +++ b/Yosemite/YosemiteTests/Stores/NotificationStoreTests.swift @@ -111,7 +111,6 @@ class NotificationStoreTests: XCTestCase { /// Initial Sync /// let initialSyncAction = NotificationAction.synchronizeNotifications() { (error) in - XCTAssertEqual(self.viewStorage.countObjects(ofType: Storage.Note.self), 40) notificationStore.onAction(nestedSyncAction) } @@ -120,6 +119,33 @@ class NotificationStoreTests: XCTestCase { wait(for: [expectation], timeout: Constants.expectationTimeout) } + /// Verifies that `NotificationAction.synchronizeNotification` will effectively request a single notification, + /// which will be stored in CoreData. + /// + func testSynchronizeSingleNotificationEffectivelyUpdatesRequestedNote() { + let expectation = self.expectation(description: "Sync notification") + let notificationStore = NotificationStore(dispatcher: dispatcher, storageManager: storageManager, network: network) + let notificationId = Int64(100001) + + network.simulateResponse(requestUrlSuffix: "notifications", filename: "notifications-load-all") + XCTAssertEqual(viewStorage.countObjects(ofType: Storage.Note.self), 0) + + let syncAction = NotificationAction.synchronizeNotification(noteId: notificationId) { error in + let note = self.viewStorage.loadNotification(noteID: notificationId) + XCTAssertNil(error) + XCTAssertNotNil(note) + + let request = self.network.requestsForResponseData[0] as! DotcomRequest + XCTAssertEqual(request.parameters?["ids"], String(notificationId)) + + expectation.fulfill() + } + + notificationStore.onAction(syncAction) + wait(for: [expectation], timeout: Constants.expectationTimeout) + } + + // MARK: - NotificationAction.updateLastSeen /// Verifies that NotificationAction.updateLastSeen handles a success response from the backend properly From 1b9c464425d660f8f64fe783a053f0aea69c4a81 Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:04:29 -0300 Subject: [PATCH 3/8] NotificationDetailsViewController: Wiring Pull to Refresh --- .../NotificationDetailsViewController.swift | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift b/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift index 2c5b080f84a..71a37360732 100644 --- a/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift @@ -17,6 +17,14 @@ class NotificationDetailsViewController: UIViewController { return EntityListener(storageManager: AppDelegate.shared.storageManager, readOnlyEntity: note) }() + /// Pull To Refresh Support. + /// + private lazy var refreshControl: UIRefreshControl = { + let refreshControl = UIRefreshControl() + refreshControl.addTarget(self, action: #selector(pullToRefresh(sender:)), for: .valueChanged) + return refreshControl + }() + /// Note to be displayed! /// private var note: Note! { @@ -87,6 +95,7 @@ private extension NotificationDetailsViewController { // Hide "Empty Rows" tableView.tableFooterView = UIView() tableView.backgroundColor = StyleManager.tableViewBackgroundColor + tableView.refreshControl = refreshControl } /// Setup: EntityListener @@ -114,6 +123,35 @@ private extension NotificationDetailsViewController { } +// MARK: - Sync +// +private extension NotificationDetailsViewController { + + /// Refresh Control's Callback. + /// + @IBAction func pullToRefresh(sender: UIRefreshControl) { + WooAnalytics.shared.track(.notificationsListPulledToRefresh) + synchronizeNotification(noteId: note.noteId) { + sender.endRefreshing() + } + } + + /// Synchronizes the Notifications associated to the active WordPress.com account. + /// + func synchronizeNotification(noteId: Int64, onCompletion: @escaping () -> Void) { + let action = NotificationAction.synchronizeNotification(noteId: noteId) { error in + if let error = error { + DDLogError("⛔️ Error synchronizing notification [\(noteId)]: \(error)") + } + + onCompletion() + } + + StoresManager.shared.dispatch(action) + } +} + + // MARK: - Private Methods // private extension NotificationDetailsViewController { From e4eecb7c63f326412fd74b41612f2e67df31dc26 Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:19:41 -0300 Subject: [PATCH 4/8] NotificationDetailsViewController: Updating title on refresh --- .../NotificationDetailsViewController.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift b/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift index 71a37360732..09f1a63736a 100644 --- a/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift @@ -29,7 +29,7 @@ class NotificationDetailsViewController: UIViewController { /// private var note: Note! { didSet { - buildDetailsRows() + reloadInterface() } } @@ -65,7 +65,7 @@ class NotificationDetailsViewController: UIViewController { configureEntityListener() registerTableViewCells() - buildDetailsRows() + reloadInterface() } } @@ -77,8 +77,6 @@ private extension NotificationDetailsViewController { /// Setup: Navigation /// func configureNavigationItem() { - title = note.title - // Don't show the Notifications title in the next-view's back button navigationItem.backBarButtonItem = UIBarButtonItem(title: String(), style: .plain, target: nil, action: nil) } @@ -131,6 +129,7 @@ private extension NotificationDetailsViewController { /// @IBAction func pullToRefresh(sender: UIRefreshControl) { WooAnalytics.shared.track(.notificationsListPulledToRefresh) + Hack.overrideTitle = false synchronizeNotification(noteId: note.noteId) { sender.endRefreshing() } @@ -156,9 +155,10 @@ private extension NotificationDetailsViewController { // private extension NotificationDetailsViewController { - /// Reloads all of the Notification Detail Rows! + /// Reloads all of the Details Interface /// - func buildDetailsRows() { + func reloadInterface() { + title = note.title rows = NoteDetailsRow.details(from: note) tableView.reloadData() } From 4d664eb4071920c50410fd1bc74bf2ee19a68697 Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:20:05 -0300 Subject: [PATCH 5/8] Note+ReadOnlyType: Updates isReadOnlyRepresentation Logic --- Yosemite/Yosemite/Model/ReadOnly/Note+ReadOnlyType.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Yosemite/Yosemite/Model/ReadOnly/Note+ReadOnlyType.swift b/Yosemite/Yosemite/Model/ReadOnly/Note+ReadOnlyType.swift index 7e4165ebbcb..504d07aa817 100644 --- a/Yosemite/Yosemite/Model/ReadOnly/Note+ReadOnlyType.swift +++ b/Yosemite/Yosemite/Model/ReadOnly/Note+ReadOnlyType.swift @@ -13,7 +13,6 @@ extension Yosemite.Note: ReadOnlyType { return false } - return storageNote.noteID == noteId && - storageNote.noteHash == hash + return storageNote.noteID == noteId } } From f8b2361a669e628e4df61a2ee529e6abc8f2e0db Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:20:50 -0300 Subject: [PATCH 6/8] Revert to this commit to test Note Details PTR --- .../Notifications/NotificationsViewController.swift | 2 +- .../Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift b/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift index 8f267742b86..ab2917187b9 100644 --- a/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift @@ -24,7 +24,7 @@ class NotificationsViewController: UIViewController { /// TODO: Remove ASAP as soon as the (pending) backend PR is merged /// private var filter: NSPredicate { - return NSPredicate(format: "type == %@ OR title == 'Product Review'", Note.Kind.storeOrder.rawValue) + return NSPredicate(format: "type == %@ OR title == 'Product Review' OR title == 'HACK Product Review'", Note.Kind.storeOrder.rawValue) } /// Pull To Refresh Support. diff --git a/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift b/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift index 8b5111c1150..5cd35f47eb3 100644 --- a/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift +++ b/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift @@ -1,6 +1,10 @@ import Foundation import Storage +public class Hack { + public static var overrideTitle = true +} + // Storage.Note: ReadOnlyConvertible Conformance. // extension Storage.Note: ReadOnlyConvertible { @@ -15,7 +19,7 @@ extension Storage.Note: ReadOnlyConvertible { noticon = note.noticon timestamp = note.timestamp url = note.url - title = note.title + title = Hack.overrideTitle ? ("HACK " + (note.title ?? "")) : note.title type = note.type subject = note.subjectAsData header = note.headerAsData From 224ce9e8026489eda857d847129d43f55614e454 Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:20:53 -0300 Subject: [PATCH 7/8] Revert "Revert to this commit to test Note Details PTR" This reverts commit f8b2361a669e628e4df61a2ee529e6abc8f2e0db. --- .../Notifications/NotificationsViewController.swift | 2 +- .../Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift b/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift index ab2917187b9..8f267742b86 100644 --- a/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Notifications/NotificationsViewController.swift @@ -24,7 +24,7 @@ class NotificationsViewController: UIViewController { /// TODO: Remove ASAP as soon as the (pending) backend PR is merged /// private var filter: NSPredicate { - return NSPredicate(format: "type == %@ OR title == 'Product Review' OR title == 'HACK Product Review'", Note.Kind.storeOrder.rawValue) + return NSPredicate(format: "type == %@ OR title == 'Product Review'", Note.Kind.storeOrder.rawValue) } /// Pull To Refresh Support. diff --git a/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift b/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift index 5cd35f47eb3..8b5111c1150 100644 --- a/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift +++ b/Yosemite/Yosemite/Model/Storage/Note+ReadOnlyConvertible.swift @@ -1,10 +1,6 @@ import Foundation import Storage -public class Hack { - public static var overrideTitle = true -} - // Storage.Note: ReadOnlyConvertible Conformance. // extension Storage.Note: ReadOnlyConvertible { @@ -19,7 +15,7 @@ extension Storage.Note: ReadOnlyConvertible { noticon = note.noticon timestamp = note.timestamp url = note.url - title = Hack.overrideTitle ? ("HACK " + (note.title ?? "")) : note.title + title = note.title type = note.type subject = note.subjectAsData header = note.headerAsData From dc33c7b7095d50f482695a45e22716bc8941d593 Mon Sep 17 00:00:00 2001 From: Jorge Leandro Perez Date: Tue, 13 Nov 2018 10:25:35 -0300 Subject: [PATCH 8/8] NotificationDetailsViewController: Removing Hack Leftover --- .../Notifications/NotificationDetailsViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift b/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift index 09f1a63736a..4effdf5e258 100644 --- a/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift +++ b/WooCommerce/Classes/ViewRelated/Notifications/NotificationDetailsViewController.swift @@ -129,7 +129,7 @@ private extension NotificationDetailsViewController { /// @IBAction func pullToRefresh(sender: UIRefreshControl) { WooAnalytics.shared.track(.notificationsListPulledToRefresh) - Hack.overrideTitle = false + synchronizeNotification(noteId: note.noteId) { sender.endRefreshing() }