-
Notifications
You must be signed in to change notification settings - Fork 376
fix: custom events now handle null object within the event properties #2537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-Authored-By: AR Abdul Azeez <abdul@onesignal.com>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Co-Authored-By: AR Abdul Azeez <abdul@onesignal.com>
| } | ||
| is List<*> -> { | ||
| val array = JSONArray() | ||
| value.forEach { array.put(convertToJson(it!!)) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am surprised how this passed detekt in the first place
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Threshold: 80% Changed Files Coverage
Overall Coverage (Changed Lines Only)3/4 changed lines covered (75.0%) ❌ Coverage Check FailedFiles below 80% threshold:
|
There was a problem hiding this 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 pull request fixes null value handling in JSON conversion for custom event properties to ensure null values are preserved throughout the serialization process.
Changes:
- Modified
JSONUtils.convertToJson()to convert null values toJSONObject.NULLinstead of leaving them as-is - Updated type signatures from
Map<String, Any>toMap<String, Any?>across the custom event interfaces and implementations to support null property values - Added comprehensive test coverage for null handling in both
JSONUtilsTests.ktand a newCustomEventControllerTests.ktfile
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/JSONUtils.kt | Updated convertToJson() and mapToJson() to handle null values by converting them to JSONObject.NULL, and removed force-unwrap operators |
| OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/JSONUtilsTests.kt | Added comprehensive test cases for null value handling in maps, arrays, and nested structures |
| OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/customEvents/impl/CustomEventControllerTests.kt | New test file with full test coverage for custom event controller including null property scenarios |
| OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/customEvents/ICustomEventController.kt | Updated type signature to Map<String, Any?>? and added documentation |
| OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/customEvents/impl/CustomEventController.kt | Updated type signature to Map<String, Any?>? and added class documentation |
| OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/IUserManager.kt | Updated type signature to Map<String, Any?>? |
| OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt | Updated type signature to Map<String, Any?>? |
| OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt | Added null property to test case |
| OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/TrackCustomEventOperation.kt | Added class documentation |
| OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/customEvents/impl/CustomEventMetadata.kt | Added class and method documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Runner failed with a test coverage for an interface. Merging now. |
…#2537) Co-authored-by: AR Abdul Azeez <abdul@onesignal.com>
Description
One Line Summary
Fix null value handling in JSON conversion for custom event properties to preserve null values in nested structures.
Details
Motivation
Custom event properties with null values were not preserved when converting to JSON. This prevented users from tracking events with null properties (e.g., someNull: null or nested nulls like someObject.ghi: null). The JSON conversion now preserves nulls by converting them to JSONObject.NULL, matching expected JSON behavior.
Scope
OPTIONAL - Other
The fix ensures that when convertToJson() encounters a null value, it returns JSONObject.NULL instead of null. This allows:
All null values are properly serialized to JSON and can be deserialized back, maintaining data integrity throughout the conversion process.
Testing
Unit testing
Updated JSONUtilsTests.kt.
Added CustomEventControllerTests.kt.
All tests verify that null values are correctly converted to JSONObject.NULL and preserved in the final JSON output.
Manual testing
Tested with emulator Pixel 9 api 35
Used some example codes in
MainApplicationKT.ktwhich tracks a custom event with the following structure:Verified that all null values are properly serialized and sent to the backend.

Affected code checklist
Checklist
Overview
Testing
Final pass
This change is