OneSignal is a free push notification service for mobile apps. This plugin makes it easy to integrate your native iOS app with OneSignal. https://onesignal.com
Other
496
stars
263
forks
source link
Location sharing with OneSignal now defaults to false #1352
Location sharing with OneSignal now defaults to false regardless of device permissions, so developers must explicitly set sharing to true.
Details
Motivation
It may be a poor developer experience to be opted-in by default. Previously, as long as device has location permission, we shared location with the OneSignal server. Now, sharing must be explicitly enabled. This aligns with recent similar Android behavior changes https://github.com/OneSignal/OneSignal-Android-SDK/pull/1942.
Scope
⚠️ Behavior changes:
Developers must now explicitly call OneSignal.Location.isShared = true (Swift) or [OneSignal.Location setShared:true] to share location with OneSignal regardless of permission or prompting for location.
Fix location module bug
Use constants for Location and IAM module classes that are used with NSClassFromString: OneSignalLocationManager and OneSignalInAppMessages
These hardcoded classes can be easily missed. When the class OneSignalLocation was renamed to OneSignalLocationManager, some of the strings were missed in NSClassFromString calls. One consequence of this is the location module's start method never ran and location not shared.
Testing
Unit testing
None
Manual testing
iPhone 13 on iOS 17.1.2
Enable and then request permission → shares location when both steps are done
Request permission and then enable → shares location when both steps are done
Something that is not working is mid-session disabling and enabling doesn't drive location sharing until next cold start.
Already sharing location
Disable sharing - location stops being tracked and shared
Enable sharing - location sharing doesn't start
New cold start - location is shared
Affected code checklist
[ ] Notifications
[ ] Display
[ ] Open
[ ] Push Processing
[ ] Confirm Deliveries
[ ] Outcomes
[ ] Sessions
[ ] In-App Messaging
[ ] REST API requests
[ ] Public API changes
[x] Location sharing
Checklist
Overview
[x] I have filled out all REQUIRED sections above
[x] PR does one thing
[x] 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
[x] All automated tests pass, or I explained why that is not possible
[x] I have personally tested this on my device, or explained why that is not possible
Final pass
[x] Code is as readable as possible.
[x] I have reviewed this PR myself, ensuring it meets each checklist item
Description
One Line Summary
Location sharing with OneSignal now defaults to
false
regardless of device permissions, so developers must explicitly set sharing totrue
.Details
Motivation
It may be a poor developer experience to be opted-in by default. Previously, as long as device has location permission, we shared location with the OneSignal server. Now, sharing must be explicitly enabled. This aligns with recent similar Android behavior changes https://github.com/OneSignal/OneSignal-Android-SDK/pull/1942.
Scope
⚠️ Behavior changes:
OneSignal.Location.isShared = true
(Swift) or[OneSignal.Location setShared:true]
to share location with OneSignal regardless of permission or prompting for location.Fix location module bug
NSClassFromString
:OneSignalLocationManager
andOneSignalInAppMessages
OneSignalLocation
was renamed toOneSignalLocationManager
, some of the strings were missed inNSClassFromString
calls. One consequence of this is the location module'sstart
method never ran and location not shared.Testing
Unit testing
None
Manual testing
iPhone 13 on iOS 17.1.2
Something that is not working is mid-session disabling and enabling doesn't drive location sharing until next cold start.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is