fix: simplified error messages for invalid inputs at profile settings #2473
Conversation
WalkthroughThe changes in this pull request involve the addition of new error messages across multiple localization files ( Changes
Assessment against linked issues
Possibly related PRs
Poem
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? 🪧 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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
public/locales/zh/errors.json (3)
11-14: Enhance validation messages for better user guidanceWhile the translations are grammatically correct, they could be more user-friendly and specific. Consider these improvements:
- "invalidPhoneNumber": "请选择一个有效的电话号码", + "invalidPhoneNumber": "请输入正确的电话号码格式", - "invalidEducationGrade": "请选择教育年级", + "invalidEducationGrade": "请从列表中选择您的教育程度", - "invalidEmploymentStatus": "请选择有效的就业状况", + "invalidEmploymentStatus": "请从列表中选择您的就业状况", - "invalidMaritalStatus": "请选择有效的婚姻状况", + "invalidMaritalStatus": "请从列表中选择您的婚姻状况"Rationale for changes:
- Phone number: More specific about format requirement
- Other fields: Clarify that options are available in a list
- Added respectful form of address (您) which is more appropriate for user interfaces in Chinese
15-16: Consider improving technical error messagesThe HTTP 400 error message is too technical for end users. Consider a more user-friendly approach:
- "error400": "响应不成功. 从服务器收到状态代码 400", + "error400": "请求无法完成，请检查您的输入是否正确",The default error message is well translated and appropriate for general use.
Translation inconsistencies found across locale files
Several inconsistencies in messaging structure and completeness were identified:
- Spanish (sp) locale has untranslated entries: The first 7 entries are still in English
- French (fr) and Hindi (hi) locales have incomplete messages:
notFoundMsgandnotAuthorisedare truncated compared to English- Chinese (zh) locale has shorter error messages that may not convey the same level of detail (e.g.,
talawaApiUnavailabledoesn't mention checking network connectivity)- The placeholder usage ({{msg}}, {{entity}}) is consistent across all locales, which is good
🔗 Analysis chain
Line range hint
1-16: Verify translations across all locale filesTo ensure consistency across all locales, we should verify that the messaging structure and tone are uniform.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Compare structure of all locale files and check for missing translations # List all locale files and their content for locale in en fr hi sp zh; do echo "=== Checking $locale locale ===" if [ -f "public/locales/$locale/errors.json" ]; then # Check if all keys exist and print their values jq -r '. | to_entries | .[] | "\(.key): \(.value)"' "public/locales/$locale/errors.json" fi doneLength of output: 4966
public/locales/fr/errors.json (1)
15-15: Consider simplifying the technical error messageThe current message
"error400": "Réponse non réussie. Code d'état 400 reçu du serveur"is too technical for end users.Consider a more user-friendly version:
- "error400": "Réponse non réussie. Code d'état 400 reçu du serveur", + "error400": "Désolé, votre requête n'a pas pu être traitée. Veuillez réessayer.",public/locales/en/errors.json (1)
10-10: Fix typo in error messageThe word "occured" is misspelled. It should be "occurred".
- "errorLoading": "Error occured while loading {{entity}} data", + "errorLoading": "Error occurred while loading {{entity}} data",public/locales/sp/errors.json (1)
11-14: LGTM! Translations are clear and consistent.The Spanish translations are grammatically correct and maintain a consistent, polite tone using "por favor" for validation messages.
Consider renaming the directory from
sptoesas it's the standard ISO language code for Spanish.src/utils/errorHandler.tsx (1)
Line range hint
1-40: Consider implementing a centralized error handling system.While the current changes improve error message clarity, consider implementing a more robust error handling system:
- Define an error registry with error codes and corresponding messages
- Implement error hierarchies for different types of validation errors
- Add error context/metadata support for better debugging
- Use proper logging infrastructure
- Consider implementing error boundaries for React components
This would make the system more maintainable and provide better error tracking capabilities.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
public/locales/en/errors.json(1 hunks)public/locales/fr/errors.json(1 hunks)public/locales/hi/errors.json(1 hunks)public/locales/sp/errors.json(1 hunks)public/locales/zh/errors.json(1 hunks)src/utils/errorHandler.tsx(1 hunks)
🔇 Additional comments (11)
public/locales/zh/errors.json (1)
10-10: LGTM: Consistent formatting of existing message
The reformatted message maintains proper Chinese grammar and clarity.
public/locales/hi/errors.json (3)
11-14: Well-structured validation messages!
The validation messages are clear, consistent, and user-friendly. They follow good practices by:
- Starting with "कृपया" (please) to maintain politeness
- Clearly indicating the specific action required
- Using consistent language structure across all validation messages
16-16: LGTM! Good fallback message
The default error message is well-crafted:
- Acknowledges the error without technical details
- Provides clear next steps for the user
- Uses appropriate and friendly language
11-16: Verify translations across all locale files
Let's ensure these new error messages are consistently implemented across all supported languages.
✅ Verification successful
Let me generate another script to get more detailed information about the actual content of these files.
This will help us see if the translations exist but with different keys, or if they're missing entirely.
All error messages are properly translated across all locale files
The verification shows that all the new error messages (invalidPhoneNumber, invalidEducationGrade, invalidEmploymentStatus, invalidMaritalStatus, error400, and defaultError) are present and properly translated in all supported languages (en, fr, hi, sp, and zh).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all new error keys exist in other locale files
# Expected: All locale files should have the same error keys
# Define the new keys to check
keys=("invalidPhoneNumber" "invalidEducationGrade" "invalidEmploymentStatus" "invalidMaritalStatus" "error400" "defaultError")
# Find all locale files
echo "Checking locale files for consistency..."
for locale_file in $(fd "errors.json" public/locales); do
echo "\nChecking $locale_file"
for key in "${keys[@]}"; do
if ! grep -q "\"$key\":" "$locale_file"; then
echo "❌ Missing key: $key"
fi
done
done
Length of output: 3657
Script:
#!/bin/bash
# Get the actual content of all error.json files to compare translations
for locale_file in $(fd "errors.json" public/locales); do
echo -e "\n=== Content of $locale_file ===\n"
cat "$locale_file"
done
Length of output: 5474
public/locales/fr/errors.json (2)
10-16: LGTM: Consistent JSON formatting
The new entries maintain consistent formatting with the existing messages and follow proper JSON structure.
11-14: Verify coverage of all profile field validations
The new validation messages cover phone, education, employment, and marital status. Let's ensure we haven't missed any other profile fields that require validation messages.
✅ Verification successful
Let me gather more information about the profile fields and their validations.
Let me search for profile-related components and forms to verify all validation fields.
Let me search for profile-related form fields and validation schemas in the codebase.
Based on the analysis of the codebase and locale files, I can now provide a final response regarding the validation messages coverage.
All required profile field validations are properly covered
The validation messages in errors.json cover all the essential profile fields that require validation:
- Phone number validation: "invalidPhoneNumber"
- Education grade validation: "invalidEducationGrade"
- Employment status validation: "invalidEmploymentStatus"
- Marital status validation: "invalidMaritalStatus"
This aligns with the profile fields found in the codebase:
- Phone number input field
- Education grade selection
- Employment status selection
- Marital status selection
The validation messages are also properly translated across different locale files (en, fr, hi, etc.) ensuring consistent user feedback across languages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for form field validations in profile components
rg -t tsx -t ts "validation|required|invalid" src/ | grep -i "profile"
# Check other locale files for additional validation messages
fd -e json . public/locales | xargs rg "invalid.*Status|invalid.*Field"
Length of output: 1126
Script:
#!/bin/bash
# Search for profile-related form fields and validations
ast-grep --pattern 'const $_ = useForm<$_>({
$$$
})'
# Search for profile-related validation schemas
rg "schema|validation" -A 5 -B 5
# Search for all error messages in English locale to compare
cat public/locales/en/errors.json
Length of output: 29227
Script:
#!/bin/bash
# Search for profile-related components and forms
fd -e tsx -e ts . src/ | grep -i "profile" | xargs cat
# Search for form field validations in any components
ast-grep --pattern 'const $_ = yup.object().shape({
$$$
})'
# Search for validation schema definitions
rg "yup.string\(\)|yup.number\(\)" -A 2 -B 2
Length of output: 65355
Script:
#!/bin/bash
# Search for profile form fields and validation
rg -t tsx -t ts "Form\." -A 3 -B 3 src/components/UserProfile
# Search for validation messages in all locale files
fd -e json . public/locales | xargs cat
# Search for form field enums or constants
rg "enum|const.*Status" -A 3 -B 3 src/
Length of output: 122957
public/locales/en/errors.json (1)
11-14: LGTM! Clear and user-friendly validation messages
The validation messages are well-structured, consistent, and provide clear guidance to users, which aligns perfectly with the PR's objective of simplifying error feedback.
Let's verify the usage of these messages in the codebase:
✅ Verification successful
Let me verify the usage of these validation messages by checking the form validation implementation.
Validation messages are properly integrated and used in error handling
The validation messages are correctly implemented in the error handler utility (src/utils/errorHandler.tsx), which displays appropriate toast notifications using the translation keys. The error handler properly maps backend validation errors to user-friendly messages for phone number, education grade, employment status, and marital status fields.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if these validation messages are properly integrated with form validation logic
# Search for validation message keys in the codebase
echo "Checking usage of validation messages in components:"
rg -l "(invalidPhoneNumber|invalidEducationGrade|invalidEmploymentStatus|invalidMaritalStatus)"
# Look for form validation logic
echo -e "\nChecking form validation implementation:"
ast-grep --pattern 'const $_ = useTranslation($$$)' -A 5
Length of output: 93686
Script:
#!/bin/bash
# Search for validation message usage in form validation logic
echo "Checking form validation implementations:"
rg -A 5 "invalidPhoneNumber|invalidEducationGrade|invalidEmploymentStatus|invalidMaritalStatus" src/utils/errorHandler.tsx
Length of output: 856
public/locales/sp/errors.json (2)
11-16: Well-aligned with PR objectives for simplified error handling.
The new error messages perfectly match the PR objectives by:
- Providing clear, user-friendly feedback
- Addressing one validation issue at a time
- Clearly indicating the required action
11-16: Verify consistency across all language files.
Let's ensure these new error messages are consistently implemented across all language files (en, fr, hi, zh) with equivalent meanings.
✅ Verification successful
All error messages are properly translated and consistent across language files
The verification shows that all the new error messages (invalidPhoneNumber, invalidEducationGrade, invalidEmploymentStatus, invalidMaritalStatus, error400, defaultError) are:
- Present in all language files (en, fr, hi, sp, zh)
- Properly translated with equivalent meanings
- Consistently formatted in valid JSON
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all language files contain the same keys and are properly formatted
# List of new keys to verify
KEYS=("invalidPhoneNumber" "invalidEducationGrade" "invalidEmploymentStatus" "invalidMaritalStatus" "error400" "defaultError")
# Find all error.json files
echo "Checking all locale files..."
fd "errors.json" public/locales
# Verify each key exists in all locale files
for key in "${KEYS[@]}"; do
echo -e "\nChecking key: $key"
rg -l "\"$key\":" public/locales/*/errors.json
done
# Validate JSON format
echo -e "\nValidating JSON format..."
fd "errors.json" public/locales -x jq '.' {} \; >/dev/null
Length of output: 3109
Script:
#!/bin/bash
# Let's check the actual content of these error messages to verify translations
echo "=== English (en) error messages ==="
jq -r 'select(.invalidPhoneNumber, .invalidEducationGrade, .invalidEmploymentStatus, .invalidMaritalStatus, .error400, .defaultError) | .' public/locales/en/errors.json
echo -e "\n=== French (fr) error messages ==="
jq -r 'select(.invalidPhoneNumber, .invalidEducationGrade, .invalidEmploymentStatus, .invalidMaritalStatus, .error400, .defaultError) | .' public/locales/fr/errors.json
echo -e "\n=== Hindi (hi) error messages ==="
jq -r 'select(.invalidPhoneNumber, .invalidEducationGrade, .invalidEmploymentStatus, .invalidMaritalStatus, .error400, .defaultError) | .' public/locales/hi/errors.json
echo -e "\n=== Chinese (zh) error messages ==="
jq -r 'select(.invalidPhoneNumber, .invalidEducationGrade, .invalidEmploymentStatus, .invalidMaritalStatus, .error400, .defaultError) | .' public/locales/zh/errors.json
Length of output: 21107
src/utils/errorHandler.tsx (2)
13-13: LGTM! Good practice for error message extraction.
Extracting the error message into a variable improves code readability and prevents repeated property access.
18-31: Consider using error codes instead of string matching.
The current implementation relies on string matching which is fragile and could break if the backend error messages change. Using error codes or constants would be more maintainable.
Let's verify the backend error messages:
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
src/utils/errorHandler.tsx (2)
24-26: Enhance 400 error handling with more contextThe generic handling of status code 400 might hide specific validation errors from the user.
Consider extracting and displaying specific validation errors from the 400 response:
if (errorMessage.includes('status code 400')) { try { const errorData = JSON.parse(errorMessage); // Extract specific validation errors if available const validationErrors = errorData.errors || []; validationErrors.forEach(error => toast.error(tErrors(error.code) || tErrors('error400')) ); } catch { toast.error(tErrors('error400')); } }
13-28: Add error boundaries for React componentsWhile the current error handler manages API errors, it doesn't handle React component errors.
Consider implementing React Error Boundaries:
interface ErrorBoundaryProps { children: React.ReactNode; fallback: React.ReactNode; } class ErrorBoundary extends React.Component<ErrorBoundaryProps, { hasError: boolean }> { constructor(props: ErrorBoundaryProps) { super(props); this.state = { hasError: false }; } static getDerivedStateFromError(error: Error) { return { hasError: true }; } componentDidCatch(error: Error, errorInfo: React.ErrorInfo) { errorHandler(null, error); } render() { if (this.state.hasError) { return this.props.fallback; } return this.props.children; } }
|
I can use map. But it will still use includes method. As backend is returning a string of all errors combined, I need to check it that way. |
|
Please fix the failing tests |
Please let me know if I need to change anything. |
|
Please submit your PRs against our |
What kind of change does this PR introduce?
A bugfix.
Issue Number:
Fixes #2464
Snapshots/Videos:
https://www.loom.com/share/0a0e7d6b5b754bbfb8e9b6b8bd6e6161?sid=074df4c5-6119-43f7-827d-6108f36f6cee
Summary
While updating user profile at Talawa User Portal, user used not to get user friendly errors. I have simplified the error which will come one by one in a user friendly readable manner helping user to understand the fields they are missing/giving wrong data into. I have also added translation of errors.
Other information
Added a 400 handler too.
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation