fix: Forward impression events and products instead of null events - #355
fix: Forward impression events and products instead of null events#355mmustafa-tse wants to merge 2 commits into
Conversation
PR SummaryMedium Risk Overview In the same New Arch product mapping, Dependency: Reviewed by Cursor Bugbot for commit e264e8d. Bugbot is set up for automated code reviews on this repo. Configure here. |
| NSMutableDictionary *productDict = [[NSMutableDictionary alloc] init]; | ||
| if (productStruct.name()) productDict[@"name"] = productStruct.name(); | ||
| if (productStruct.sku()) productDict[@"sku"] = productStruct.sku(); | ||
| productDict[@"price"] = @(productStruct.price()); |
There was a problem hiding this comment.
Any reason price doesn't have a conditional whereas all the others do?
There was a problem hiding this comment.
@rmi22186 - I pretty much copied the same logic on for commerceEvent.products().has_value() in line 513, I can add the if statement for both to be on the safer side
rmi22186
left a comment
rmi22186
left a comment
There was a problem hiding this comment.
Note this is new architecture that doesn't support it, not the old arch, the old arch supports, so the PR description is slightly off.
|
Also the iOS test app is not building. Take a look at that. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit e264e8d. Configure here.
| if (productStruct.name()) productDict[@"name"] = productStruct.name(); | ||
| if (productStruct.sku()) productDict[@"sku"] = productStruct.sku(); | ||
| productDict[@"price"] = @(productStruct.price()); | ||
| if (productStruct.price()) productDict[@"price"] = @(productStruct.price()); |
There was a problem hiding this comment.
Zero price silently drops products
Medium Severity
price is a required non-optional number on Product, unlike optional fields that use .has_value(). Guarding with if (productStruct.price()) treats 0 as absent, so the key is omitted and createMPProductFromDict returns nil. Free products are then dropped from commerce events and impressions.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e264e8d. Configure here.
2 participants
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:
Master Issue
Closes https://rokt.atlassian.net/browse/SDKE-1210