[codex] Add pre-chat regex validation support#1522
Conversation
WalkthroughAdds per-field regex-driven validation for pre-chat lead inputs (helpers, input wiring, runtime validators, unified submit guard) and normalizes/sorts pasted and loaded messages by createdAtTime in the sidebox composer. ChangesPre-chat Custom Field Validation
Sidebox Composer: Paste Normalization & Message Sorting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@webplugin/js/app/prechat/km-prechat.js`:
- Around line 736-737: handleCustomValidation currently calls setError('') when
custom fields pass, which can clear unrelated email/phone errors; remove any
setError('') calls from handleCustomValidation (and the identical calls at the
other block around the same logic) so the function only returns true/false.
Instead, clear the global error in the centralized submit chain after all
validators succeed—i.e., in the form submission handler (e.g.,
handleSubmit/onSubmit) call setError('') only once after running
handleCustomValidation and the other validators and confirming they all passed.
- Around line 818-821: The submit flow calls handlePhoneValidation() but ignores
its boolean result, so failed phone validation won't stop submission; update the
submit logic (the block using handleEmailValidation and handlePhoneValidation)
to check the return value of handlePhoneValidation exactly like
handleEmailValidation (e.g., if (handlePhoneValidation &&
!handlePhoneValidation()) return;), or otherwise combine both results and return
early when either validation fails; ensure you reference the existing
handleEmailValidation and handlePhoneValidation functions so behavior remains
consistent.
- Around line 720-723: The catch block that logs "Invalid pre-chat validation
regex" currently returns true (allowing the field), which makes regex parse
failures bypass validation; change that catch to return false so a regex parse
error fails validation (fail-closed) and also invoke the existing user-facing
validation error path (e.g., call the same error/reporting helper used for other
validation failures or push a message to the field's errors array) instead of
silently only logging; update the catch around the console.error('Invalid
pre-chat validation regex', error) to log the error and then trigger the normal
validation-failure flow so users see an error.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31149cac-f6b7-49d6-8b4d-76bd1ed0108c
📒 Files selected for processing (1)
webplugin/js/app/prechat/km-prechat.js
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d1f6316f4
ℹ️ 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".
| if (!handleCustomValidation()) { | ||
| return; |
There was a problem hiding this comment.
When a custom regex is configured on a textarea pre-chat field, pattern is not enforced by native HTML constraint validation, so this return only exits the click listener while the form still proceeds to the existing #km-form-chat-login submit handler and initializes the chat with the invalid value. Pass the click event into the handler and call preventDefault() (or move the custom check into the submit handler) whenever handleCustomValidation() returns false.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68b900451f
ℹ️ 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".
| if (regex instanceof RegExp) { | ||
| regex = regex.source; |
There was a problem hiding this comment.
When a widget config passes validation.regex as a RegExp with flags, such as /^[a-z]+$/i for a case-insensitive field, converting it to regex.source drops the flags before both the pattern attribute and data-km-validation-regex are populated. Those fields then become case-sensitive and reject values that the configured validator was meant to accept, so either preserve the flags for the JS check or avoid accepting RegExp objects without flag handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7f299a5bb
ℹ️ 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".
| if (!input || !validation || !validation.regex) { | ||
| return; | ||
| } | ||
| input.setAttribute('pattern', validation.regex); |
There was a problem hiding this comment.
Although the new data attribute now preserves RegExp flags for the JS check, this line still installs only the regex source as the HTML pattern. For a configured validator like /^[a-z]+$/i, entering ABC passes handleCustomValidation() but the browser’s native pattern check is case-sensitive and prevents the form submit before the submit listener can proceed, so values accepted by the supplied validator remain blocked.
Useful? React with 👍 / 👎.
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 `@webplugin/js/app/mck-sidebox-1.0.js`:
- Around line 4366-4369: The current assignment to text calls (e.clipboardData
|| window.clipboardData).getData('text') directly which can throw when clipboard
APIs are absent; change it to first capture the data source (e.clipboardData ||
window.clipboardData), check that it's truthy, call getData('text') only when
present, and then run the .replace(...) on a safe string (e.g., raw || '') so
that e.clipboardData/window.clipboardData and getData are guarded; update the
assignment where text is computed (the const text = ... line) to use this safe
pattern referencing e.clipboardData, window.clipboardData and getData.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 84732426-1245-4aa8-a78e-9d835518cf35
📒 Files selected for processing (2)
webplugin/js/app/mck-sidebox-1.0.jswebplugin/js/app/prechat/km-prechat.js
🚧 Files skipped from review as they are similar to previous changes (1)
- webplugin/js/app/prechat/km-prechat.js
| const text = (e.clipboardData || window.clipboardData) | ||
| .getData('text') | ||
| .replace(/\r\n/g, '\n') | ||
| .replace(/\n+$/, ''); |
There was a problem hiding this comment.
Guard clipboard access before calling getData()
Line 4366 can throw when clipboard APIs are unavailable, which breaks paste handling entirely in affected environments. Add a safe fallback before .replace(...).
Proposed fix
- const text = (e.clipboardData || window.clipboardData)
- .getData('text')
- .replace(/\r\n/g, '\n')
- .replace(/\n+$/, '');
+ var clipboard = e.clipboardData || window.clipboardData;
+ var rawText =
+ clipboard && typeof clipboard.getData === 'function'
+ ? clipboard.getData('text') || clipboard.getData('Text') || ''
+ : '';
+ const text = String(rawText).replace(/\r\n/g, '\n').replace(/\n+$/, '');📝 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 text = (e.clipboardData || window.clipboardData) | |
| .getData('text') | |
| .replace(/\r\n/g, '\n') | |
| .replace(/\n+$/, ''); | |
| var clipboard = e.clipboardData || window.clipboardData; | |
| var rawText = | |
| clipboard && typeof clipboard.getData === 'function' | |
| ? clipboard.getData('text') || clipboard.getData('Text') || '' | |
| : ''; | |
| const text = String(rawText).replace(/\r\n/g, '\n').replace(/\n+$/, ''); |
🤖 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 `@webplugin/js/app/mck-sidebox-1.0.js` around lines 4366 - 4369, The current
assignment to text calls (e.clipboardData ||
window.clipboardData).getData('text') directly which can throw when clipboard
APIs are absent; change it to first capture the data source (e.clipboardData ||
window.clipboardData), check that it's truthy, call getData('text') only when
present, and then run the .replace(...) on a safe string (e.g., raw || '') so
that e.clipboardData/window.clipboardData and getData are guarded; update the
assignment where text is computed (the const text = ... line) to use this safe
pattern referencing e.clipboardData, window.clipboardData and getData.
1 participant
Summary
Jira
Validation
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes