[WIP] Rules #108

viktorprogger · 2018-11-30T18:12:52Z

Q	A
Is bugfix?	no
New feature?	no
Breaks BC?	yes
Tests pass?	no
Fixed issues	#96

Here is an example of my vision about how it should work. I've changed NumberValidator instead of abstract Validator to simplfy this example. If all is ok, I'll change all the core validators and the relevant tests.

samdark · 2018-12-01T21:51:57Z

@miolae any usage syntax examples?

viktorprogger · 2018-12-03T09:00:56Z

@samdark Usage axample

$validator = new NumberValidator();
$validator->attributes = $this->getAttributesForValidator();
$validator->min(1);

return [
    [['login', 'pass'], (new NumberValidator())->min(6)->max(255)],
   $validator,
];

I'd prefer a more convenient way to set attributes, but I didn't find it globally in the system.

viktorprogger · 2018-12-03T13:06:26Z

I've read the issue again and I'm confused. There is also a good variant $this->property => Validator::number()->min(9)->required().

But there could be some difficulties with equal keys (when property1 and property2 has the same value but different validators).

On the other hand the right part is looking very tasty. I can add some code to implement this. Because now there is no way to set several validators to a single set of attributes (I understood it just now).

samdark · 2018-12-03T21:57:21Z

Could be the following as well:

public function rules(Validator $is)
{
    return [
        [['name', 'email', 'subject', 'body'], $is->required()],
        ['email', $is->email()],
        [['login', 'pass'], $is->number()->min(6)->max(255)],
    ];
}

viktorprogger · 2018-12-04T08:11:24Z

I think built-in and custom validator using shouldn't differ too much. You're now showing only built-in variant. And how will look a custom one?

samdark · 2018-12-04T19:16:44Z

Right. Then:

public function rules()
{
    return [
        [['name', 'email', 'subject', 'body'], new RequiredValidator()],
        ['email', new EmailValidator()],
        [['login', 'pass'], (new NumberValidator())->min(6)->max(255)],
        ['branch', new CustomBranchValidator()->primaryOnly()],
    ];
}

viktorprogger · 2018-12-05T08:43:37Z

This variant is already realized for NumberValidator 👍

samdark · 2018-12-05T10:42:11Z

I think it's good enough. Let's do other validators and docs.

[skip ci]

viktorprogger · 2018-12-25T17:33:22Z

@samdark (or anybody of yiisoft) make a code review please. There are some significant changes. I've described them in the changelog. If all is ok I'll continue with other validators (now only NumberValidator is completely rewritten).
Please pay attention also to the new test I've written. I will make an abstract test with this logic, if you like it, and rewrite other validation tests in a such way. The main idea is to check all available validation rules (see the changelog) combination with equal set of values to be completely sure that all validations are ok.

machour · 2018-12-25T18:46:14Z

src/validators/Validator.php

@@ -466,4 +542,48 @@ public function getAttributeNames()
            return ltrim($attribute, '!');
        }, $this->attributes);
    }
+
+    private function ruleEsixts(string $name)


src/validators/Validator.php

rob006 · 2018-12-28T17:50:23Z

src/base/Model.php

