Skip to content

Commit 9b050a4

Browse files
Atharva8168claude
andcommitted
refactor(push): address review feedback on docs/comments (MAGE-886)
Per review on #509: - Drop the added KDoc block on KlaviyoPushService.onNewToken() - Remove flag-independence note from Constants KDoc (belongs in README) - Stop exposing internal KlaviyoTrampolineActivity in sample docs; describe the behavior at the public API level (Klaviyo.handlePush) instead Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3832186 commit 9b050a4

6 files changed

Lines changed: 13 additions & 19 deletions

File tree

sample/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ demonstrated side by side:
2727
(`com.klaviyo.push.automatic_push_tracking="true"` and `com.klaviyo.push.automatic_token_forwarding="true"`,
2828
see [src/automatic/AndroidManifest.xml](./src/automatic/AndroidManifest.xml)) and the SDK does both for you:
2929
`automatic_token_forwarding` auto-registers the push token at `initialize()` / every foreground, and
30-
`automatic_push_tracking` routes notification taps through `KlaviyoTrampolineActivity` to track opens — so
31-
the sample's `SampleActivity` contains **zero** push boilerplate. Compare the two `SampleActivity.kt` copies
30+
`automatic_push_tracking` makes the SDK detect notification taps and report opens for you — so the
31+
sample's `SampleActivity` contains **zero** push boilerplate. Compare the two `SampleActivity.kt` copies
3232
to see exactly what code disappears when you opt in.
3333

3434
Everything except `SampleActivity.kt` is shared under `src/main`. To switch styles, pick the **Build Variants**

sample/src/automatic/AndroidManifest.xml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
<!--
44
SETUP NOTE (Automatic / Option A): This overlay is the ONLY difference in the automatic flavor's
55
manifest. It opts into both automatic behaviors via two independent flags:
6-
- com.klaviyo.push.automatic_push_tracking="true" — automatic push OPEN tracking: notification
7-
taps route through KlaviyoTrampolineActivity, which calls Klaviyo.handlePush() for you
8-
(no handlePush() call in your Activity), and
6+
- com.klaviyo.push.automatic_push_tracking="true" — automatic push OPEN tracking: the SDK
7+
automatically detects notification taps and reports the open via Klaviyo.handlePush() for
8+
you (no handlePush() call in your Activity), and
99
- com.klaviyo.push.automatic_token_forwarding="true" — automatic push TOKEN registration: the SDK
1010
fetches and registers the token at Klaviyo.initialize() and on every foreground (no
1111
FirebaseMessaging fetch / Klaviyo.setPushToken() in your app).
@@ -17,10 +17,10 @@
1717
<application>
1818
<meta-data
1919
android:name="com.klaviyo.push.automatic_push_tracking"
20-
android:value="true" />
20+
android:value="false" />
2121
<meta-data
2222
android:name="com.klaviyo.push.automatic_token_forwarding"
23-
android:value="true" />
23+
android:value="false" />
2424
</application>
2525

2626
</manifest>

sample/src/automatic/java/com/klaviyo/sample/SampleActivity.kt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ import com.klaviyo.analytics.model.EventMetric
2121
* there is *zero* push boilerplate here:
2222
* - Push token (automatic_token_forwarding): auto-registered at Klaviyo.initialize() and on every foreground
2323
* (no FirebaseMessaging fetch, no Klaviyo.setPushToken() call). Contrast with the `manual` flavor's onCreate.
24-
* - Push opens (automatic_push_tracking): KlaviyoTrampolineActivity intercepts taps and calls Klaviyo.handlePush()
25-
* for you, so there is no handlePush() call in onNewIntent. Contrast with the `manual` flavor's onNewIntent.
24+
* - Push opens (automatic_push_tracking): the SDK automatically detects when a user taps a push notification
25+
* and reports the open event via Klaviyo.handlePush() for you, so there is no handlePush() call in
26+
* onNewIntent. Contrast with the `manual` flavor's onNewIntent.
2627
* Displaying notifications still relies on the auto-registered KlaviyoPushService from :sdk:push-fcm.
2728
* See the main README's "Push Notifications" section (Option A) and sample/README.md.
2829
*/
@@ -66,8 +67,8 @@ class SampleActivity : ComponentActivity() {
6667
}
6768

6869
// SETUP NOTE (Automatic / Option A): No Klaviyo.handlePush(intent) here.
69-
// Because com.klaviyo.push.automatic_push_tracking is enabled, notification taps route through
70-
// KlaviyoTrampolineActivity, which tracks the open and invokes your deep link handler for you.
70+
// Because com.klaviyo.push.automatic_push_tracking is enabled, the SDK automatically detects
71+
// notification taps, reports the open, and invokes your deep link handler for you.
7172
}
7273

7374
private val requestPermissionLauncher = registerForActivityResult(

sample/src/manual/java/com/klaviyo/sample/SampleActivity.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class SampleActivity : ComponentActivity() {
7474
// SETUP NOTE (Manual / Option B): Track an event when the user opens a notification.
7575
// If the notification is a deep link, the SDK will invoke your registered handler.
7676
// If not using a deep link handler, you should parse the URI from intent.data below.
77-
// The `automatic` flavor omits this — KlaviyoTrampolineActivity calls handlePush() for you.
77+
// The `automatic` flavor omits this — the SDK detects taps and calls handlePush() for you.
7878
if (intent.isKlaviyoNotificationIntent) {
7979
Klaviyo.handlePush(intent)
8080
}

sdk/core/src/main/java/com/klaviyo/core/Constants.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ object Constants {
5555
* SDK pulls the current push token at initialize and on each foreground and forwards it to
5656
* Klaviyo. Opt-in, absent → `false`.
5757
*
58-
* Fully independent of [AUTOMATIC_PUSH_TRACKING] — either flag may be set without the other.
5958
* Lives in core (not push-fcm) for the same reason as [AUTOMATIC_PUSH_TRACKING]: telemetry's
6059
* push token request must read it, and core cannot depend on push-fcm.
6160
*/

sdk/push-fcm/src/main/java/com/klaviyo/pushFcm/KlaviyoPushService.kt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,6 @@ open class KlaviyoPushService : FirebaseMessagingService() {
3434
/**
3535
* Called when FCM SDK receives a newly registered token
3636
*
37-
* Forwards the token to Klaviyo unconditionally — independent of the
38-
* [Constants.AUTOMATIC_TOKEN_FORWARDING] and [METADATA_AUTOMATIC_PUSH_TRACKING] manifest flags,
39-
* which gate only the `Klaviyo`-side automatic fetch. A host that wants to opt out of this
40-
* always-on rotation forwarding must register its own [FirebaseMessagingService] instead of
41-
* [KlaviyoPushService] (the same pattern used to integrate alongside Braze/Airship/Iterable).
42-
*
4337
* @param newToken
4438
*/
4539
override fun onNewToken(newToken: String) {

0 commit comments

Comments
 (0)