[Main] Fix event tracking#11499
Conversation
📝 WalkthroughSummaryUpdated event tracking functionality in the documentation template's base JavaScript file. The Google Tag Manager script is now injected unconditionally without domain-based conditional checks. Removed prior conditional JavaScript blocks related to domain restrictions. Replaced the Moesif initialization configuration with a lightweight Changes:
WalkthroughThe base HTML template's JavaScript is updated in two areas. First, the Google Tag Manager script loader is now injected unconditionally, removing the prior 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
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 `@en/theme/material/partials/javascripts/base.html`:
- Around line 44-54: The trackEvent function's conditional check `if (moesif)`
will throw a ReferenceError if the moesif variable is not defined in the global
scope, preventing the guard from working as intended. Replace the condition with
either `typeof moesif !== 'undefined'` or use `window.moesif` instead of the
bare `moesif` reference. The window.moesif approach is preferred as it safely
returns undefined for missing properties without throwing an error. Update both
the condition and the subsequent moesif.track call to use the same safe
reference pattern.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f579aae5-a496-4bd2-9ec9-62360a362c6e
📒 Files selected for processing (1)
en/theme/material/partials/javascripts/base.html
| function trackEvent(eventName, properties) { | ||
| if (moesif) { | ||
| moesif.track(eventName, { | ||
| product: "api", | ||
| asset_type: "docs", | ||
| domain: window.location.hostname, | ||
| page_url: window.location.pathname, | ||
| ...properties, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Use typeof check or window.moesif to avoid ReferenceError.
The current if (moesif) guard will throw a ReferenceError if moesif is not defined in the global scope, defeating the purpose of the conditional check.
Suggested fix
function trackEvent(eventName, properties) {
- if (moesif) {
+ if (typeof moesif !== 'undefined') {
moesif.track(eventName, {
product: "api",
asset_type: "docs",
domain: window.location.hostname,
page_url: window.location.pathname,
...properties,
});
}
}Alternatively, use window.moesif which safely returns undefined for missing properties.
🤖 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 `@en/theme/material/partials/javascripts/base.html` around lines 44 - 54, The
trackEvent function's conditional check `if (moesif)` will throw a
ReferenceError if the moesif variable is not defined in the global scope,
preventing the guard from working as intended. Replace the condition with either
`typeof moesif !== 'undefined'` or use `window.moesif` instead of the bare
`moesif` reference. The window.moesif approach is preferred as it safely returns
undefined for missing properties without throwing an error. Update both the
condition and the subsequent moesif.track call to use the same safe reference
pattern.
Purpose
$subject
Security checks