feat: braze ecommerce events onboarding#3076
Open
ItsSudip wants to merge 8 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA new ChangesBraze Recommended Ecommerce Events
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in path in the Braze web integration to emit Braze “recommended ecommerce events” (e.g., ecommerce.order_placed) with a structured properties payload, metadata pass-through, and warnings for missing required fields.
Changes:
- Introduces
ecommerceUtil.jsto map RS ecommerce track events to Braze recommended event names and build compliant event properties (includingproducts[],source,action, andmetadata). - Updates Braze
track()to use the recommended ecommerce mapping whenuseRecommendedEcommerceEventsis enabled, otherwise falling back to legacy behavior. - Adds unit tests covering mapping behavior, warnings for missing required fields, source override, and legacy fall-through.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| packages/analytics-js-integrations/src/integrations/Braze/ecommerceUtil.js | New mapping + payload builder for Braze recommended ecommerce events, including required-field warnings and metadata routing. |
| packages/analytics-js-integrations/src/integrations/Braze/browser.js | Adds feature flag and routes eligible ecommerce track events through the new recommended-event path. |
| packages/analytics-js-integrations/tests/integrations/Braze/browser.test.js | Adds coverage for the new recommended ecommerce events behavior and legacy fall-through scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/analytics-js-integrations/__tests__/integrations/Braze/browser.test.js (1)
1090-1094: 💤 Low valueConsider mocking before instantiation for clarity.
The
mockBrazeSDK()call happens after creating the Braze instance. While this works correctly (the constructor doesn't accessglobalThis.braze), it's unconventional and could confuse maintainers. The typical pattern is to set up mocks before instantiating the object under test.♻️ Suggested refactor
const buildBraze = (configOverrides = {}) => { + mockBrazeSDK(); const braze = new Braze({ ...baseConfig, ...configOverrides }, {}, {}); - mockBrazeSDK(); return braze; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/analytics-js-integrations/__tests__/integrations/Braze/browser.test.js` around lines 1090 - 1094, The buildBraze function is calling mockBrazeSDK() after instantiating the Braze object, but the conventional testing pattern is to set up mocks before object instantiation. Move the mockBrazeSDK() call to occur before the new Braze() constructor call within the buildBraze function to follow standard test setup patterns and improve code clarity for maintainers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/analytics-js-integrations/src/integrations/Braze/ecommerceUtil.js`:
- Around line 196-200: The condition that marks top-level product fields as
consumed for cart_updated events is incorrect because it checks only the event
type (which is always true for cart_updated) rather than whether a products
array actually exists. When a products array is present, the top-level product
fields are not used and should not be marked as consumed, preventing data loss.
Refactor the condition in the block where
consumedKeysFromMapping(ECOMMERCE_PRODUCT_MAPPING) is called to additionally
check whether a products array is actually absent in the properties before
consuming those top-level product field keys.
---
Nitpick comments:
In
`@packages/analytics-js-integrations/__tests__/integrations/Braze/browser.test.js`:
- Around line 1090-1094: The buildBraze function is calling mockBrazeSDK() after
instantiating the Braze object, but the conventional testing pattern is to set
up mocks before object instantiation. Move the mockBrazeSDK() call to occur
before the new Braze() constructor call within the buildBraze function to follow
standard test setup patterns and improve code clarity for maintainers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2c987b33-411b-464d-850e-a01dbba578be
📒 Files selected for processing (3)
packages/analytics-js-integrations/__tests__/integrations/Braze/browser.test.jspackages/analytics-js-integrations/src/integrations/Braze/browser.jspackages/analytics-js-integrations/src/integrations/Braze/ecommerceUtil.js
size-limit report 📦
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/analytics-js-integrations/__tests__/integrations/Braze/browser.test.js`:
- Line 1427: Replace all instances of window.braze with globalThis.braze in the
test assertions to maintain consistency with the actual integration code that
dispatches via globalThis.braze. This change is necessary in the assertion at
line 1427 and the range at lines 1440-1446 to ensure the tests remain compatible
with service worker environments where the window object does not exist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a64b8feb-8842-450e-9d83-41796bbc5b96
📒 Files selected for processing (3)
packages/analytics-js-integrations/.size-limit.mjspackages/analytics-js-integrations/__tests__/integrations/Braze/browser.test.jspackages/analytics-js-integrations/src/integrations/Braze/ecommerceUtil.js
✅ Files skipped from review due to trivial changes (1)
- packages/analytics-js-integrations/.size-limit.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/analytics-js-integrations/src/integrations/Braze/ecommerceUtil.js
Comment on lines
+209
to
+214
| case FIELD_TYPE.INTEGER: | ||
| // `value % 1 === 0` is true only for whole numbers; NaN/Infinity yield NaN and fail. | ||
| return typeof value === 'number' && value % 1 === 0; | ||
| case FIELD_TYPE.FLOAT: | ||
| return typeof value === 'number' && !Number.isNaN(value); | ||
| case FIELD_TYPE.STRING_ARRAY: |
Comment on lines
+329
to
+331
| .map(raw => { | ||
| const item = raw && typeof raw === 'object' ? raw : {}; | ||
| const product = removeUndefinedAndNullAndEmptyValues( |
|
achettyiitr
approved these changes
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
Adds support for Braze's recommended ecommerce events in the web device mode integration, mirroring the cloud-mode mapping. Gated behind a new opt-in config flag
useRecommendedEcommerceEvents(default off); when off, the existing legacy custom-event /logPurchasepath is unchanged.When enabled, mapped RudderStack ecommerce events are emitted via
globalThis.braze.logCustomEvent(name, properties)(includingOrder Completed, which maps toecommerce.order_placedrather thanlogPurchase).Event mapping (RS → Braze):
ecommerce.product_viewedproducts[]ecommerce.cart_updatedaction: 'add', single-product wrapecommerce.cart_updatedaction: 'remove'ecommerce.checkout_startedecommerce.order_placedecommerce.order_refundedecommerce.order_cancelledCart Viewed/Cart Updatedare intentionally unmapped and fall through to the legacy path.Changes:
ecommerceUtil.js(new) — event-name resolution, per-event + per-product field mappings,products[]construction, unmapped-key →metadatapass-through, andsourcederivation ('web', with validproperties.sourcetaking precedence).browser.js— reads theuseRecommendedEcommerceEventsflag and routes mapped events through the new builder.logger.warnand still calllogCustomEvent.products[]and other empty/null fields are stripped from the payload before sending (the missing-field warning still fires).Linear task
INT-4303
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
Summary by CodeRabbit