Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements offline tracking functionality for the Radar SDK, allowing location tracking and geofence event generation when network connectivity is unavailable.
- Adds offline tracking capabilities with fallback when network requests fail
- Implements notification deduplication system to prevent repeated notifications
- Introduces file storage system for offline data persistence
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| RadarOfflineManager.kt | Core offline tracking logic with geofence event generation and user state management |
| RadarFileStorage.kt | File storage utility for persisting offline data |
| RadarState.kt | Notification deduplication tracking and state management |
| RadarSettings.kt | User object persistence methods |
| RadarSdkConfiguration.kt | Configuration flags for offline features |
| RadarApiClient.kt | Integration of offline manager as fallback when network fails |
| RadarLocationManager.kt | Enhanced geofence registration with notification metadata |
| RadarNotificationHelper.kt | Notification parsing and delivery for geofence campaigns |
| RadarLocationReceiver.kt | Geofence notification triggering on entry events |
| RadarUtils.kt | Live environment detection utility |
| Various test files | Import cleanup and code style improvements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| fun isLive(context: Context) { | ||
| RadarSettings.getPublishableKey(context)?.contains("_live_") |
There was a problem hiding this comment.
Function isLive() returns Unit instead of Boolean. The contains() call result should be returned.
| fun isLive(context: Context) { | |
| RadarSettings.getPublishableKey(context)?.contains("_live_") | |
| fun isLive(context: Context): Boolean { | |
| return RadarSettings.getPublishableKey(context)?.contains("_live_") == true |
|
|
||
| // offline track in case online track fails | ||
| fun track(context: Context, params: JSONObject): JSONObject? { | ||
| print("Tracked offline") |
There was a problem hiding this comment.
Use proper logging instead of print(). Replace with Radar.logger.d() or similar logging method for consistency with the rest of the codebase.
| print("Tracked offline") | |
| Radar.logger.d("Tracked offline") |
| put("offline", true) | ||
| } | ||
|
|
||
| print(response) |
There was a problem hiding this comment.
Use proper logging instead of print(). Replace with Radar.logger.d() or similar logging method for consistency with the rest of the codebase.
| print(response) | |
| Radar.logger.d(response.toString()) |
| fun readJSON(context: Context, filename: String): JSONObject? { | ||
| return readFile(context, filename)?.let { | ||
| try { | ||
| return JSONObject(it.toString()) |
There was a problem hiding this comment.
Converting ByteArray to String without specifying charset may cause encoding issues. Use String(it, Charsets.UTF_8) for consistent UTF-8 encoding.
| return JSONObject(it.toString()) | |
| return JSONObject(String(it, Charsets.UTF_8)) |
| // User | ||
| // set user fields | ||
| val newUser: JSONObject = user.toJson() // the json value for the updated user | ||
| newUser.put("geofences", newUserGeofences.map{ it.toJson() }) |
There was a problem hiding this comment.
Missing space after 'map'. Should be map { it.toJson() }.
| newUser.put("geofences", newUserGeofences.map{ it.toJson() }) | |
| newUser.put("geofences", newUserGeofences.map { it.toJson() }) |
| var res = res // shadows res from parameter with a var so it can be re-assigned at offline manager | ||
| var status = status |
There was a problem hiding this comment.
Variable shadowing creates confusion. Use different names like responseResult and requestStatus to avoid shadowing the parameters.
1 participant
No description provided.