fix: conversion not being tracked properly with builder action #4153
Conversation
🦋 Changeset detectedLatest commit: 2ff9c7f The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx test @snippet/angular-17-ssr |
❌ Failed | 6m 23s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-01 05:38:09 UTC
midhunadarvin
changed the title
contentId to builder class
sidmohanty11
left a comment
sidmohanty11
left a comment
There was a problem hiding this comment.
looks good to me, can we add some tests for this?
there can be 3 e2e tests that we add:
- one page with the button and button calls track and we check amount,variationid,contentid etc
- one page with section and inside that a button calling track
- one page with symbol and calling track inside that
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 21273596 | Triggered | Generic High Entropy Secret | 9245f6d | packages/sdks-tests/src/specs/track-conversion.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
| export function setTestsFromUrl() { | ||
| if (typeof window === 'undefined') return; | ||
|
|
||
| try { | ||
| // Use native URL object to parse current page URL | ||
| const params = parseUrlParams(window.location.href); | ||
|
|
||
| // Look for parameters that start with 'builder.tests.' | ||
| for (const [key, value] of params) { | ||
| if (key.startsWith(`${testCookiePrefix}.`)) { | ||
| const testKey = key.replace(`${testCookiePrefix}.`, ''); | ||
| setTestCookie(testKey, value); | ||
| } | ||
| } | ||
| } catch (e) { | ||
| console.debug('Error parsing tests from URL', e); | ||
| } |
There was a problem hiding this comment.
all of these functions are not used anymore right?
There was a problem hiding this comment.
yes, not used anymore. Thanks for catching. removed the unused functions
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| type: 'conversion', | ||
| apiHost: builderContext?.apiHost, | ||
| apiKey: builderContext?.apiKey || '', | ||
| amount: amount || undefined, |
There was a problem hiding this comment.
Bug: Zero Amounts Falsely Converted to Undefined
The trackConversion function incorrectly converts a 0 amount to undefined because amount || undefined treats 0 as falsy. This prevents valid zero-amount conversions from being tracked as intended.
| export function initializeGlobalBuilderContext(): void { | ||
| // Detect environment and get the appropriate global object | ||
| const isServer = typeof window === 'undefined'; | ||
| const globalObject = isServer | ||
| ? typeof globalThis !== 'undefined' | ||
| ? globalThis | ||
| : (function () { | ||
| try { | ||
| return global; | ||
| } catch (e) { | ||
| return {}; | ||
| } | ||
| })() | ||
| : window; | ||
|
|
||
| if ((globalObject as any).GlobalBuilderContext) { | ||
| // if already exists, don't re-initialize | ||
| return; | ||
| } |
There was a problem hiding this comment.
As discussed on the call, I recommend avoiding a new global context, as maintaining it would create unpredictable problems across SDKs.
- SSR frameworks could behave in unexpected ways (two different global contexts, also cookie isn't present on server)
- This will be an extra addition to the customer’s HTML as we're injecting inlined scripts
- It’s extra code that we have to maintain.
The simpler solution is to expose just track and trackConversion on the Gen2 SDK globals obj and update every evaluate call to include apiKey and contentId.
Description
Gen 1 :
contentIdon the builder classtrackConversionmethod to figure outcontentIdandvariantionIdif they are undefined from the SDK value and cookies.Gen 2:
BuilderGlobalContextclass to share context variables between components and helper functions in the gen2 sdksIssue :
https://www.loom.com/share/6bcecbfc1b284331a867f162223e4c20
Loom - Gen 1 fix
https://www.loom.com/share/3712e28c436b4d9eac9bd3e6a2a68325
Loom - Gen 2 fix
https://www.loom.com/share/fc3b894372d244d8b0c2a14e67ed93ec
Note
Ensure conversions capture contentId and variationId via instance/global context and cookies, adding tracking APIs, inlined context setup, and comprehensive tests.
Builder.contentId(reactively) and set it in React components on content load/inline render.Builder.trackConversionto derivecontentIdfrom instance andvariationIdfrom test cookie; ignore variant when equal to content.initializeGlobalBuilderContext) and inject setup script fromContentto storeapiKey,apiHost, andcontentId.builderglobals withtrackandtrackConversion, using global context andgetTestCookie.watch:sdkscript; document alternative Yarn link workflow in SDK develop docs.@builder.io/*packages.Written by Cursor Bugbot for commit 2ff9c7f. This will update automatically on new commits. Configure here.