@@ -448,8 +448,10 @@ public function createValidators()
        foreach ($this->rules() as $rule) {
            if ($rule instanceof Validator) {
                $validators->append($rule);
-            } elseif (is_array($rule) && isset($rule[0], $rule[1])) { // attributes, validator type
-                $validator = Validator::createValidator($rule[1], $this, (array) $rule[0], array_slice($rule, 2));
+            } elseif (is_array($rule) && isset($rule[0], $rule[1]) && $rule[1] instanceof Validator) {


What is the reason for removing support for plain arrays with validator config? Looks like unnecessary BC break.

Because this configuration way is deprecated.

There was a discussion about this in Slack. Somebody said that the deprecation will be removed. So, I'll return arrays for configuration.

rob006 · 2018-12-28T17:52:42Z

src/validators/Validator.php

+     * @return mixed
+     * @throws InvalidConfigException
+     */
+    public function __call($name, $params)


Why rely on magic __call() and processing some constant with configuration? It is less flexible and less readable than regular getters and setters.

samdark · 2018-12-30T21:02:50Z

CHANGELOG.md

+    ```php
+    public function rules() {
+      return [
+        [['login', 'pass'], (new NumberValidator())->min(6)->integerOnly(true)],


true could be default.

samdark · 2018-12-30T21:03:02Z

CHANGELOG.md

+    public function rules() {
+      return [
+        [['login', 'pass'], (new NumberValidator())->min(6)->integerOnly(true)],
+        ['email', new RequiredValidator],


() should be specified.

samdark · 2018-12-30T21:03:40Z

CHANGELOG.md

+      ];
+    }
+    ```
+  - Validators don't contain validation logic anymore. They are containers for _validation rules_ (see `BaseRule` class).


samdark · 2018-12-30T21:04:42Z

CHANGELOG.md

+    - either with a given callback
+    - or with a given regular expression
+  - In all other cases you just need to extend `BaseRule` and change or add the needed bit of logic
+  - Validation rules themselves are described in the `RULES` constant of your validator


That looks like validator and its rule are coupled very tightly... is there a reason for these to be separate classes?

samdark · 2018-12-30T21:09:22Z

src/validators/Validator.php

+     *
+     * @throws InvalidConfigException
+     */
+    public function setRule(string $name, $value, ?string $message = null, $validation = null)


What is the reason introducing such flexibility/complexity? While re-configuring rules and overriding standard ones is definitely cool, it may be easily abused to change standard predictable behavior. Are there use cases for it?

viktorprogger · 2019-01-10T15:13:12Z

Thank you very much for your feedback. Now I have a lot of work in different directions and can't take time for this. A bit later I'll reconsider my decisions.

viktorprogger · 2019-02-15T14:21:07Z

Will begin from draft with all requested changes.

samdark · 2019-05-22T20:14:16Z

@viktorprogger thank you for ideas. There's now https://github.com/yiisoft/validator

#96 Rules

2d0446b

samdark added the type:enhancement Enhancement label Dec 5, 2018

samdark added this to the 3.0.0 milestone Dec 5, 2018

viktorprogger added 2 commits December 17, 2018 17:27

Moving logic to the abstract Validator

faf3ebd

Preparing to validate by Rule class

e5a7669

viktorprogger force-pushed the validators branch from 6f777ce to 5a87eab Compare December 19, 2018 13:58

Validation is ready. Error messages are the rest.

1768da8

viktorprogger force-pushed the validators branch from 5a87eab to 1768da8 Compare December 19, 2018 14:02

viktorprogger changed the title ~~#96 Rules~~ [WIP] Rules Dec 24, 2018

viktorprogger added 8 commits December 24, 2018 14:25

Fix getting rule class

4bdc590

[skip ci]

Refactor Validator::setRule()

e3ffba7

[skip ci]

idle

eb99b97

[skip ci]

Working with error messages

596e577

[skip ci]

Add a test for NumberValidator and fix bugs

343e4de

[skip ci]

minor fixes for abstract Validator compatibility

bd09f1e

[skip ci]

Merge remote-tracking branch 'upstream/master' into validators

1197db1

Changelog

b472369

[skip ci]

machour reviewed Dec 25, 2018

View reviewed changes

src/validators/Validator.php Show resolved Hide resolved

rob006 suggested changes Dec 28, 2018

View reviewed changes

samdark suggested changes Dec 30, 2018

View reviewed changes

viktorprogger closed this Feb 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Rules #108

[WIP] Rules #108

viktorprogger commented Nov 30, 2018 •

edited

samdark commented Dec 1, 2018

viktorprogger commented Dec 3, 2018

viktorprogger commented Dec 3, 2018

samdark commented Dec 3, 2018

viktorprogger commented Dec 4, 2018 •

edited

samdark commented Dec 4, 2018

viktorprogger commented Dec 5, 2018

samdark commented Dec 5, 2018

viktorprogger commented Dec 25, 2018 •

edited

machour Dec 25, 2018

rob006 Dec 28, 2018

viktorprogger Jan 10, 2019

rob006 Dec 28, 2018

samdark Dec 30, 2018

samdark Dec 30, 2018

samdark Dec 30, 2018

samdark Dec 30, 2018

samdark Dec 30, 2018

viktorprogger commented Jan 10, 2019

viktorprogger commented Feb 15, 2019

samdark commented May 22, 2019

[WIP] Rules #108

[WIP] Rules #108

Conversation

viktorprogger commented Nov 30, 2018 • edited

samdark commented Dec 1, 2018

viktorprogger commented Dec 3, 2018

viktorprogger commented Dec 3, 2018

samdark commented Dec 3, 2018

viktorprogger commented Dec 4, 2018 • edited

samdark commented Dec 4, 2018

viktorprogger commented Dec 5, 2018

samdark commented Dec 5, 2018

viktorprogger commented Dec 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viktorprogger commented Jan 10, 2019

viktorprogger commented Feb 15, 2019

samdark commented May 22, 2019

viktorprogger commented Nov 30, 2018 •

edited

viktorprogger commented Dec 4, 2018 •

edited

viktorprogger commented Dec 25, 2018 •

edited