Skip to content

feat: Java enum for MPCommerceEventActionType was base 1#336

Merged
BrandonStalnaker merged 2 commits into
mainfrom
fix/TRIAGE-719-Java-Enum-is-Base-1-unlike-iOS
Jun 5, 2026
Merged

feat: Java enum for MPCommerceEventActionType was base 1#336
BrandonStalnaker merged 2 commits into
mainfrom
fix/TRIAGE-719-Java-Enum-is-Base-1-unlike-iOS

Conversation

@BrandonStalnaker
Copy link
Copy Markdown
Contributor

Summary

On iOS with the New Architecture (RCT_NEW_ARCH_ENABLED), logCommerceEvent set MPCommerceEvent.action by casting the TurboModule / codegen number straight to MPCommerceEventAction. That number is the JavaScript ProductActionType (1-based MPReactCommerceEventAction: AddToCart = 1, …), which does not match the Apple SDK’s native enum raw values or ordering. The result was wrong commerce actions (e.g. AddToCart → remove_from_cart, and generally scrambled types including wishlist vs checkout).

The legacy bridge path already fixed this by routing productActionType through [RCTConvert MPCommerceEventAction:], which maps each JS constant to the correct MPCommerceEventAction.

This PR aligns the New Architecture path with that behavior by:

  • Using [RCTConvert MPCommerceEventAction:@(productActionType)] in the New Arch logCommerceEvent implementation instead of a direct cast.
  • Adding a forward declaration for + [RCTConvert MPCommerceEventAction:] near the top of RNMParticle.mm so the call is valid before the RCTConvert (MPCommerceEvent) category implementation later in the file.
  • Updating RCTConvert (MParticle) + MPCommerceEvent: (dictionary-based helper) to set commerceEvent.action via MPCommerceEventAction: as well, instead of casting the integer directly—same bug class if that code path is used.
    Android is unchanged (already maps JS ints explicitly in Kotlin). Promotion action types were not part of this issue (JS 0/1 matches existing native usage).

Master Issue

Closes https://go.mparticle.com/work/TRIAGE-719

@BrandonStalnaker BrandonStalnaker self-assigned this Jun 4, 2026
@BrandonStalnaker BrandonStalnaker requested a review from a team as a code owner June 4, 2026 20:32
@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 4, 2026

PR Summary

Medium Risk
Changes only analytics event mapping on iOS, but wrong commerce actions affect downstream attribution and reporting; scope is limited to RNMParticle.mm conversion paths.

Overview
Fixes incorrect iOS commerce analytics when React Native’s New Architecture is enabled: TurboModule integers were cast directly to Apple SDK enums, but JS ProductActionType is 1-based (AddToCart = 1, …) while native MPCommerceEventAction uses different raw values—so actions like add-to-cart could be logged as the wrong type.

Product actions now go through existing [RCTConvert MPCommerceEventAction:] in the New Arch logCommerceEvent path and in the dictionary-based +MPCommerceEvent: helper (same as the legacy bridge), with a forward declaration so the converter is visible earlier in RNMParticle.mm.

Promotion actions get a new +MPPromotionAction: mapper (JS View=0 / Click=1 vs Apple Click=0 / View=1) and replace raw casts in New Arch logCommerceEvent and MPPromotionContainer:.

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

Comment thread ios/RNMParticle/RNMParticle.mm
thomson-t
thomson-t previously approved these changes Jun 4, 2026
@jamesnrokt
Copy link
Copy Markdown
Contributor

Also wondering if there's any tests we can be adding here as a regression on this type of error can easily occur

@BrandonStalnaker
Copy link
Copy Markdown
Contributor Author

Also wondering if there's any tests we can be adding here as a regression on this type of error can easily occur

I agree we should add some but currently there's no existing tests in that vein. I figured with the priority it made sense to focus on the fix and then follow up adding tests later. Here's the breakdown of existing tests.

Package root (react-native-mparticle)

  • yarn test / npm test → runs lint only (eslint on js + lib TypeScript). No Jest, no unit assertions on SDK behavior.

Sample app (sample/)

  • Jest – sample/package.json has "test": "jest". The only spec is sample/tests/index.js, which renders the sample Index screen with react-test-renderer and asserts the tree is truthy.
  • iOS (XCTest) – sample/ios/MParticleSampleTests/MParticleSampleTests.m: default RN-style UI test that looks for the “Welcome to React Native!” label and checks for RedBox errors. Not tied to mParticle APIs.

Android library (android/src/test/)

  • IdentityApiTest.java – JUnit tests around identity API usage.
  • MParticleUserTest.java – JUnit tests around MParticleUser / related behavior.
  • testutils/ – mocks (MockMap, MockReadableArray, MockMParticleUser, etc.) used by those tests.

Not automated tests

  • ExpoTestApp/ – manual/demo app; not a test suite wired into root test.

@BrandonStalnaker BrandonStalnaker merged commit 4ae1903 into main Jun 5, 2026
11 checks passed
@BrandonStalnaker BrandonStalnaker deleted the fix/TRIAGE-719-Java-Enum-is-Base-1-unlike-iOS branch June 5, 2026 14:04
@rokt-releases rokt-releases Bot mentioned this pull request Jun 5, 2026
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.

4 participants