-
Notifications
You must be signed in to change notification settings - Fork 0
feat: ✨ handle grouped errors under the enum constraint #183
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
Conversation
lwjohnst86
left a comment
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.
Neat! Just some fairly small comments
lwjohnst86
left a comment
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.
Nice! Some fairly small comments 👍
lwjohnst86
left a comment
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.
Very minor suggestion
|
@martonvago plus resolving conflicts |
Description
This PR handles grouped errors at
$.resources[x].schema.fields[x].constraints.enum. It's the worst one yet!This schema node is deeply nested, so handling errors here is a little more complex.
The situation is the following: for each field, you can list the allowed values for that field under
"constraints": {"enum": [...]}. Depending on the type of the field, the type of the values inenumcan be different. Sometimes it's straightforward (e.g. for string fields, enum values have to be strings), but sometimes the schema forenumis more permissive (e.g. for number fields, enum values can be either numbers or strings). However, you cannot mix types.If
enumhas mixed types or if some of the values are the wrong type, all sub-schemas underenumflag errors. These are usually contradictory, e.g., for a number field[1, "inf"]will alert both because 1 is not a string and because "inf" is not a number...The logic in the function:
enumproperty itself (not an array, duplicate items, etc.) -> keep only thesePart of #15
Needs an in-depth review.
Checklist
just run-all