DavidePastore / Slim-Validation

A validation library for the Slim Framework. It internally uses Respect/Validation.
170 stars 30 forks source link

useTemplate option is confusing #34

Closed thibaudcolas closed 6 years ago

thibaudcolas commented 6 years ago

I've been trying to use templates for my validators with this middleware and respect/validation, and I have trouble understanding what useTemplate does exactly.

Looking at the middleware code:

https://github.com/DavidePastore/Slim-Validation/blob/c28d6e2545dab51c9c4330d4db1dc55a75632f56/src/Validation.php#L136-L139

useTemplate just switches between only returning the main error message (the one for the last validator in the chain), and all messages. As far as I can tell there is nothing here related to templates, unless I'm missing something?

I was wondering if this previous implementation might be in order to circumvent the issue fixed by https://github.com/Respect/Validation/pull/1047. With this fixed, I seem to be able to use templates as I would expect without needing the middleware's useTemplate.

DavidePastore commented 6 years ago

Hi @thibaudcolas. I think this is something related to what is written here:

After that, if you call getMainMessage() or getFullMessage() (for nested), the message will be translated.

Note that getMessage() will keep the original message.

I just tried to use the latest version (1.1.23), to use templates and to set useTemplate to false and the result is the right one, so it seems that this is something related to https://github.com/Respect/Validation/pull/1047. I suppose the thing to do here is to update the respect/validation package version and to create another release.

thibaudcolas commented 6 years ago

Hey @DavidePastore, thanks for taking the time to look into this. I haven't used the middleware or respect/Validation with translators yet, so don't have any thoughts on how this getMainMessage / getFullMessage difference might play into this.

If it helps I'm happy to submit a PR with the respect/Validation upgrade. I'm using the upgraded version in my project and it seems to work well. Updating it directly within your project would also require fixing broken unit tests – I think they are just because Validation changed one of the rules' error message.

``` ......F......F......F.FF... Time: 281 ms, Memory: 6.00Mb There were 5 failures: 1) DavidePastore\Slim\Validation\Tests\ValidationTest::testValidation with data set #6 (array(Respect\Validation\Validator Object (...), Respect\Validation\Validator Object (...)), null, true, array(array('"davidepastore" must have a l... and 5'), array('"89" must be lower than or equals 60'))) Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'username' => Array (...) 'age' => Array ( - 0 => '"89" must be lower than or equals 60' + 0 => '"89" must be less than or equal to 60' ) ) /Users/thibaudcolas/Dev/forks/Slim-Validation/tests/ValidationTest.php:134 2) DavidePastore\Slim\Validation\Tests\ValidationTest::testValidation with data set #13 (array(array(array(array(Respect\Validation\Validator Object (...))))), null, true, array(array('321 must be lower than or equals 200')), 'JSON', array(22, array(33, array(97, array(321))))) Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'email.sub.sub-sub.finally' => Array ( - 0 => '321 must be lower than or equals 200' + 0 => '321 must be less than or equal to 200' ) ) /Users/thibaudcolas/Dev/forks/Slim-Validation/tests/ValidationTest.php:134 3) DavidePastore\Slim\Validation\Tests\ValidationTest::testValidation with data set #20 (array(array(array(array(Respect\Validation\Validator Object (...))))), null, true, array(array('"321" must be lower than or equals 200')), 'XML', '\n ') Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'email.sub.sub-sub.finally' => Array ( - 0 => '"321" must be lower than or equals 200' + 0 => '"321" must be less than or equal to 200' ) ) /Users/thibaudcolas/Dev/forks/Slim-Validation/tests/ValidationTest.php:134 4) DavidePastore\Slim\Validation\Tests\ValidationTest::testValidation2 Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'payer.consumer' => Array ( + 0 => 'Key country must be present' ) ) /Users/thibaudcolas/Dev/forks/Slim-Validation/tests/ValidationTest.php:181 5) DavidePastore\Slim\Validation\Tests\ValidationTest::testSetValidators Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'username' => Array (...) 'age' => Array ( - 0 => '"89" must be lower than or equals 20' + 0 => '"89" must be less than or equal to 20' ) ) /Users/thibaudcolas/Dev/forks/Slim-Validation/tests/ValidationTest.php:682 FAILURES! Tests: 27, Assertions: 95, Failures: 5 ```
DavidePastore commented 6 years ago

Yes, the tests need to be updated accordingly. Can you please check #38 and see if it contains the code you expect? Thanks 🐱