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

Custom callback validation against array #20

Closed
ghost opened this issue Aug 7, 2013 · 13 comments
Closed

Custom callback validation against array #20

ghost opened this issue Aug 7, 2013 · 13 comments

Comments

@ghost
Copy link

ghost commented Aug 7, 2013

I have been trying to add a custom rule using "addRule" which uses an array as an extra parameter.

This is what I am validating with

$v->addRule('validNewsletter', array($this, 'validNewsletters'));
$valid_keys = array('news','sport');
$v->rule('validNewsletter', 'newsletters', $valid_keys)->message('Invalid newsletter selected.');

Unfortunately this currently breaks on line 611 of the Validator class

$label = isset($params[$k]) && isset($this->_labels[$params[$k]]) ? $this->_labels[$params[$k]] : $tag;

It complains that the isset value is not a string

Could this line be changed to this - which does work in this use case?

$label = isset($params[$k]) && !is_array($params[$k]) && isset($this->_labels[$params[$k]]) ? $this->_labels[$params[$k]] : $tag;
@ghost
Copy link
Author

ghost commented Aug 7, 2013

To get around it I have imploded the array when sending it to the callback and explode it in the callback to avoid the 'isset' error on line 611.

It might be worth documenting the callback method a bit more to mention this issue ?

@vlucas
Copy link
Owner

vlucas commented Aug 7, 2013

Looks like I will have to add some more tests around adding custom callbacks to debug this issue a bit more. Thanks for bringing this to my attention.

@ghost
Copy link
Author

ghost commented Aug 7, 2013

That would be good - I noticed there was no test for "addRule". It must be the only function without one :-)

I love the Validator btw

@vlucas
Copy link
Owner

vlucas commented Aug 7, 2013

I added a bunch of tests with this commit: 0f96c88, and I cannot reproduce this error. I also checked line 611, and the code is different for me on the current release than what you show. I just released v1.1.1 with the tests included. Please make sure you are up to date.

@ghost
Copy link
Author

ghost commented Aug 8, 2013

Ok thank you. I will update the package today and try it out.

@ghost
Copy link
Author

ghost commented Aug 8, 2013

@vlucas Yes I still get the error but it is line 615 now.

Illegal offset type in isset or empty

...\vendor\vlucas\valitron\src\Valitron\Validator.php:615
...\vendor\vlucas\valitron\src\Valitron\Validator.php:454
...\vendor\vlucas\valitron\src\Valitron\Validator.php:516
...

It happens when I pass an array as the third parameter of the rule. It looks like it is generating the error message and trying to use the third parameter to build the message?

@vlucas
Copy link
Owner

vlucas commented Aug 8, 2013

This test should cover your exact use case given the code you provided: 0f96c88 (line 576)

I cannot reproduce the error with it, and neither can Travis CI: https://travis-ci.org/vlucas/valitron/builds/9965676 - both PHP 5.3 and 5.4 pass all tests.

@vlucas
Copy link
Owner

vlucas commented Aug 8, 2013

I need you to clone Valitron, cd into the directory, and run phpunit and let me know what the output is. If all the tests pass, I need you to open a text editor, and add a new test that reproduces your error. Then show me that test here, and I will add it to the test suite to get this issue fixed once and for all (or preferably, just fork the project and submit a pull request with the test and the fix).

@ghost
Copy link
Author

ghost commented Aug 9, 2013

@vlucas Ok I can reproduce it now - it is when a specific label is added

    public function testAddRuleCallbackArrayWithArrayAsExtraParameterAndCustomMessageTwo()
    {
        $v = new Validator(array('name' => 'Chester Tester'));
        $v->labels(array('name' => 'Name'));
        $v->addRule('testRule', array($this, 'sampleObjectCallbackFalse'));
        $v->rule('testRule', 'name', array('foo', 'bar'))->message('Invalid name selected.');
        $this->assertFalse($v->validate());
    }

@vlucas
Copy link
Owner

vlucas commented Aug 9, 2013

Ah! So we were missing a labels call :). I will add this to the test suite and get a fix today.

@ghost
Copy link
Author

ghost commented Aug 9, 2013

excellent - thank you

@vlucas vlucas closed this as completed in f0aa501 Aug 9, 2013
@vlucas
Copy link
Owner

vlucas commented Aug 9, 2013

Just released v1.1.2 with a fix for your issue in it. Thanks again for taking the time to report this issue and help me investigate it.

https://github.com/vlucas/valitron/releases/tag/v1.1.2

@ghost
Copy link
Author

ghost commented Aug 9, 2013

Excellent, that worked for me perfectly now. Thanks for getting it in.

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

No branches or pull requests

1 participant