Update how the app decides whether to accept a push notification by irfano · Pull Request #15222 · woocommerce/woocommerce-android

irfano · 2026-01-21T00:07:35Z

Closes WOOMOB-1957

Description

This PR adds deduplication logic to NotificationMessageHandler to prevent duplicate push notifications. When WOO_PUSH_NOTIFICATIONS_SYSTEM feature flag is enabled, we check if the incoming notification is from WPCOM (note_id > 0) and if the device is already registered with Woo Core (hasPushToken()). If both conditions are true, we skip the WPCOM notification since the same event will also arrive via Woo Core.

Test Steps

Set up a store with the new Woo Core endpoints by following these steps: p91TBi-dyW-p2#comment-14564
Install the app and log into your store.
Submit a product review from the web interface.
Regression: Confirm you receive the notification, because the feature flag is disabled.
Apply this patch to enable the feature flag: Enable_WOO_PUSH_NOTIFICATIONS_SYSTEM_feature_flag_for_debug_builds.patch
Rebuild the app.
Logout and log in again the app to trigger new push token registration. (you may need to kill and restart after login)
Submit another product review.
Verify that you do not receive a notification. (Note: You won't receive it yet because the development plugin is still a work in progress.)

Images/gif

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.

wpmobilebot · 2026-01-21T00:12:49Z

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`891dff4`
	Direct Download	`woocommerce-wear-prototype-build-pr15222-891dff4.apk`

wpmobilebot · 2026-01-21T00:28:17Z

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`891dff4`
	Direct Download	`woocommerce-prototype-build-pr15222-891dff4.apk`

codecov-commenter · 2026-01-21T00:37:56Z

Codecov Report

❌ Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.77%. Comparing base (1308abb) to head (891dff4).
⚠️ Report is 129 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...d/notifications/push/NotificationMessageHandler.kt	73.33%	0 Missing and 4 partials ⚠️
...merce/android/notifications/push/RegisterDevice.kt	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##              trunk   #15222   +/-   ##
=========================================
  Coverage     38.77%   38.77%           
- Complexity    10611    10613    +2     
=========================================
  Files          2201     2201           
  Lines        125148   125157    +9     
  Branches      17354    17358    +4     
=========================================
+ Hits          48530    48535    +5     
- Misses        71698    71700    +2     
- Partials       4920     4922    +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JorgeMucientes · 2026-01-22T19:19:44Z

Hey @irfano the code changes look good, however, I think we can do this in a cleaner way once we merge the changes from the PR I'll open tomorrow early morning (I'm wrapping up testing changes): #15238.
With the changes from that PR we can directly use PushNotificationRegistrationStatus class to verify if the user is registered in Woo system. That will avoid having to change the code you added here once we remove the feature flag. In fact relying on PushNotificationRegistrationStatus use case is feature flag agnostic,
Let me know what you think.

…system - Bypass user ID and `remoteNoteId` validation checks when `WOO_PUSH_NOTIFICATIONS_SYSTEM` feature flag is enabled. - Skip WPCOM notifications when the app is registered with Woo Core to avoid duplicates.

…ComPushNotificationStore

…tatus in NotificationMessageHandlerTest

irfano · 2026-01-22T20:51:03Z

I've rebased onto your branch. Let's merge yours first since this is smaller. I agree that using PushNotificationRegistrationStatus in this PR is cleaner.

… issue/WOOMOB-1957-update-how-decide-whether-accept-pn

JorgeMucientes · 2026-01-29T17:05:55Z

...rce/src/main/kotlin/com/woocommerce/android/notifications/push/NotificationMessageHandler.kt

        val notification = notificationModel.toAppModel(resourceProvider)
