-
Notifications
You must be signed in to change notification settings - Fork 168
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
fix: fixed validation errors duplicates #2368
base: dev
Are you sure you want to change the base?
Conversation
|
WalkthroughThe Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
PR Description updated to latest commit (b4ef6cc) |
1 similar comment
PR Description updated to latest commit (b4ef6cc) |
PR Description updated to latest commit (b4ef6cc) |
PR Review 🔍(Review updated until commit b4ef6cc)
Code feedback:
|
PR Review 🔍
Code feedback:
|
Persistent review updated to latest commit b4ef6cc |
PR Code Suggestions ✨
|
PR Code Suggestions ✨
|
PR Code Suggestions ✨
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/kyb-app/src/components/organisms/DynamicUI/rule-engines/json-schema.rule-engine.ts (2 hunks)
Files not reviewed due to errors (1)
- apps/kyb-app/src/components/organisms/DynamicUI/rule-engines/json-schema.rule-engine.ts (no review received)
return uniqBy( | ||
result?.flat()?.filter(result => Boolean(result.message)), | ||
'message', | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't a message connected to a field? It seems the real issue is that more than one of the same message is being pushed to the same field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate messages are coming from ajv, the way we parsing errors and converting them to field errors didnt handled this case. So uniqBy should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If 3 fields have the same message, would only the first field have the message post this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why in the first place we get couple of messages on a single input field? @chesterkmr
User description
Description
Checklist
Summary by CodeRabbit
PR Type
Bug fix
Description
uniqBy
function.Changes walkthrough 📝
json-schema.rule-engine.ts
Implement Unique Error Message Filtering in JSON Schema Rule Engine
apps/kyb-app/src/components/organisms/DynamicUI/rule-engines/json-schema.rule-engine.ts
uniqBy
from lodash to ensure unique error messages.uniqBy
based on the'message' property.