Skip to content

Conversation

@ShiCheng-Lu
Copy link
Contributor

@ShiCheng-Lu ShiCheng-Lu commented Aug 29, 2025

Client side geofence notification for Android

Test step: (using mock GPS)

  1. create campaign on dashboard.
  2. set location to outside geofence.
  3. track once,
  4. turn off WIFI (or not)
  5. set location inside geofence
  6. notification should trigger right away.
  7. notification should not trigger any more when moving out + in geofence.
  8. turn on wifi again.
  9. move in and out geofence should not trigger more notificaitons.

With no frequency cap: Turn on override frequency cap on dashboard

  1. track once again,
  2. move out of geofence, (should not trigger notificaiton)
  3. move in, should trigger, (frequency cap overridden)

There is an issue with this:

  • currently our pending intents all look the same (to the system) so only the last pending intent's data is returned every time. so the geofence stored in the extras is not correct.

@Copilot Copilot AI review requested due to automatic review settings August 29, 2025 22:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we this this new comment?

if (geofences.size == 0) {
logger.d("No synced geofences")
return@mapNotNull RadarAbstractLocationClient.RadarAbstractGeofence(
requestId = geofence._id,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestId changed to geofence._id instead of radar_sync_1, radar_sync_2, etc. because we want to know which geofence it is

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.

2 participants