Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

checks for username and username in card overflow fix #8940

Conversation

SwanandBhuskute
Copy link
Contributor

@SwanandBhuskute SwanandBhuskute commented Oct 28, 2024

Proposed Changes

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced validation for ABHA address input, ensuring proper user input before submission.
    • Real-time password strength feedback in the Reset Password component.
    • Improved username and password validation in the User Add component.
    • Added password validation rules in the User Profile component.
  • Bug Fixes

    • Refined error handling for user management functions, improving user feedback during actions like updating working hours and deleting users.
    • Adjusted layout handling for long usernames in the Manage Users component.

These updates enhance user experience by providing clearer validation messages and improving overall functionality.

@SwanandBhuskute SwanandBhuskute requested a review from a team as a code owner October 28, 2024 06:17
Copy link

netlify bot commented Oct 28, 2024

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 14d145c
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/672a68aa61cfec0008d362f2
😎 Deploy Preview https://deploy-preview-8940--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nihal467 nihal467 added the question Further information is requested label Oct 28, 2024
@Jacobjeevan Jacobjeevan added needs testing needs review and removed question Further information is requested labels Oct 28, 2024
Copy link
Contributor

@Jacobjeevan Jacobjeevan left a comment

Choose a reason for hiding this comment

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

LGTM

@nihal467
Copy link
Member

nihal467 commented Nov 4, 2024

LGTM

@bodhish
Copy link
Member

bodhish commented Nov 5, 2024

@coderabbitai full review

Copy link

coderabbitai bot commented Nov 5, 2024

✅ Actions performed

Full review triggered.

Copy link

coderabbitai bot commented Nov 5, 2024

Walkthrough

The changes implemented in this pull request enhance validation and error handling across multiple components, including CreateWithAadhaar, ResetPassword, ManageUsers, UserAdd, and UserProfile. Notable updates include expanded validation rules for user inputs such as ABHA numbers, passwords, and usernames, with improved feedback mechanisms. The user interface is also refined to prevent overflow of content in the user management section. Overall, these modifications aim to strengthen user input validation and enhance the user experience.

Changes

File Path Change Summary
src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx Enhanced validation logic for ABHA address input with new rules and error handling improvements.
src/components/Auth/ResetPassword.tsx Added password strength validation rules and improved feedback during input focus.
src/components/Users/ManageUsers.tsx Introduced a wrapper for name display to prevent overflow and refined error handling for user updates.
src/components/Users/UserAdd.tsx Updated username and password validation with new rules and visual feedback adjustments.
src/components/Users/UserProfile.tsx Enhanced password validation and error handling for profile updates.

Assessment against linked issues

Objective Addressed Explanation
Improve user validation feedback for username input (#7307)
Prevent overflow of content in user management UI (#8883)

🐇 In the fields where we play,
Validation rules lead the way.
Passwords strong, and names align,
Overflow issues, now all fine!
With every check, we hop with glee,
A smoother path for you and me! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ 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.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate unit testing code.
    • @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.

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 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

@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 and nitpick comments (6)
src/components/Auth/ResetPassword.tsx (1)

131-147: LGTM! Consider extracting validation rules to constants.

The password validation implementation looks good and fixes the issue of premature validation messages. The rules provide comprehensive password strength requirements.

Consider extracting the validation rules to named constants for better maintainability:

const PASSWORD_VALIDATION_RULES = [
  {
    validate: (password: string) => password.length >= 8,
    message: "Password should be atleast 8 characters long"
  },
  // ... other rules
];
src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx (1)

685-700: Consider adding validation for consecutive special characters

The current validation allows consecutive dots and underscores (e.g., "user..name" or "user__name"), which might not be desirable for an ABHA address.

Add an additional validation rule:

+ {validateRule(
+   !/[._]{2,}/.test(healthId),
+   t("abha_address_validation_consecutive_special_chars_error"),
+   false
+ )}
🧰 Tools
🪛 Biome

[error] 688-688: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

src/components/Users/UserAdd.tsx (2)

798-819: Consider extracting regex patterns as constants.

While the username validation rules are well-implemented, consider extracting the regex patterns as named constants at the top of the file. This would improve maintainability and make the patterns more reusable.

+const USERNAME_PATTERNS = {
+  ALLOWED_CHARS: /^[a-z0-9._-]*$/,
+  START_END: /^[a-z0-9].*[a-z0-9]$/i,
+  NO_CONSECUTIVE_SPECIAL: /(?:[._-]{2,})/
+};

851-866: Consider adding a password strength indicator.

While the password validation rules are comprehensive, consider enhancing the user experience by:

  1. Adding a visual password strength indicator (weak/medium/strong)
  2. Implementing a more robust password strength algorithm

Example implementation:

const getPasswordStrength = (password: string): 'weak' | 'medium' | 'strong' => {
  const hasLength = password.length >= 8;
  const hasLower = password !== password.toUpperCase();
  const hasUpper = password !== password.toLowerCase();
  const hasNumber = /\d/.test(password);
  const hasSpecial = /[!@#$%^&*(),.?":{}|<>]/.test(password);
  
  const score = [hasLength, hasLower, hasUpper, hasNumber, hasSpecial]
    .filter(Boolean).length;
    
  return score <= 2 ? 'weak' : score <= 4 ? 'medium' : 'strong';
};
src/components/Users/UserProfile.tsx (2)

951-951: Improve password confirmation validation.

The current implementation only validates password match when the confirmation field is focused. This could lead to a confusing user experience as the validation state is not immediately visible.

Consider showing the validation message whenever both password fields have values:

- {changePasswordForm.new_password_2.length > 0 && (
+ {(changePasswordForm.new_password_1.length > 0 && changePasswordForm.new_password_2.length > 0) && (
    <div className="text-small mb-2 hidden pl-2 text-secondary-500 peer-focus-within:block">
      {validateRule(
        changePasswordForm.new_password_1 === changePasswordForm.new_password_2,
        "Confirm password should match the new password",
        !changePasswordForm.new_password_1,
      )}
    </div>
  )}

Line range hint 910-951: Add rate limiting for password change attempts.

The current implementation doesn't protect against brute force attacks on the password change functionality.

Consider implementing rate limiting for password change attempts:

  1. Track failed password change attempts
  2. Implement exponential backoff
  3. Lock the account temporarily after multiple failed attempts
  4. Add CAPTCHA verification after a certain number of failures

Would you like me to provide a detailed implementation for these security enhancements?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between bc15ed9 and fef7d70.

📒 Files selected for processing (5)
  • src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx (1 hunks)
  • src/components/Auth/ResetPassword.tsx (2 hunks)
  • src/components/Users/ManageUsers.tsx (1 hunks)
  • src/components/Users/UserAdd.tsx (4 hunks)
  • src/components/Users/UserProfile.tsx (2 hunks)
🧰 Additional context used
🪛 Biome
src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx

[error] 688-688: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

🔇 Additional comments (5)
src/components/Auth/ResetPassword.tsx (1)

Line range hint 42-60: Verify server-side password validation.

While the client-side password validation is comprehensive, ensure that these same validation rules are enforced on the server side for security.

src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx (1)

685-700: 🛠️ Refactor suggestion

Verify consistency between UI validation rules and submit validation

The validation rules displayed in the UI don't exactly match the regex pattern used in the submit button's disabled state (/^(?![\d.])[a-zA-Z0-9._]{4,}(?<!\.)$/). This could lead to user confusion where the UI shows all validations as passed, but the submit button remains disabled.

Consider extracting validation logic into a shared function:

+ const validateAbhaAddress = (value: string) => {
+   const rules = [
+     { valid: value.length >= 4, message: t("abha_address_validation_length_error") },
+     { valid: Number.isNaN(Number(value[0])) && value[0] !== ".", message: t("abha_address_validation_start_error") },
+     { valid: value[value.length - 1] !== ".", message: t("abha_address_validation_end_error") },
+     { valid: /^[0-9a-zA-Z._]+$/.test(value), message: t("abha_address_validation_character_error") }
+   ];
+   return {
+     isValid: rules.every(r => r.valid),
+     rules
+   };
+ };

Then use it consistently:

- disabled={memory?.isLoading || !/^(?![\d.])[a-zA-Z0-9._]{4,}(?<!\.)$/.test(healthId)}
+ disabled={memory?.isLoading || !validateAbhaAddress(healthId).isValid}
🧰 Tools
🪛 Biome

[error] 688-688: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

src/components/Users/UserAdd.tsx (2)

148-166: LGTM! Well-implemented validation feedback system.

The enhanced validateRule function now provides better user feedback with three distinct states (initial, valid, invalid), improving the user experience through progressive disclosure.


887-887: LGTM! Confirm password validation is well-implemented.

The confirm password validation provides appropriate feedback and follows the consistent pattern used throughout the form.

src/components/Users/ManageUsers.tsx (1)

249-251: LGTM! Overflow fix implemented correctly.

The addition of max-w-full break-words classes effectively prevents username overflow in cards, addressing issue #8883.

src/components/Auth/ResetPassword.tsx Outdated Show resolved Hide resolved
src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx Outdated Show resolved Hide resolved
src/components/Users/UserProfile.tsx Outdated Show resolved Hide resolved
Copy link

@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 and nitpick comments (1)
src/components/Users/UserProfile.tsx (1)

196-218: Consider enhancing password validation rules and performance.

The password validation implementation could be improved in two ways:

  1. Add special character requirement for better security
  2. Memoize the rules array to prevent unnecessary re-creation on each render
 const validatePassword = (password: string) => {
-  const rules = [
+  const rules = useMemo(() => [
     {
       test: (p: string) => p.length >= 8,
       message: "Password should be at least 8 characters long",
     },
     {
       test: (p: string) => p !== p.toUpperCase(),
       message: "Password should contain at least 1 lowercase letter",
     },
     {
       test: (p: string) => p !== p.toLowerCase(),
       message: "Password should contain at least 1 uppercase letter",
     },
     {
       test: (p: string) => /\d/.test(p),
       message: "Password should contain at least 1 number",
     },
+    {
+      test: (p: string) => /[!@#$%^&*(),.?":{}|<>]/.test(p),
+      message: "Password should contain at least 1 special character",
+    },
-  ];
+  ], []);
   return rules.map((rule) =>
     validateRule(rule.test(password), rule.message, !password),
   );
 };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between fef7d70 and c101bea.

📒 Files selected for processing (3)
  • src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx (1 hunks)
  • src/components/Auth/ResetPassword.tsx (2 hunks)
  • src/components/Users/UserProfile.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/components/ABDM/LinkAbhaNumber/CreateWithAadhaar.tsx
  • src/components/Auth/ResetPassword.tsx
🔇 Additional comments (1)
src/components/Users/UserProfile.tsx (1)

931-931: LGTM!

The password validation feedback implementation provides clear visual feedback to users.

src/components/Users/UserProfile.tsx Outdated Show resolved Hide resolved
Copy link

@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 and nitpick comments (1)
src/components/Users/UserProfile.tsx (1)

954-954: Consider UX improvements for password confirmation.

The password confirmation validation works but could be enhanced:

  1. Use a more user-friendly message like "Passwords must match"
  2. Consider showing validation state persistently, not just on focus

Apply this diff to improve the message:

   validateRule(
     changePasswordForm.new_password_1 === changePasswordForm.new_password_2,
-    "Confirm password should match the new password",
+    "Passwords must match",
     !changePasswordForm.new_password_2,
   )
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c101bea and 14d145c.

📒 Files selected for processing (1)
  • src/components/Users/UserProfile.tsx (3 hunks)
🔇 Additional comments (1)
src/components/Users/UserProfile.tsx (1)

931-931: LGTM!

The password validation UI implementation correctly displays validation rules to users.

src/components/Users/UserProfile.tsx Show resolved Hide resolved
@rithviknishad rithviknishad merged commit 1e45600 into ohcnetwork:develop Nov 6, 2024
6 checks passed
Copy link

github-actions bot commented Nov 6, 2024

@SwanandBhuskute Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌

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

Successfully merging this pull request may close these issues.

Content overflows from the card in /users Issue in adding new user in users section
5 participants