Re-Add webinar section#163
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR refactors the form submission architecture and adds a new webinar registration feature. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 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: 3
🤖 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 `@layouts/partials/nav.html`:
- Around line 7-11: The mobile menu max-height needs to account for the webinar
banner when .Scratch.Get "webinarBannerActive" is true; update the logic in
layouts/partials/nav.html to adjust the mobile menu's max-h (currently using
max-h-[calc(100vh-48px)]) when the banner is rendered (i18n "webinar_banner"
block) by subtracting the banner height (e.g. calc(100vh - 48px -
<bannerHeight>) or by toggling a CSS variable/class that sets --banner-height
and using max-height: calc(100vh - 48px - var(--banner-height))); ensure the
mobile menu element/class that uses max-h-[calc(100vh-48px)] is updated to use
the conditional value or variable when .Scratch.Get "webinarBannerActive" is
true so the nav viewport is not clipped.
In `@layouts/webinar/single.html`:
- Around line 173-176: The altcha module scripts are being accessed via
$altchaJs and $altchaWorkerJs and their templates reference .Data.Integrity even
though fingerprinting was not applied; update the resource pipeline to apply the
fingerprint function to these resources (or fingerprint after any intended
minification) so that .Data.Integrity is populated for the integrity attributes
on altcha.js and js/altcha/worker.js; specifically, locate the assignments to
$altchaJs and $altchaWorkerJs in the template and pipe them through |
fingerprint (or through your existing minify + fingerprint sequence) before
using .RelPermalink and .Data.Integrity.
- Line 6: The fetch chain that loads webinar metadata (fetch(API_BASE_URL +
'/connect/contact/webinar/' + submitData.webinarId)) lacks error handling and
assumes d.dateStart exists; update the chain to first check response.ok before
parsing JSON, wrap JSON parsing/use of d in a try/catch, and add a .catch(...)
to handle network/parse errors; inside the success path ensure you only call
d.dateStart.replace(...) when d.dateStart is a non-null string (or provide a
fallback), and on any error set feedbackData.errorMessage (and
feedbackData.inProgress = false) and optionally leave webinar with safe defaults
so the form/UI does not break; reference ApiForm, submitData.webinarId, webinar,
dateStart.replace and feedbackData when making these changes.
🪄 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: 93433305-03f1-4f73-b983-41046200c317
📒 Files selected for processing (13)
assets/js/apiform.jsassets/js/webinar-datetime.jsconfig/_default/params.yamlcontent/webinar.de.htmlcontent/webinar.en.htmli18n/de.yamli18n/en.yamllayouts/_default/baseof.htmllayouts/become-a-partner/single.htmllayouts/book-a-demo/single.htmllayouts/contact-sales/single.htmllayouts/partials/nav.htmllayouts/webinar/single.html
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
layouts/webinar/single.html (1)
6-6: ⚡ Quick winEscape template values used inside JS literals in
x-data/x-init.On Line 6, interpolated values (
webinarId,$lang, i18n strings) are inserted into JS single-quoted strings without JS escaping. A quote/backslash in translations or config can break Alpine initialization.Proposed fix
- <section x-data="{feedbackData: {success: false, inProgress: false, errorMessage: ''}, submitData: {webinarId: '{{ .Site.Params.webinar.webinarId }}', firstName: '', lastName: '', email: '', company: '', captcha: null, acceptNewsletter: false}, webinar: {name: '', language: '', dateStart: null, lead: '', learnTitle: '', learnItems: []}, acceptTerms: false, apiForm: null, captchaState: null}" x-init="apiForm = new ApiForm($refs.form, feedbackData, submitData, API_BASE_URL + '/connect/contact/register-webinar'); new Webinar(submitData.webinarId, '{{ $lang }}', {en: '{{ i18n "webinar_language_en" }}', de: '{{ i18n "webinar_language_de" }}'}, webinar)" class="container py-12"> + <section x-data="{feedbackData: {success: false, inProgress: false, errorMessage: ''}, submitData: {webinarId: '{{ .Site.Params.webinar.webinarId | js }}', firstName: '', lastName: '', email: '', company: '', captcha: null, acceptNewsletter: false}, webinar: {name: '', language: '', dateStart: null, lead: '', learnTitle: '', learnItems: []}, acceptTerms: false, apiForm: null, captchaState: null}" x-init="apiForm = new ApiForm($refs.form, feedbackData, submitData, API_BASE_URL + '/connect/contact/register-webinar'); new Webinar(submitData.webinarId, '{{ $lang | js }}', {en: '{{ i18n "webinar_language_en" | js }}', de: '{{ i18n "webinar_language_de" | js }}'}, webinar)" class="container py-12">🤖 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 `@layouts/webinar/single.html` at line 6, The Alpine x-data/x-init contains unescaped template interpolations that are inserted into JS single-quoted strings (submitData.webinarId, the second arg to new Webinar which uses $lang, and the i18n strings passed as the language map), which can break JS if those values contain quotes/backslashes; escape these values for JS by applying Hugo's JS-safe escaping (e.g., the | js template function or jsonify) to each interpolation used inside JS literals: wrap '{{ .Site.Params.webinar.webinarId }}', '{{ $lang }}', '{{ i18n "webinar_language_en" }}', and '{{ i18n "webinar_language_de" }}' with the JS-escaping helper so ApiForm and new Webinar receive safe string literals.
🤖 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 `@assets/js/webinar.js`:
- Line 9: Guard the dereference of d.dateStart before calling .replace() to
avoid runtime exceptions: in the code that sets data.dateStart (referencing
d.dateStart.replace), check that d.dateStart is a non-empty string (or otherwise
truthy) and only call replace when safe; otherwise set data.dateStart to a
sensible fallback (e.g., null, empty string, or a normalized ISO default) so the
webinar state initialization cannot crash if the API returns missing or
malformed dateStart.
---
Nitpick comments:
In `@layouts/webinar/single.html`:
- Line 6: The Alpine x-data/x-init contains unescaped template interpolations
that are inserted into JS single-quoted strings (submitData.webinarId, the
second arg to new Webinar which uses $lang, and the i18n strings passed as the
language map), which can break JS if those values contain quotes/backslashes;
escape these values for JS by applying Hugo's JS-safe escaping (e.g., the | js
template function or jsonify) to each interpolation used inside JS literals:
wrap '{{ .Site.Params.webinar.webinarId }}', '{{ $lang }}', '{{ i18n
"webinar_language_en" }}', and '{{ i18n "webinar_language_de" }}' with the
JS-escaping helper so ApiForm and new Webinar receive safe string literals.
🪄 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: f6737826-518f-487c-b389-48e25282e341
📒 Files selected for processing (4)
assets/js/webinar.jscontent/webinar.de.htmlcontent/webinar.en.htmllayouts/webinar/single.html
✅ Files skipped from review due to trivial changes (1)
- content/webinar.de.html
No description provided.