Skip to content

feat: [SDK-4775] add location opt-out support#184

Merged
fadi-george merged 14 commits into
mainfrom
fadi/sdk-4775
Jun 26, 2026
Merged

feat: [SDK-4775] add location opt-out support#184
fadi-george merged 14 commits into
mainfrom
fadi/sdk-4775

Conversation

@fadi-george

@fadi-george fadi-george commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Adds an MSBuild-controlled way to exclude the native OneSignal location module and includes a no-location MAUI demo.

Expect in Console.app:
Screenshot 2026-06-25 at 3 07 00 PM

Expect in ADB/LogCat:
Screenshot 2026-06-25 at 3 20 45 PM

Demo app:
Screenshot 2026-06-26 at 10 32 06 AM

Details

Motivation

Apps that do not use OneSignal location features should be able to omit native location dependencies while keeping notification, user, IAM, outcome, and live activity behavior available.

Scope

This introduces <OneSignalDisableLocation>true</OneSignalDisableLocation> for source/package builds, conditionally excludes iOS and Android location native assets, and makes location API calls gracefully no-op when the native module is absent. It also adds examples/demo-no-location with APNs capability and updates demo scripts/styles shared by the examples.

Testing

Unit testing

No unit tests were added; this change primarily affects MSBuild item inclusion and native app packaging paths.

Manual testing

From examples/demo-no-location:

  • Android: Ran ./run-android.sh on an emulator, tapped TEST LOCATION REQUEST, and confirmed in Logcat that the location module was excluded (missing-module error, no location dependency linked).
  • iOS: Ran ./run-ios.sh on a simulator, tapped TEST LOCATION REQUEST, and confirmed in Console that the location module was missing (missing-module error, no OneSignalLocation.framework in the app).

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 fadi-george requested a review from a team as a code owner June 25, 2026 22:22
@abdulraqeeb33

Copy link
Copy Markdown

One thing I'd flag before merge: on iOS the missing-module diagnostic uses System.Diagnostics.Debug.WriteLine, which gets compiled out in Release builds. So the log line you relied on during manual testing won't actually show up in a production build — and that's a bit ironic since the whole point here is to leave a breadcrumb when someone calls a location API without the module.

Android is fine since Android.Util.Log.Error always logs, which is really the inconsistency: same situation, two different logging behaviors.

Suggest switching iOS to Console.WriteLine/NSLog, or ideally routing both platforms through the SDK's own OneSignalSDK.DotNet.Core.Debug logging so the message respects the configured log level and shows up consistently.

(Smaller, related: the catch (Exception) is broad enough that a genuine error with the module present would also get logged as "module not available" — might be worth narrowing or softening that wording.)

Comment thread OneSignalSDK.DotNet.Android/AndroidLocationManager.cs
Comment thread OneSignalSDK.DotNet.iOS/iOSLocationManager.cs
@fadi-george fadi-george requested a review from sherwinski June 26, 2026 18:28
@fadi-george fadi-george merged commit 4970aa5 into main Jun 26, 2026
5 checks passed
@fadi-george fadi-george deleted the fadi/sdk-4775 branch June 26, 2026 18:34
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