Skip to content

fix(push): re-register the device token on opt-in (#675) - #689

Merged
dmarchuk merged 8 commits into
mainfrom
fix/push-optin-reregister-675
Aug 11, 2026
Merged

fix(push): re-register the device token on opt-in (#675)#689
dmarchuk merged 8 commits into
mainfrom
fix/push-optin-reregister-675

Conversation

@dmarchuk

@dmarchuk dmarchuk commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

The opt-in half of #675. After a logout unregisterPushNotificationToken() clears the stored device token, optIn() only flips consent back on. It never refetches the FCM token or re-registers the device, so a user who opts out and back in stays unsubscribed and Workflows can't target them until the next app launch re-runs registration (or onNewToken happens to fire).

Change

Adds an onOptIn() lifecycle hook to PostHogIntegration (default no-op). PostHog.optIn() now notifies every integration, mirroring how onRemoteConfig is dispatched. PostHogPushSubscriptionIntegration.onOptIn() refetches the FCM token and re-registers the device, reusing the same executor and token fetcher as install.

The push integration is only registered when capturePushNotificationSubscriptions is enabled, so the refetch is gated to auto-capture apps. Manual push mode is unchanged: the host owns the token lifecycle.

How tested

  • Unit test: onOptIn refetches the token and re-registers the device (a second registration after install), in PostHogPushSubscriptionIntegrationTest.
  • Emulator E2E against a local stack with a real FCM token (Firebase on the sample's classpath, adb reverse to the backend). The app auto-registered its FCM token on launch, opt out cleared it, then opt in refetched the FCM token and re-registered the device (POST /api/push_subscriptions, 200). Rebuilt with the opt-in dispatch disabled and the same opt-in produced no re-register.
  • The iOS counterpart (fix(push): re-register the device token on opt-in (#746) posthog-ios#760) is confirmed the same way on a real device.

Not in this PR

The strand-the-unregister-DELETE half of #675 is fixed separately in #688. This PR is independent and targets main.

Note

Draft. Directed by @dmarchuk; implemented and verified locally by Claude while @ioannisj is out.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

posthog-android Compliance Report

Date: 2026-08-11 10:58:50 UTC
Duration: 118404ms

✅ All Tests Passed!

46/46 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 401ms
Format Validation.Event Has Uuid 30ms
Format Validation.Event Has Lib Properties 29ms
Format Validation.Distinct Id Is String 23ms
Format Validation.Token Is Present 24ms
Format Validation.Custom Properties Preserved 23ms
Format Validation.Event Has Timestamp 36ms
Retry Behavior.Retries On 503 7029ms
Retry Behavior.Does Not Retry On 400 4022ms
Retry Behavior.Does Not Retry On 401 4025ms
Retry Behavior.Respects Retry After Header 7025ms
Retry Behavior.Implements Backoff 17033ms
Retry Behavior.Retries On 500 7019ms
Retry Behavior.Retries On 502 7017ms
Retry Behavior.Retries On 504 7020ms
Retry Behavior.Max Retries Respected 17021ms
Deduplication.Generates Unique Uuids 38ms
Deduplication.Preserves Uuid On Retry 7018ms
Deduplication.Preserves Uuid And Timestamp On Retry 12031ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7019ms
Deduplication.No Duplicate Events In Batch 45ms
Deduplication.Different Events Have Different Uuids 22ms
Compression.Sends Gzip When Enabled 18ms
Batch Format.Uses Proper Batch Structure 21ms
Batch Format.Flush With No Events Sends Nothing 12ms
Batch Format.Multiple Events Batched Together 32ms
Error Handling.Does Not Retry On 403 4021ms
Error Handling.Does Not Retry On 413 4020ms
Error Handling.Retries On 408 5026ms

Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 34ms
Request Payload.Flags Request Uses V2 Query Param 26ms
Request Payload.Flags Request Hits Flags Path Not Decide 20ms
Request Payload.Flags Request Omits Authorization Header 27ms
Request Payload.Token In Flags Body Matches Init 21ms
Request Payload.Groups Round Trip 22ms
Request Payload.Groups Default To Empty Object 23ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 23ms
Request Payload.Disable Geoip Omitted Defaults To False 27ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 21ms
Request Lifecycle.No Flags Request On Init Alone 11ms
Request Lifecycle.No Flags Request On Normal Capture 20ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 36ms
Request Lifecycle.Mock Response Value Is Returned To Caller 23ms
Retry Behavior.Retries Flags On 502 323ms
Retry Behavior.Retries Flags On 504 321ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 21ms

@dmarchuk
dmarchuk marked this pull request as ready for review August 10, 2026 16:27
@dmarchuk
dmarchuk requested a review from a team as a code owner August 10, 2026 16:27
@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
posthog/src/main/java/com/posthog/PostHogIntegration.kt:39
**Breaks integration interface compatibility**

When a Java consumer upgrades with a custom `PostHogIntegration`, the new method is abstract in the generated JVM interface, causing source compilation failures; previously compiled implementations encounter `AbstractMethodError` when `optIn()` dispatches the method and therefore miss the lifecycle notification.

### Issue 2
posthog/src/main/java/com/posthog/PostHog.kt:1106
**Logger failure escapes dispatch**

When a custom integration and the configured logger both throw, this direct logging call lets the secondary exception escape `optIn()` and prevents later integrations from receiving the notification; use the existing non-throwing `safeLog` helper here.

```suggestion
                safeLog("Failed to notify integration of opt-in: $e.")
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "chore: record onOptIn in posthog-android..." | Re-trigger Greptile

Comment thread posthog/src/main/java/com/posthog/PostHogIntegration.kt Outdated
Comment thread posthog/src/main/java/com/posthog/PostHog.kt Outdated
Comment thread posthog/src/main/java/com/posthog/PostHogIntegration.kt Outdated
@marandaneto
marandaneto requested a review from a team August 10, 2026 19:04
@posthog

posthog Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

🦔 ReviewHog reviewed this pull request

Found 0 must fix, 2 should fix, 2 consider.

Published 4 findings (view the review).

@posthog

posthog Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ReviewHog Report

Changes

Issues: 4 issues

Files (5)
  • .changeset/push-optin-reregister.md
  • posthog-android/src/main/java/com/posthog/android/internal/PostHogPushSubscriptionIntegration.kt
  • posthog/api/posthog.api
  • posthog/src/main/java/com/posthog/PostHog.kt
  • posthog/src/main/java/com/posthog/PostHogOptInReceiver.kt

Comment thread posthog/src/main/java/com/posthog/PostHog.kt
Comment thread .changeset/push-optin-reregister.md
Comment thread .changeset/push-optin-reregister.md
Comment thread posthog/src/main/java/com/posthog/PostHog.kt
@dmarchuk
dmarchuk requested a review from marandaneto August 11, 2026 12:57
@dmarchuk
dmarchuk merged commit 71b632d into main Aug 11, 2026
16 checks passed
@dmarchuk
dmarchuk deleted the fix/push-optin-reregister-675 branch August 11, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants