Skip to content

feat(frontend): add useForm hook#1040

Draft
yassinedorbozgithub wants to merge 6 commits intomainfrom
1025-issue---enhance-useform-hook
Draft

feat(frontend): add useForm hook#1040
yassinedorbozgithub wants to merge 6 commits intomainfrom
1025-issue---enhance-useform-hook

Conversation

@yassinedorbozgithub
Copy link
Collaborator

@yassinedorbozgithub yassinedorbozgithub commented May 26, 2025

Motivation

The main motivation is to add a new useForm hook.

Fixes #1025

Checklist:

  • I have performed a self-review of my own code
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • Refactor

    • Unified and streamlined form validation across all forms by introducing a custom form hook, resulting in centralized and consistent validation rules.
    • Simplified form field registration and validation logic, reducing redundancy and improving maintainability.
    • Minor updates such as updating copyright years.
  • New Features

    • Added new custom hooks and type definitions to enhance form handling and validation capabilities.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 26, 2025

Walkthrough

A custom useStrictForm hook was introduced to centralize and enforce validation rules for forms, replacing the direct usage of useForm from react-hook-form across many components. Validation rules are now defined in a single location per form, and new utility types and hooks were added to support this stricter, more maintainable approach.

Changes

File(s) Change Summary
frontend/src/hooks/useStrictForm.ts, frontend/src/hooks/useStrictRegister.ts Added new custom hooks: useStrictForm and useStrictRegister for stricter, centralized form handling.
frontend/src/types/react-hook-form.types.ts Added new TypeScript types to support strict form registration and validation.
frontend/src/hooks/useValidationRules.ts Refactored validation rules structure, updated copyright.
frontend/src/app-components/auth/Login.tsx
frontend/src/app-components/auth/Register.tsx
frontend/src/app-components/auth/ResetPassword.tsx
frontend/src/app-components/auth/resetPasswordRequest.tsx
Refactored to use useStrictForm, centralized validation rules, removed external validation dependencies.
frontend/src/components/Menu/MenuForm.tsx
frontend/src/components/categories/CategoryForm.tsx
frontend/src/components/content-types/ContentTypeForm.tsx
frontend/src/components/contents/ContentForm.tsx
frontend/src/components/context-vars/ContextVarForm.tsx
frontend/src/components/labels/LabelForm.tsx
frontend/src/components/languages/LanguageForm.tsx
frontend/src/components/nlp/components/NlpEntityForm.tsx
frontend/src/components/nlp/components/NlpTrainForm.tsx
frontend/src/components/nlp/components/NlpValueForm.tsx
frontend/src/components/profile/profile.tsx
frontend/src/components/roles/RoleForm.tsx
frontend/src/components/subscribers/SubscriberForm.tsx
frontend/src/components/translations/TranslationForm.tsx
frontend/src/components/users/EditUserForm.tsx
frontend/src/components/users/InviteUserForm.tsx
frontend/src/components/visual-editor/BlockEditForm.tsx
Replaced useForm with useStrictForm, moved validation rules to hook config, simplified registration.

Sequence Diagram(s)

sequenceDiagram
    participant Component
    participant useStrictForm
    participant useStrictRegister
    participant useForm (react-hook-form)
    participant useValidationRules

    Component->>useStrictForm: Initialize form with { rules, ...props }
    useStrictForm->>useForm (react-hook-form): Call useForm(props)
    useStrictForm->>useStrictRegister: Wrap register with rules
    useStrictRegister->>useValidationRules: Get default rules
    useStrictRegister->>useForm (react-hook-form): Register fields with merged rules
    useStrictForm-->>Component: Return form methods with strict register
    Component->>useStrictForm: Register fields (no inline rules)
Loading

Assessment against linked issues

Objective Addressed Explanation
Enhance useForm hook to centralize repeated code and strengthen typing (#1025)

Poem

In fields of forms where errors hide,
A rabbit hops with rules beside.
With stricter hooks and types anew,
Validation’s clear, concise, and true.
No more rules that roam or stray—
Centralized, they guide the way!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (2)
frontend/src/components/nlp/components/NlpEntityForm.tsx (1)

142-153: 🛠️ Refactor suggestion

Inconsistent validation pattern for the weight field.

The weight field still uses inline validation rules in the register call, which is inconsistent with the new pattern where validation rules should be moved to the rules object in the useForm configuration.

Consider moving these validation rules to the rules object for consistency:

  } = useForm<INlpEntityAttributes>({
    defaultValues: {
      name: nlpEntity?.name || "",
      doc: nlpEntity?.doc || "",
      lookups: nlpEntity?.lookups || ["keywords"],
      weight: nlpEntity?.weight || 1,
    },
    rules: {
      name: {
        required: t("message.name_is_required"),
      },
+     weight: {
+       valueAsNumber: true,
+       required: t("message.weight_required_error"),
+       min: {
+         value: 0.01,
+         message: t("message.weight_positive_number_error"),
+       },
+       validate: (value) =>
+         value && value > 0
+           ? true
+           : t("message.weight_positive_number_error"),
+     },
    },
  });

And simplify the register call:

-              {...register("weight", {
-                valueAsNumber: true,
-                required: t("message.weight_required_error"),
-                min: {
-                  value: 0.01,
-                  message: t("message.weight_positive_number_error"),
-                },
-                validate: (value) =>
-                  value && value > 0
-                    ? true
-                    : t("message.weight_positive_number_error"),
-              })}
+              {...register("weight")}
frontend/src/hooks/useForm.ts (1)

1-70: 🛠️ Refactor suggestion

Well-designed wrapper hook with potential field name extraction issue.

The custom useForm hook implementation is well-structured and provides good type safety. However, there's a potential issue with the field name extraction logic.

Issue with field name extraction (line 42):

const fieldName = args[0].split(".").at(-1) || "";

This approach has several concerns:

  1. For nested field paths like "user.profile.email", it only extracts "email", which may not match the intended validation rule key
  2. The at(-1) method may not be available in all JavaScript environments (ES2022 feature)
  3. It assumes the field name is always a simple string

Suggested improvements:

-    const fieldName = args[0].split(".").at(-1) || "";
+    const fieldName = args[0];
+    // Alternative: Extract last segment more safely
+    // const fieldName = args[0].split(".").pop() || args[0];

Consider using the full field path as the key or implementing a more robust field name resolution strategy that matches your validation rules structure.

The rest of the implementation looks solid with proper TypeScript generics, correct rule merging logic (field-specific rules override defaults), and maintaining the same interface as the original useForm hook.

🧹 Nitpick comments (1)
frontend/src/components/users/InviteUserForm.tsx (1)

76-78: Consider moving roles validation to the centralized rules for consistency.

The roles field validation is still defined directly in the Controller component, which is inconsistent with the new pattern used for the email field. For better consistency and maintainability, consider moving this validation to the rules property in the useForm call.

  rules: {
    email: {
      required: t("message.email_is_required"),
    },
+   roles: {
+     required: t("message.roles_is_required"),
+   },
  },

Then remove the validation from the Controller:

  <Controller
    name="roles"
-   rules={{
-     required: t("message.roles_is_required"),
-   }}
    control={control}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 933daaa and a9be452.

📒 Files selected for processing (20)
  • frontend/src/app-components/auth/Login.tsx (4 hunks)
  • frontend/src/app-components/auth/Register.tsx (8 hunks)
  • frontend/src/app-components/auth/ResetPassword.tsx (5 hunks)
  • frontend/src/app-components/auth/resetPasswordRequest.tsx (4 hunks)
  • frontend/src/components/Menu/MenuForm.tsx (4 hunks)
  • frontend/src/components/categories/CategoryForm.tsx (3 hunks)
  • frontend/src/components/content-types/ContentTypeForm.tsx (3 hunks)
  • frontend/src/components/contents/ContentForm.tsx (1 hunks)
  • frontend/src/components/context-vars/ContextVarForm.tsx (4 hunks)
  • frontend/src/components/labels/LabelForm.tsx (5 hunks)
  • frontend/src/components/languages/LanguageForm.tsx (3 hunks)
  • frontend/src/components/nlp/components/NlpEntityForm.tsx (3 hunks)
  • frontend/src/components/nlp/components/NlpTrainForm.tsx (3 hunks)
  • frontend/src/components/profile/profile.tsx (8 hunks)
  • frontend/src/components/roles/RoleForm.tsx (3 hunks)
  • frontend/src/components/translations/TranslationForm.tsx (1 hunks)
  • frontend/src/components/users/InviteUserForm.tsx (3 hunks)
  • frontend/src/components/visual-editor/BlockEditForm.tsx (3 hunks)
  • frontend/src/hooks/useForm.ts (1 hunks)
  • frontend/src/hooks/useValidationRules.ts (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
frontend/src/hooks/useValidationRules.ts (1)
frontend/src/hooks/useForm.ts (1)
  • TRegisterOption (21-25)
frontend/src/components/visual-editor/BlockEditForm.tsx (1)
api/src/i18n/services/i18n.service.ts (1)
  • t (27-57)
frontend/src/hooks/useForm.ts (1)
frontend/src/hooks/useValidationRules.ts (1)
  • useValidationRules (15-48)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Frontend Tests
  • GitHub Check: API-Tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (39)
frontend/src/components/contents/ContentForm.tsx (2)

24-24: Import change looks good, but validation migration appears incomplete.

The import has been successfully updated to use the custom @/hooks/useForm hook.


151-160: 🛠️ Refactor suggestion

Inconsistent validation pattern - consider completing the migration.

The validation rules are still defined locally (lines 151-160) and passed inline to the Controller (lines 209-215), which contradicts the stated goal of centralizing validation rules in the custom useForm hook. This creates an inconsistent pattern compared to other files in this PR.

Consider moving these validation rules into the rules option of the useForm hook call:

  } = useForm<IContentAttributes & { [key: string]: any }>({
    defaultValues: {
      entity: content?.entity || "",
      status: content?.status || false,
      title: content?.title || "",
    },
+   rules: {
+     title: {
+       required: t("message.title_is_required"),
+     },
+     url: {
+       required: t("message.url_is_invalid"),
+       validate: (value: string) =>
+         isAbsoluteUrl(value) || t("message.url_is_invalid"),
+     },
+   },
  });
- const validationRules = {
-   title: {
-     required: t("message.title_is_required"),
-   },
-   url: {
-     required: t("message.url_is_invalid"),
-     validate: (value: string) =>
-       isAbsoluteUrl(value) || t("message.url_is_invalid"),
-   },
- };

And simplify the Controller rules:

                 rules={
-                  contentField.name === "title"
-                    ? validationRules.title
-                    : contentField.type === ContentFieldType.URL
-                    ? validationRules.url
-                    : undefined
+                  undefined
                 }

Also applies to: 209-215

Likely an incorrect or invalid review comment.

frontend/src/components/translations/TranslationForm.tsx (1)

11-11: LGTM! Import strategy is appropriate.

The mixed import approach is correct - Controller and ControllerRenderProps remain from react-hook-form while useForm uses the custom hook. This maintains backward compatibility while adopting the new centralized validation pattern.

Also applies to: 17-17

frontend/src/components/nlp/components/NlpTrainForm.tsx (2)

26-26: LGTM! Correct import migration pattern.

The import changes are consistent with the refactoring strategy - useForm migrated to the custom hook while Controller and useFieldArray remain from react-hook-form. This maintains proper separation of concerns.

Also applies to: 35-35


326-326: Minor formatting improvement.

The comment formatting change from multi-line to single-line is a minor improvement for consistency.

frontend/src/components/roles/RoleForm.tsx (3)

15-15: LGTM! Correct import migration.

Successfully updated to use the custom useForm hook.


48-53: Excellent implementation of centralized validation rules.

This demonstrates the intended pattern perfectly - validation rules are now centralized within the useForm hook configuration via the rules option. This improves consistency and maintainability across the codebase.


83-83: LGTM! Simplified register call.

The register("name") call is now clean and simple, with validation rules handled centrally in the hook configuration. This demonstrates the benefits of the refactoring effort.

frontend/src/components/languages/LanguageForm.tsx (3)

17-17: LGTM: Import change aligns with the custom useForm hook pattern.

The import change from react-hook-form to the custom @/hooks/useForm is consistent with the PR objective to centralize form validation.


56-64: Well-structured validation rules centralization.

The validation rules for title and code fields are properly centralized within the useForm hook configuration. This approach improves maintainability and consistency across the codebase.


91-91: Good UX improvement with required attributes and simplified register calls.

Adding the required attribute to Input components provides visual feedback to users, while simplifying the register calls reduces code duplication since validation is now handled centrally.

Also applies to: 94-94, 102-102, 105-105

frontend/src/hooks/useValidationRules.ts (2)

9-9: Good addition of proper imports for type safety.

The new imports of FieldValues and TRegisterOption enable proper TypeScript typing for the validation rules object.

Also applies to: 13-13


17-17: Excellent type safety improvement with satisfies operator.

The renaming to rules and the addition of the satisfies type assertion provides compile-time validation that the rules object conforms to the expected TRegisterOption type structure. This prevents type mismatches and improves developer experience.

Also applies to: 45-45, 47-47

frontend/src/components/content-types/ContentTypeForm.tsx (3)

18-18: Consistent import change following the new pattern.

The import change to the custom useForm hook maintains consistency with the overall refactoring effort.


48-53: Proper validation rule centralization for name field.

The validation rule for the name field is correctly moved to the centralized rules configuration, maintaining the required validation with proper error messaging.


122-122: Clean register call simplification.

The register call is properly simplified since validation rules are now handled centrally through the rules configuration.

frontend/src/app-components/auth/ResetPassword.tsx (2)

15-15: Consistent import change following the refactoring pattern.

The import change to the custom useForm hook aligns with the overall refactoring effort.


75-75: Simplified register calls following the new pattern.

The register calls are properly simplified since validation is now handled through the centralized rules configuration.

Also applies to: 86-86

frontend/src/app-components/auth/resetPasswordRequest.tsx (2)

2-2: LGTM: Copyright year update.

The copyright year update to 2025 is appropriate and timely.


13-13: Excellent migration to the custom useForm hook.

The refactor successfully centralizes validation rules within the useForm configuration, removing the need for inline validation rules in the register call. This improves maintainability and follows a consistent pattern across the codebase.

The email validation requirement is properly preserved in the new structure.

Also applies to: 30-35, 63-63

frontend/src/components/nlp/components/NlpEntityForm.tsx (1)

22-22: Good migration to the custom useForm hook.

The refactor properly centralizes the name field validation within the useForm configuration, improving consistency with the new pattern.

Also applies to: 65-70, 123-123

frontend/src/components/categories/CategoryForm.tsx (1)

16-16: Clean migration to the custom useForm hook.

The refactor successfully centralizes validation rules and simplifies the register call. The label validation requirement is properly preserved in the new structure, following the consistent pattern established across the codebase.

Also applies to: 51-54, 81-81

frontend/src/components/labels/LabelForm.tsx (1)

15-15: Excellent migration to the custom useForm hook.

The refactor successfully centralizes the title validation rules while preserving all existing functionality. All register calls are consistently simplified, and the custom InputProps.onChange handler for the title field is properly maintained. This follows the established pattern perfectly.

Also applies to: 55-60, 91-91, 105-105, 114-114

frontend/src/components/context-vars/ContextVarForm.tsx (1)

17-17: LGTM! Excellent refactoring to use the custom useForm hook.

The changes successfully centralize validation rules into the useForm configuration while maintaining the same validation logic:

  • name field validates the regex pattern for valid context variable names
  • label field enforces the required constraint
  • register calls are appropriately simplified

This improves code maintainability and follows the new centralized validation pattern.

Also applies to: 64-75, 106-106, 121-121

frontend/src/app-components/auth/Login.tsx (1)

19-19: LGTM! Clean migration to the custom useForm hook.

The validation logic is properly centralized with:

  • Required validation for identifier field
  • Required validation for password field
  • Simplified register calls maintaining the same functionality

The refactoring enhances code consistency across the authentication flow.

Also applies to: 66-74, 105-105, 116-116

frontend/src/components/users/InviteUserForm.tsx (1)

16-16: LGTM! Clean adoption of the custom useForm hook for email validation.

The email field validation is properly centralized in the useForm rules configuration, and the register call is appropriately simplified.

Also applies to: 50-55, 69-69

frontend/src/components/profile/profile.tsx (1)

25-25: LGTM! Excellent implementation of complex validation with the custom useForm hook.

The refactoring successfully handles cross-field validation for password confirmation while simplifying all other field registrations:

  • Password confirmation validation properly uses watch() and trigger() for dynamic validation
  • All register calls are appropriately simplified, relying on the custom hook's default validation rules
  • The complex validation logic is well-contained within the rules configuration

This demonstrates how the custom useForm hook can effectively handle both simple and complex validation scenarios.

Also applies to: 61-72, 118-118, 129-129, 160-160, 172-172, 184-184

frontend/src/components/visual-editor/BlockEditForm.tsx (3)

13-13: LGTM: Import changes align with the custom useForm hook pattern.

The import changes correctly switch from the external react-hook-form to the custom useForm hook while retaining the needed Controller import.

Also applies to: 20-20


86-90: LGTM: Validation rules properly centralized.

The validation rules have been correctly moved into the rules object within the useForm hook configuration, following the new pattern established in this refactoring.


125-127: LGTM: Register call simplified and UX improved.

The register call has been appropriately simplified to rely on the centralized validation rules, and the addition of the required attribute improves form accessibility and user experience.

frontend/src/components/Menu/MenuForm.tsx (4)

11-11: LGTM: Import changes follow the established pattern.

The import modifications correctly adopt the custom useForm hook while maintaining the necessary Controller import.

Also applies to: 18-18


64-75: LGTM: Comprehensive validation rules centralization.

The validation rules for type, title, and url fields have been properly centralized. The URL validation using isAbsoluteUrl is appropriate for ensuring valid web URLs.


106-125: LGTM: Component change from Controller to Input is valid.

The change from Controller to Input component for the type field is acceptable since it still uses the register function for form control. The onChange handler for resetting the URL field when type changes is a good UX pattern.


133-133: LGTM: Register calls simplified appropriately.

The register calls have been correctly simplified to rely on the centralized validation rules defined in the useForm hook.

Also applies to: 144-144

frontend/src/app-components/auth/Register.tsx (3)

26-26: LGTM: Import change aligns with refactoring pattern.

The import change to use the custom useForm hook is consistent with the codebase-wide refactoring.


73-98: LGTM: Comprehensive validation rules with proper cross-field validation.

The validation rules cover all form fields appropriately. The password confirmation validation correctly implements cross-field validation using watch() and trigger() to ensure passwords match.


152-152: LGTM: Register calls appropriately simplified.

All register calls have been correctly simplified to rely on the centralized validation rules, maintaining the same functionality while reducing code duplication.

Also applies to: 166-166, 178-178, 190-190, 204-204, 216-216

frontend/src/hooks/useForm.ts (2)

21-25: LGTM: Well-defined TypeScript types.

The type definitions are comprehensive and properly constrained. TRegisterOption and TRules provide good type safety for the validation rules structure.

Also applies to: 31-33


46-49: LGTM: Proper rule merging logic.

The rule merging logic correctly prioritizes field-specific options over default options, allowing for proper customization while maintaining sensible defaults.

Comment on lines +33 to 41
rules: {
password: {
required: t("message.password_is_required"),
},
password2: {
required: t("message.password_is_required"),
},
},
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Potential loss of password validation logic.

The current implementation only includes required validation for both password fields. However, based on the AI summary mentioning that useValidationRules was previously used, this form likely had more comprehensive validation including:

  • Password minimum length requirements
  • Password confirmation matching validation

This appears to be a significant loss of validation logic during the migration.

Please verify that the password validation rules from useValidationRules are properly applied. The custom useForm hook should merge these default rules automatically. Check if the validation rules for password and password2 from useValidationRules.ts are being applied:

#!/bin/bash
# Verify that password validation rules are being merged correctly
echo "Checking useValidationRules for password validation:"
cat frontend/src/hooks/useValidationRules.ts | grep -A 10 "password"

echo -e "\nChecking the custom useForm implementation:"
cat frontend/src/hooks/useForm.ts
🤖 Prompt for AI Agents
In frontend/src/app-components/auth/ResetPassword.tsx around lines 33 to 41, the
password validation rules currently only enforce the fields as required, missing
important validations like minimum length and password confirmation matching. To
fix this, ensure that the validation rules from useValidationRules.ts for both
password and password2 are properly merged or applied in this form. Review the
useForm hook implementation to confirm it merges default validation rules
automatically, and update the rules object here to include all necessary
validations from useValidationRules, not just the required check.

@yassinedorbozgithub yassinedorbozgithub marked this pull request as draft May 26, 2025 05:59
@yassinedorbozgithub yassinedorbozgithub added the enhancement New feature or request label May 26, 2025
@marrouchi marrouchi marked this pull request as ready for review May 30, 2025 15:03
Copy link
Contributor

@medchedli medchedli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think EditUserForm.tsx component also need to updated

@yassinedorbozgithub
Copy link
Collaborator Author

password_is_required

@medchedli Thank you for your feedback. Fixes are pushed 🪄

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (1)
frontend/src/components/users/EditUserForm.tsx (1)

50-57: 🛠️ Refactor suggestion

Inconsistent validation pattern - missing rules in useStrictForm call.

The validationRules object is defined but not passed to useStrictForm. According to the new pattern, validation rules should be centralized within the hook configuration.

Apply this diff to properly implement the useStrictForm pattern:

  } = useStrictForm<IUserAttributes>({
    defaultValues: { roles: roles?.map((role) => role.id) },
+   rules: validationRules,
  });
- const validationRules = {
-   roles: {
-     required: t("message.roles_is_required"),
-   },
- };

And update the Controller to remove inline validation:

            <Controller
              name="roles"
-             rules={validationRules.roles}
              control={control}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2a0fc8 and ae08a7f.

📒 Files selected for processing (25)
  • frontend/src/app-components/auth/Login.tsx (4 hunks)
  • frontend/src/app-components/auth/Register.tsx (8 hunks)
  • frontend/src/app-components/auth/ResetPassword.tsx (4 hunks)
  • frontend/src/app-components/auth/resetPasswordRequest.tsx (4 hunks)
  • frontend/src/components/Menu/MenuForm.tsx (4 hunks)
  • frontend/src/components/categories/CategoryForm.tsx (3 hunks)
  • frontend/src/components/content-types/ContentTypeForm.tsx (3 hunks)
  • frontend/src/components/contents/ContentForm.tsx (2 hunks)
  • frontend/src/components/context-vars/ContextVarForm.tsx (4 hunks)
  • frontend/src/components/labels/LabelForm.tsx (5 hunks)
  • frontend/src/components/languages/LanguageForm.tsx (3 hunks)
  • frontend/src/components/nlp/components/NlpEntityForm.tsx (3 hunks)
  • frontend/src/components/nlp/components/NlpTrainForm.tsx (3 hunks)
  • frontend/src/components/nlp/components/NlpValueForm.tsx (5 hunks)
  • frontend/src/components/profile/profile.tsx (8 hunks)
  • frontend/src/components/roles/RoleForm.tsx (3 hunks)
  • frontend/src/components/subscribers/SubscriberForm.tsx (2 hunks)
  • frontend/src/components/translations/TranslationForm.tsx (2 hunks)
  • frontend/src/components/users/EditUserForm.tsx (2 hunks)
  • frontend/src/components/users/InviteUserForm.tsx (3 hunks)
  • frontend/src/components/visual-editor/BlockEditForm.tsx (3 hunks)
  • frontend/src/hooks/useStrictForm.ts (1 hunks)
  • frontend/src/hooks/useStrictRegister.ts (1 hunks)
  • frontend/src/hooks/useValidationRules.ts (2 hunks)
  • frontend/src/types/react-hook-form.types.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • frontend/src/types/react-hook-form.types.ts
🚧 Files skipped from review as they are similar to previous changes (19)
  • frontend/src/components/translations/TranslationForm.tsx
  • frontend/src/components/content-types/ContentTypeForm.tsx
  • frontend/src/components/nlp/components/NlpTrainForm.tsx
  • frontend/src/app-components/auth/Login.tsx
  • frontend/src/components/languages/LanguageForm.tsx
  • frontend/src/components/labels/LabelForm.tsx
  • frontend/src/app-components/auth/ResetPassword.tsx
  • frontend/src/components/contents/ContentForm.tsx
  • frontend/src/components/profile/profile.tsx
  • frontend/src/components/context-vars/ContextVarForm.tsx
  • frontend/src/components/nlp/components/NlpEntityForm.tsx
  • frontend/src/components/roles/RoleForm.tsx
  • frontend/src/components/users/InviteUserForm.tsx
  • frontend/src/components/visual-editor/BlockEditForm.tsx
  • frontend/src/hooks/useValidationRules.ts
  • frontend/src/components/Menu/MenuForm.tsx
  • frontend/src/app-components/auth/resetPasswordRequest.tsx
  • frontend/src/components/categories/CategoryForm.tsx
  • frontend/src/app-components/auth/Register.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/src/components/subscribers/SubscriberForm.tsx (1)
frontend/src/hooks/useStrictForm.ts (1)
  • useStrictForm (20-36)
frontend/src/components/users/EditUserForm.tsx (2)
frontend/src/hooks/useStrictForm.ts (1)
  • useStrictForm (20-36)
frontend/src/types/user.types.ts (1)
  • IUserAttributes (15-24)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: API-Tests
  • GitHub Check: Frontend Tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
frontend/src/components/subscribers/SubscriberForm.tsx (1)

51-51: Verify if validation rules are needed for form fields.

The migration to useStrictForm is complete, but no validation rules are applied. Please verify if the labels field requires validation (e.g., required field validation).

If validation is needed, consider adding rules to the useStrictForm configuration:

  } = useStrictForm<ISubscriberAttributes>({
+   rules: {
+     labels: {
+       // Add validation rules if needed
+     },
+   },
  });
frontend/src/components/nlp/components/NlpValueForm.tsx (2)

92-108: Excellent implementation of the useStrictForm pattern.

This correctly demonstrates the intended pattern:

  • Validation rules are centralized in the useStrictForm configuration
  • The register call relies on the centralized rules
  • Clean separation of concerns between validation definition and field registration

146-146: Correct usage of register without explicit validation.

The register("value") call correctly omits explicit validation rules, relying on the centralized rules defined in the useStrictForm configuration. This follows the intended pattern.

frontend/src/hooks/useStrictForm.ts (1)

20-36: Well-designed hook architecture with proper type safety.

The useStrictForm implementation demonstrates excellent design principles:

  • Proper generic constraints: Uses appropriate TypeScript generics with sensible defaults
  • Clean prop extraction: Cleanly separates rules from standard UseFormProps
  • Delegation pattern: Delegates enhanced registration to useStrictRegister
  • Type preservation: Maintains the same return interface as useForm while enhancing functionality
  • Backwards compatibility: Works as a drop-in replacement for useForm

This provides a solid foundation for centralized form validation across the application.

frontend/src/hooks/useStrictRegister.ts (3)

9-13: LGTM! Clean import structure.

The imports are well-organized and follow a logical structure: external dependencies first, then internal types, and finally local hooks.


15-18: Good generic typing and parameter structure.

The function signature properly uses generics to maintain type safety across different form field types, and the optional rules parameter provides flexibility for per-field overrides.


26-30:

Details

❓ Verification inconclusive

Verify the option precedence logic.

The merging logic looks correct - args[1] takes precedence over rules?.[fieldName], and both override defaultOptions. However, ensure this precedence aligns with the intended behavior documented in the broader form validation strategy.


🏁 Script executed:

#!/bin/bash
# Description: Verify how useStrictRegister is used across the codebase to confirm the precedence logic aligns with usage patterns

# Search for useStrictRegister usage patterns
rg -A 5 -B 2 "useStrictRegister"

# Search for register calls with validation options to understand current patterns
rg -A 3 "register\(" --type ts --type tsx

Length of output: 1517


Verify option precedence in useStrictRegister

Please confirm that the merged options follow the intended priority in your form-validation design:

• frontend/src/hooks/useStrictRegister.ts (lines 26–30):

const fieldOptions = args[1] || rules?.[fieldName];
const mergedOptions = { ...defaultOptions, ...fieldOptions };
return register(args[0], mergedOptions);

– Ensure args[1] (call-site override) wins over rules[fieldName] (hook-level defaults), and both trump defaultOptions.
• frontend/src/hooks/useStrictForm.ts:

return { …restMethods, register: useStrictRegister(register, rules) };

– Verify that any form-level rules passed here behave as expected given the above precedence.

If this ordering differs from your documented validation strategy, please adjust accordingly.

Comment on lines +23 to +25
const defaultOptions = (defaultRules[fieldName] || {}) as
| TFieldValues
| undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect type assertion.

The type assertion as TFieldValues | undefined is incorrect. The defaultRules[fieldName] should contain validation options, not field values.

-    const defaultOptions = (defaultRules[fieldName] || {}) as
-      | TFieldValues
-      | undefined;
+    const defaultOptions = defaultRules[fieldName] || {};
📝 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.

Suggested change
const defaultOptions = (defaultRules[fieldName] || {}) as
| TFieldValues
| undefined;
const defaultOptions = defaultRules[fieldName] || {};
🤖 Prompt for AI Agents
In frontend/src/hooks/useStrictRegister.ts around lines 23 to 25, the type
assertion on defaultRules[fieldName] is incorrect because it asserts to
TFieldValues or undefined, but defaultRules[fieldName] should represent
validation options, not field values. Update the type assertion to reflect the
correct type for validation options instead of TFieldValues, ensuring the type
matches the expected validation rules structure.

const defaultRules = useValidationRules();

return (...args: TRegisterProps<TFieldValues>) => {
const fieldName = args[0].split(".").at(-1) || "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add input validation for field name extraction.

The field name extraction logic could fail if args[0] is not a string or is undefined, potentially causing runtime errors.

-    const fieldName = args[0].split(".").at(-1) || "";
+    const fieldName = typeof args[0] === 'string' ? args[0].split(".").at(-1) || "" : "";
📝 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.

Suggested change
const fieldName = args[0].split(".").at(-1) || "";
- const fieldName = args[0].split(".").at(-1) || "";
+ const fieldName = typeof args[0] === 'string'
+ ? args[0].split(".").at(-1) || ""
+ : "";
🤖 Prompt for AI Agents
In frontend/src/hooks/useStrictRegister.ts at line 22, the code extracts the
field name by splitting args[0] without validating its type or existence. To fix
this, add a check to ensure args[0] is a defined string before calling split and
at; if not, handle the case gracefully by assigning an empty string or a default
value to fieldName to prevent runtime errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request file conflicts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🤔 [FEATURE REQUEST] - Enhance useForm with useStrictForm and useStrictRegister for improved type safety and cleaner usage

2 participants