feat(notifications): per-session push subscription snapshot on native - #2510
feat(notifications): per-session push subscription snapshot on native#2510innolope-dev wants to merge 3 commits into
Conversation
… on native Records subscription id, token presence, opt-in, permission, and external_id link state to Sentry ~10s after OneSignal init and on the first subscription change. Gives fleet-wide visibility into where native push breaks (FCM/APNs registration vs external_id linking vs delivery) without OneSignal dashboard access.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe native OneSignal adapter now captures subscription state for Sentry after initialization and subscription changes. The related test mock exposes additional identity and push subscription methods required by the snapshot logic. ChangesOneSignal subscription snapshots
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant OneSignalPlugin
participant NativeAdapter
participant Sentry
OneSignalPlugin->>NativeAdapter: Emit subscription change
NativeAdapter->>OneSignalPlugin: Read subscription and identity state
OneSignalPlugin-->>NativeAdapter: Return snapshot fields
NativeAdapter->>Sentry: Report subscription-change snapshot
NativeAdapter->>NativeAdapter: Schedule init snapshot after 10 seconds
NativeAdapter->>Sentry: Report init snapshot
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code-analysis diffPainscore total: 6270.06 → 6271.17 (+1.11) 🆕 New findings (3)
✅ Resolved (2)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/onesignal/native.adapter.test.ts (1)
20-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd assertions for the snapshot behavior, not only mock support.
Please cover delayed initialization capture, first-change deduplication,
linked: true, token presence without recording the raw token, and rejected OneSignal calls. The current fixednullvalues only prove that the expanded mock can execute.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/onesignal/native.adapter.test.ts` around lines 20 - 26, Expand the tests using the OneSignal mock around the native adapter’s snapshot behavior: assert delayed initialization capture, deduplicate the first change, verify the snapshot reports linked: true, confirm token presence without exposing the raw token, and cover rejected OneSignal calls. Replace the fixed null-only setup with scenario-specific mock values and assertions that validate these behaviors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/onesignal/native.adapter.ts`:
- Line 64: Update the Sentry event metadata in the native adapter to remove
externalId from the extra payload, while retaining subscriptionId and
onesignalId and the existing onesignal.linked status.
---
Nitpick comments:
In `@src/services/onesignal/native.adapter.test.ts`:
- Around line 20-26: Expand the tests using the OneSignal mock around the native
adapter’s snapshot behavior: assert delayed initialization capture, deduplicate
the first change, verify the snapshot reports linked: true, confirm token
presence without exposing the raw token, and cover rejected OneSignal calls.
Replace the fixed null-only setup with scenario-specific mock values and
assertions that validate these behaviors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4408cfac-4304-4155-bc7c-b0452ac6d65c
📒 Files selected for processing (2)
src/services/onesignal/native.adapter.test.tssrc/services/onesignal/native.adapter.ts
1 participant
What
Captures a once-per-session-per-trigger OneSignal subscription snapshot to Sentry on native builds: subscription id, push-token presence (boolean only — never the raw token), opt-in state, notification permission, and whether
login()linked anexternal_id.Fires ~10s after
OneSignal.initialize()(token registration and login are async) and again on the first push-subscription change (opt-in usually lands after the init snapshot).Why
Native push doesn't deliver while web push to the same user works, and we have no OneSignal dashboard access. The visibility we shipped in #2473 only surfaces thrown errors — a device that silently ends up with no FCM/APNs token, or unlinked from its
external_id, produces nothing. This snapshot distinguishes, fleet-wide in Sentry (onesignal:subscription-snapshot,environment:native):has_token:false→ FCM/APNs registration failing on devicelinked:false→external_idlogin not attaching (pushes hit 0 recipients)Testing
pnpm jest src/services/onesignal/native.adapter.test.tsgreen (mock extended with the new getters)pnpm typecheckcleanprettiercleanSummary by CodeRabbit