-
Notifications
You must be signed in to change notification settings - Fork 27
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
Feature: Syntax sugars for parameter validators #108
Conversation
We injected nunjucks environments and their extensions together into compiler, but they can be injected after they have been initialized properly. - Compose compiler environments - Fix circular dependencies issues
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
72034c8
to
f7ddbf8
Compare
Codecov ReportBase: 92.57% // Head: 92.68% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #108 +/- ##
===========================================
+ Coverage 92.57% 92.68% +0.10%
===========================================
Files 259 265 +6
Lines 3799 3964 +165
Branches 482 517 +35
===========================================
+ Hits 3517 3674 +157
- Misses 185 193 +8
Partials 97 97
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
b3f7bfb
to
be6f6c5
Compare
3e9160c
to
eca492d
Compare
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.
Beside one question, other is LGTM 👍 👍
|
||
if (existedParam) { | ||
if (!existedParam.validators) existedParam.validators = []; | ||
existedParam.validators.push(...(parameter.validators || [])); |
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 there are the same name validators in metadata and schema requests, will we exclude one of them?
Because we may face some cases which the args are different in schema request and SQL syntax validation filter:
- schema request has integer validator with args min=10, and max=20
- metadata exists in the validator with args min=5, and max=90 which parsed from SQL
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.
I pushed all validators to the list because I didn't know how to compose all of the validators, e.g.
string(format="\d+")
string(format="[1-5]+")
So I prefer to execute all of these validators.
P.S. Our constraints have composed logic so that we won't duplicate them.
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.
Thanks so much
Description
I made validation filters for every validator, named with prefix
is_
, e.g.integer
validator has theis_integer
filter. Those validators can be used with any variables:With dynamic parameters
With predefined queries' result
To add arguments:
Please check the document here.
I've also added a new validator
enum
and implementedgetConstraints
function for all validators.Issue ticket number
closes #99
Additional Context
is_xxxx
, we can improve it in the future.