Skip to content
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

Only evaluating the first rule? #36

Closed
hbarroso opened this issue Dec 19, 2015 · 4 comments · Fixed by #41
Closed

Only evaluating the first rule? #36

hbarroso opened this issue Dec 19, 2015 · 4 comments · Fixed by #41
Labels

Comments

@hbarroso
Copy link

Hi,

Either I'm missing something completely obvious or might there be bug in the version 2.1.
It seems that, when passing an array to the add method, it only adds or evaluates the first rule.

    $validator = new Validator();

    $input = ['title' => 'Some title'];

    $validator->add(array(
        'title:Title' => 'required',
        'content:Content' => 'required | minlength(5)',
    ));

    if ($validator->validate($input)) {
        echo "all good";
    } else {
       var_dump($validator->getMessages();
    }

If you run this code, it returns true even though the content is not included in the $input array. But the funny thing is, even if I change the $input to:

    $input = ['title' => 'Some title', 'content'  => 'foo'];

This still returns true, even though content is less than 5 chars.

What am I missing?

thanks,

@adrianmiu adrianmiu added the bug label Jan 20, 2016
@adrianmiu
Copy link
Contributor

Yes. It is a problem when adding multiple rules using the string definition
Adding multiple rules works if they are passed as array (not strings separated by |):
https://github.com/siriusphp/validation/blob/master/tests/src/ValidatorTest.php#L113

Adding single rules as strings works
https://github.com/siriusphp/validation/blob/master/tests/src/ValidatorTest.php#L159

Will fix as soon as I have some time.

@mscreenie
Copy link

Any eta on a fix? The example in docs does not seem to work either?

@adrianmiu
Copy link
Contributor

@mscreenie I'm extremely busy at the moment but I would approve at a pull-request :)

@vikkio88
Copy link
Contributor

Hello, I just opened a PR which fixes this Issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants