-
Notifications
You must be signed in to change notification settings - Fork 138
Issue/woomob 2042 add fallback to wpcom pn system #15238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Issue/woomob 2042 add fallback to wpcom pn system #15238
Conversation
When the Woo Core push token registration fails, the app will now fall back to the legacy WPCom registration system. This ensures that users can still receive push notifications even if the new registration method encounters an error.
This commit refactors the push notification registration status logic to account for both WPCom and Woo Core systems simultaneously. The `PushNotificationRegistrationStatus` class is updated to return a more detailed status: `WOO_REGISTERED`, `WPCOM_REGISTERED`, `REGISTERED_IN_BOTH`, or `UNREGISTERED`. The `RegisterDevice` use case is enhanced to use this new detailed status. It now triggers registration with the Woo Core system if the device is only registered with WPCom and the feature flag is enabled, ensuring a smoother transition between notification systems. Additional logging has also been added to improve traceability.
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #15238 +/- ##
============================================
+ Coverage 38.69% 38.70% +0.01%
- Complexity 10550 10563 +13
============================================
Files 2194 2194
Lines 124969 124984 +15
Branches 17299 17305 +6
============================================
+ Hits 48353 48379 +26
+ Misses 71720 71708 -12
- Partials 4896 4897 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
...main/kotlin/com/woocommerce/android/notifications/push/PushNotificationRegistrationStatus.kt
Show resolved
Hide resolved
The `PushNotificationRegistrationStatus` class no longer directly depends on `SelectedSite` to get the current site ID. Instead, the `siteId` is now passed as an argument to its `invoke` method. This change simplifies the `PushNotificationRegistrationStatus` class and makes its dependencies more explicit. Classes that use it, such as `RegisterDevice` and `DashboardViewModel`, are updated to provide the `siteId` when checking the push notification registration status.
When Woo Core push notification registration fails, the system attempts to fall back to the WPCom registration system. This change prevents the fallback from occurring if the device is already registered with the WPCom system.
| if (!isWpComPushRegistered()) { | ||
| registerPushTokenInWpComSystem(token) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something I forgot in my prior PR. If user is already WP.com registered avoid registering again.
|
|
||
| trackLoginEvent(currentStep = UnifiedLoginTracker.Step.SUCCESS) | ||
| appPrefsWrapper.removeLoginSiteAddress() | ||
| registerDevice(RegisterDevice.Mode.FORCEFULLY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes several issues:
- Not registering automatically in Woo Core system when first time loggin in, because the site was not selected yet.
- Automatically trigger PN token registration when switching sites.
Closes WOOMOB-2042
Description
This PR adds a fallback mechanism to the WP.com push notification registration system when the Woo Core push notification registration fails. It also refactors the push notification registration status logic to track both registration systems simultaneously.
Changes
Push Notification Registration Status Refactoring:
PushNotificationRegistrationStatusto return a more granular status instead of a simple binary state:WOO_REGISTERED- Only registered in Woo Core systemWPCOM_REGISTERED- Only registered in WP.com systemREGISTERED_IN_BOTH- Registered in both systemsUNREGISTERED- Not registered in either systemFeatureFlag.WOO_PUSH_NOTIFICATIONS_SYSTEMfor status checks, now evaluating both systems independentlyFallback Mechanism:
Test Steps
Case 1: Register into WP.com as usual
WOO_PUSH_NOTIFICATIONS_SYSTEM -> falseCase 2:
WOO_PUSH_NOTIFICATIONS_SYSTEMfeature flag in the codeCase 3:
Missing tasks to address
Mitigate attempting to register in Woo PN system every time the app is resumed from background. Solution TBD
I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.