-
Notifications
You must be signed in to change notification settings - Fork 775
Only create Length with subsequents when it's possible #1485
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
424adb9 to
f22eb36
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1485 +/- ##
============================================
+ Coverage 96.54% 96.56% +0.01%
- Complexity 973 977 +4
============================================
Files 201 201
Lines 2403 2414 +11
============================================
+ Hits 2320 2331 +11
Misses 83 83 ☔ View full report in Codecov by Sentry. |
f22eb36 to
aa293de
Compare
| - O comprimento de 0 deve possuir de 2 a 15 caracteres | ||
| - 0 deve ser um número de telefone válido para o país Estados Unidos | ||
| - Todas as regras requeridas devem passar para `[]` | ||
| - `[]` deve ser uma string |
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 changed this just because it was wrong, but it doesn't have anything to do with the changes.
| )); | ||
|
|
||
| test('Chained wrapped rule', expectAll( | ||
| fn() => v::length(v::between(5, 7)->odd())->assert([]), |
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.
This is pretty much the essence of the changes.
If you see the other tests, length() receives only 1 rule, this test receives two rules, and the two of them fail. That means that we can't simply add the prefix ("The length of") to the result of that rule because the result has two children.
When Length receives a rule that produces a result with children, we need to add the prefix ("The length of") to all the children of that result.
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.
@dmjohnsson23 I wrote some comments to this merge request. If you have any questions, you can just write here, I'll be happy to answer them!
| // Message: The length of "rose" must not be equal to 4 | ||
| ``` | ||
|
|
||
| ### `Length::TEMPLATE_WRONG_TYPE` |
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.
Because this is a new template, I had to add it to the documentation.
| $length = $this->extractLength($input); | ||
| if ($length === null) { | ||
| return Result::failed($input, $this, [], self::TEMPLATE_WRONG_TYPE) | ||
| ->withId('length' . ucfirst($this->rule->evaluate($input)->id)); |
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.
This is just a way to get the id of the result
Since I updated the validation engine1, it became possible to create results with siblings. This commit changes the "Length", allowing it to create a result with a sibling when possible. That will improve the clarity of the error messages.