Skip to content

chore: [SDK-4788] add local notification compatibility demo - #188

Merged
fadi-george merged 7 commits into
mainfrom
fadi/sdk-4788
Jul 16, 2026
Merged

chore: [SDK-4788] add local notification compatibility demo#188
fadi-george merged 7 commits into
mainfrom
fadi/sdk-4788

Conversation

@fadi-george

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Adds a .NET MAUI demo for reproducing and testing OneSignal with Plugin.LocalNotification.

Details

Motivation

Provide a focused reproduction for the iOS notification delegate crash reported in issues #110 and #132, while also covering Android compatibility.

Scope

Adds the examples/plugin-local-notif app, iOS and Android platform setup, .env app ID configuration, run scripts, notification test controls, and push subscription diagnostics. This PR does not change SDK runtime behavior or public APIs.

Testing

Manual testing

  • Built and launched the iOS simulator app using run-ios.sh.
  • Verified local notification foreground display and notification permission flows.
  • Built the Android target and verified local and OneSignal notification flows.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible
  • I have reviewed this PR myself, ensuring it meets each checklist item

Made with Cursor

@fadi-george

Copy link
Copy Markdown
Contributor Author

@claude review once

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — self-contained example app addition with no SDK runtime or public API changes.

What was reviewed: the new examples/plugin-local-notif MAUI demo (App.cs, MainPage.cs, MauiProgram.cs, DotEnv.cs, platform setup for Android/iOS). Checked the .env loading/parsing logic in DotEnv.cs for correctness. Confirmed the app only wires up existing public OneSignal SDK APIs (Notifications, User.PushSubscription) with no changes to SDK source. Reviewed the flagged namespace usage and app ID collision concerns — both are cosmetic/demo-scoped, not functional blockers.

Extended reasoning...

Overview

This PR adds a new standalone example app, examples/plugin-local-notif, a .NET MAUI demo used to reproduce an iOS notification-delegate interaction between OneSignal and Plugin.LocalNotification (referencing issues #110/#132). It includes app entry points (App.cs, MauiProgram.cs), a UI page wiring OneSignal SDK calls (MainPage.cs), a small hand-rolled .env loader (DotEnv.cs), Android/iOS platform scaffolding, and run scripts. No files under the core OneSignalSDK.DotNet library are touched, and no public APIs are added or changed.

Security risks

None of note. The .env loader (DotEnv.cs) reads a local app-bundled file and does simple string parsing (no shell execution, no network calls, no injection surface). The only externally-configurable value is the OneSignal App ID, which is expected to be non-sensitive and is loaded from a local, gitignored-by-convention .env file rather than committed secrets. iOS entitlements/Info.plist and Android manifest only declare the minimal permissions needed for local/push notifications.

Level of scrutiny

This warrants light scrutiny — it's example/demo code, isolated to its own directory, with no impact on the SDK's runtime behavior, build output, or consumers. The blast radius is limited to this one sample app used for manual reproduction/testing.

Other factors

The bug-hunting system found no bugs. The three candidate issues raised (Plugin.LocalNotification namespace usage possibly not compiling, and ApplicationId/bundle ID reuse colliding with the existing demo app) were investigated and refuted as not real, functional issues — at most demo-app packaging nits that wouldn't block merge. The PR author (via Cursor) also self-reported manual testing on both iOS and Android.

@abdulraqeeb33 abdulraqeeb33 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review

Examples-only, no SDK runtime impact — fine as a scaffold. Two gaps before treating this as the official #110/#132 repro:

  1. Evidence gap (blocking for “repro” claim) — Testing describes successful foreground local display/permission flows, but not the onesignalUserNotificationCenter:* ObjCRuntime.RuntimeException. #110 was closed by the reporter as fixed on latest; #132 is still open and reports remote push receive/tap. Please either paste the crash log from these steps, or state explicitly that it no longer reproduces on Plugin.LocalNotification 14.1.1 + current SDK.

  2. #132 path coverage — UI only schedules local notifications. If remote push still matters for #132, add a minimal send-to-self path (or document using the shown subscription ID from the dashboard).

Nits (non-blocking)

  • Shared com.onesignal.example ApplicationId with examples/demo — README mentions shared App UUID but not bundle-ID collision.
  • ClearAllNotifications() is OneSignal-only; Plugin.LocalNotification delivered items may remain.
  • android:allowBackup="true" with full push token on screen (same pattern as main demo).

Otherwise structure looks solid (DotEnv/appenv, run scripts, iOS + Android scaffolding).

@fadi-george
fadi-george merged commit e75ecab into main Jul 16, 2026
6 checks passed
@fadi-george
fadi-george deleted the fadi/sdk-4788 branch July 16, 2026 17:25
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.

3 participants