-        if (notification.remoteNoteId == 0L) {
+        val isRegisteredForWooPush = runBlocking {
+            registrationStatus(notification.remoteSiteId) == PushNotificationRegistrationStatus.Status.WOO_REGISTERED


I think this check should also include the case were user is registered in both systems, right?

Suggested change

registrationStatus(notification.remoteSiteId) == PushNotificationRegistrationStatus.Status.WOO_REGISTERED

val isRegisteredForWooPush = runBlocking {

val registrationStatus = registrationStatus(notification.remoteSiteId)

registrationStatus == PushNotificationRegistrationStatus.Status.REGISTERED_IN_BOTH

|| registrationStatus == PushNotificationRegistrationStatus.Status.WOO_REGISTERED

}

The most likely case were we get duplicates is when the user is registered in both systems (even though PN should be turned off in WP.com system)

🤔 You're right. I'm confused about WOO_REGISTERED would be true even though it's registered in both. I renamed (82eb608) the enums and added ONLY suffix to avoid any confusion in the future. Does that sound good?
Also fixed the logic: 0f667f4

Please don’t follow commits for the review, I’ve synced with the base branch many times since this PR has been open for a while.

JorgeMucientes · 2026-01-30T07:10:24Z

...rce/src/main/kotlin/com/woocommerce/android/notifications/push/NotificationMessageHandler.kt

        val notification = notificationModel.toAppModel(resourceProvider)
-        if (notification.remoteNoteId == 0L) {
+        val isRegisteredForWooPush = runBlocking {
+            registrationStatus(notification.remoteSiteId) == PushNotificationRegistrationStatus.Status.REGISTERED_BOTH


I think we should still include PushNotificationRegistrationStatus.Status.REGISTERED_WOO_ONLY in this check too.
While it might be a corner case I can see how we can end in a situation were locally, the app thinks the user is only registered in WOO system only, however their site might still be registered in both PN systems in the backend side. For example, if user is registered in WP.com system and fails to unregister device on log out and then logs in again and registers successfully in Woo system but fails to turn off notifications in WP.com.
That's just one case but I think there could be others. To stay on the safe side I'd check either values REGISTERED_BOTH || REGISTERED_WOO_ONLY

I'm not sure what happened exactly, but when I added this comment to the PR this morning I would've sworn that the PushNotificationRegistrationStatus.Status.REGISTERED_WOO_ONLY was not present in the code. Look like it is now. So all good 👍🏼

I actually included PushNotificationRegistrationStatus.Status.REGISTERED_WOO_ONLY and PushNotificationRegistrationStatus.Status.REGISTERED_BOTH after your feedback, but I think I messed it up while resolving conflicts.
I re-added it today in commit 891dff474a77593f0da8181b34a65cd48905144a. That's why I commented "don't follow my commits" 😅
Anyway, thanks for the review.

JorgeMucientes

LGTM! Nice work @irfano

JorgeMucientes · 2026-01-30T15:03:29Z

...rce/src/main/kotlin/com/woocommerce/android/notifications/push/NotificationMessageHandler.kt

        if (!accountStore.hasAccessToken()) {
            wooLog.e(NOTIFICATIONS, "User is not logged in!")
            return
        }


Out of scope for this PR but we probably need to update this as this condition will be false for all users loggin in with site credentials.
Can you create a nwe Linear task and tackle that @irfano 🙏🏼

👍🏻 Opened: WOOMOB-2113

irfano added this to the 24.0 milestone Jan 21, 2026

irfano requested a review from JorgeMucientes January 21, 2026 00:07

irfano added feature: notifications Related to notifications or notifs. Task (new) labels Jan 21, 2026

JorgeMucientes self-assigned this Jan 21, 2026

irfano modified the milestones: 24.0, 24.1 Jan 22, 2026

irfano added 5 commits January 22, 2026 23:09

Update NotificationMessageHandler to support new push notification …

43d225b

…system - Bypass user ID and `remoteNoteId` validation checks when `WOO_PUSH_NOTIFICATIONS_SYSTEM` feature flag is enabled. - Skip WPCOM notifications when the app is registered with Woo Core to avoid duplicates.

Use DataStore to check Woo Core push token registration status

9acff25

Update NotificationMessageHandler to use NOTIFICATIONS log tag and Wp…

7cef698

…ComPushNotificationStore

Replace PushNotificationRepository with PushNotificationRegistrationS…

ae84593

…tatus in NotificationMessageHandlerTest

Update NotificationMessageHandlerTest

8a42fa9

irfano force-pushed the issue/WOOMOB-1957-update-how-decide-whether-accept-pn branch from 823bd27 to 8a42fa9 Compare January 22, 2026 20:44

irfano changed the base branch from trunk to issue/woomob-2042-add-fallback-to-wpcom-pn-system January 22, 2026 20:44

irfano marked this pull request as draft January 22, 2026 20:45

irfano added 2 commits January 27, 2026 17:48

Merge branch 'issue/woomob-2042-add-fallback-to-wpcom-pn-system' into…

d02f410

… issue/WOOMOB-1957-update-how-decide-whether-accept-pn

Update registration status check to use site ID

ad70295

irfano marked this pull request as ready for review January 27, 2026 15:45

JorgeMucientes requested changes Jan 29, 2026

View reviewed changes

Base automatically changed from issue/woomob-2042-add-fallback-to-wpcom-pn-system to trunk January 29, 2026 17:49

irfano added 3 commits January 29, 2026 20:58

Fix NotificationMessageHandlerTest

5589148

Rename PushNotificationRegistrationStatus enum values

82eb608

Update registration status check for push notifications

0f667f4

irfano requested a review from JorgeMucientes January 29, 2026 18:17

Fix NotificationMessageHandlerTest

891dff4

JorgeMucientes reviewed Jan 30, 2026

View reviewed changes

JorgeMucientes approved these changes Jan 30, 2026

View reviewed changes

irfano merged commit 6461df3 into trunk Jan 30, 2026
19 checks passed

irfano deleted the issue/WOOMOB-1957-update-how-decide-whether-accept-pn branch January 30, 2026 20:24

-            registrationStatus(notification.remoteSiteId) == PushNotificationRegistrationStatus.Status.WOO_REGISTERED
+        val isRegisteredForWooPush = runBlocking {
+            val registrationStatus = registrationStatus(notification.remoteSiteId)
+            registrationStatus == PushNotificationRegistrationStatus.Status.REGISTERED_IN_BOTH
+                || registrationStatus == PushNotificationRegistrationStatus.Status.WOO_REGISTERED
+        }

Conversation

irfano commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Steps

Images/gif

Uh oh!

wpmobilebot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JorgeMucientes commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

irfano commented Jan 22, 2026

Uh oh!

JorgeMucientes Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irfano Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irfano Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

JorgeMucientes Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

JorgeMucientes Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

irfano Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

JorgeMucientes left a comment

Choose a reason for hiding this comment

Uh oh!

JorgeMucientes Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

irfano Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

irfano commented Jan 21, 2026 •

edited

Loading

wpmobilebot commented Jan 21, 2026 •

edited

Loading

wpmobilebot commented Jan 21, 2026 •

edited

Loading

codecov-commenter commented Jan 21, 2026 •

edited

Loading

JorgeMucientes commented Jan 22, 2026 •

edited

Loading

JorgeMucientes Jan 29, 2026 •

edited

Loading

irfano Jan 29, 2026 •

edited

Loading