-
Notifications
You must be signed in to change notification settings - Fork 84
fix(push): capture $push_notification_opened on iOS cold start #556
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: main
Are you sure you want to change the base?
Changes from all commits
283050c
3da4395
f8e8246
688674e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'posthog_flutter': patch | ||
| --- | ||
|
|
||
| Fix `$push_notification_opened` not being captured on iOS when the app is cold-launched from a notification tap. Requires posthog-ios 3.72.0, and your app must set `UNUserNotificationCenter.current().delegate` β without one iOS reports the tap to nobody. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'posthog_flutter': patch | ||
| --- | ||
|
|
||
| Change `com.posthog.posthog.CAPTURE_PUSH_NOTIFICATION_OPENED` to also suppress the new iOS cold-start prewarm in apps that do not use `com.posthog.posthog.AUTO_INIT`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,7 @@ public class PosthogFlutterPlugin: NSObject, FlutterPlugin { | |
| let instance = PosthogFlutterPlugin() | ||
| instance.channel = methodChannel | ||
| PosthogFlutterPlugin.instance = instance | ||
| prewarmPushNotificationOpenCapture() | ||
| initPlugin() | ||
| registrar.addMethodCallDelegate(instance, channel: methodChannel) | ||
| } | ||
|
|
@@ -94,6 +95,21 @@ public class PosthogFlutterPlugin: NSObject, FlutterPlugin { | |
| private let dispatchQueue = DispatchQueue(label: "com.posthog.PosthogFlutterPlugin", | ||
| target: .global(qos: .utility)) | ||
|
|
||
| /// A cold launch from a notification tap delivers the response about 150ms in, before Dart can | ||
| /// reach `Posthog().setup()`. Registration runs inside `didFinishLaunchingWithOptions`, early | ||
| /// enough for the native SDK to hold that response until setup() installs the integration. | ||
| /// | ||
| /// The Info.plist key is the only opt-out reachable this early. `setup()` releases an unwanted | ||
| /// prewarm afterwards, except when PostHog is already set up with push-open capture disabled and | ||
| /// a later `FlutterEngine` registers β that setup() has already run, so use the plist key there. | ||
| private static func prewarmPushNotificationOpenCapture() { | ||
| let capturePushNotificationOpened = Bundle.main.object(forInfoDictionaryKey: "com.posthog.posthog.CAPTURE_PUSH_NOTIFICATION_OPENED") as? Bool ?? true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Small one: this is a copy of the plist read in |
||
| guard capturePushNotificationOpened else { return } | ||
| if #available(iOS 14.0, macOS 11.0, *) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The package still supports iOS 13, but this availability check skips the prewarm there. When a notification tap cold-launches an iOS 13 app, the response can still arrive before Dart setup without being buffered, so the advertised cold-start fix remains ineffective on a supported deployment target. Prompt To Fix With AIThis is a comment left during a code review.
Path: posthog_flutter/darwin/posthog_flutter/Sources/posthog_flutter/PosthogFlutterPlugin.swift
Line: 108
Comment:
**Prewarm skips supported iOS 13**
The package still supports iOS 13, but this availability check skips the prewarm there. When a notification tap cold-launches an iOS 13 app, the response can still arrive before Dart setup without being buffered, so the advertised cold-start fix remains ineffective on a supported deployment target.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
| PostHogSDK.prewarmPushNotificationOpenCapture() | ||
| } | ||
| } | ||
|
|
||
| public static func initPlugin() { | ||
| let autoInit = Bundle.main.object(forInfoDictionaryKey: "com.posthog.posthog.AUTO_INIT") as? Bool ?? true | ||
| if !autoInit { | ||
|
|
||
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.
The new registration-time prewarm and its plist opt-out have no automated regression coverage. Please add a native test seam that verifies registration invokes prewarm when the key is enabled or absent and suppresses it when false; otherwise, removing the early call or reversing the guard could silently reintroduce the cold-start failure.
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!