Added Validations to Valueset form Name and Slug fields along with translations for error messages. #10838
Conversation
WalkthroughThis pull request introduces enhancements to slug validation and messaging. A new entry for slug validation is added in the JSON locale file. Additionally, the validation schema in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(1 hunks)src/components/ValueSet/ValueSetForm.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: CodeQL-Build
- GitHub Check: lint
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
public/locale/en.json (1)
2216-2217: New validation messages added for slug field.Good addition of translation messages for the slug validation. These will provide clear feedback to users when entering invalid or too lengthy slugs.
src/components/ValueSet/ValueSetForm.tsx (1)
350-352: Good improvement with string trimming.Adding
.trim()to the name field validation is a good practice to prevent whitespace-only inputs or accidental leading/trailing spaces.
| slug: z | ||
| .string() | ||
| .trim() | ||
| .min(5, t("field_required")) | ||
| .max(25, t("slug_lengthy_message")) | ||
| .regex(/^[-\w]+$/, { | ||
| message: t("slug_format_message"), | ||
| }), |
There was a problem hiding this comment.
Error message key mismatch in slug validation.
There's a mismatch between the translation key used here (slug_lengthy_message) and the actual key defined in en.json (slug_lengthy). This will cause the error message to not display correctly.
- .max(25, t("slug_lengthy_message"))
+ .max(25, t("slug_lengthy"))The additional validation requirements for the slug field (min length, max length, and regex pattern) are well implemented and will help ensure consistent slug formats.
📝 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.
| slug: z | |
| .string() | |
| .trim() | |
| .min(5, t("field_required")) | |
| .max(25, t("slug_lengthy_message")) | |
| .regex(/^[-\w]+$/, { | |
| message: t("slug_format_message"), | |
| }), | |
| slug: z | |
| .string() | |
| .trim() | |
| .min(5, t("field_required")) | |
| .max(25, t("slug_lengthy")) | |
| .regex(/^[-\w]+$/, { | |
| message: t("slug_format_message"), | |
| }), |
|
@Jacobjeevan @rithviknishad can you review this again I have added translations as suggested by @Jacobjeevan |
| export function ValueSetForm({ | ||
| initialData, | ||
| onSubmit, | ||
| isSubmitting, | ||
| }: ValueSetFormProps) { | ||
| const { t } = useTranslation(); | ||
|
|
||
| const valuesetFormSchema = z.object({ |
There was a problem hiding this comment.
You can import t function directly:
import { t } from "i18next"
This allows to keep the schema outside as before.
|
@Jacobjeevan I have tried that first but the associated translation was not being displayed. It was displaying field_required. It was not fetching the translation. I have checked it that is the reason why I changed like that |
|
@hrit2773 , refresh the page then check |
|
@AdityaJ2305 yes I checked that. Surprisingly when I removed trim function the error translations are showing correctly but when I added trim its not |
I'm saying about the translation key is it fixed after refresh ? |
|
@AdityaJ2305 no its not displaying |
working fine on my side |
|
@AdityaJ2305 Ohh really.. let me try once again |
|
@Jacobjeevan can you check it now |
Jacobjeevan
added
needs testing
and removed
question
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/ValueSet/ValueSetForm.tsx (1)
4-4: Consider consolidating translation approachesThe code currently uses two different approaches to access translations:
- Importing
tdirectly (line 4) for use in the schema- Using
useTranslation()hook inside the component (line 410)While this works, it's better to use a consistent approach. Consider one of these options:
Option 1: Define the schema inside the component to use the same
tinstance:-const valuesetFormSchema = z.object({...}); export function ValueSetForm({ initialData, onSubmit, isSubmitting, }: ValueSetFormProps) { const { t } = useTranslation(); + + const valuesetFormSchema = z.object({ + name: z.string().trim().min(1, t("field_required")), + slug: z + .string() + .trim() + .min(5, t("field_required")) + .max(25, t("slug_lengthy_message")) + .regex(/^[-\w]+$/, { + message: t("slug_format_message"), + }), + // rest of the schema... + });Option 2: Use a factory function to create the schema with the translation function:
-const valuesetFormSchema = z.object({...}); +const createValuesetFormSchema = (t: any) => z.object({ + name: z.string().trim().min(1, t("field_required")), + slug: z + .string() + .trim() + .min(5, t("field_required")) + .max(25, t("slug_lengthy_message")) + .regex(/^[-\w]+$/, { + message: t("slug_format_message"), + }), + // rest of the schema... +}); export function ValueSetForm({ initialData, onSubmit, isSubmitting, }: ValueSetFormProps) { const { t } = useTranslation(); + const valuesetFormSchema = createValuesetFormSchema(t);Also applies to: 410-410
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ValueSet/ValueSetForm.tsx(2 hunks)
🔇 Additional comments (3)
src/components/ValueSet/ValueSetForm.tsx (3)
344-353: Improved validation for name and slug fieldsThe enhanced validation for name and slug fields with trimming whitespace and regex pattern validation is a good addition that will ensure consistent data format. The regex pattern
/^[-\w]+$/correctly enforces that slugs only contain letters, numbers, underscores, and hyphens.
350-351:✅ Verification successful
Verify translation key consistency
Make sure the translation key
slug_lengthy_messageis properly defined in your locale files to match what you're using in the code.
🏁 Script executed:
#!/bin/bash # Check if the translation key exists in locale files grep -r "slug_lengthy_message" public/locale/Length of output: 116
Translation key 'slug_lengthy_message' is properly defined in the locale files.
The verification confirmed that the key exists inpublic/locale/en.jsonwith the appropriate value ("Slug is too lengthy"). No changes are necessary.
347-353:❓ Verification inconclusive
Verify the translation implementation
Based on the PR comments, there were issues with translations not displaying correctly. Ensure that the translations are working as expected when the form validation is triggered.
🌐 Web query:
What are best practices for handling translations in React form validation messages?💡 Result:
When handling translations in React form validation messages, follow these best practices to ensure a seamless multilingual experience:
1. Use Internationalization Libraries
Integrate robust i18n libraries like react-i18next or FormatJS (react-intl)[3] to manage translation keys and locale switching efficiently. These tools provide:
- Centralized translation files (e.g., JSON)
- Dynamic message interpolation
- Pluralization and formatting support
2. Decouple Validation Messages from Logic
Store all validation messages as translation keys instead of hardcoding strings:
// en.json { "validation.required": "This field is required", "validation.email": "Invalid email format" }3. Integrate with Validation Libraries
For Yup (commonly used with Formik):
const schema = Yup.object().shape({ email: Yup.string() .email(t('validation.email')) .required(t('validation.required')) });Listen to language changes and force revalidation:
i18n.on('languageChanged', () => { Object.keys(formik.errors).forEach(fieldName => { formik.setFieldTouched(fieldName); }); });Source[2]
For React Hook Form:
const { t } = useTranslation(); const schema = yup.object().shape({ password: yup.string() .min(8, t('validation.password.min')) }); useForm({ resolver: yupResolver(schema) });4. Handle Dynamic Language Changes
Use hooks or HOCs to sync form errors with active locale:
const useTranslateFormErrors = (form) => { const { t } = useTranslation(); useEffect(() => { Object.keys(form.errors).forEach(field => { form.setFieldError(field, t(form.errors[field])); }); }, [i18n.language]); };Source[2]
5. Consider Validation Library Support
Some libraries like Fonk provide built-in i18n integration:
Validators.required.setErrorMessage(t('required'));Source[5]
6. Implement Accessible Error Display
Combine translation with ARIA live regions:
<Formik> {({ errors }) => ( <div role="alert" aria-live="polite"> {t(errors.email)} </div> )} </Formik>Comparison Table: Approach Strategies
Method Pros Cons Yup + Formik Tight integration Manual language change handling React Hook Form Uncontrolled performance Requires custom resolvers Native Constraints Browser-native Limited customization Key Recommendations
- Use memoized validation schemas to prevent unnecessary recalculations
- Centralize all form copy in translation files
- Test edge cases like right-to-left languages
- Combine client-side + server-side validation messages
By following these patterns, you'll create forms that smoothly adapt to locale changes while maintaining clean code structure[4][7][9].
Citations:
- 1: https://formspree.io/blog/react-form-validation/
- 2: https://dev.to/codedivoire/how-to-internationalize-a-yup-validation-schema-in-a-react-formik-and-react-i18next-app-cj7
- 3: https://phrase.com/blog/posts/react-i18n-format-js/
- 4: https://daily.dev/blog/form-on-react-best-practices
- 5: https://lemoncode.github.io/fonk-doc/messages/internationalization/
- 6: https://css-tricks.com/two-ways-to-create-custom-translated-messaging-for-html-forms/
- 7: https://devaradise.com/react-form-handling-validation/
- 8: https://ej2.syncfusion.com/react/documentation/form-validator/localization
- 9: https://github.com/orgs/react-hook-form/discussions/2632
- 10: https://www.dhiwise.com/post/react-form-validation-best-practices-with-tips-and-tricks
Verify Translation Implementation
The current use of the
tfunction within the Yup validation schema aligns with best practices for using translation keys in form validations. However, please verify the following to ensure translations display correctly when validation is triggered:
- Confirm that the translation keys (
"field_required","slug_lengthy_message","slug_format_message") are correctly defined in your localization files.- If the form supports dynamic language switching, ensure that the validation schema is recreated or that error messages are refreshed when the active language changes.
- Double-check that the i18n library (e.g., react-i18next) is properly configured and that translations are correctly injected at runtime.
| .string() | ||
| .trim() | ||
| .min(5, t("field_required")) | ||
| .max(25, t("slug_lengthy_message")) |
There was a problem hiding this comment.
let's use variables to keep the message generic so that it can be re-used elsewhere instead of creating too many translations keys specific for each one
| .max(25, t("slug_lengthy_message")) | |
| .max(25, t("max_character_validation", { length: 25 })) |
There was a problem hiding this comment.
Complete the merge checklist as well @hrit2773
There was a problem hiding this comment.
Complete the merge checklist as well @hrit2773
Yup
|
@rithviknishad @Jacobjeevan made the suggested change |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/components/ValueSet/ValueSetForm.tsx (1)
499-515:⚠️ Potential issueFix undefined
goBackfunction errorThe
goBackfunction is referenced in the Cancel button's click handler but is not defined, causing a TypeScript error. According to the AI summary, thegoBackfunctionality was intended to be removed.You have two options to fix this issue:
Option 1: Import and use the useNavigate hook (recommended):
- onClick={() => goBack("/admin/valuesets")} + onClick={() => navigate("/admin/valuesets")}With the corresponding import at the top:
import { useNavigate } from "react-router-dom";And adding this inside the component:
const navigate = useNavigate();Option 2: If you want to completely remove the Cancel button functionality as mentioned in the summary:
- <Button - variant="outline" - disabled={isSubmitting} - type="button" - onClick={() => goBack("/admin/valuesets")} - > - {t("cancel")} - </Button>🧰 Tools
🪛 GitHub Check: cypress-run (1)
[failure] 504-504:
Cannot find name 'goBack'.🪛 GitHub Actions: Cypress Tests
[error] 504-504: TS2304: Cannot find name 'goBack'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(1 hunks)src/components/ValueSet/ValueSetForm.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locale/en.json
🧰 Additional context used
🪛 GitHub Check: lint
src/components/ValueSet/ValueSetForm.tsx
[failure] 516-516:
Delete ··⏎······
🪛 ESLint
src/components/ValueSet/ValueSetForm.tsx
[error] 516-517: Delete ··⏎······
(prettier/prettier)
🪛 GitHub Actions: Lint Code Base
src/components/ValueSet/ValueSetForm.tsx
[error] 516-516: Prettier formatting check failed. Delete ··⏎······.
🪛 GitHub Actions: Cypress Tests
src/components/ValueSet/ValueSetForm.tsx
[error] 504-504: TS2304: Cannot find name 'goBack'.
🔇 Additional comments (2)
src/components/ValueSet/ValueSetForm.tsx (2)
342-351: Well-implemented validation enhancements with proper internationalizationThe validation schema for name and slug fields has been appropriately enhanced:
- Added
.trim()to both fields to handle whitespace properly- Implemented minimum and maximum length validations for the slug field
- Added a regex pattern to enforce proper slug format (alphanumeric characters, hyphens, and underscores only)
- Used translation functions consistently for all error messages
This implementation aligns with the PR objectives to add validations to the Name and Slug fields.
347-349: Verify minimum length validation error messageThe slug field has a minimum length of 5 characters but uses the generic
field_requirederror message which may not accurately convey the minimum length requirement to users.Consider using a more specific error message that communicates the minimum length requirement:
- .min(5, t("field_required")) + .min(5, t("min_character_validation", { length: 5 }))This assumes you have a translation key for minimum character validation similar to the maximum character validation key you're already using.
| @@ -394,15 +399,14 @@ | |||
| ), | |||
| }), | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove trailing blank line
There's an extra blank line after the schema definition which needs to be removed to comply with the project's code style.
-📝 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.
| // (The file now has no trailing blank line) |
| @@ -509,6 +513,7 @@ | |||
| {isSubmitting ? t("saving") : t("save_valueset")} | |||
| </Button> | |||
| </div> | |||
|
|
|||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove trailing whitespace
There's trailing whitespace on this line that needs to be removed to comply with the project's code style.
-
+📝 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.
🧰 Tools
🪛 GitHub Check: lint
[failure] 516-516:
Delete ··⏎······
🪛 ESLint
[error] 516-517: Delete ··⏎······
(prettier/prettier)
🪛 GitHub Actions: Lint Code Base
[error] 516-516: Prettier formatting check failed. Delete ··⏎······.
|
|
|
@nihal467 Now it should work can you validate it again. |
Proposed Changes
@rithviknishad @Jacobjeevan
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes