fix(livechat): prevent whitespace-only values in Name, Description and Messages #38609
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
|
WalkthroughAdded client-side validation to form fields in omnichannel triggers to prevent values starting with spaces. Validation checks were added to the Name and Description fields in EditTrigger and the Message field in SendMessageActionForm, with inline error display for failed validation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
|
Hi, could someone review this PR when possible? Thanks 🙂 |
![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
🤖 Fix all issues with AI agents
In
`@apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx`:
- Around line 35-37: Replace the current custom validate that blocks leading
ASCII spaces in the rules for the message input in SendMessageActionForm.tsx
with a trimming transform and let the existing required rule handle empty
values: apply a setValueAs (or onChange/onBlur transform) on the message form
field to trimStart/trim (or trim on submit) so leading/trailing whitespace is
removed before validation, remove the validate that uses startsWith(" "), and
replace the literal error key t('Message cannot start with space') with a real
i18n key or parameterized existing key (e.g., use the registered translation key
for an empty/invalid message) so all locales render correctly.
In `@apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx`:
- Around line 196-206: The Description TextInput is missing the accessibility
linking used by the Name field; update the Controller render for 'description'
in EditTrigger.tsx to pass the same error and ARIA props as Name: set
error={Boolean(errors?.description)} aria-invalid={Boolean(errors?.description)}
and when there is an error set aria-describedby={`${descriptionField}-error`}
(otherwise omit or undefined), so the TextInput is visually and programmatically
associated with the FieldError element.
- Line 199: The description field Controller uses an inconsistent lowercase
error string and is on one long line; update the validation message used in
Controller name='description' (the validate rule) to title case ("Description
cannot start with space") to match the other fields, and reformat the Controller
+ rules + render (the Controller with control and render that returns TextInput
id={descriptionField}) across multiple lines to match the Name field's
formatting for readability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsxapps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsxapps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx
⏰ 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). (1)
- GitHub Check: cubic · AI code reviewer
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| rules={{ required: t('Required_field', { field: t('Message') }), | ||
| validate: (value) => {if(value.startsWith(" ")) return t('Message cannot start with space');} | ||
| }} |
There was a problem hiding this comment.
Validation rejects leading spaces instead of trimming — contradicts the issue objective.
The linked issue (#38512) asks to "trim inputs before validation and treat whitespace-only values as empty." The current approach blocks any input starting with a space, which is a different (and more restrictive) UX: a user who accidentally types a leading space gets an error instead of having it silently trimmed on submit.
Additionally:
- Non-i18n key strings:
t('Message cannot start with space')is not a registered translation key — it will render as the literal key string in non-English locales. Use a proper key registered in the i18n files, or parameterize an existing key. - Only checks ASCII space:
startsWith(" ")won't catch tabs, non-breaking spaces, or other whitespace. Use.trimStart()or a regex like/^\s/if you keep this approach.
The recommended approach is to trim on submit (or via setValueAs / onChange transform) and let the existing required rule reject empty strings:
Suggested approach — trim instead of reject
rules={{ required: t('Required_field', { field: t('Message') }),
- validate: (value) => {if(value.startsWith(" ")) return t('Message cannot start with space');}
- }}
+ }}
render={({ field }) => (
<TextAreaInput
error={messageError?.message}
aria-invalid={Boolean(messageError)}
aria-describedby={`${messageFieldId}-error`}
aria-required={true}
{...field}
+ onChange={(e: React.ChangeEvent<HTMLTextAreaElement>) => {
+ field.onChange(e.target.value.trimStart());
+ }}
+ onBlur={(e: React.FocusEvent<HTMLTextAreaElement>) => {
+ field.onChange(e.target.value.trim());
+ field.onBlur();
+ }}
rows={3}
placeholder={`${t('Message')}*`}
/>This trims leading whitespace on change and trailing whitespace on blur, and the existing required rule handles whitespace-only values naturally.
🤖 Prompt for AI Agents
In
`@apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx`
around lines 35 - 37, Replace the current custom validate that blocks leading
ASCII spaces in the rules for the message input in SendMessageActionForm.tsx
with a trimming transform and let the existing required rule handle empty
values: apply a setValueAs (or onChange/onBlur transform) on the message form
field to trimStart/trim (or trim on submit) so leading/trailing whitespace is
removed before validation, remove the validate that uses startsWith(" "), and
replace the literal error key t('Message cannot start with space') with a real
i18n key or parameterized existing key (e.g., use the registered translation key
for an empty/invalid message) so all locales render correctly.
| <Field> | ||
| <FieldLabel htmlFor={descriptionField}>{t('Description')}</FieldLabel> | ||
| <FieldRow> | ||
| <Controller name='description' control={control} render={({ field }) => <TextInput id={descriptionField} {...field} />} /> | ||
| <Controller name='description' control={control} rules={{validate:(value)=>{if (value.startsWith(" ")) return t('description cannot start with space')}}} render={({ field }) => <TextInput id={descriptionField} {...field} />} /> | ||
| </FieldRow> | ||
| {errors?.description && ( | ||
| <FieldError aria-live='assertive' id={`${descriptionField}-error`}> | ||
| {errors?.description.message} | ||
| </FieldError> | ||
| )} | ||
| </Field> |
There was a problem hiding this comment.
Description field missing aria-invalid and aria-describedby attributes.
The Name field (Lines 181-184) sets error, aria-invalid, and aria-describedby on its TextInput so screen readers can associate the error message. The Description field's TextInput on Line 199 lacks all of these, so the newly added FieldError (Lines 201-205) won't be announced or visually linked to the input.
Suggested fix
- render={({ field }) => <TextInput id={descriptionField} {...field} />}
+ render={({ field }) => (
+ <TextInput
+ id={descriptionField}
+ {...field}
+ error={errors?.description?.message}
+ aria-invalid={Boolean(errors?.description)}
+ aria-describedby={`${descriptionField}-error`}
+ />
+ )}📝 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.
| <Field> | |
| <FieldLabel htmlFor={descriptionField}>{t('Description')}</FieldLabel> | |
| <FieldRow> | |
| <Controller name='description' control={control} render={({ field }) => <TextInput id={descriptionField} {...field} />} /> | |
| <Controller name='description' control={control} rules={{validate:(value)=>{if (value.startsWith(" ")) return t('description cannot start with space')}}} render={({ field }) => <TextInput id={descriptionField} {...field} />} /> | |
| </FieldRow> | |
| {errors?.description && ( | |
| <FieldError aria-live='assertive' id={`${descriptionField}-error`}> | |
| {errors?.description.message} | |
| </FieldError> | |
| )} | |
| </Field> | |
| <Field> | |
| <FieldLabel htmlFor={descriptionField}>{t('Description')}</FieldLabel> | |
| <FieldRow> | |
| <Controller name='description' control={control} rules={{validate:(value)=>{if (value.startsWith(" ")) return t('description cannot start with space')}}} render={({ field }) => ( | |
| <TextInput | |
| id={descriptionField} | |
| {...field} | |
| error={errors?.description?.message} | |
| aria-invalid={Boolean(errors?.description)} | |
| aria-describedby={`${descriptionField}-error`} | |
| /> | |
| )} /> | |
| </FieldRow> | |
| {errors?.description && ( | |
| <FieldError aria-live='assertive' id={`${descriptionField}-error`}> | |
| {errors?.description.message} | |
| </FieldError> | |
| )} | |
| </Field> |
🤖 Prompt for AI Agents
In `@apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx` around lines
196 - 206, The Description TextInput is missing the accessibility linking used
by the Name field; update the Controller render for 'description' in
EditTrigger.tsx to pass the same error and ARIA props as Name: set
error={Boolean(errors?.description)} aria-invalid={Boolean(errors?.description)}
and when there is an error set aria-describedby={`${descriptionField}-error`}
(otherwise omit or undefined), so the TextInput is visually and programmatically
associated with the FieldError element.
| <FieldLabel htmlFor={descriptionField}>{t('Description')}</FieldLabel> | ||
| <FieldRow> | ||
| <Controller name='description' control={control} render={({ field }) => <TextInput id={descriptionField} {...field} />} /> | ||
| <Controller name='description' control={control} rules={{validate:(value)=>{if (value.startsWith(" ")) return t('description cannot start with space')}}} render={({ field }) => <TextInput id={descriptionField} {...field} />} /> |
There was a problem hiding this comment.
Inconsistent error message casing and formatting.
'description cannot start with space' uses a lowercase d, while the Name and Message fields use title case ('Name cannot start with space', 'Message cannot start with space'). If these strings end up as i18n keys, casing differences mean different keys. Even as display text, it's inconsistent.
Also, this entire Controller with its rules, render, and props is crammed into a single ~190-character line, making it hard to read and review. Please break it across multiple lines consistent with the Name field's formatting above.
Formatting suggestion
- <Controller name='description' control={control} rules={{validate:(value)=>{if (value.startsWith(" ")) return t('description cannot start with space')}}} render={({ field }) => <TextInput id={descriptionField} {...field} />} />
+ <Controller
+ name='description'
+ control={control}
+ rules={{
+ validate: (value) => {
+ if (value.startsWith(' ')) return t('Description cannot start with space');
+ },
+ }}
+ render={({ field }) => <TextInput id={descriptionField} {...field} />}
+ />📝 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.
| <Controller name='description' control={control} rules={{validate:(value)=>{if (value.startsWith(" ")) return t('description cannot start with space')}}} render={({ field }) => <TextInput id={descriptionField} {...field} />} /> | |
| <Controller | |
| name='description' | |
| control={control} | |
| rules={{ | |
| validate: (value) => { | |
| if (value.startsWith(' ')) return t('Description cannot start with space'); | |
| }, | |
| }} | |
| render={({ field }) => <TextInput id={descriptionField} {...field} />} | |
| /> |
🤖 Prompt for AI Agents
In `@apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx` at line 199,
The description field Controller uses an inconsistent lowercase error string and
is on one long line; update the validation message used in Controller
name='description' (the validate rule) to title case ("Description cannot start
with space") to match the other fields, and reformat the Controller + rules +
render (the Controller with control and render that returns TextInput
id={descriptionField}) across multiple lines to match the Name field's
formatting for readability.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx:36">
P3: Missing i18n key for new validation message; the literal string is not defined in locale files, so it won’t be translated.</violation>
<violation number="2" location="apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx:36">
P2: Validation only rejects a leading literal space. Multiline inputs can start with other whitespace (e.g., newline/tab) and bypass the check, allowing whitespace-only/leading-whitespace messages despite the PR goal.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| defaultValue='' | ||
| rules={{ required: t('Required_field', { field: t('Message') }) }} | ||
| rules={{ required: t('Required_field', { field: t('Message') }), | ||
| validate: (value) => {if(value.startsWith(" ")) return t('Message cannot start with space');} |
There was a problem hiding this comment.
P2: Validation only rejects a leading literal space. Multiline inputs can start with other whitespace (e.g., newline/tab) and bypass the check, allowing whitespace-only/leading-whitespace messages despite the PR goal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx, line 36:
<comment>Validation only rejects a leading literal space. Multiline inputs can start with other whitespace (e.g., newline/tab) and bypass the check, allowing whitespace-only/leading-whitespace messages despite the PR goal.</comment>
<file context>
@@ -32,7 +32,9 @@ export const SendMessageActionForm = ({ control, index, ...props }: SendMessageA
defaultValue=''
- rules={{ required: t('Required_field', { field: t('Message') }) }}
+ rules={{ required: t('Required_field', { field: t('Message') }),
+ validate: (value) => {if(value.startsWith(" ")) return t('Message cannot start with space');}
+ }}
render={({ field }) => (
</file context>
| validate: (value) => {if(value.startsWith(" ")) return t('Message cannot start with space');} | |
| validate: (value) => {if (/^\s/.test(value)) return t('Message cannot start with space');} |
| defaultValue='' | ||
| rules={{ required: t('Required_field', { field: t('Message') }) }} | ||
| rules={{ required: t('Required_field', { field: t('Message') }), | ||
| validate: (value) => {if(value.startsWith(" ")) return t('Message cannot start with space');} |
There was a problem hiding this comment.
P3: Missing i18n key for new validation message; the literal string is not defined in locale files, so it won’t be translated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/omnichannel/triggers/actions/SendMessageActionForm.tsx, line 36:
<comment>Missing i18n key for new validation message; the literal string is not defined in locale files, so it won’t be translated.</comment>
<file context>
@@ -32,7 +32,9 @@ export const SendMessageActionForm = ({ control, index, ...props }: SendMessageA
defaultValue=''
- rules={{ required: t('Required_field', { field: t('Message') }) }}
+ rules={{ required: t('Required_field', { field: t('Message') }),
+ validate: (value) => {if(value.startsWith(" ")) return t('Message cannot start with space');}
+ }}
render={({ field }) => (
</file context>
2 participants
Proposed changes
Problem
The Livechat Trigger form accepts whitespace-only values (" ") in Name, Description, and Message fields, creating invalid triggers.
Solution
Result
Invalid triggers can no longer be saved.
Issue(s)
Fixes #38512
Steps to test or reproduce
Before: saved successfully
After: validation error shown
Further comments
Small validation improvement. No breaking changes.
Summary by CodeRabbit
Release Notes
Bug Fixes