Skip to content

feat: [SDK-5180] expose notification click event JSON - #2742

Merged
fadi-george merged 2 commits into
mainfrom
fadi/sdk-5180
Sep 9, 2026
Merged

feat: [SDK-5180] expose notification click event JSON#2742
fadi-george merged 2 commits into
mainfrom
fadi/sdk-5180

Conversation

@fadi-george

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Expose notification click events as JSON through the public Android SDK API.

Details

Motivation

NotificationClickEvent already had internal JSON serialization, but INotificationClickEvent did not expose it. Applications therefore received the default object identity when logging the event and had to duplicate serialization logic.

Scope

  • Adds toJSONObject() to INotificationClickEvent.
  • Returns the wrapper-compatible notification and result event shape.
  • Serializes notification rawPayload as a JSON object, including grouped notifications.
  • Updates the demo to use the base SDK serializer and removes its duplicate helper.
  • Changes the prior internal action key to result and the serialized rawPayload type from string to object.
  • Adds an abstract member to the public event interface; external custom implementations must implement it.

Testing

Unit testing

Added notification click serialization tests covering notification fields, grouped notifications, action ID, URL, nullable fields, and nested raw payload objects.

Manual testing

No device testing was performed. The targeted unit tests, Spotless checks, IDE lint diagnostics, and notifications release build passed.

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
  • 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.
  • I have reviewed this PR myself, ensuring it meets each checklist item

Made with Cursor

@fadi-george
fadi-george requested a review from a team as a code owner September 8, 2026 23:18
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

  • Notification.kt: 1/1 touched executable lines (100.0%) (1 touched lines in diff)
  • NotificationClickEvent.kt: 1/1 touched executable lines (100.0%) (2 touched lines in diff)

Overall (aggregate gate)

2/2 touched executable lines covered (100.0% — requires ≥ 80%)

📥 View workflow run

@abdulraqeeb33 abdulraqeeb33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved.

Stay native for KMP. This serializer sits on Android org.json and Android-only notification fields (androidNotificationId, icons, LED, lock-screen visibility). :kmp is still logger-only, and there is no iOS sibling. Revisit when the notifications model itself moves.

One non-blocking follow-up: JSONObject(rawPayload) throws when rawPayload is empty (initPayloadData returns early if custom is missing). The existing catch then omits the key. Happy-path OneSignal clicks always have a valid payload string.

@fadi-george
fadi-george merged commit e284faf into main Sep 9, 2026
9 checks passed
@fadi-george
fadi-george deleted the fadi/sdk-5180 branch September 9, 2026 16:48
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