Jetpack Focus: Disable WordPress app notifications from Jetpack app by using URL schemes

[staskus]

Part of: #19619

Description

This is a continuation of #19531 PR to allow disabling WordPress app notifications when notifications are enabled on the Jetpack app.

The original solution was built based on using Notification Filtering Entitlement but decided against it due to waiting times to approve this entitlement.

The alternative solution is to briefly jump to WordPress app, disable notifications, and comeback to the Jetpack app.

Solution

1. Add new URLSchemes for Jetpack and WordPress apps. New and separate URL schemes for migration ensures that we're not opening WordPress app unless it supports notification disabling
2. Rename NotificationFilteringService to JetpackNotificationMigrationService to represent the purpose better, also updated the description
3. Added disableWordPressNotificationsFromJetpack method that is called after Jetpack notifications are enabled and jumps from Jetpack to WordPress app
4. Added notification handling in WordPressAppDelegate
5. Added handleNotificationMigrationOnWordPress to disable notifications on WordPress and come back to Jetpack

Additionally:

1. Removed local notification disabling from FeatureFlag and added separate service checks
2. Updated JP to WP Migration Strings

Testing instructions

Case 1: WordPress + Receive Notifications

1. Install and log in to the WordPress app
2. Enable notifications
3. Confirm that push notifications are received

Case 2: WordPress + Jetpack

1. Install and log in to the WordPress app, enable notifications
2. Install and log in to the Jetpack app
3. In both the Jetpack and WordPress apps, enable the feature flag†
4. Enable notifications in the Jetpack app
5. Jetpack app should briefly jump to WordPress app and come back again to Jetpack app
6. Receive push notifications
7. Jetpack notifications should appear, WordPress notifications should not appear

Case 3: WordPress + Reenable Notifications

1. Continue Case 2
2. Go to WordPress
3. Notifications
4. Notifications Settings
5. Enable "Allow Notifications" Switch
6. Receive push notifications
7. Both Jetpack and WordPress notifications should appear

Case 4: WordPress + Jetpack migration flow

1. Install and log in to the WordPress app, enable notifications
2. In the WordPress app, enable the feature flag†
3. In JetpackWindowManager, set shouldShowMigrationUI to true (based on JP to WP Migration - Complete notifications permissions and success screen UI #19572)
4. Install and log in to the Jetpack app
5. In the Jetpack app, enable the feature flag†
6. Kill the app
7. Reopen Jetpack app
8. Follow migration steps, We’ll disable notifications for the WordPress app. should be shown
9. Allow notifications
10. Jetpack app should briefly jump to WordPress app and come back again to Jetpack app
11. Receive push notifications
12. Jetpack notifications should appear, WordPress notifications should not appear

Case 5: Incompatible WordPress + Jetpack migration flow

1. Install and log in to old WordPress app (From AppStore), enable notifications
2. Repeat Case 4 steps
3. No We’ll disable notifications for the WordPress app. should be visible, Jetpack app shouldn't open WordPress app, duplicate notification will appear

† To turn on the feature flag, tap the profile pic, then App Settings, then Debug, and enable "Jetpack Migration prevent duplicate WordPress app notifications when Jetpack is installed"

Regression Notes

1. Potential unintended areas of impact

Affecting local notifications with some logic changes

2. What I did to test those areas of impact (or what existing automated tests I relied on)

Manual testing

3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding unit tests for my changes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Images & Videos

Migration Flow

Migration.flow.MP4

Incompatible WordPress app + Migration Flow

Incompatible.WordPress.app.mov

[wpmobilebot]

You can test the changes in Jetpack from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19616-b5fe4d2 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[wpmobilebot]

You can test the changes in WordPress from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr19616-b5fe4d2 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[staskus]

@guarani

The remote notification solution is ready to review and test.

P.S. There's still tasks left with local notifications #19619

[guarani]

Hey @staskus, I didn't have time to test the code but I reviewed the code and have some thoughts which I shared above. I'm getting up to speed with this approach (plan B) and I might not have the full picture yet.
Either way I sent an invite for pair programming tomorrow if you're available. Otherwise will continue reviewing and testing tomorrow.

[staskus]

@guarani I fixed the issues we talked about, except for moving notification logic under FeatureFlags. Either way, the solution could be tested if you have capacity.

[guarani]

Debug build

• ✅ Case 1: WordPress + Receive Notifications
• ✅ Case 2: WordPress + Jetpack
• ✅ Case 3: WordPress + Reenable Notifications
• ⚠️ Case 4: WordPress + Jetpack migration flow
  • The flow itself seemed to work well except for the following issue
  • ⚠️To test, I was commenting on a site using a different account. I noticed that in this test, the comment itself (i.e. the body of the push notification) was no longer shown in the push notification (only the push notification title was visible). Do you see this?
• ✅ Case 5: Incompatible WordPress + Jetpack migration flow

@staskus, I haven't dug into the above issue, but perhaps the notification body is being removed when the feature flag is on.

[staskus]

Thanks for testing!

⚠️To test, I was commenting on a site using a different account. I noticed that in this test, the comment itself (i.e. the body of the push notification) was no longer shown in the push notification (only the push notification title was visible). Do you see this?

@staskus, I haven't dug into the above issue, but perhaps the notification body is being removed when the feature flag is on.

I assume the issue is as follows:

• WordPress app doesn't show notifications
• Jetpack app does show notifications but without notification body

Expected:

• WordPress app doesn't show notifications
• Jetpack app does show notifications with notification body

I haven't noticed (or specifically looked for) such an issue, so I need to check. As you can see in the attached video, Jetpack shows comments in the notification. For now, it's hard to see how the Feature flag could be related, I'll check.

[guarani]

I assume the issue is as follows:

Yes, that description is accurate.

I can reproduce it again today 😕. It happens in "Case 4: WordPress + Jetpack migration flow" when I go through the migration flow. This points it to being an issue not coming from this PR.

I noticed that the body of the push notification should be fetched from the server when the notification arrives (see code). After going through the migration flow, this code isn't reached because it aborts when calling readExtensionToken() (see code).

Since that code uses the keychain and app groups, I suspect something needs to be fixed there but I'm not sure yet what's wrong.

[guarani]

I confirmed that the above issue is present on trunk so it isn't related to this PR.
I think this change looks good so I'm going ahead and approving based on my previous tests here and code review.
Thanks @staskus!

[peril-wordpress-mobile]

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS