Skip to content

Conversation

@jinliu9508
Copy link
Contributor

@jinliu9508 jinliu9508 commented Jan 30, 2026

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

  • Changed: JSONUtils.convertToJson() now converts null values to JSONObject.NULL instead of leaving them as-is
  • Changed: Updated all related unit tests to verify null handling
  • Added: Comprehensive test coverage for CustomEventController including null property scenarios
  • Not changed: Other JSON conversion behavior remains the same; only null handling was modified

OPTIONAL - Other

The fix ensures that when convertToJson() encounters a null value, it returns JSONObject.NULL instead of null. This allows:

  • Null values at the root level: {"someNull": null}
  • Null values in nested objects: {"someObject": {"ghi": null}}
  • Null values in arrays: [1, "2", {"abc": "123"}, null]
    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.kt which tracks a custom event with the following structure:

val properties = mapOf(
    "someNum" to 123,
    "someFloat" to 3.14159,
    "someString" to "abc",
    "someBool" to true,
    "someObject" to mapOf(
        "abc" to "123",
        "nested" to mapOf("def" to "456"),
        "ghi" to null,  // null in nested object
    ),
    "someArray" to listOf(1, 2),
    "someMixedArray" to listOf(1, "2", mapOf("abc" to "123"), null),  // null in array
    "someNull" to null,  // null at root level
)

Verified that all null values are properly serialized and sent to the backend.
image

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

Co-Authored-By: AR Abdul Azeez <abdul@onesignal.com>
@claude
Copy link

claude bot commented Jan 30, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

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