Skip to content
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

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

Merged
merged 24 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
70fb2f5
Add separate url schemes for migration
staskus Nov 17, 2022
bbca2c1
Disable notifications using URLSchemes
staskus Nov 17, 2022
ca65a1b
Do not re-register for push notifications on app launch
staskus Nov 17, 2022
9817ec4
Rename service to JetpackNotificationMigrationService
staskus Nov 17, 2022
8acdeef
Moving NotificationMigrationService away from FeatureFlags
staskus Nov 17, 2022
3fa08f4
Do not show notification disabling label if disabling is not supported
staskus Nov 17, 2022
a26dcd7
Reusing the same instance of the JetpackNotificationMigrationService
staskus Nov 17, 2022
ed4a838
Improving notification setting switch description
staskus Nov 17, 2022
e52e6c9
Update RELEASE-NOTES.txt
staskus Nov 17, 2022
f94c558
Update SiteStatsPinnedItemStoreTests
staskus Nov 17, 2022
fe76ff5
rename shouldDisableWordPressNotifications to shouldDisableNotifications
staskus Nov 18, 2022
62b27a0
Change test names to describe expected results
staskus Nov 18, 2022
016daaa
Use isWordPress local variable to test register/unregister notifications
staskus Nov 18, 2022
23f385f
Update RELEASE-NOTES.txt
staskus Nov 18, 2022
dce06dc
Mocking isRegisteredForRemoteNotifications in test
staskus Nov 18, 2022
271365a
Rename method and feature flag to avoid negations
staskus Nov 18, 2022
404692a
Added migration url scheme to alpha and internal builds
staskus Nov 18, 2022
3de628e
Add additional blogging reminders
staskus Nov 18, 2022
1dca560
Use URLComponents building Jetpack and WordPress URLs
staskus Nov 18, 2022
0c57e27
Remove unneeded AppGroups code form JetpackNotificationMigrationService
staskus Nov 18, 2022
076d732
Update SiteStatsPinnedItemStoreTests.swift
staskus Nov 21, 2022
e32e1ba
Merge branch 'trunk' into task/disable-notifications-for-wp-using-url…
staskus Nov 23, 2022
e0e47e9
Added support for remote feature flag
staskus Nov 24, 2022
b5fe4d2
Merge branch 'trunk' into task/disable-notifications-for-wp-using-url…
staskus Nov 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
21.3
-----
* [*] [internal] Blogging Reminders notifications and scheduling can be turned off for WordPress as part of Jetpack focus behind a feature flag. [#19611]
* [*] [internal] Weekly Roundup notifications and scheduling can be turned off for WordPress as part of Jetpack focus behind a feature flag. [#19590]
* [*] [internal] When a user migrates to the Jetpack app and allows notifications, WordPress app notifications are disabled. This is released disabled and is behind a feature flag. [#19616, #19611, #19590]
* [*] Fixed a minor UI issue where the segmented control under My SIte was being clipped when "Home" is selected. [#19595]

21.2
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Classes/Models/Blog+Capabilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,6 @@ extension Blog {
}

public func areBloggingRemindersAllowed() -> Bool {
return Feature.enabled(.bloggingReminders) && isUserCapableOf(.EditPosts)
return Feature.enabled(.bloggingReminders) && isUserCapableOf(.EditPosts) && JetpackNotificationMigrationService.shared.shouldPresentNotifications()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ open class NotificationSettings {
fileprivate var blogPreferenceKeys: [String] {
var keys = [Keys.commentAdded, Keys.commentLiked, Keys.postLiked, Keys.follower, Keys.achievement, Keys.mention]

if Feature.enabled(.weeklyRoundup) {
if Feature.enabled(.weeklyRoundup) && JetpackNotificationMigrationService.shared.shouldPresentNotifications() {
keys.append(Keys.weeklyRoundup)
}

Expand Down
149 changes: 149 additions & 0 deletions WordPress/Classes/Services/JetpackNotificationMigrationService.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import UIKit

protocol JetpackNotificationMigrationServiceProtocol {
func shouldPresentNotifications() -> Bool
}

/// The service is created to support disabling WordPress notifications when Jetpack app enables notifications
/// The service uses URLScheme to determine from Jetpack app if WordPress app is installed, open it, disable notifications and come back to Jetpack app
/// This is a temporary solution to avoid duplicate notifications during the migration process from WordPress to Jetpack app
/// This service and its usage can be deleted once the migration is done
final class JetpackNotificationMigrationService: JetpackNotificationMigrationServiceProtocol {
private let notificationSettingsLoader: NotificationSettingsLoader
private let remoteNotificationRegister: RemoteNotificationRegister
private var notificationsEnabled: Bool = false
private let isWordPress: Bool

static let shared = JetpackNotificationMigrationService()

static let wordPressScheme = "wordpressnotificationmigration"
static let jetpackScheme = "jetpacknotificationmigration"

var wordPressNotificationsEnabled: Bool {
get {
return remoteNotificationRegister.isRegisteredForRemoteNotifications
}

set {
if newValue, isWordPress {
remoteNotificationRegister.registerForRemoteNotifications()
} else if isWordPress {
remoteNotificationRegister.unregisterForRemoteNotifications()
}

if isWordPress && !newValue {
cancelAllPendingWordPressLocalNotifications()
}
}
}

/// Migration is supported if WordPress is compatible with the notification migration URLScheme
var isMigrationSupported: Bool {
guard let url = URL(string: "\(JetpackNotificationMigrationService.wordPressScheme)://") else {
return false
}

return UIApplication.shared.canOpenURL(url)
}

init(notificationSettingsLoader: NotificationSettingsLoader = UNUserNotificationCenter.current(),
remoteNotificationRegister: RemoteNotificationRegister = UIApplication.shared,
isWordPress: Bool = AppConfiguration.isWordPress) {
self.notificationSettingsLoader = notificationSettingsLoader
self.remoteNotificationRegister = remoteNotificationRegister
self.isWordPress = isWordPress

notificationSettingsLoader.getNotificationAuthorizationStatus { [weak self] status in
self?.notificationsEnabled = status == .authorized
}
}

func shouldShowNotificationControl() -> Bool {
return Feature.enabled(.jetpackMigrationPreventDuplicateNotifications) && isWordPress && notificationsEnabled
}


func shouldPresentNotifications() -> Bool {
let disableNotifications = Feature.enabled(.jetpackMigrationPreventDuplicateNotifications)
&& isWordPress
&& !wordPressNotificationsEnabled

if disableNotifications {
cancelAllPendingWordPressLocalNotifications()
}

return !disableNotifications
}

// MARK: - Only executed on Jetpack app

func disableWordPressNotificationsFromJetpack() {
guard Feature.enabled(.jetpackMigrationPreventDuplicateNotifications), !isWordPress else {
return
}

let wordPressUrl: URL? = {
var components = URLComponents()
components.scheme = JetpackNotificationMigrationService.wordPressScheme
return components.url
}()

/// Open WordPress app to disable notifications
if let url = wordPressUrl, UIApplication.shared.canOpenURL(url) {
UIApplication.shared.open(url)
}
}

// MARK: - Only executed on WordPress app

func handleNotificationMigrationOnWordPress() -> Bool {
guard isWordPress else {
return false
}

wordPressNotificationsEnabled = false

let jetpackUrl: URL? = {
var components = URLComponents()
components.scheme = JetpackNotificationMigrationService.jetpackScheme
return components.url
}()

/// Return to Jetpack app
if let url = jetpackUrl, UIApplication.shared.canOpenURL(url) {
UIApplication.shared.open(url)
}

return true
}

// MARK: - Local notifications

private func cancelAllPendingWordPressLocalNotifications(notificationCenter: UNUserNotificationCenter = UNUserNotificationCenter.current()) {
if isWordPress {
notificationCenter.removeAllPendingNotificationRequests()
}
}
}

// MARK: - Helpers

protocol NotificationSettingsLoader: AnyObject {
func getNotificationAuthorizationStatus(completionHandler: @escaping (UNAuthorizationStatus) -> Void)
}

extension UNUserNotificationCenter: NotificationSettingsLoader {
func getNotificationAuthorizationStatus(completionHandler: @escaping (UNAuthorizationStatus) -> Void) {
getNotificationSettings { settings in
completionHandler(settings.authorizationStatus)
}
}
}

protocol RemoteNotificationRegister {
func registerForRemoteNotifications()
func unregisterForRemoteNotifications()
var isRegisteredForRemoteNotifications: Bool { get }
}

extension UIApplication: RemoteNotificationRegister {}
87 changes: 0 additions & 87 deletions WordPress/Classes/Services/NotificationFilteringService.swift

This file was deleted.

1 change: 0 additions & 1 deletion WordPress/Classes/System/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ extern NSString *const WPNotificationServiceExtensionKeychainServiceName;
extern NSString *const WPNotificationServiceExtensionKeychainTokenKey;
extern NSString *const WPNotificationServiceExtensionKeychainUsernameKey;
extern NSString *const WPNotificationServiceExtensionKeychainUserIDKey;
extern NSString *const WPNotificationsEnabledKey;

/// Share Extension Constants
///
Expand Down
1 change: 0 additions & 1 deletion WordPress/Classes/System/Constants.m
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
NSString *const WPNotificationServiceExtensionKeychainTokenKey = @"OAuth2Token";
NSString *const WPNotificationServiceExtensionKeychainUsernameKey = @"Username";
NSString *const WPNotificationServiceExtensionKeychainUserIDKey = @"UserID";
NSString *const WPNotificationsEnabledKey = @"WordPressNotificationsEnabled";

/// Share Extension Constants
///
Expand Down
4 changes: 4 additions & 0 deletions WordPress/Classes/System/WordPressAppDelegate+openURL.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import AutomatticTracks
return true
}

if url.scheme == JetpackNotificationMigrationService.wordPressScheme {
return JetpackNotificationMigrationService.shared.handleNotificationMigrationOnWordPress()
}

guard url.scheme == WPComScheme else {
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class BackgroundTasksCoordinator {
}

scheduler.register(forTaskWithIdentifier: type(of: task).identifier, using: nil) { osTask in
guard Feature.enabled(.weeklyRoundup) else {
guard Feature.enabled(.weeklyRoundup) && JetpackNotificationMigrationService.shared.shouldPresentNotifications() else {
osTask.setTaskCompleted(success: false)
eventHandler.handle(.taskCompleted(identifier: type(of: task).identifier, cancelled: true))
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ class WeeklyRoundupBackgroundTask: BackgroundTask {
private let eventTracker: NotificationEventTracker
let runDateComponents: DateComponents
let notificationScheduler: WeeklyRoundupNotificationScheduler
private let notificationFilteringService = NotificationFilteringService()
private let jetpackNotificationMigrationService = JetpackNotificationMigrationService()

init(
eventTracker: NotificationEventTracker = NotificationEventTracker(),
Expand All @@ -262,7 +262,7 @@ class WeeklyRoundupBackgroundTask: BackgroundTask {

self.eventTracker = eventTracker
notificationScheduler = WeeklyRoundupNotificationScheduler(staticNotificationDateComponents: staticNotificationDateComponents,
notificationFilteringService: notificationFilteringService)
jetpackNotificationMigrationService: jetpackNotificationMigrationService)
self.store = store

self.runDateComponents = runDateComponents ?? {
Expand Down Expand Up @@ -361,7 +361,7 @@ class WeeklyRoundupBackgroundTask: BackgroundTask {

// This will no longer run for WordPress as part of JetPack migration.
// This can be removed once JetPack migration is complete.
if notificationFilteringService.shouldFilterWordPressNotifications() {
guard jetpackNotificationMigrationService.shouldPresentNotifications() else {
notificationScheduler.cancellAll()
notificationScheduler.cancelStaticNotification()
return
Expand Down Expand Up @@ -495,11 +495,11 @@ class WeeklyRoundupNotificationScheduler {

init(
staticNotificationDateComponents: DateComponents? = nil,
notificationFilteringService: NotificationFilteringService,
jetpackNotificationMigrationService: JetpackNotificationMigrationService,
userNotificationCenter: UNUserNotificationCenter = UNUserNotificationCenter.current()) {

self.userNotificationCenter = userNotificationCenter
self.notificationFilteringService = notificationFilteringService
self.jetpackNotificationMigrationService = jetpackNotificationMigrationService

self.staticNotificationDateComponents = staticNotificationDateComponents ?? {
var dateComponents = DateComponents()
Expand All @@ -518,7 +518,7 @@ class WeeklyRoundupNotificationScheduler {

let staticNotificationDateComponents: DateComponents
let userNotificationCenter: UNUserNotificationCenter
let notificationFilteringService: NotificationFilteringService
let jetpackNotificationMigrationService: JetpackNotificationMigrationService

enum NotificationSchedulingError: Error {
case staticNotificationSchedulingError(error: Error)
Expand Down Expand Up @@ -624,7 +624,7 @@ class WeeklyRoundupNotificationScheduler {
dateComponents: DateComponents,
completion: @escaping (Result<Void, Error>) -> Void) {

if notificationFilteringService.shouldFilterWordPressNotifications() {
guard jetpackNotificationMigrationService.shouldPresentNotifications() else {
return
}

Expand Down
Loading