-
Notifications
You must be signed in to change notification settings - Fork 491
feat: integrate Impact telemetry with checkout attribution for subscriptions #8688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
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:
📝 WalkthroughWalkthroughAdds async checkout attribution capture (URL + external queue), persists merged attribution, integrates attribution into checkout and telemetry flows, introduces ImpactTelemetryProvider, updates types/global window typings, and adds extensive tests validating parsing, persistence, queue interactions, and identity hashing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Checkout as Checkout Flow
participant AttribUtil as Attribution Utility
participant Queue as Impact Queue (window.ire)
participant Storage as Local Attribution State
participant Provider as ImpactTelemetryProvider
User->>Checkout: start subscription or visit page
Checkout->>AttribUtil: getCheckoutAttribution() (await)
AttribUtil->>AttribUtil: parse URL params & read stored attribution
AttribUtil->>Queue: request generated click id (if queue present)
Queue-->>AttribUtil: im_ref / click id (or timeout)
AttribUtil->>Storage: merge & persist attribution (if changed)
AttribUtil-->>Checkout: resolved attribution object
Checkout->>Checkout: include attribution in checkout POST
User->>Provider: trackPageView(path)
Provider->>AttribUtil: captureCheckoutAttributionFromSearch(path)
AttribUtil->>Storage: merge & persist attribution
Provider->>Provider: resolve current user & hash email
Provider->>Queue: ire.identify({customerId, customerEmail})
Queue-->>Provider: acknowledge identify
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/10/2026, 01:03:03 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ✅ 521 passed, 0 failed · 3 flaky 📊 Browser Reports
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive checkout attribution tracking to capture marketing attribution data (UTM parameters, Impact affiliate tracking, and Google Ads click IDs) during user sessions. The attribution data is persisted to localStorage, captured on page views via a dedicated telemetry provider, and sent to the backend during subscription checkout flows.
Changes:
- Expanded attribution tracking from just Google Ads click IDs (gclid, gbraid, wbraid) to include UTM parameters and Impact affiliate tracking (im_ref)
- Added a new CheckoutAttributionTelemetryProvider that captures attribution data on page views
- Modified subscription checkout flows to send attribution data to the backend API
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/platform/telemetry/utils/checkoutAttribution.ts | Core attribution logic: renamed functions/types, added UTM and Impact tracking, added deduplication logic to prevent unnecessary storage writes |
| src/platform/telemetry/utils/tests/checkoutAttribution.test.ts | Comprehensive test coverage for attribution capture, storage, and deduplication scenarios |
| src/platform/telemetry/types.ts | New CheckoutAttributionMetadata interface with all attribution fields, extended BeginCheckoutMetadata |
| src/platform/telemetry/providers/cloud/CheckoutAttributionTelemetryProvider.ts | New telemetry provider that captures attribution from page view paths |
| src/platform/telemetry/initTelemetry.ts | Registered new CheckoutAttributionTelemetryProvider in telemetry initialization |
| src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.test.ts | Updated tests with new attribution fields |
| src/platform/cloud/subscription/composables/useSubscription.ts | Modified initiateSubscriptionCheckout to send attribution data in request body |
| src/platform/cloud/subscription/composables/useSubscription.test.ts | Added mock for getCheckoutAttribution and verified attribution is sent in checkout requests |
| import { captureCheckoutAttributionFromSearch } from '@/platform/telemetry/utils/checkoutAttribution' | ||
|
|
||
| import type { PageViewMetadata, TelemetryProvider } from '../../types' | ||
|
|
||
| /** | ||
| * Internal cloud telemetry provider used to persist checkout attribution | ||
| * from query parameters during page view tracking. | ||
| */ | ||
| export class CheckoutAttributionTelemetryProvider implements TelemetryProvider { | ||
| trackPageView(_pageName: string, properties?: PageViewMetadata): void { | ||
| const search = this.extractSearchFromPath(properties?.path) | ||
|
|
||
| if (search) { | ||
| captureCheckoutAttributionFromSearch(search) | ||
| return | ||
| } | ||
|
|
||
| if (typeof window !== 'undefined') { | ||
| captureCheckoutAttributionFromSearch(window.location.search) | ||
| } | ||
| } | ||
|
|
||
| private extractSearchFromPath(path?: string): string { | ||
| if (!path || typeof window === 'undefined') return '' | ||
|
|
||
| try { | ||
| const url = new URL(path, window.location.origin) | ||
| return url.search | ||
| } catch { | ||
| const queryIndex = path.indexOf('?') | ||
| return queryIndex >= 0 ? path.slice(queryIndex) : '' | ||
| } | ||
| } | ||
| } |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CheckoutAttributionTelemetryProvider lacks test coverage. While the underlying captureCheckoutAttributionFromSearch function is well-tested, the provider's specific behavior should also be tested, particularly:
- The extractSearchFromPath method with various path formats
- The fallback to window.location.search when no path is provided
- Integration with the trackPageView lifecycle
This is important because the provider has its own logic for extracting search parameters from paths and choosing between the provided path and window.location.search.
a8df879 to
e38875c
Compare
0f32622 to
dafccab
Compare
dafccab to
4fb386d
Compare
72c94e9 to
3eccf3e
Compare
4fb386d to
73c0451
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/platform/telemetry/providers/cloud/CheckoutAttributionTelemetryProvider.ts`:
- Around line 23-33: The SSR guard currently returns early in
extractSearchFromPath and blocks the manual fallback; instead ensure you only
early-return when path is falsy, then attempt to construct new URL only if
typeof window !== 'undefined' (wrap new URL in try/catch), and if window is
undefined or URL construction fails fall back to using path.indexOf('?') to
slice and return the query string; update extractSearchFromPath to perform path
check first, then try the window-dependent URL parsing, and finally the manual
indexOf fallback.
| private extractSearchFromPath(path?: string): string { | ||
| if (!path || typeof window === 'undefined') return '' | ||
|
|
||
| try { | ||
| const url = new URL(path, window.location.origin) | ||
| return url.search | ||
| } catch { | ||
| const queryIndex = path.indexOf('?') | ||
| return queryIndex >= 0 ? path.slice(queryIndex) : '' | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SSR guard on Line 24 also blocks the window-independent fallback path.
When window is undefined but a valid path with query params is provided (e.g. "/checkout?utm_source=google"), the early return on Line 24 prevents reaching the manual indexOf('?') fallback (lines 30–31), which doesn't need window at all. This silently drops attribution params in SSR/test environments.
Move the window guard to only protect the new URL call:
Suggested fix
private extractSearchFromPath(path?: string): string {
- if (!path || typeof window === 'undefined') return ''
+ if (!path) return ''
try {
+ if (typeof window === 'undefined') throw new Error('no window')
const url = new URL(path, window.location.origin)
return url.search
} catch {
const queryIndex = path.indexOf('?')
return queryIndex >= 0 ? path.slice(queryIndex) : ''
}
}🤖 Prompt for AI Agents
In
`@src/platform/telemetry/providers/cloud/CheckoutAttributionTelemetryProvider.ts`
around lines 23 - 33, The SSR guard currently returns early in
extractSearchFromPath and blocks the manual fallback; instead ensure you only
early-return when path is falsy, then attempt to construct new URL only if
typeof window !== 'undefined' (wrap new URL in try/catch), and if window is
undefined or URL construction fails fall back to using path.indexOf('?') to
slice and return the query string; update extractSearchFromPath to perform path
check first, then try the window-dependent URL parsing, and finally the manual
indexOf fallback.
|
Taking draft to make Impact specific |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/platform/cloud/subscription/components/PricingTable.vue`:
- Line 417: The call to getCheckoutAttribution in PricingTable.vue can throw and
currently bubbles up through wrapWithErrorHandlingAsync causing a user-facing
error; wrap the call in a local try/catch (around the getCheckoutAttribution
call that assigns checkoutAttribution) so any failure is swallowed for UX
purposes: on catch set checkoutAttribution to undefined (or a safe default) and
log/debug the error internally (or send non-blocking telemetry), but do not
rethrow or surface a toast; mirror the defensive pattern used in
subscriptionCheckoutUtil.ts to keep this telemetry call non-fatal.
In `@src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.ts`:
- Around line 49-50: getCheckoutAttribution() can throw and currently will block
checkout; wrap the call in a try/catch inside subscriptionCheckoutUtil (where
checkoutAttribution and checkoutPayload are built) so any error is caught,
logged (non-fatal) and you fall back to an empty attribution object (e.g., {}).
Then build checkoutPayload from the safe fallback so attribution failures cannot
prevent the checkout flow.
🧹 Nitpick comments (5)
global.d.ts (1)
8-11:ImpactQueueFunctionis duplicated inImpactTelemetryProvider.ts(lines 6-9).Since this interface is declared globally here, the local re-declaration in the provider file is redundant. Consider removing the local copy and referencing this global type directly.
src/platform/telemetry/providers/cloud/ImpactTelemetryProvider.ts (2)
6-9: DuplicateImpactQueueFunctioninterface — already declared inglobal.d.ts.This local definition is identical to the one in
global.d.ts(lines 8-11). Since the global declaration is ambient and available everywhere, this can be removed.♻️ Remove local duplicate
-interface ImpactQueueFunction { - (...args: unknown[]): void - a?: unknown[][] -} -
37-37: Unhandled rejection risk from fire-and-forget async call.
void this.identifyCurrentUser()discards the promise. While the individual helpers (resolveCustomerIdentity,hashSha1) have their own try/catch, an unexpected throw fromwindow.ire?.()or future code changes could surface as an unhandled promise rejection.🛡️ Add a catch to the voided promise
- void this.identifyCurrentUser() + void this.identifyCurrentUser().catch(() => {})src/platform/telemetry/utils/checkoutAttribution.ts (2)
89-122: Timeout handle leaks if the external callback never fires.If
impactQueue('generateClickId', cb)retains a reference tocbindefinitely and the timeout fires first, the closure (and everything it captures —timeoutHandle,settled,resolve) stays alive as long as the external queue holds the callback. This is unlikely to matter in practice (single invocation, small closure), but worth noting.Separately, the
return awaiton Line 99 is unnecessary for an async function returning a promise directly, though it does marginally improve async stack traces — fine either way.
124-131:captureCheckoutAttributionFromSearchonly persists the click ID, not full attribution.The function name suggests it "captures checkout attribution" broadly, but it only extracts and stores the impact click ID. Other attribution params (UTMs, gclid, etc.) parsed by
readAttributionFromUrlare discarded. If this is intentional (becausegetCheckoutAttributionre-reads them from the current URL at checkout time), consider a brief doc comment clarifying the scoping so future maintainers don't assume all attribution is being persisted here.
| try { | ||
| if (isActiveSubscription.value) { | ||
| const checkoutAttribution = getCheckoutAttribution() | ||
| const checkoutAttribution = await getCheckoutAttribution() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same risk: attribution failure surfaces as a user-facing error on subscription change.
wrapWithErrorHandlingAsync will catch and display the error, but users shouldn't see an error toast because a non-essential telemetry call failed. Wrap defensively, consistent with the fix suggested in subscriptionCheckoutUtil.ts.
🛡️ Proposed fix
- const checkoutAttribution = await getCheckoutAttribution()
+ const checkoutAttribution = await getCheckoutAttribution().catch(
+ () => ({})
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const checkoutAttribution = await getCheckoutAttribution() | |
| const checkoutAttribution = await getCheckoutAttribution().catch( | |
| () => ({}) | |
| ) |
🤖 Prompt for AI Agents
In `@src/platform/cloud/subscription/components/PricingTable.vue` at line 417, The
call to getCheckoutAttribution in PricingTable.vue can throw and currently
bubbles up through wrapWithErrorHandlingAsync causing a user-facing error; wrap
the call in a local try/catch (around the getCheckoutAttribution call that
assigns checkoutAttribution) so any failure is swallowed for UX purposes: on
catch set checkoutAttribution to undefined (or a safe default) and log/debug the
error internally (or send non-blocking telemetry), but do not rethrow or surface
a toast; mirror the defensive pattern used in subscriptionCheckoutUtil.ts to
keep this telemetry call non-fatal.
src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/platform/telemetry/providers/cloud/ImpactTelemetryProvider.ts`:
- Around line 22-29: The trackPageView implementation incorrectly falls back to
window.location.search whenever extractSearchFromPath returns a falsy value
(including empty string), causing browser query params to be used even when a
path was explicitly provided; change the logic in trackPageView so you first
check whether a path was supplied (properties?.path !== undefined &&
properties?.path !== null) and only call captureCheckoutAttributionFromSearch
with the result of extractSearchFromPath(properties.path) if that returned a
non-empty string, otherwise do nothing; only when no path was supplied at all
should you fall back to using window.location.search and call
captureCheckoutAttributionFromSearch(window.location.search). Use the existing
functions trackPageView, extractSearchFromPath, and
captureCheckoutAttributionFromSearch to locate and update the branch.
- Around line 99-116: resolveCustomerIdentity currently calls useCurrentUser()
each time it runs (e.g., from trackPageView), leaking reactive subscriptions and
risking errors outside setup; refactor by calling useCurrentUser() once (or
lazily once) and storing its returned refs on the class instance during
construction or first access, then have resolveCustomerIdentity read from those
stored refs (customerId/email computed values) instead of re-invoking
useCurrentUser(); update the constructor (or a private lazy getter) to
initialize the stored refs and remove the try/catch in resolveCustomerIdentity
so it simply reads storedRef.value?.id ?? EMPTY_CUSTOMER_VALUE and
userEmailRef.value ?? EMPTY_CUSTOMER_VALUE.
- Around line 118-135: Add a short inline comment immediately above the hashSha1
method stating that SHA-1 is required by Impact.com's UTT specification for the
customerEmail field (vendor mandate), so the use of SHA-1 here is intentional
and must not be replaced; reference the function name hashSha1 and ensure the
comment is clear that this is an external requirement from Impact.com.
| trackPageView(_pageName: string, properties?: PageViewMetadata): void { | ||
| const search = this.extractSearchFromPath(properties?.path) | ||
|
|
||
| if (search) { | ||
| captureCheckoutAttributionFromSearch(search) | ||
| } else if (typeof window !== 'undefined') { | ||
| captureCheckoutAttributionFromSearch(window.location.search) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback to window.location.search fires even when a path is explicitly provided.
When properties.path is set to a valid path without query parameters (e.g., "/checkout"), extractSearchFromPath returns '' (falsy), so the code falls through to window.location.search. This means attribution is captured from the browser URL even though a specific path was intentionally provided — likely not the desired behavior.
If the intent is "use the provided path's query string, or fall back to the current URL only when no path is provided," the condition should distinguish between "no path given" and "path given but has no query":
Suggested fix
trackPageView(_pageName: string, properties?: PageViewMetadata): void {
- const search = this.extractSearchFromPath(properties?.path)
-
- if (search) {
- captureCheckoutAttributionFromSearch(search)
- } else if (typeof window !== 'undefined') {
- captureCheckoutAttributionFromSearch(window.location.search)
- }
+ const search = properties?.path
+ ? this.extractSearchFromPath(properties.path)
+ : (typeof window !== 'undefined' ? window.location.search : '')
+
+ if (search) {
+ captureCheckoutAttributionFromSearch(search)
+ }
void this.identifyCurrentUser()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| trackPageView(_pageName: string, properties?: PageViewMetadata): void { | |
| const search = this.extractSearchFromPath(properties?.path) | |
| if (search) { | |
| captureCheckoutAttributionFromSearch(search) | |
| } else if (typeof window !== 'undefined') { | |
| captureCheckoutAttributionFromSearch(window.location.search) | |
| } | |
| trackPageView(_pageName: string, properties?: PageViewMetadata): void { | |
| const search = properties?.path | |
| ? this.extractSearchFromPath(properties.path) | |
| : (typeof window !== 'undefined' ? window.location.search : '') | |
| if (search) { | |
| captureCheckoutAttributionFromSearch(search) | |
| } |
🤖 Prompt for AI Agents
In `@src/platform/telemetry/providers/cloud/ImpactTelemetryProvider.ts` around
lines 22 - 29, The trackPageView implementation incorrectly falls back to
window.location.search whenever extractSearchFromPath returns a falsy value
(including empty string), causing browser query params to be used even when a
path was explicitly provided; change the logic in trackPageView so you first
check whether a path was supplied (properties?.path !== undefined &&
properties?.path !== null) and only call captureCheckoutAttributionFromSearch
with the result of extractSearchFromPath(properties.path) if that returned a
non-empty string, otherwise do nothing; only when no path was supplied at all
should you fall back to using window.location.search and call
captureCheckoutAttributionFromSearch(window.location.search). Use the existing
functions trackPageView, extractSearchFromPath, and
captureCheckoutAttributionFromSearch to locate and update the branch.
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.2 kB (baseline 22.2 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 855 kB (baseline 855 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 10 added / 10 removed Panels & Settings — 451 kB (baseline 451 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 751 B (baseline 751 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 36.6 kB (baseline 36.6 kB) • ⚪ 0 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 2.12 MB (baseline 2.12 MB) • 🔴 +186 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 235 kB (baseline 237 kB) • 🟢 -1.72 kBHelpers, composables, and utility bundles
Status: 13 added / 13 removed Vendor & Third-Party — 8.77 MB (baseline 8.77 MB) • 🟢 -23 BExternal libraries and shared vendor chunks
Status: 1 added / 1 removed Other — 7.21 MB (baseline 7.21 MB) • 🔴 +59 BBundles that do not match a named category
Status: 67 added / 67 removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
ComfyUI_frontend/src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.ts
Line 69 in 283fbb4
| body: JSON.stringify(checkoutPayload) |
This now always sends a JSON request body for checkout creation, but the current registry schema still defines both createCloudSubscriptionCheckout and createCloudSubscriptionCheckoutTier as requestBody?: never (see @comfyorg/registry-types/src/comfyRegistryTypes.ts), so deployments that enforce that schema can reject these calls with 400 and block users from starting checkout; please add a compatibility fallback (e.g., retry without body) or ship this only with the backend contract update.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Implement Impact telemetry and checkout attribution through cloud subscription checkout flows.
This PR adds Impact.com tracking support and carries attribution context from landing-page visits into subscription checkout requests so conversion attribution can be validated end-to-end.
ImpactTelemetryProviderduring cloud telemetry initialization.ire) and load the Universal Tracking Tag script once.ire('identify', ...)on page views with dynamiccustomerIdand SHA-1customerEmail(or empty strings when unknown).im_ref, UTM fields, and Google click IDs, with local persistence across navigation.ire('generateClickId')with a timeout and fall back to URL/local attribution when unavailable./customers/cloud-subscription-checkout/customers/cloud-subscription-checkout/{tier}Tradeoffs / constraints:
Screenshots