[WIP] Geofencing feature branch #436
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Starting point from hackathon branch * Revise spi access and use KlaviyoInternal method instead * Uncomment preview snippet * Create KlaviyoLocationManager singleton to call in new initializer * Refactor redundant instances of CLLocationManager * Add stubs for public methods * Remove unnecessary geofenceService reference, use directly * Remove unused code * Add TODOs and deinit
* Add fetchGeofences endpoint * Add local Geofences.json file * Add Geofence data struct * Decode JSON response into Geofence data object * Add tests and some organizational cleanup * Remove placeholder test file * Remove ghost file * Move toKlaviyoGeofence helper to Geofence model, add to test * Update dummy geofences to have identifiers more similar to expected format * Unused but move fetchGeofences for maxRetries to 1 * remove palceholder companyId from mock data * Make Geofence conform to Codable * Revise access control, make things internal * change identifier to id, change shape of mock data, fix tests
* Monitor location permissions * Track locationAuthorizationStatus on environment * Add tests for location monitoring behavior * Revise access control * Rename to LocationManagerProtocol * Remove test specific code from main file, generalize to dependency injection * use LocationManagerProtocol instead of CLLocationManager in KlaviyoGeofenceManager
* Replace Geofences.json with real shape of data * Decode JSON and identify geofences with companyId * Add test utils file * Fix utils file * Separate fetch call and data transformation with companyId * Add id validation * Implement do catch * Add tests * Simplify error handling and change some logs * Remove redundant tests * Make GeofenceJSON structs private * Change hyphen split to colon split for geofence id
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
* Add KlaviyoLocation podspec * Bump version to 5.1.0
* Use expected metric and dimension keys for geofence event * Fix test util * Add optional dwell field on Geofence object * Add default of 5 minutes * Implement dwell events * Simplify by unwrapping klaviyoLocationId at top level and use that in dwell timers * Remove unnecessary dwellEnterTimes * Don't actually change the apiUrl on the environment to mock fetch * Make dwell optional with default 5 min * Immediately flush queue for geofence events * Add helper in KlaviyoSDK+Location to ensure background support * Hook up real fetch geofences endpoint (#438) * Use client/geofences and add apiKey to request * Use 2025-10-15.pre api revision * Remove mock call and mock data * Fix tests * include error message in log Co-authored-by: Andrew Balmer <[email protected]> --------- Co-authored-by: Andrew Balmer <[email protected]> * Remove dwell (for now) * Clean up warnings, helper functions, PR feedback * startMonitoringSignificantLocationChanges should support terminated events * Add initialized check to fetchGeofences to protect initial launches * OSLog privacy changes * Remove mock data * Remove locationManagerDelegate, reorganize, simplify background support * Remove redundant internal keywords from KlaviyoLocation module (#443) - Changed KlaviyoLocationManager from public to internal class - Removed redundant internal keywords from all type declarations: - KlaviyoLocationManager, KlaviyoGeofenceManager - GeofenceServiceProvider, GeofenceService - Geofence, GeofenceError - LocationManagerProtocol - Removed redundant internal keywords from all members (properties, methods, inits) - Types now rely on Swift's default internal access level for clarity All 13 tests passing. * Add query to endpoint test --------- Co-authored-by: Andrew Balmer <[email protected]> Co-authored-by: Ajay Subramanya <[email protected]>
* Add map view to sample app with hardcoded geofences * WIP remove storyboard with SwiftUI views * Remove Storyboard, use SwiftUI, clean up sample app * Connect up the AppDelegate, other fixes * Remove last unused table view cells * Add temporary KlaviyoLocation podspec and update CocoapodsExample * Add start and stop buttons, show current regions * Clean up SwiftUI, refactor AppState, remove MapViewController * Cleanup MapView * Thanks XCode for the recommended upgrades + getting rid of some warnings * Swap event type * Remove duplicative KlaviyoLocation podspec * PR feedback, more SwiftUI cleanup * sample app geofencing suggestions (#445) * changed `NavigationView` to `NavigationStack` `NavigationView` is deprecated * added .ignoresSafeArea and background for toolbars. this makes the map fill is parent container. The toolbar background ensures that the buttons are visible above the map on pre-iOS-26 devices * changed Close button to show an image instead of text * Changed "Stop" button to show an icon instead of text this resolves an issue on iOS 26 where the "Stop" button text was getting truncated * consolidated styles into `locationStatusLabel` tuple * minor formatting fix * added `geofenceMonitoringLabel` and revised geofence monitoring indicator style this fixes an issue on iOS 26+ where the monitoring label was getting truncated * removed geofencing monitoring label from bottom toolbar and placed it in navigation bar * moved all controls into a menu button at the top of the screen * moved monitoring buttons to bottom bar --------- Co-authored-by: Belle Lim <[email protected]> * Replace straggler NavigationView --------- Co-authored-by: Andrew Balmer <[email protected]>
* Use expected metric and dimension keys for geofence event * Fix test util * Add optional dwell field on Geofence object * Add default of 5 minutes * Implement dwell events * Simplify by unwrapping klaviyoLocationId at top level and use that in dwell timers * Remove unnecessary dwellEnterTimes * Don't actually change the apiUrl on the environment to mock fetch * Make dwell optional with default 5 min * Immediately flush queue for geofence events * Add helper in KlaviyoSDK+Location to ensure background support * Hook up real fetch geofences endpoint (#438) * Use client/geofences and add apiKey to request * Use 2025-10-15.pre api revision * Remove mock call and mock data * Fix tests * include error message in log Co-authored-by: Andrew Balmer <[email protected]> --------- Co-authored-by: Andrew Balmer <[email protected]> * Remove dwell (for now) * Clean up warnings, helper functions, PR feedback * startMonitoringSignificantLocationChanges should support terminated events * Add initialized check to fetchGeofences to protect initial launches * OSLog privacy changes * Remove mock data * Remove locationManagerDelegate, reorganize, simplify background support * Remove redundant internal keywords from KlaviyoLocation module (#443) - Changed KlaviyoLocationManager from public to internal class - Removed redundant internal keywords from all type declarations: - KlaviyoLocationManager, KlaviyoGeofenceManager - GeofenceServiceProvider, GeofenceService - Geofence, GeofenceError - LocationManagerProtocol - Removed redundant internal keywords from all members (properties, methods, inits) - Types now rely on Swift's default internal access level for clarity All 13 tests passing. * Add query to endpoint test * Consolidate KlaviyoGeofenceManager into KlaviyoLocationManager ## Changes - Merged KlaviyoGeofenceManager functionality into KlaviyoLocationManager - Removed unnecessary class separation and simplified architecture - Improved dependency injection: API key now passed down from manager to service ## Architecture improvements - GeofenceService is now a pure service with no SDK dependencies - Takes apiKey as parameter instead of fetching internally - parseGeofences() is now synchronous (no longer needs async) - API key fetched once in setupGeofencing() instead of 3 times - Better testability: GeofenceService can be tested with any API key string ## Test updates - Removed MockKlaviyoGeofenceManager (no longer needed) - Simplified tests to verify behavior directly - Enhanced MockLocationManager to track stopped regions * Make device capability check testable via protocol - Add isMonitoringAvailable method to LocationManagerProtocol - Implement instance method in CLLocationManager extension - Update KlaviyoLocationManager to use protocol method instead of static call - Add mockIsMonitoringAvailable property to MockLocationManager for testing This removes the leaky abstraction where we were calling CLLocationManager static methods directly, making the capability check fully mockable in tests. * Clean up KlaviyoLocationManager organization - Move destroyGeofencing() after syncGeofences() for better code organization - Remove redundant error logging in syncGeofences() catch block - Minor formatting improvements 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Improve readability of active geofences collection Simplified the compactMap closure by: - Combining type check and conversion in a single guard statement - Using try? for cleaner error handling - Removing unnecessary do-catch block 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Extract active geofences logic into private method - Created getActiveGeofences() method to encapsulate conversion logic - Makes syncGeofences() method more concise and focused - Improves code organization and reusability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Improve geofence sync performance and code quality - Optimize region removal with dictionary lookup (O(1) vs O(n)) - Improve variable naming: regionsToRemove -> geofencesToRemove - Refactor destroyGeofencing with early return and better logging - Use method reference syntax for cleaner code Performance: Changes region removal from O(n*m) to O(n+m) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Extract CLLocationManagerDelegate to separate file - Move CLLocationManagerDelegate extension to dedicated file - Improves code organization and readability - KlaviyoLocationManager.swift now 109 lines (was 206) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Rename geofencing methods for clarity - setupGeofencing() -> startGeofenceMonitoring() - destroyGeofencing() -> stopGeofenceMonitoring() Better method names that: - Use clear action verbs (start/stop) - Align with CoreLocation's startMonitoring/stopMonitoring pattern - More accurately describe what the methods do Updated all references in source and tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Add MARK comment to API response models Improves code readability by clearly separating service logic from private API response data structures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Simplify parseGeofences method for better readability Removed redundant do-catch wrapper and inlined variable. Error handling still occurs at the top level in fetchGeofences(). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Remove redundant success log in fetchGeofenceData The success case doesn't need a log since it's the normal path. Error cases are already logged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Rename handleCLAuthorizationStatusChange to handleAuthorizationChange Simplified method name by removing redundant "CL" prefix and "Status" - both are clear from context. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Remove unnecessary default case from authorization status switch Switch statement is exhaustive - all CLAuthorizationStatus cases are covered. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Extract common geofence event handling logic Reduced duplication by extracting handleGeofenceEvent helper method. Both didEnterRegion and didExitRegion now delegate to this shared implementation. Also simplified Task/MainActor pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Remove question comments from code Cleaned up all Q: comments that were added during development. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Tidy up CLLocationManagerDelegate, remove question * Remove id validation * create more helpful method to call monitorGeofencesFromBackground with * Fix main thread declarations * Remove deinit since this is a singleton, avoid concurrency issues * Fix merge conflict in Info.plist --------- Co-authored-by: Isobelle Lim <[email protected]> Co-authored-by: Andrew Balmer <[email protected]> Co-authored-by: Claude <[email protected]>
* Implement cooldown period for geofence events * Fix OSLogger and refactor save and filter in cooldown map * Remove internal * Update test, add clean method
…t) (#444) * Use expected metric and dimension keys for geofence event * Fix test util * Add optional dwell field on Geofence object * Add default of 5 minutes * Implement dwell events * Simplify by unwrapping klaviyoLocationId at top level and use that in dwell timers * Remove unnecessary dwellEnterTimes * Don't actually change the apiUrl on the environment to mock fetch * Make dwell optional with default 5 min * Immediately flush queue for geofence events * Add helper in KlaviyoSDK+Location to ensure background support * Hook up real fetch geofences endpoint (#438) * Use client/geofences and add apiKey to request * Use 2025-10-15.pre api revision * Remove mock call and mock data * Fix tests * include error message in log Co-authored-by: Andrew Balmer <[email protected]> --------- Co-authored-by: Andrew Balmer <[email protected]> * Remove dwell (for now) * Clean up warnings, helper functions, PR feedback * startMonitoringSignificantLocationChanges should support terminated events * Add initialized check to fetchGeofences to protect initial launches * OSLog privacy changes * Remove mock data * Remove locationManagerDelegate, reorganize, simplify background support * Remove redundant internal keywords from KlaviyoLocation module (#443) - Changed KlaviyoLocationManager from public to internal class - Removed redundant internal keywords from all type declarations: - KlaviyoLocationManager, KlaviyoGeofenceManager - GeofenceServiceProvider, GeofenceService - Geofence, GeofenceError - LocationManagerProtocol - Removed redundant internal keywords from all members (properties, methods, inits) - Types now rely on Swift's default internal access level for clarity All 13 tests passing. * Add query to endpoint test * Support pre-initialized events (sorta) * Add company observer * Add lifecycle monitoring * Add unit tests for observers * Remove separate event creator * Add geofence events in front of queue * Restore observer stoppers * Refactor to avoid loopish call, other cleanup * Fix tests * Prevent triggering double syncs * Remove fetchInProgress and remove dispatch to Tasks * Remove lifecycle observer (for now) --------- Co-authored-by: Andrew Balmer <[email protected]> Co-authored-by: Ajay Subramanya <[email protected]>
…455) * Implement getCurrentGeofences * Create isKlaviyoGeofence helper, only handle klaviyo geofences * Add _k prefix and use that to check klaviyo geofences * Use filter in stopGeofenceMonitoring
* Add stopMonitoringSignificantLocationChanges to unregister * Try to reduce monitoring significant location changes
* Initialize on geofence event * Add tests to check priority insertion, instant flush, and init effect * guard against empty companyIds form malformed geofences * Reject geofence events if they have a mismatching API key to the one in store
* Add monitoringDidFailFor error logs * Don't fail to parse geofence data if one fails * Very explicitly only affect Klaviyo geofences with 20 limit * Swap some errors for warnings and reorganize
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.