feat: track captcha analytics#7822
Open
kernelwhisperer wants to merge 6 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
WalkthroughThis PR introduces ChangesAnalytics tracking rework and CAPTCHA integration
Vercel Cross-Origin-Opener-Policy header
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Turnstile
participant CaptchaWidget
participant trackCaptcha
participant CowAnalytics
Turnstile->>CaptchaWidget: onWidgetLoad
CaptchaWidget->>trackCaptcha: captcha_challenge_started
trackCaptcha->>CowAnalytics: sendEvent
Turnstile->>CaptchaWidget: onSuccess(jwt)
CaptchaWidget->>trackCaptcha: captcha_challenge_solved
trackCaptcha->>CowAnalytics: sendEvent
Turnstile->>CaptchaWidget: onError / onExpire / onUnsupported
CaptchaWidget->>trackCaptcha: captcha_challenge_failed(reason)
trackCaptcha->>CowAnalytics: sendEvent
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
kernelwhisperer
force-pushed
the
feat/captcha-analytics
branch
from
July 7, 2026 15:22
167762f to
84ba4c3
Compare
Deploying explorer-dev with
|
| Latest commit: |
60b2a0f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://df84b8dd.explorer-dev-dxz.pages.dev |
| Branch Preview URL: | https://feat-captcha-analytics.explorer-dev-dxz.pages.dev |
Deploying swap-dev with
|
| Latest commit: |
60b2a0f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d32d34db.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://feat-captcha-analytics.swap-dev-5u6.pages.dev |
kernelwhisperer
force-pushed
the
feat/captcha-analytics
branch
from
July 8, 2026 07:47
84ba4c3 to
1badc1d
Compare
kernelwhisperer
force-pushed
the
feat/captcha-analytics
branch
from
July 8, 2026 08:11
1badc1d to
024868d
Compare
## Summary - Sets `Cross-Origin-Opener-Policy` to `unsafe-none` for non-production Vercel deployments. - Keeps production on `same-origin-allow-popups`. ## Why Google Tag Assistant preview can lose its cross-origin opener/debug connection when the preview page sends a stricter COOP header. This branch gives us an isolated Vercel preview URL to verify whether relaxing COOP fixes the Tag Assistant connection without changing the original captcha analytics PR branch. ## Preview - https://swap-dev-git-agent-gtm-tag-assistant-coop-preview-cowswap-dev.vercel.app/?gtm_debug=x - Verified response header: `cross-origin-opener-policy: unsafe-none` ## Checks - `pnpm exec prettier --check apps/cowswap-frontend/vercel.ts` - `pnpm exec eslint apps/cowswap-frontend/vercel.ts` - `pnpm exec tsc --noEmit --pretty false --skipLibCheck --moduleResolution node16 --module Node16 --target ES2022 apps/cowswap-frontend/vercel.ts` - `curl -sI 'https://swap-dev-git-agent-gtm-tag-assistant-coop-preview-cowswap-dev.vercel.app/?gtm_debug=x'`
kernelwhisperer
marked this pull request as ready for review
July 9, 2026 14:14
![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 (3)
apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx (2)
141-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude
errorCodein the analytics event.
onErrorreceiveserrorCodeand logs it, but doesn't pass it totrackCaptcha. Including it as a custom param would makecaptcha_challenge_failedevents withreason: 'turnstileError'actionable in analytics dashboards without cross-referencing logs.♻️ Pass errorCode to trackCaptcha
onError={(errorCode) => { exchangeRequestIdRef.current += 1 logCaptcha.error('Challenge errored', { errorCode, hostname: window.location.hostname }) - trackCaptcha({ action: 'captcha_challenge_failed', reason: 'turnstileError' }) + trackCaptcha({ action: 'captcha_challenge_failed', reason: 'turnstileError', errorCode }) setCaptchaJwt(null) }}🤖 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 `@apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx` around lines 141 - 147, The CaptchaWidget.container.tsx onError handler logs errorCode but does not send it with the captcha analytics event, so the failure data is incomplete. Update the onError callback in CaptchaWidget.container to pass errorCode through to trackCaptcha as an additional custom param alongside action and reason, keeping the existing logCaptcha.error call unchanged.
29-31: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider memoizing
trackCaptcha.
createCowTrackeris called in the component body, producing a new closure on every render. SinceTurnstileuseskey={siteKey}, callbacks are refreshed onsiteKeychange regardless, so this is not a bug — but wrapping inuseMemokeyed onsiteKeyavoids needless allocations and makes theenabled-dependency explicit.♻️ Wrap in useMemo
+ const trackCaptcha = useMemo( + () => createCowTracker(CowSwapAnalyticsCategory.CAPTCHA, { + enabled: siteKey !== TURNSTILE_DEMO_INTERACTIVE_SITE_KEY, + }), + [siteKey] + ) - const trackCaptcha = createCowTracker(CowSwapAnalyticsCategory.CAPTCHA, { - enabled: siteKey !== TURNSTILE_DEMO_INTERACTIVE_SITE_KEY, - })🤖 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 `@apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx` around lines 29 - 31, The `trackCaptcha` callback in `CaptchaWidget.container` is recreated on every render because `createCowTracker` is called directly in the component body. Wrap the `createCowTracker(CowSwapAnalyticsCategory.CAPTCHA, { enabled: siteKey !== TURNSTILE_DEMO_INTERACTIVE_SITE_KEY })` call in `useMemo`, keyed on `siteKey`, so the closure is only rebuilt when the site key changes and the `enabled` dependency is explicit.libs/analytics/src/CowAnalytics.ts (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant union member in
AnalyticsEvent.
EventOptions & Record<string, unknown>is assignable toEventOptions, so the third union member adds no new type-level coverage. The union collapses tostring | EventOptions. If the intent is to signal that arbitrary extra keys are allowed, a comment or a dedicated branded type would be clearer than a redundant intersection.♻️ Simplify the union
-export type AnalyticsEvent = string | EventOptions | (EventOptions & Record<string, unknown>) +export type AnalyticsEvent = string | EventOptions🤖 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 `@libs/analytics/src/CowAnalytics.ts` at line 3, Simplify the AnalyticsEvent type by removing the redundant EventOptions & Record<string, unknown> union member, since it is already covered by EventOptions. Update the exported type in CowAnalytics to use only string | EventOptions, and if extra arbitrary fields are intended, make that intent explicit elsewhere with a clearer type or comment rather than keeping an equivalent union branch.
🤖 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
`@apps/cowswap-frontend/src/modules/affiliate/analytics/affiliateAnalytics.utils.ts`:
- Around line 31-38: The `trackAffiliateEvent` helper is masking a `chainId`
type mismatch with an `as GtmEvent` cast, allowing invalid numeric values to
reach `analytics.sendEvent`. Fix this in `trackAffiliateEvent` by removing the
cast-based bypass and either narrowing `TrackAffiliateEventParams.chainId` to
`SupportedChainId` or validating/coercing `chainId` before building the event
payload so the `GtmEvent` type is satisfied without unsafe assertions.
---
Nitpick comments:
In
`@apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx`:
- Around line 141-147: The CaptchaWidget.container.tsx onError handler logs
errorCode but does not send it with the captcha analytics event, so the failure
data is incomplete. Update the onError callback in CaptchaWidget.container to
pass errorCode through to trackCaptcha as an additional custom param alongside
action and reason, keeping the existing logCaptcha.error call unchanged.
- Around line 29-31: The `trackCaptcha` callback in `CaptchaWidget.container` is
recreated on every render because `createCowTracker` is called directly in the
component body. Wrap the `createCowTracker(CowSwapAnalyticsCategory.CAPTCHA, {
enabled: siteKey !== TURNSTILE_DEMO_INTERACTIVE_SITE_KEY })` call in `useMemo`,
keyed on `siteKey`, so the closure is only rebuilt when the site key changes and
the `enabled` dependency is explicit.
In `@libs/analytics/src/CowAnalytics.ts`:
- Line 3: Simplify the AnalyticsEvent type by removing the redundant
EventOptions & Record<string, unknown> union member, since it is already covered
by EventOptions. Update the exported type in CowAnalytics to use only string |
EventOptions, and if extra arbitrary fields are intended, make that intent
explicit elsewhere with a clearer type or comment rather than keeping an
equivalent union branch.
🪄 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: a4f68aa5-6f3e-42ff-9343-5cbe96d382e8
📒 Files selected for processing (14)
apps/cowswap-frontend/jest.setup.tsapps/cowswap-frontend/src/common/analytics/types.tsapps/cowswap-frontend/src/modules/affiliate/analytics/affiliateAnalytics.test.tsapps/cowswap-frontend/src/modules/affiliate/analytics/affiliateAnalytics.utils.tsapps/cowswap-frontend/src/modules/affiliate/hooks/useAffiliatePartnerCodeCreate.test.tsxapps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsxapps/cowswap-frontend/vercel.tslibs/analytics/src/CowAnalytics.tslibs/analytics/src/createCowTracker.tslibs/analytics/src/gtm/CowAnalyticsGtm.test.tslibs/analytics/src/gtm/CowAnalyticsGtm.tslibs/analytics/src/index.tslibs/analytics/src/noop/createNoopCowAnalytics.tslibs/common-utils/src/logger.ts
💤 Files with no reviewable changes (1)
- apps/cowswap-frontend/src/modules/affiliate/hooks/useAffiliatePartnerCodeCreate.test.tsx
Comment on lines
31
to
38
| export function trackAffiliateEvent({ analytics, action, chainId, ...customParams }: TrackAffiliateEventParams): void { | ||
| try { | ||
| analytics.sendEvent( | ||
| compactRecord({ | ||
| category: CowSwapAnalyticsCategory.AFFILIATE, | ||
| action, | ||
| chainId, | ||
| ...customParams, | ||
| }) as GtmEvent<CowSwapAnalyticsCategory.AFFILIATE>, | ||
| ) | ||
| } catch (error) { | ||
| logAffiliate('Failed to send analytics event', { action, error }) | ||
| } | ||
| analytics.sendEvent({ | ||
| category: CowSwapAnalyticsCategory.AFFILIATE, | ||
| action, | ||
| chainId, | ||
| ...customParams, | ||
| } as GtmEvent<CowSwapAnalyticsCategory.AFFILIATE>) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
as GtmEvent cast bypasses chainId type mismatch.
TrackAffiliateEventParams.chainId is typed SupportedChainId | number, but EventOptions.chainId expects SupportedChainId. The cast silently widens invalid numeric values through to the analytics payload. Either narrow the parameter type or validate at the boundary.
🛡️ Narrow chainId type instead of casting
interface TrackAffiliateEventParams {
analytics: CowAnalytics
action: AffiliateAnalyticsAction
- chainId?: SupportedChainId | number
+ chainId?: SupportedChainId
[key: string]: unknown
}If raw number support is intentional, validate before sending:
export function trackAffiliateEvent({ analytics, action, chainId, ...customParams }: TrackAffiliateEventParams): void {
+ if (chainId !== undefined && !Object.values(SupportedChainId).includes(chainId as SupportedChainId)) {
+ return
+ }
analytics.sendEvent({
category: CowSwapAnalyticsCategory.AFFILIATE,
action,
chainId,
...customParams,
} as GtmEvent<CowSwapAnalyticsCategory.AFFILIATE>)
}🤖 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
`@apps/cowswap-frontend/src/modules/affiliate/analytics/affiliateAnalytics.utils.ts`
around lines 31 - 38, The `trackAffiliateEvent` helper is masking a `chainId`
type mismatch with an `as GtmEvent` cast, allowing invalid numeric values to
reach `analytics.sendEvent`. Fix this in `trackAffiliateEvent` by removing the
cast-based bypass and either narrowing `TrackAffiliateEventParams.chainId` to
`SupportedChainId` or validating/coercing `chainId` before building the event
payload so the `GtmEvent` type is satisfied without unsafe assertions.
fairlighteth
self-requested a review
July 9, 2026 14:45
fairlighteth
reviewed
Jul 9, 2026
fairlighteth
left a comment
fairlighteth
left a comment
Contributor
There was a problem hiding this comment.
⚠️ AI Review (Codex GPT-5, worked 10m): question on reserved GTM custom-param names
Finding: [QUESTION] Should custom analytics params protect GTM’s reserved names?
- Location:
libs/analytics/src/gtm/CowAnalyticsGtm.ts:264 - I do not see a current regression in this PR: the new captcha/affiliate events use custom fields like
reason,result, andfailureReason. - The edge case is the new generic custom-param path. GTM uses
eventas the event name, and this code builds official fields first, then spreadsgetAdditionalEventParams(...)last. A future caller passing{ event: 'wrong_name' }could overwrite the canonicalevent: event.action. - This pattern is not new only in this PR:
toGtmEvent()has a similar “official fields first, extra fields last” shape. The difference is that this PR makes arbitrary custom params a first-class path throughAnalyticsEvent/createCowTracker, so this may be a good moment to define the reserved-key behavior centrally.
Suggested fix
- Filter reserved GTM output keys like
eventfrom custom params, or spread custom params before the official GTM fields so official fields always win. - Add a small
CowAnalyticsGtmunit test showing{ action: 'captcha_challenge_failed', event: 'wrong_name' }still sendsevent: 'captcha_challenge_failed'.
Review scope and related context
This is separate from existing review comments:
- CodeRabbit already covers the affiliate
chainIdcast. - CodeRabbit’s captcha
errorCodenote is about richer analytics metadata; this question is about protecting reserved GTM output fields. - This is not framed as a current PR regression, only as an API-contract question now that custom params are easier to send.
🤖 Prompt for AI agents
Verify this finding against current code. Fix only if still valid, keep the change minimal, and validate with a targeted unit test.
Context:
- libs/analytics/src/CowAnalytics.ts now allows custom params through AnalyticsEvent.
- libs/analytics/src/createCowTracker.ts forwards arbitrary custom params.
- libs/analytics/src/gtm/CowAnalyticsGtm.ts spreads getAdditionalEventParams(gtmEvent) after canonical fields, so a custom `event` key could override the GTM event name.
- toGtmEvent() has a similar shape, so prefer a central reserved-key policy if practical.
Generated using the pr-review skill from the CoW Protocol skills repo.
shoom3301
reviewed
Jul 13, 2026
| const [siteKey, setSiteKey] = useState(TURNSTILE_SITE_KEY) | ||
| const theme = useTheme() | ||
|
|
||
| const trackCaptcha = createCowTracker(CowSwapAnalyticsCategory.CAPTCHA, { |
Collaborator
There was a problem hiding this comment.
createCowTracker returns a new function every time, so it should be memoized
shoom3301
reviewed
Jul 13, 2026
| // Isolates the browsing context so cross-origin pages cannot access window.opener, | ||
| // while still allowing this page to open popups (needed for wallet connections). | ||
| { key: 'Cross-Origin-Opener-Policy', value: 'same-origin-allow-popups' }, | ||
| // Preview deploys use unsafe-none so Tag Assistant can keep its cross-origin debug connection. |
Collaborator
There was a problem hiding this comment.
It might be a bit dangerous in case if something depends on Cross-Origin-Opener-Policy and we don't notice it locally because it's unsafe-none
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
3 participants
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.
Summary
Adds captcha analytics and simplifies shared analytics tracking.
createCowTracker()helper for category-scoped analytics eventsreasonconsole.debugwithlogAnalyticsSummary by CodeRabbit
New Features
Bug Fixes