Skip to content

fix: Forward impression events and products instead of null events#355

Open
mmustafa-tse wants to merge 1 commit into
mParticle:mainfrom
mmustafa-tse:fix/impressionEvent
Open

fix: Forward impression events and products instead of null events#355
mmustafa-tse wants to merge 1 commit into
mParticle:mainfrom
mmustafa-tse:fix/impressionEvent

Conversation

@mmustafa-tse

Copy link
Copy Markdown
Contributor

Summary

A customer reported that Impression events sent from React Native SDK are being received as null in the UI, after further investigation it seems the iOS react native bridge was missing the logic to handle impressions in logCommerceEvents.

Testing Plan

Tested E2E by changing the code locally:
Screenshot 2026-07-06 at 2 49 15 PM

Screenshot 2026-07-06 at 2 06 18 PM Screenshot 2026-07-06 at 2 08 48 PM

Master Issue

Closes https://rokt.atlassian.net/browse/SDKE-1210

@mmustafa-tse mmustafa-tse requested a review from a team as a code owner July 6, 2026 21:53
@cursor

cursor Bot commented Jul 6, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Localized bridge mapping for analytics commerce impressions; no auth, persistence, or API contract changes beyond fixing dropped impression data on the New Arch iOS path.

Overview
Fixes impression commerce events from React Native on iOS showing as null in the dashboard by mapping commerceEvent.impressions() in the New Architecture logCommerceEvent bridge.

When impressions are present, each list’s impressionListName and nested products are converted with the same field mapping as regular products (createMPProductFromDict), then attached via addImpression:product listName: before the event is logged. Impression lists without a list name are skipped.

This aligns the Turbo Module path with the existing old architecture RCTConvert (MPCommerceEvent) impression handling, which was already implemented but not wired in the struct-based New Arch method.

Reviewed by Cursor Bugbot for commit 557df1f. Bugbot is set up for automated code reviews on this repo. Configure here.

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.

1 participant