Added Validation block for validation errors #11152
Conversation
WalkthroughThis change introduces an additional conditional block in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant N as notifyError()
participant T as Toast Notification
U->>N: Call notifyError(error)
alt error type is "validation_error"
N->>N: Check if error.msg exists and is object
N->>N: Iterate over error.msg values and concatenate messages
alt concatenated messages exist
N->>T: Trigger toast notification with concatenated message
else
N->>T: Trigger toast with default error message
end
else Other error types
N->>N: Process error using the existing error handling logic
N->>T: Trigger toast notification with generic message
end
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/Utils/Notifications.ts (3)
25-44: New validation error block looks good, but has minor formatting issues.The added validation block effectively handles errors of type "validation_error" by processing error messages from the error.msg object. However, there are a few improvements that could be made:
- When joining array elements with commas in line 32, consider adding a space after each comma for better readability.
- When appending non-array messages in line 34, there's no separator between different error messages, which could make the final message difficult to read.
- The default error message has an unnecessary space before the comma and is specifically about uploading images, which might not apply to all validation errors.
if ( error?.type === "validation_error" && error?.msg && typeof error.msg === "object" ) { Object.values(error.msg).forEach((messages) => { if (Array.isArray(messages)) { - errorMsg += messages.join(", "); + errorMsg += messages.join(", ") + ". "; } else { - errorMsg += messages; + errorMsg += messages + ". "; } }); if (errorMsg.length > 0) { toast.error(errorMsg); } else { - toast.error("Something went wrong , please upload different image"); + toast.error("Something went wrong, please try again"); } return; }
25-30: Consider adding type checking for the error messages.While handling validation errors, it's important to ensure that the messages being processed are of the expected type. Currently, there's no type checking for the values returned by
Object.values(error.msg).Adding type guards would make the code more robust against unexpected data structures:
if ( error?.type === "validation_error" && error?.msg && typeof error.msg === "object" ) { - Object.values(error.msg).forEach((messages) => { + Object.values(error.msg).forEach((messages: any) => { if (Array.isArray(messages)) {
38-42: Generic error message may be more appropriate.The default error message is specifically about uploading a different image, but validation errors could be related to other fields or actions. A more generic error message would be more versatile.
if (errorMsg.length > 0) { toast.error(errorMsg); } else { - toast.error("Something went wrong , please upload different image"); + toast.error("Validation failed. Please check your input and try again."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Utils/Notifications.ts(1 hunks)
🔇 Additional comments (1)
src/Utils/Notifications.ts (1)
25-44:❓ Verification inconclusive
Verify integration with validation error sources.
Since this code is specifically handling validation errors for image uploads (based on the default error message), it's important to verify that the error structure matches what's expected from the API or validation library.
🏁 Script executed:
#!/bin/bash # Look for places where validation errors might be created rg "validation_error" --type ts # Look for API calls related to image uploads that might return this error structure rg "upload.*image" --type tsLength of output: 951
Action Required: Verify image upload validation error integration.
Please ensure that the error structure returned by the image upload API matches the expected format in this function. In particular:
- Confirm that for image uploads, the API returns a validation error where
error?.msgis an object (with values as arrays or strings) so that the error messages are concatenated correctly.- Double-check that the fallback error message ("Something went wrong , please upload different image") is appropriate given the API response, and consider aligning it with similar messages used elsewhere (e.g., in
AvatarEditModal.tsx).- Verify that the integration of validation error handling in this file is consistent with how other validation errors (such as the phone number errors in
validators.tsandPatientLogin.tsx) are managed across the codebase.
| if ( | ||
| error?.type === "validation_error" && | ||
| error?.msg && | ||
| typeof error.msg === "object" | ||
| ) { | ||
| Object.values(error.msg).forEach((messages) => { | ||
| if (Array.isArray(messages)) { | ||
| errorMsg += messages.join(", "); | ||
| } else { | ||
| errorMsg += messages; | ||
| } | ||
| }); | ||
|
|
||
| if (errorMsg.length > 0) { | ||
| toast.error(errorMsg); | ||
| } else { | ||
| toast.error("Something went wrong , please upload different image"); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Test coverage for the new validation block.
The new validation error handling block handles a specific error format. Make sure to add tests for this scenario to ensure it works as expected for various validation error structures.
Please verify that there are unit tests covering this new validation error handling logic, especially with:
- Various message structures (arrays, strings)
- Empty msg object
- Different validation error types
🏁 Script executed:
#!/bin/bash
# Search for test files that might test the notifyError function
fd -t f "test|spec" --exec grep -l "notifyError" {} \;Length of output: 57
Attention: Missing Unit Tests for New Validation Logic
It appears that our automated search didn’t locate any test files referencing the validation error handling (e.g. via “notifyError” or “validation_error”) in our test or spec directories. Please manually verify if tests exist under alternate naming conventions. If not, add unit tests covering the following cases:
- Array messages: Verify that an error message constructed from an array (joined via commas) is correctly handled.
- String messages: Confirm that single-message (non-array) errors are processed properly.
- Empty msg object: Ensure that an empty or missing message object falls back to the default error.
- Different error types: Test that only errors with
error?.type === "validation_error"trigger this logic, while others fall back accordingly.
Proposed Changes
of image file types
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit