diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 7c8d690a340..b45b94dafa8 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -6,6 +6,7 @@ - [*] Better support site credential login for sites with captcha plugins [https://github.com/woocommerce/woocommerce-ios/pull/16372] - [*] Crash fix attempt to resolve the race condition in request authenticator swapping [https://github.com/woocommerce/woocommerce-ios/pull/16370] - [*] Update products on the Top Performers card when there are changes made to displayed products [https://github.com/woocommerce/woocommerce-ios/pull/16380] +- [*] Fixed issue unregistering device from push notifications [https://github.com/woocommerce/woocommerce-ios/pull/16386] - [Internal] Fix broken navigation to a variable product selector [https://github.com/woocommerce/woocommerce-ios/pull/16363] 23.7 diff --git a/WooCommerce/Classes/Notifications/PushNotificationsManager.swift b/WooCommerce/Classes/Notifications/PushNotificationsManager.swift index bf52c038e32..d9cc5e466f6 100644 --- a/WooCommerce/Classes/Notifications/PushNotificationsManager.swift +++ b/WooCommerce/Classes/Notifications/PushNotificationsManager.swift @@ -167,18 +167,19 @@ extension PushNotificationsManager { /// Unregisters the Application from WordPress.com Push Notifications Service. /// - func unregisterForRemoteNotifications() { + func unregisterForRemoteNotifications(onCompletion: @escaping () -> Void) { DDLogInfo("📱 Unregistering For Remote Notifications...") unregisterDotcomDeviceIfPossible() { error in if let error = error { DDLogError("⛔️ Unable to unregister from WordPress.com Push Notifications: \(error)") - return + } else { + DDLogInfo("📱 Successfully unregistered from WordPress.com Push Notifications!") + self.deviceID = nil + self.deviceToken = nil } - - DDLogInfo("📱 Successfully unregistered from WordPress.com Push Notifications!") - self.deviceID = nil - self.deviceToken = nil + // Always call completion, even on error + onCompletion() } } @@ -249,7 +250,7 @@ extension PushNotificationsManager { /// func registrationDidFail(with error: Error) { DDLogError("⛔️ Push Notifications Registration Failure: \(error)") - unregisterForRemoteNotifications() + unregisterForRemoteNotifications {} } /// Handles a notification while the app is in foreground diff --git a/WooCommerce/Classes/ServiceLocator/PushNotesManager.swift b/WooCommerce/Classes/ServiceLocator/PushNotesManager.swift index 874cf608bbc..9d2507b1ac7 100644 --- a/WooCommerce/Classes/ServiceLocator/PushNotesManager.swift +++ b/WooCommerce/Classes/ServiceLocator/PushNotesManager.swift @@ -50,8 +50,9 @@ protocol PushNotesManager { func registerForRemoteNotifications() /// Unregisters the Application from WordPress.com Push Notifications Service. + /// - Parameter onCompletion: Closure to be executed on completion. /// - func unregisterForRemoteNotifications() + func unregisterForRemoteNotifications(onCompletion: @escaping () -> Void) /// Requests Authorization to receive Push Notifications, *only* when the current Status is not determined. /// diff --git a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift index 032dd2369e9..e243847e98e 100644 --- a/WooCommerce/Classes/Yosemite/AuthenticatedState.swift +++ b/WooCommerce/Classes/Yosemite/AuthenticatedState.swift @@ -237,11 +237,6 @@ class AuthenticatedState: StoresManagerState { /// Executed before the current state is deactivated. /// func willLeave() { - let pushNotesManager = ServiceLocator.pushNotesManager - - pushNotesManager.unregisterForRemoteNotifications() - pushNotesManager.resetBadgeCountForAllStores(onCompletion: {}) - resetServices() } diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index 16dc1eece35..78360ab2ff5 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -270,7 +270,7 @@ class DefaultStoresManager: StoresManager { sessionManager.deleteApplicationPassword(locally: true) ServiceLocator.analytics.refreshUserData() ZendeskProvider.shared.reset() - ServiceLocator.pushNotesManager.unregisterForRemoteNotifications() + ServiceLocator.pushNotesManager.unregisterForRemoteNotifications {} } /// Fully deauthenticates the user, if needed. @@ -290,6 +290,20 @@ class DefaultStoresManager: StoresManager { /// @discardableResult func deauthenticate() -> StoresManager { + let pushNotesManager = ServiceLocator.pushNotesManager + pushNotesManager.resetBadgeCountForAllStores(onCompletion: {}) + + // Keep a strong reference to the current state to prevent it from being deallocated + // until the unregister completes + let currentState = state + + // Unregister from remote notifications asynchronously + pushNotesManager.unregisterForRemoteNotifications { + DDLogInfo("📱 Push notification unregistration completed after logout") + // Keep `currentState` until the end of the closure + _ = currentState + } + applicationPasswordGenerationFailureObserver = nil invalidWPCOMTokenNotificationObserver = nil stopObservingNetworkNotifications() diff --git a/WooCommerce/WooCommerceTests/Mocks/MockPushNotificationsManager.swift b/WooCommerce/WooCommerceTests/Mocks/MockPushNotificationsManager.swift index a74d61371e9..94d248aae44 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockPushNotificationsManager.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockPushNotificationsManager.swift @@ -78,7 +78,7 @@ final class MockPushNotificationsManager: PushNotesManager { } - func unregisterForRemoteNotifications() { + func unregisterForRemoteNotifications(onCompletion: @escaping () -> Void) { } diff --git a/WooCommerce/WooCommerceTests/Notifications/PushNotificationsManagerTests.swift b/WooCommerce/WooCommerceTests/Notifications/PushNotificationsManagerTests.swift index 0b6ae019672..9b0438992e2 100644 --- a/WooCommerce/WooCommerceTests/Notifications/PushNotificationsManagerTests.swift +++ b/WooCommerce/WooCommerceTests/Notifications/PushNotificationsManagerTests.swift @@ -134,7 +134,7 @@ final class PushNotificationsManagerTests: XCTestCase { /// func testUnregisterForRemoteNotificationsDoesNothingWhenThereIsNoDeviceIdStored() { XCTAssert(storesManager.receivedActions.isEmpty) - manager.unregisterForRemoteNotifications() + manager.unregisterForRemoteNotifications {} XCTAssert(storesManager.receivedActions.isEmpty) } @@ -143,7 +143,7 @@ final class PushNotificationsManagerTests: XCTestCase { /// func testUnregisterForRemoteNotificationsEffectivelyDispatchesUnregisterDeviceAction() { defaults.set(Sample.deviceID, forKey: .deviceID) - manager.unregisterForRemoteNotifications() + manager.unregisterForRemoteNotifications {} guard case let .unregisterDevice(deviceID, _) = storesManager.receivedActions.first as! NotificationAction else { XCTFail() @@ -161,7 +161,7 @@ final class PushNotificationsManagerTests: XCTestCase { defaults.set(Sample.deviceID, forKey: .deviceID) defaults.set(Sample.deviceToken, forKey: .deviceToken) - manager.unregisterForRemoteNotifications() + manager.unregisterForRemoteNotifications {} guard case let .unregisterDevice(_, onCompletion) = storesManager.receivedActions.first as! NotificationAction else { XCTFail()