Skip to content

[Woo POS][Surveys] Add feature flag. Define "potential merchant" notification #16229

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 10, 2025
Merged

[Woo POS][Surveys] Add feature flag. Define "potential merchant" notification #16229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5a3a26
add pointOfSaleSurveys feature flag
iamgabrielma Oct 9, 2025
86f8991
update notifications manager to handle local-sent notifications as well
iamgabrielma Oct 9, 2025
4b81f10
add new potential merchant case to LocalNotifications
iamgabrielma Oct 9, 2025
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
2 changes: 2 additions & 0 deletions Modules/Sources/Experiments/DefaultFeatureFlagService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ public struct DefaultFeatureFlagService: FeatureFlagService {
return buildConfig == .localDeveloper || buildConfig == .alpha
case .ciabBookings:
return buildConfig == .localDeveloper || buildConfig == .alpha
case .pointOfSaleSurveys:
return buildConfig == .localDeveloper || buildConfig == .alpha
default:
return true
}
Expand Down
4 changes: 4 additions & 0 deletions Modules/Sources/Experiments/FeatureFlag.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,8 @@ public enum FeatureFlag: Int {
/// Enables a new Bookings tab for CIAB sites
///
case ciabBookings

/// Enables surveys for potential and current POS merchants
///
case pointOfSaleSurveys
}
18 changes: 18 additions & 0 deletions WooCommerce/Classes/Notifications/LocalNotification.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ struct LocalNotification {
case blazeNoCampaignReminder
case blazeAbandonedCampaignCreationReminder
case productImageBackgroundUpload
case pointOfSalePotentialMerchant
case unknown(siteID: Int64)

var identifier: String {
Expand All @@ -32,6 +33,8 @@ struct LocalNotification {
return "blaze_abandoned_campaign_reminder"
case .productImageBackgroundUpload:
return "product_image_background_upload"
case .pointOfSalePotentialMerchant:
return "point_of_sale_potential_merchant"
case let .unknown(siteID):
return "unknown_" + "\(siteID)"
}
Expand Down Expand Up @@ -95,6 +98,9 @@ extension LocalNotification {
case .productImageBackgroundUpload:
title = Localization.ProductImageBackgroundUpload.title
body = Localization.ProductImageBackgroundUpload.body
case .pointOfSalePotentialMerchant:
title = Localization.PointOfSalePotentialMerchant.title
body = Localization.PointOfSalePotentialMerchant.body
case .unknown:
title = ""
body = ""
Expand Down Expand Up @@ -147,5 +153,17 @@ extension LocalNotification {
comment: "Message on the local notification to inform the user about the background upload of product images."
)
}
enum PointOfSalePotentialMerchant {
static let title = NSLocalizedString(
"localNotification.PointOfSalePotentialMerchant.title",
value: "Thinking about in-person sales?",
comment: "Title of the local notification sent to potential Point of Sale merchants"
)
static let body = NSLocalizedString(
"localNotification.PointOfSalePotentialMerchant.body",
value: "Take a quick 2-minute survey to help us shape features you’ll love.",
comment: "Message body of the local notification sent to potential Point of Sale merchants"
)
}
}
}
19 changes: 12 additions & 7 deletions WooCommerce/Classes/Notifications/PushNotificationsManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,19 +252,24 @@ extension PushNotificationsManager {
unregisterForRemoteNotifications()
}

/// Handles a Notification while in Foreground Mode. Currently, only remote notifications are handled in the foreground.
/// Handles a notification while the app is in foreground
///
/// - Parameters:
/// - userInfo: The Notification's Payload
/// - completionHandler: A callback, to be executed on completion
///
/// - Returns: True when handled. False otherwise
/// - Parameter notification: The delivered `UNNotification`
/// - Returns: `UNNotificationPresentationOptions` indicating how (if at all) the system should present its own UI for this notification
///
@MainActor
func handleNotificationInTheForeground(_ notification: UNNotification) async -> UNNotificationPresentationOptions {
let content = notification.request.content
if ServiceLocator.featureFlagService.isFeatureFlagEnabled(.pointOfSaleSurveys) {
Copy link
Contributor

@staskus staskus Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if in this PR, but we could expand our LocalNotification definition that would also define how it needs to be handled, possibly through userInfo + helper extensions such as content.isRemoteNotification. Then, this handler method could be made tidier, and not contain feature flag or content.isRemoteNotification checks.

E.g

if content.showSystemNotification {
     
}

Copy link
Contributor Author

@iamgabrielma iamgabrielma Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review and suggestion!

Yes, this class will need some further updates when handling these, for example, one case I've already found is that if we call handleNotificationInTheForeground when the merchant is in POS mode it will not trigger anything, because tabBarController is not in the window hierarchy due POS being presented full-screen, so we need to reach to the topmost VC instead. Feature flag will also be called outside on some sort of eligibility checker before reaching this point 👍

// Check if this is a local notification
if !content.isRemoteNotification {
// Display local notifications with banner and sound when app is in foreground
return [.banner, .sound]
}
}

guard applicationState == .active, content.isRemoteNotification, inAppNotices == true else {
// Local notifications are currently not handled when the app is in the foreground.
// Remote notifications not handled when the app is not active, or in-app notices are disabled
return UNNotificationPresentationOptions(rawValue: 0)
}

Expand Down
Loading