Create Geofence events

[belleklaviyo]

Description

So this does a bit more than just creating geofence events but it all makes sense together:

• Adds the apiKey to the fetchGeofences call
• Updates path for fetchGeofences to /client/events
• Uses expected API revision
• Removes mock API call
• Adds guard to setupGeofencing() call to ensure we are initialized and not trying to fetch geofences for a nil/invalid companyId* (this is doubly necessary due to some lifecycle weirdness with CLLocationManager)
• Calls startMonitoringSignificantLocationChanges to ensure we can monitor location events in the backgrounded and terminated states
• Introduces an API function monitorGeofencesFromBackground to be put in didFinishLaunchingWithOptions to ensure we can monitor location events in the backgrounded and terminated states
• Modifies the location event to use the expected dimension names
• Modifies the enqueueEvent so if its a goefence event it automatically flushes the queue, regardless of initialization state

Notes

• Dwell was originally going to be implemented in this PR as well, but that will be done in a separate PR
• CoreLocation sometimes triggers two identical enter/exit events depending if you're foregrounded or not when triggering an event. We will implement a cooldown period to make sure 2 identical events are not fired within an unreasonable amount of time (more here). This will be done in a separate PR.
• We want to doubly ensure geofence events are delivered in a timely manner, so we want to insert those events at the front of the queue. This will be done in a separate PR.

Due Diligence

• I have tested this on a simulator or a physical device.
• I have added sufficient unit/integration tests of my changes.
• I have adjusted or added new test cases to team test docs, if applicable.
• I am confident these changes are compatible with all iOS and XCode versions the SDK currently supports.

Release/Versioning Considerations

• Patch Contains internal changes or backwards-compatible bug fixes.
• Minor Contains changes to the public API.
• Major Contains breaking changes.
• Contains readme or migration guide changes.
  • If so, please merge to a feature branch so documentation updates only go live upon official release.
• This is planned work for an upcoming release.
  • If no, author or reviewer should account for this in a release plan, or describe why not below.

Changelog / Code Overview

This updates the event metrics going to client/events to the expected dimension names.

Test Plan

Related Issues/Tickets

https://klaviyo.atlassian.net/browse/CHNL-25309

[wiz-inc-faae60d47d]

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	-
Sensitive Data	-
Total	-

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

[ab1470]

overall looks great, just some minor comments and suggested changes

[ab1470]

Added some suggested changes regarding OSLog privacy

[belleklaviyo]

So more context about this -- it's a sensible guard to put in anyways where we would expect there to be a valid API key to use with our fetchGeofences call. But there's some weirdness where when CLLocationManager() is instantiated (which we do on launch to ensure background/terminated support) it also triggers didChangeAuthorization even if the actual auth level has not changed. This fires before initialization is able to complete, so as a result when we trigger setupGeofences because we have the correct auth level, we may not have the actual API key. This results in fetching the geofences for a nil company which returns empty which then clears any existing geofences.

tldr; we need this to make sure we can support being responsive to authorization level changes (such as if they revoke it and we need to unregister our geofences) while also carefully handling the timing issues between when CoreLocation is setting up and the app can be initialized

[ajaysubra]

Hey did a quick pass. Will look into this in more detail tomorrow.

[ajaysubra]

Can't we do this within our SDK instead of having the developer add this in?

[belleklaviyo]

Not really, from my understanding, didFinishLaunchingWithOptions is the very very very first thing called in the lifecycle when waking up an app from the terminated state. There isn't any UIApplication notification we can hook into reliably like how we have other app life cycle events that will be an equivalent catch-all for this case.

[ajaysubra]

Can't we either hook into foregrounded life cycle event and or have something in initialize since that gets called when the app is launch and have a hook in location that observers something in KlaviyoSwift and does this?

[belleklaviyo]

Hmm the foregrounded life cycle event only is fired when the app is actually opened, so this wouldn't work in the terminated state. As for the initialize hook, I've been keeping geofencing methods pretty separate from it with the late initialization case in mind. (Part of that will be making KlaviyoLocation observe companyId changes and update the geofences then but unrelated to this monitorGeofencesFromBackground necessity.) But as far as I've explored, I think since we don't actually have any direct insurance to hook into didFinishLaunchingWithOptions from the SDK side, it's most assured to instruct developers to implement it themselves. Let me know if I'm misunderstanding/missing something else here

[cursor]

Bug: Unconditional Background Location Causes App Crashhrase

Setting allowsBackgroundLocationUpdates = true unconditionally in the initializer will cause a runtime crash if the host app's Info.plist doesn't include "location" in the UIBackgroundModes array. This is a required configuration per Apple's documentation. The SDK initializes this singleton when monitorGeofencesFromBackground() is called (which just accesses the shared instance), meaning apps will crash immediately if they haven't configured their Info.plist properly. This property should only be set when geofencing is actively being used, or the SDK should add proper error handling and documentation about this requirement.

 

[belleklaviyo]

this will be part of our documentation/setup requirements so....

[ajaysubra]

Hey @belleklaviyo while reviewing your PR, I noticed that we had the internal keyword in code in multiple locations and since by default swift's access level is internal I removed it. Please let me know what you think.

#443

[ajaysubra]

Nice work, Belle!