From 7364ae20bc35bd9cc33125150f8c214d85daa228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boz=CC=8Cidar=20S=CC=8Cevo?= Date: Tue, 10 Sep 2024 14:58:30 +0200 Subject: [PATCH 1/3] Use orderID from push notification note if there is some --- ...shNotificationBackgroundSynchronizer.swift | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift b/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift index ec724936ab1..32076dfcd89 100644 --- a/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift +++ b/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift @@ -27,22 +27,27 @@ struct PushNotificationBackgroundSynchronizer { /// @MainActor func sync() async -> UIBackgroundFetchResult { - guard let pushNotification = PushNotification.from(userInfo: userInfo) else { return .noData } do { - let startTime = Date.now - - // I'm not sure why we need to sync all notifications instead of only the current one. - // This is legacy code copied from PushNotificationsManager - try await synchronizeNotifications() - - // Find the orderID from the previously synced notification. - guard let orderID = getOrderID(noteID: pushNotification.noteID) else { - return .newData + let orderID: Int64 + + if let note = pushNotification.note, + let noteOrderID = note.meta.identifier(forKey: .order) { + orderID = Int64(noteOrderID) + } else { + // I'm not sure why we need to sync all notifications instead of only the current one. + // This is legacy code copied from PushNotificationsManager + try await synchronizeNotifications() + + // Find the orderID from the previously synced notification. + guard let pushNotificationOrderID = getOrderID(noteID: pushNotification.noteID) else { + return .newData + } + orderID = pushNotificationOrderID } // Sync the order list data @@ -58,7 +63,6 @@ struct PushNotificationBackgroundSynchronizer { ServiceLocator.analytics.track(event: .BackgroundUpdates.orderPushNotificationSynced(timeTaken: timeTaken)) return .newData - } catch { DDLogError("⛔️ Error synchronizing notification dependencies: \(error)") ServiceLocator.analytics.track(event: .BackgroundUpdates.orderPushNotificationSyncError(error)) From fdca1f38bae4dd07e956d4d35b1966bfaf0f3db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boz=CC=8Cidar=20S=CC=8Cevo?= Date: Wed, 11 Sep 2024 15:13:18 +0200 Subject: [PATCH 2/3] Update PushNotificationBackgroundSynchronizer.swift --- .../Notifications/PushNotificationBackgroundSynchronizer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift b/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift index 32076dfcd89..30407ddbc2a 100644 --- a/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift +++ b/WooCommerce/Classes/Notifications/PushNotificationBackgroundSynchronizer.swift @@ -3,7 +3,7 @@ import Yosemite /// Type that fetches the necessary resources when a push notification arrives in the background. /// Current it fetches: -/// - Notifications +/// - Notifications (If needed) /// - Orders List (If needed) /// - Notification Order (If needed) /// From 096108a67693beb71841e18437a359d0a786d6a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boz=CC=8Cidar=20S=CC=8Cevo?= Date: Fri, 13 Sep 2024 11:15:52 +0200 Subject: [PATCH 3/3] Add test_synchronizer_stores_order_notification_when_order_comes_inside_notification --- ...ificationBackgroundSynchronizerTests.swift | 89 +++++++++++++++---- 1 file changed, 71 insertions(+), 18 deletions(-) diff --git a/WooCommerce/WooCommerceTests/Notifications/PushNotificationBackgroundSynchronizerTests.swift b/WooCommerce/WooCommerceTests/Notifications/PushNotificationBackgroundSynchronizerTests.swift index da6e16a873a..5020a10a295 100644 --- a/WooCommerce/WooCommerceTests/Notifications/PushNotificationBackgroundSynchronizerTests.swift +++ b/WooCommerce/WooCommerceTests/Notifications/PushNotificationBackgroundSynchronizerTests.swift @@ -18,7 +18,7 @@ final class PushNotificationBackgroundSynchronizerTests: XCTestCase { @MainActor func test_synchronizer_stores_order_notification_when_exists_in_order_list() async { // Given - mockOrderInOrderListResponses() + mockOrderInOrderListResponses(order: Self.sampleOrder) // When let synchronizer = PushNotificationBackgroundSynchronizer(userInfo: Self.sampleUserInfo, stores: stores, storage: storage.viewStorage) @@ -33,7 +33,7 @@ final class PushNotificationBackgroundSynchronizerTests: XCTestCase { @MainActor func test_synchronizer_stores_order_notification_when_does_not_exists_in_order_list() async { // Given - mockOrderNotInOrdersList() + mockOrderNotInOrdersList(order: Self.sampleOrder) // When let synchronizer = PushNotificationBackgroundSynchronizer(userInfo: Self.sampleUserInfo, stores: stores, storage: storage.viewStorage) @@ -44,21 +44,41 @@ final class PushNotificationBackgroundSynchronizerTests: XCTestCase { XCTAssertNotNil(storedOrder) XCTAssertEqual(syncResult, .newData) } + + @MainActor + func test_synchronizer_stores_order_notification_when_order_comes_inside_notification() async { + // Given + mockOrderNotInOrdersForSampleWithNoteFullData() + + // When + let synchronizer = PushNotificationBackgroundSynchronizer(userInfo: Self.sampleUserInfoWithNoteFullData, stores: stores, storage: storage.viewStorage) + let syncResult = await synchronizer.sync() + + // Then + let storedOrder = storage.viewStorage.loadOrder(siteID: Self.sampleSiteIDFromNotificationWithNoteFullData, + orderID: Self.sampleOrderIDFromNotificationWithNoteFullData) + XCTAssertNotNil(storedOrder) + XCTAssertEqual(syncResult, .newData) + XCTAssertEqual(storedOrder?.orderID, Self.sampleOrderIDFromNotificationWithNoteFullData) + XCTAssertEqual(storedOrder?.siteID, Self.sampleSiteIDFromNotificationWithNoteFullData) + } } // MARK: Helpers private extension PushNotificationBackgroundSynchronizerTests { /// Responses when the order exists in the order list /// - func mockOrderInOrderListResponses() { - // Mock sync notifications - stores.whenReceivingAction(ofType: NotificationAction.self) { action in - switch action { - case let .synchronizeNotifications(onCompletion): - self.storage.insertSampleNote(readOnlyNote: Self.sampleNote) - onCompletion(nil) - default: - break + func mockOrderInOrderListResponses(order: Networking.Order, synchronizeNotifications: Bool = true) { + if synchronizeNotifications { + // Mock sync notifications + stores.whenReceivingAction(ofType: NotificationAction.self) { action in + switch action { + case let .synchronizeNotifications(onCompletion): + self.storage.insertSampleNote(readOnlyNote: Self.sampleNote) + onCompletion(nil) + default: + break + } } } @@ -66,7 +86,7 @@ private extension PushNotificationBackgroundSynchronizerTests { stores.whenReceivingAction(ofType: AppSettingsAction.self) { action in switch action { case let .loadOrdersSettings(_, onCompletion): - onCompletion(.success(.init(siteID: 123, orderStatusesFilter: nil, dateRangeFilter: nil, productFilter: nil, customerFilter: nil))) + onCompletion(.success(.init(siteID: order.siteID, orderStatusesFilter: nil, dateRangeFilter: nil, productFilter: nil, customerFilter: nil))) default: break } @@ -76,8 +96,8 @@ private extension PushNotificationBackgroundSynchronizerTests { stores.whenReceivingAction(ofType: OrderAction.self) { action in switch action { case let .fetchFilteredOrders(_, _, _, _, _, _, _, _, _, onCompletion): - self.storage.insertSampleOrder(readOnlyOrder: Self.sampleOrder) - onCompletion(0, .success([Self.sampleOrder])) + self.storage.insertSampleOrder(readOnlyOrder: order) + onCompletion(0, .success([order])) default: break } @@ -86,8 +106,8 @@ private extension PushNotificationBackgroundSynchronizerTests { /// Responses when the order does not exists the order list. /// - func mockOrderNotInOrdersList() { - mockOrderInOrderListResponses() + func mockOrderNotInOrdersList(order: Networking.Order) { + mockOrderInOrderListResponses(order: order) // Mock sync order list & single order stores.whenReceivingAction(ofType: OrderAction.self) { action in @@ -95,8 +115,25 @@ private extension PushNotificationBackgroundSynchronizerTests { case let .fetchFilteredOrders(_, _, _, _, _, _, _, _, _, onCompletion): onCompletion(0, .success([])) case let .retrieveOrderRemotely(_, _, onCompletion): - self.storage.insertSampleOrder(readOnlyOrder: Self.sampleOrder) - onCompletion(.success(Self.sampleOrder)) + self.storage.insertSampleOrder(readOnlyOrder: order) + onCompletion(.success(order)) + default: + break + } + } + } + + func mockOrderNotInOrdersForSampleWithNoteFullData() { + mockOrderInOrderListResponses(order: Self.sampleOrderWithNoteFullDataOrderID, synchronizeNotifications: false) + + // Mock sync order list & single order + stores.whenReceivingAction(ofType: OrderAction.self) { action in + switch action { + case let .fetchFilteredOrders(_, _, _, _, _, _, _, _, _, onCompletion): + onCompletion(0, .success([])) + case let .retrieveOrderRemotely(_, _, onCompletion): + self.storage.insertSampleOrder(readOnlyOrder: Self.sampleOrderWithNoteFullDataOrderID) + onCompletion(.success(Self.sampleOrderWithNoteFullDataOrderID)) default: break } @@ -134,4 +171,20 @@ private extension PushNotificationBackgroundSynchronizerTests { }() static var sampleOrder = Order.fake().copy(siteID: sampleSiteID, orderID: sampleOrderID) + + // Samples with note_full_data + static var sampleUserInfoWithNoteFullData: [String: Any] = [ + APNSKey.siteID: 205617935, + APNSKey.identifier: 8300031174, + APNSKey.aps: [APNSKey.alert: [ + APNSKey.alertTitle: "" + ]], + // swiftlint:disable line_length + APNSKey.noteFullData: "eNqVkc1O6zAQhV/FjNiRH+enTZqHgA0bdI0qN522hsSx4jEFobz7nYRSwZJN7MjnfDpn5hPsQOih+fcJZg9NXUgpiyyrygjowyE04GkYcTuMexwhgmBH1Cy0oesi8NY4h/T92yNpaGaSnw9viAG5XK2zalOsIviCNIVcTxF0xr7+kMGJyPlGpSp1YdeZNtbOJGe2uBG9T9qhVynfSKVvmUpnk1fpFQ5X+h9Bi4tJHAomjuVJU/DXgmH3gi0t8yF85ws8DUGc9BsKLSyexeK/ESrs66Ll76HewBRd1fffEnEYRnGbJ1KKwQo6oVgGC9MzT9r0nEf3jg25zMtYVnG+fswkj6opizspGylh1lGHF+jDZSGmHeyv0j45u6+SZxfzI6Hlqn2IXReOxnLVZeUqNb0+zmdwe00YO/3RszTO3xNnj0xm2QWuwqEsqyVqpz1tPaLdzqH5Lavysi7qzaqYLaHfzTvIpv+sG8QM", + APNSKey.type: sampleOrderType + ] + static let sampleSiteIDFromNotificationWithNoteFullData: Int64 = 205617935 + static let sampleOrderIDFromNotificationWithNoteFullData: Int64 = 306 + static var sampleOrderWithNoteFullDataOrderID = Order.fake().copy(siteID: sampleSiteIDFromNotificationWithNoteFullData, + orderID: sampleOrderIDFromNotificationWithNoteFullData) }