feat: add analytic scripts#2208
Conversation
wilsonrivera
requested review from
JivusAyrus,
StarpTech and
thisisnithin
as code owners
WalkthroughAdds Osano consent loading, Google Tag Manager and ActiveCampaign consent-aware script components, updates _document to use these components, and changes analytics environment variables (adds Osano, GTM, ActiveCampaign; removes LinkedIn). All integrations are gated for production and Osano consent. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
studio/.env.local.example (2)
20-24: Standardize “Osano” naming and avoid future confusion.Service is “Osano”, but the key uses “OSANA”. Rename now (new feature) to keep consistency across code and docs.
-# analytics config -NEXT_PUBLIC_OSANA_SCRIPT_ID= -NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID= -NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT= +# analytics config +NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT= +NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID= +NEXT_PUBLIC_OSANO_SCRIPT_ID=Follow-up: update code that reads
NEXT_PUBLIC_OSANA_SCRIPT_IDtoNEXT_PUBLIC_OSANO_SCRIPT_ID.
21-23: Satisfy dotenv-linter ordering warnings.Alphabetize keys within the section as suggested by the linter.
- NEXT_PUBLIC_OSANA_SCRIPT_ID= - NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID= - NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT= + NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT= + NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID= + NEXT_PUBLIC_OSANO_SCRIPT_ID=studio/src/components/layout/analytics/osana-script.tsx (3)
1-3: Align env var name to “OSANO”.Keep naming consistent with the provider and the env example.
-export function OsanaScript() { - const osanaScriptId = process.env.NEXT_PUBLIC_OSANA_SCRIPT_ID; +export function OsanaScript() { + const osanoScriptId = process.env.NEXT_PUBLIC_OSANO_SCRIPT_ID; - if (!osanaScriptId || process.env.NODE_ENV !== 'production') { + if (!osanoScriptId || process.env.NODE_ENV !== 'production') { return null; }And:
- src={`https://cmp.osano.com/${osanaScriptId}/osano.js`} + src={`https://cmp.osano.com/${osanoScriptId}/osano.js`}
10-12: Tighten script attrs.
type="text/javascript"is unnecessary (HTML5 default). Prefer one of async or defer (both set is redundant; async wins).- type="text/javascript" - async - defer + defer
7-15: Consider Next.js Script for early, ordered loading.Using
next/scriptwithstrategy="beforeInteractive"ensures the CMP loads before other scripts without manual attribute juggling.+import Script from "next/script"; ... - return ( - <script - id="osano-cmp" - defer - src={`https://cmp.osano.com/${osanoScriptId}/osano.js`} - /> - ); + return ( + <Script + id="osano-cmp" + src={`https://cmp.osano.com/${osanoScriptId}/osano.js`} + strategy="beforeInteractive" + /> + );studio/src/components/layout/analytics/gtm-script.tsx (1)
9-21: Give the GTM loader a unique id and drop obsolete type attribute.Avoid id collisions with Osano/GTM consent script and remove
type="text/javascript".Applied in the diff above via
id="gtm-base"; notypeattr needed.studio/src/pages/_document.tsx (1)
125-125: Add per-request CSP nonce to the ActiveCampaign inline scriptGood: production/env gating and Osano-driven lazy load. If you enforce a CSP without 'unsafe-inline', generate a per-request nonce in middleware/server, set the CSP header (script-src 'nonce-') and an X-Nonce header, read that nonce in pages/_document (getInitialProps) and apply it to inline scripts and .
File: studio/src/pages/_document.tsx (around line 125)
- <script + <script nonce={props.cspNonce} dangerouslySetInnerHTML={{ __html: `(...)` }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
studio/.env.local.example(1 hunks)studio/src/components/layout/analytics/active-campaign-script.tsx(1 hunks)studio/src/components/layout/analytics/gtm-script.tsx(1 hunks)studio/src/components/layout/analytics/osana-script.tsx(1 hunks)studio/src/pages/_document.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
studio/src/pages/_document.tsx (3)
studio/src/components/layout/analytics/osana-script.tsx (1)
OsanaScript(1-16)studio/src/components/layout/analytics/gtm-script.tsx (2)
GtmScript(1-60)GtmNoScript(62-78)studio/src/components/layout/analytics/active-campaign-script.tsx (1)
ActiveCampaignScript(3-59)
🪛 Biome (2.1.2)
studio/src/components/layout/analytics/gtm-script.tsx
[error] 14-14: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 25-25: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
studio/src/components/layout/analytics/active-campaign-script.tsx
[error] 11-11: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 ast-grep (0.38.6)
studio/src/components/layout/analytics/gtm-script.tsx
[warning] 13-13: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
[warning] 24-24: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
studio/src/components/layout/analytics/active-campaign-script.tsx
[warning] 10-10: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 dotenv-linter (3.3.0)
studio/.env.local.example
[warning] 22-22: [UnorderedKey] The NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID key should go before the NEXT_PUBLIC_OSANA_SCRIPT_ID key
(UnorderedKey)
[warning] 23-23: [UnorderedKey] The NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT key should go before the NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
studio/src/components/layout/analytics/gtm-script.tsx (3)
62-77: LGTM on noscript fallback.The hidden iframe is standard for GTM noscript usage; gating on production and presence of GTM ID is correct.
44-55: Osano event names are valid; no change required.Osano exposes category-specific events (osano-cm-analytics, osano-cm-marketing — they fire when a category becomes ACCEPT) plus lifecycle events (osano-cm-initialized, osano-cm-consent-changed). Use Osano.cm.getConsent() or osano-cm-consent-changed for the full consent state. This file already listens to the category events and osano-cm-consent-changed, so the current handlers are appropriate.
24-55: ActiveCampaign loader present — no action required.
Found loadActiveCampaign in studio/src/components/layout/analytics/active-campaign-script.tsx; ActiveCampaignScript is included in studio/src/pages/_document.tsx and gtm-script.tsx wires Osano events to call it.studio/src/pages/_document.tsx (2)
106-106: GTM noscript placement looks goodAt top of body and production-gated via component logic. LGTM.
3-5: Standardize naming: “Osano”, not “Osana” (env var, file, symbol, imports)
- Matches found: studio/.env.local.example:21 (NEXT_PUBLIC_OSANA_SCRIPT_ID), studio/src/components/layout/analytics/osana-script.tsx:2 (process.env.NEXT_PUBLIC_OSANA_SCRIPT_ID).
- Action: Rename NEXT_PUBLIC_OSANA_SCRIPT_ID → NEXT_PUBLIC_OSANO_SCRIPT_ID; rename file osana-script.tsx → osano-script.tsx and exported symbol OsanaScript → OsanoScript; update all imports/usages (e.g., studio/src/pages/_document.tsx).
- NEXT_PUBLIC_OSANA_SCRIPT_ID + NEXT_PUBLIC_OSANO_SCRIPT_IDstudio/src/components/layout/analytics/active-campaign-script.tsx (2)
4-7: Production & env gating: LGTMReturns null when account is missing or not production. Correct.
12-56: Add a per-request CSP nonce and apply it to this inline scriptYou already JSON.stringify dynamic values (good). Add a nonce attribute to this script and ensure a fresh, cryptographically-random nonce is generated per request in middleware, included in the CSP header (script-src 'nonce-'), returned to the renderer (header or pageProps) and passed into this component (or use next/script nonce). Nonces require per-request rendering (force-dynamic / no ISR).
- return ( - <script + return ( + <script id="active-campaign" nonce={props?.cspNonce} dangerouslySetInnerHTML={{ __html: `(...)` }} />
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
studio/src/components/layout/analytics/active-campaign-script.tsx (1)
36-40: Remove non-existent Osano.cm.marketing/analytics fallbacks; rely solely on getConsent().Those properties don’t exist; keep a single source of truth via Osano.cm.getConsent(). This also avoids accidental truthiness in future API changes.
Apply:
- function updateConsentFromOsano() { - const consent = window.Osano && Osano.cm && Osano.cm.getConsent ? Osano.cm.getConsent() : null; - const marketing = (consent && consent.MARKETING === 'ACCEPT' || !!(window.Osano && Osano.cm && Osano.cm.marketing)); - const analytics = (consent && consent.ANALYTICS === 'ACCEPT' || !!(window.Osano && Osano.cm && Osano.cm.analytics)); - - if (marketing || analytics) { - loadActiveCampaign(); - } - } + function updateConsentFromOsano() { + var consent = (window.Osano && Osano.cm && Osano.cm.getConsent) ? Osano.cm.getConsent() : null; + var marketing = consent && consent.MARKETING === 'ACCEPT'; + var analytics = consent && consent.ANALYTICS === 'ACCEPT'; + if (marketing || analytics) { + loadActiveCampaign(); + } + }studio/src/components/layout/analytics/osana-script.tsx (1)
11-11: Thanks for fixing the duplicate id
id="osano-cmp"avoids the earlier “gtm” collision. LGTM.
🧹 Nitpick comments (8)
studio/src/components/layout/analytics/active-campaign-script.tsx (3)
52-56: Listen on document for osano-cm-initialized for reliability.Some CMPs dispatch on document; listening on window can miss non-bubbling custom events.
- } else { - window.addEventListener('osano-cm-initialized', onOsanoReady, { once: true }); - } + } else { + document.addEventListener('osano-cm-initialized', onOsanoReady, { once: true }); + }
45-51: Guard against duplicate listener registration.If this script runs more than once (e.g., misplacement in a client component), you’ll stack listeners. Add a simple once-guard.
- - function onOsanoReady() { + var listenersRegistered = false; + function onOsanoReady() { + if (listenersRegistered) return; + listenersRegistered = true; updateConsentFromOsano();
12-12: CSP compatibility: support nonce.If you enforce a strict CSP, expose an optional nonce prop and pass it to <Script nonce={nonce}>.
If you want, I can wire this into your _document/_app setup.
studio/src/components/layout/analytics/gtm-script.tsx (2)
11-20: Use a unique Script id to avoid dedupe collisionsRename to a more specific id to prevent clashes if this is rendered from multiple places. If you enforce CSP with nonces, consider plumbing a nonce prop into these Scripts.
- <Script id="gtm" strategy="afterInteractive">{` + <Script id="gtm-base" strategy="afterInteractive">{`
41-49: Confirmed Osano events & consent keys — add Consent Mode v2 signals
- osano-cm-initialized and osano-cm-consent-changed are the correct runtime events; no change needed in studio/src/components/layout/analytics/gtm-script.tsx (lines 41–49).
- Osano.cm.getConsent() contains "ANALYTICS" and "MARKETING" with value "ACCEPT" when granted — ensure updateConsentFromOsano checks for 'ACCEPT' (or use Osano.cm.analytics / Osano.cm.marketing).
- For Google Consent Mode v2, send ad_user_data and ad_personalization in addition to ad_storage and analytics_storage when updating consent signals.
studio/src/components/layout/analytics/osana-script.tsx (3)
5-7: Don’t restrict CMP to production-only; allow staging/preview QAGate by the env var presence so consent flows can be validated in preview/staging.
- if (!osanaScriptId || process.env.NODE_ENV !== 'production') { + if (!osanaScriptId) { return null; }If you still want an explicit toggle, add
NEXT_PUBLIC_ENABLE_CMP_IN_DEV=trueand check it here.
10-14: Optional: reduce referrer leakageAdd a referrer policy to minimize Referer header to the CMP endpoint.
<Script id="osano-cmp" src={`https://cmp.osano.com/${osanaScriptId}/osano.js`} + referrerPolicy="no-referrer" strategy="afterInteractive" />
3-5: Use 'Osano' as the canonical name or add non‑breaking aliases for 'Osana'Repo has mixed spellings — Osana appears in: studio/src/components/layout/analytics/osana-script.tsx, studio/src/pages/_document.tsx, studio/.env.local.example; Osano appears in gtm-script.tsx and active-campaign-script.tsx. Align names or add aliases. Example non‑breaking changes:
const osanoScriptId = process.env.NEXT_PUBLIC_OSANO_SCRIPT_ID ?? process.env.NEXT_PUBLIC_OSANA_SCRIPT_ID;
export function OsanoScript() { /* ... */ }
// legacy alias
export { OsanoScript as OsanaScript };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
studio/src/components/layout/analytics/active-campaign-script.tsx(1 hunks)studio/src/components/layout/analytics/gtm-script.tsx(1 hunks)studio/src/components/layout/analytics/osana-script.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: build_test
- GitHub Check: build_push_image
🔇 Additional comments (6)
studio/src/components/layout/analytics/active-campaign-script.tsx (2)
6-9: Prod + env guard looks good.Only loads in production with a defined account — solid default-safe behavior.
12-34: ActiveCampaign loader and queueing are implemented correctly.Defers script, queues vgo calls, and enables tracking only after consent — good.
studio/src/components/layout/analytics/gtm-script.tsx (3)
3-7: Production gating + env var: LGTMCorrectly avoids loading GTM when missing ID or outside production.
62-71: noscript iframe: LGTMMatches GTM’s recommended fallback.
3-72: Sanity check — no remaining inline GTM/LinkedIn snippets foundSearch results: studio/src/components/layout/analytics/gtm-script.tsx (this PR's GTM scripts), studio/.env.local.example (NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID placeholder), studio/next.config.mjs (CSP/allowlist includes googletagmanager.com and snap.licdn.com). No other inline GTM or LinkedIn Insight snippets found in _document or legacy pages.
studio/src/components/layout/analytics/osana-script.tsx (1)
1-16: Verified ordering & gating — no action requiredOsana is rendered before GTM in ; Osano, GTM and ActiveCampaign all use strategy="afterInteractive" (no beforeInteractive found); GTM and ActiveCampaign register/listen for Osano initialization/consent events and env-var + NODE_ENV guards are present.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
studio/src/components/layout/analytics/gtm-script.tsx (1)
22-51: Harden Osano/gtag references to avoid ReferenceErrors; clarify boolean precedenceUse window.Osano/window.gtag, guard cm APIs, and make precedence explicit. This also aligns with previous feedback.
- <Script id="gtm-consent" strategy="afterInteractive">{` -(function(){ - function updateConsentFromOsano() { - const consent = window.Osano && Osano.cm && Osano.cm.getConsent ? Osano.cm.getConsent() : null; - const marketing = (consent && consent.MARKETING === 'ACCEPT' || !!(window.Osano && Osano.cm && Osano.cm.marketing)); - const analytics = (consent && consent.ANALYTICS === 'ACCEPT' || !!(window.Osano && Osano.cm && Osano.cm.analytics)); - - if (typeof gtag === 'function') { - gtag('consent', 'update', { + <Script id="gtm-consent" strategy="afterInteractive">{` +(function(){ + function updateConsentFromOsano() { + var osano = window.Osano; + var cm = osano && osano.cm ? osano.cm : null; + var consent = (cm && typeof cm.getConsent === 'function') ? cm.getConsent() : null; + var marketing = (!!consent && consent.MARKETING === 'ACCEPT') || !!(cm && cm.marketing); + var analytics = (!!consent && consent.ANALYTICS === 'ACCEPT') || !!(cm && cm.analytics); + + if (typeof window.gtag === 'function') { + window.gtag('consent', 'update', { ad_storage: marketing ? 'granted' : 'denied', ad_user_data: marketing ? 'granted' : 'denied', ad_personalization: marketing ? 'granted' : 'denied', analytics_storage: analytics ? 'granted' : 'denied', functionality_storage: 'granted', security_storage: 'granted' }); } } function onOsanoReady() { updateConsentFromOsano(); - Osano.cm.addEventListener('osano-cm-consent-changed', updateConsentFromOsano); + var osano = window.Osano; + var cm = osano && osano.cm ? osano.cm : null; + if (cm && typeof cm.addEventListener === 'function') { + cm.addEventListener('osano-cm-consent-changed', updateConsentFromOsano); + } } - if (window.Osano && Osano.cm) { + if (window.Osano && window.Osano.cm) { onOsanoReady(); } else { window.addEventListener('osano-cm-initialized', onOsanoReady, { once: true }); } })(); `}</Script>
🧹 Nitpick comments (4)
studio/.env.local.example (1)
21-23: Reorder env keys to satisfy dotenv-linter warningsCurrent order triggers UnorderedKey warnings. Reorder as below.
- NEXT_PUBLIC_OSANO_SCRIPT_ID= -NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID= -NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT= +NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT= +NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID= +NEXT_PUBLIC_OSANO_SCRIPT_ID=studio/src/pages/_document.tsx (2)
135-148: Avoid index keys for custom scripts; always use stable idsPrevents unnecessary re-mounts on list changes.
- <Script - key={i} + <Script + key={script.id} id={script.id} src={script.src} strategy="afterInteractive" />
9-16: Return empty array on JSON parse failurePrevents silent undefined and keeps iteration consistent.
} catch { - // ignore + // ignore parse errors + return []; }studio/src/components/layout/analytics/gtm-script.tsx (1)
11-20: Optional: unique, specific script idConsider renaming id="gtm" to "gtm-base" for clarity; you already use "gtm-default" elsewhere.
- <Script id="gtm" strategy="afterInteractive">{` + <Script id="gtm-base" strategy="afterInteractive">{`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
studio/.env.local.example(1 hunks)studio/src/components/layout/analytics/gtm-script.tsx(1 hunks)studio/src/pages/_document.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
studio/src/pages/_document.tsx (2)
studio/src/components/layout/analytics/gtm-script.tsx (2)
GtmScript(3-54)GtmNoScript(56-72)studio/src/components/layout/analytics/active-campaign-script.tsx (1)
ActiveCampaignScript(5-60)
🪛 dotenv-linter (3.3.0)
studio/.env.local.example
[warning] 22-22: [UnorderedKey] The NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID key should go before the NEXT_PUBLIC_OSANO_SCRIPT_ID key
(UnorderedKey)
[warning] 23-23: [UnorderedKey] The NEXT_PUBLIC_ACTIVE_CAMPAIGN_ACCOUNT key should go before the NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
studio/src/pages/_document.tsx (1)
27-36: Follow-up after fixing isProduction: verify Osano loads before GTMOnce the env check is fixed, this block will correctly load Osano beforeInteractive in prod. Please verify in a staging build that osano.js is requested before gtm.js.
…ays-enforced-no-cookie-placement
…ays-enforced-no-cookie-placement
…nsent-is-always-enforced-no-cookie-placement' into wilson/eng-8069-ensure-cookie-consent-is-always-enforced-no-cookie-placement
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
studio/src/components/layout/analytics/gtm-script.tsx (2)
3-7: Gate GTM by production AND valid ID (avoid loading gtm.js?id=undefined).Return null when the ID is missing or not prod.
export function GtmScript() { const gtmId = process.env.NEXT_PUBLIC_GOOGLE_TAG_MANAGER_ID; - return ( + if (!gtmId || process.env.NODE_ENV !== 'production') { + return null; + } + + return (
21-47: Fix ReferenceError risks: use window.Osano everywhere and guard APIs.Unqualified
Osanocan throw; also guardaddEventListenerand clarify boolean precedence.(function(){ function updateConsentFromOsano() { - const consent = window.Osano && Osano.cm && Osano.cm.getConsent ? Osano.cm.getConsent() : null; - const marketing = (consent && consent.MARKETING === 'ACCEPT' || !!(window.Osano && Osano.cm && Osano.cm.marketing)); - const analytics = (consent && consent.ANALYTICS === 'ACCEPT' || !!(window.Osano && Osano.cm && Osano.cm.analytics)); + const osano = window.Osano; + const consent = osano && osano.cm && typeof osano.cm.getConsent === 'function' + ? osano.cm.getConsent() + : null; + const marketing = + (!!consent && consent.MARKETING === 'ACCEPT') || + !!(osano && osano.cm && osano.cm.marketing); + const analytics = + (!!consent && consent.ANALYTICS === 'ACCEPT') || + !!(osano && osano.cm && osano.cm.analytics); - if (typeof gtag === 'function') { + if (typeof gtag === 'function') { gtag('consent', 'update', { ad_storage: marketing ? 'granted' : 'denied', ad_user_data: marketing ? 'granted' : 'denied', ad_personalization: marketing ? 'granted' : 'denied', analytics_storage: analytics ? 'granted' : 'denied', functionality_storage: 'granted', security_storage: 'granted' }); } } function onOsanoReady() { updateConsentFromOsano(); - Osano.cm.addEventListener('osano-cm-consent-changed', updateConsentFromOsano); + const osano = window.Osano; + if (osano && osano.cm && typeof osano.cm.addEventListener === 'function') { + osano.cm.addEventListener('osano-cm-consent-changed', updateConsentFromOsano); + } } - if (window.Osano && Osano.cm) { + if (window.Osano && window.Osano.cm) { onOsanoReady(); } else { window.addEventListener('osano-cm-initialized', onOsanoReady, { once: true }); } })();studio/src/pages/_document.tsx (1)
51-51: Also gate by presence of ID.Prevents loading GTM with
id=undefinedin prod.- {isProduction && <GtmScript />} + {isProduction && gtmId ? <GtmScript /> : null}
🧹 Nitpick comments (4)
studio/src/components/layout/analytics/gtm-script.tsx (1)
61-66: Improve a11y: add title to noscript iframe.Helps assistive tech users.
<iframe src={`https://www.googletagmanager.com/ns.html?id=${gtmId}`} height="0" width="0" - style={{ display: "none", visibility: "hidden" }} + style={{ display: "none", visibility: "hidden" }} + title="Google Tag Manager (noscript)" />studio/src/pages/_document.tsx (3)
37-49: Set Consent Mode defaults regardless of GTM ID.Defaults should apply even if the GTM ID is absent/misconfigured; keeps storage denied until consent.
- {gtmId && ( - <Script id="gtm-default" strategy="beforeInteractive">{`window.dataLayer = window.dataLayer || []; + <Script id="gtm-default" strategy="beforeInteractive">{`window.dataLayer = window.dataLayer || []; function gtag(){ dataLayer.push(arguments); } gtag('consent', 'default', { ad_storage: 'denied', ad_user_data: 'denied', ad_personalization: 'denied', analytics_storage: 'denied', functionality_storage: 'granted', // essentials security_storage: 'granted' -});`}</Script> - )} +});`}</Script>
141-146: Use stable keys for mapped scripts.Prefer deterministic keys for reconciliation.
<Script - key={i} + key={script.id} id={script.id} src={script.src} strategy="afterInteractive" />
18-23: Minor: simplify getCustomScripts return type.Since you already default to
[], consider returning[]on parse error to avoidundefinedunion.-const getCustomScripts = (): - | { src: string; id: string; inline?: boolean }[] - | undefined => { +const getCustomScripts = (): { src: string; id: string; inline?: boolean }[] => { try { return process.env.CUSTOM_HEAD_SCRIPTS ? JSON.parse(process.env.CUSTOM_HEAD_SCRIPTS) : []; } catch { - // ignore + // ignore and fall back + return []; } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
studio/src/components/layout/analytics/gtm-script.tsx(1 hunks)studio/src/pages/_document.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
studio/src/pages/_document.tsx (2)
studio/src/components/layout/analytics/gtm-script.tsx (2)
GtmScript(3-51)GtmNoScript(53-69)studio/src/components/layout/analytics/active-campaign-script.tsx (1)
ActiveCampaignScript(5-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
studio/src/components/layout/analytics/gtm-script.tsx (1)
53-69: LGTM: Noscript variant guarded correctly inside component.studio/src/pages/_document.tsx (2)
28-36: CMP load order/LGTM.Osano loads with beforeInteractive in prod — correct placement.
150-150: ActiveCampaign is safely gated inside component.No action needed here.
wilsonrivera
deleted the
wilson/eng-8069-ensure-cookie-consent-is-always-enforced-no-cookie-placement
branch
2 participants
Summary by CodeRabbit
New Features
Refactor
Chores
Checklist