Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR appears to implement campaign-based notifications functionality with enhanced geofence handling and notification parsing capabilities.
- Enhanced geofence management with individual metadata-based notification handling
- Added notification parsing and creation functionality for campaign notifications
- Refactored color parsing to use extension functions for consistency
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| RadarGeofence.kt | Added blank line for formatting |
| RadarNotificationHelper.kt | Added notification parsing/sending methods and updated color parsing |
| RadarLocationReceiver.kt | Split geofence handling logic and added notification processing |
| RadarLocationManager.kt | Modified geofence management to handle individual metadata |
| Radar.kt | Added geofence replacement call |
| AndroidManifest.xml | Added new intent filter actions |
| MyRadarReceiver.kt | Disabled notification functionality |
| MainActivity.kt | Updated API key and added debug logging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Just noting that the behaviors are not explicitly tested.
I also concede that adding test coverage for this is non trivial due to the extensive mocking required.
Given the urgency of this ask, maybe as a fast follow up?
|
|
||
| return | ||
| } | ||
| // track successful |
There was a problem hiding this comment.
do we this this new comment?
| if (geofences.size == 0) { | ||
| logger.d("No synced geofences") | ||
| return@mapNotNull RadarAbstractLocationClient.RadarAbstractGeofence( | ||
| requestId = geofence._id, |
There was a problem hiding this comment.
requestId changed to geofence._id instead of radar_sync_1, radar_sync_2, etc. because we want to know which geofence it is
Client side geofence notification for Android
Test step: (using mock GPS)
With no frequency cap: Turn on override frequency cap on dashboard
There is an issue with this: