Added new option to fix a little issue originated from last PR #3844

Closed
wants to merge 3 commits into
from

Projects

None yet

5 participants

@iquabius

After I sent this PR, I realised a little problem:
When identical validator is used in a form scope, the form will always provide a context, because of this, it wouldn't be possible to validate a literal token, see the example.

use Zend\Form\Form;
use Zend\InputFilter\InputFilter;

$form = new Form('form');
$form->add(array(
    'name' => 'humanCheck',
    'options' => array(
        'label' => 'How much is 1 + 2?',
    ),
));

$inputFilter = new InputFilter();
$inputFilter->add(array(
    'name' => 'humanCheck',
    'required' => true,
    'validators' => array(
        array(
            'name' => 'Identical',
            'options' => array(
                'token' => '3',
            ),
        ),
    ),
));

$form->setInputFilter($inputFilter);
...

This would not work because on validation the validator would try to find '3' in the context, and then rise an exception when it doesn't find it.

To solve this I add a new option, literal. Setting this to true (it's false by default) would allow a 'literal' validation even when the context is provided. So the above input filter would look like this:

$inputFilter->add(array(
    'name' => 'humanCheck',
    'required' => true,
    'validators' => array(
        array(
            'name' => 'Identical',
            'options' => array(
                'literal' => true,
                'token' => '3',
            ),
        ),
    ),
));

This would tell the validator to skip the look up in the context and just use the string '3'.
Note: this option has no affect when context is not provided.

@blanchonvincent blanchonvincent commented on the diff Feb 21, 2013
library/Zend/Validator/Identical.php
@@ -43,7 +43,8 @@ class Identical extends AbstractValidator
*/
protected $tokenString;
protected $token;
- protected $strict = true;
+ protected $strict = true;
@blanchonvincent
blanchonvincent Feb 21, 2013

why you add a space ?

@iquabius
iquabius Feb 21, 2013

To align with the line below!

@iquabius

@bakura10 sent this PR (#3869), to fix a BC break originated from this PR (#3803). If his PR is really necessary, this one isn't.

I think this new option is a good idea, if it can't be in this version, it could be added in some next version (from what I see just in ZF 3.0, right!?)

So someone take a good look and decide what is the best solution!

@bakura10

I think we need to merge my PR, yours still introduce a BC that is not necessary. As you said, you need to keep this for ZF 3 :).

@Freeaqingme
Zend Framework member

As per the comments, I'll assign this issue to version 3.0.0, that will give us plenty of time to think about it and review it ;)

Nonetheless I'd like to thank you for your PR, we have plenty of other issues that you may be willing to work on in the meanwhile! =)

@weierophinney
Zend Framework member

I think I'm missing something; I don't see anything backwards incompatible in this PR. I think we can merge this for 2.2.0.

@weierophinney weierophinney added a commit that referenced this pull request Mar 25, 2013
@weierophinney weierophinney [#3844] Remove tests no longer present in develop
- Two tests were present on master but removed in develop -- and brought
  back in during merge. Removed.
a1512a7
@weierophinney weierophinney added a commit that referenced this pull request Mar 25, 2013
@weierophinney weierophinney Merge branch 'feature/3844' into develop
Close #3844
9c848db
@iquabius

The BC break was fixed with @bakura10's PR. When I first introduced the ability to validate in fieldsets, the line 174 was throwing an exeption - that happened in the case a $context was provided and the given token didn't existed. Now the behavior is to use the token literally if this happens - the bad thing about this is that if you type a wrong token, you will never know why your fields are not validating, but this was necessary to remove the BC break.

I actually didn't realize this PR could still be used.

@weierophinney
Zend Framework member

Forgot to close when I merged!

@iquabius iquabius deleted the iquabius:fix-identical-validator branch Mar 27, 2013
@iquabius iquabius referenced this pull request in zendframework/zf2-documentation Apr 6, 2013
Merged

Docs for new options added #831

@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney [#3844] Remove tests no longer present in develop
- Two tests were present on master but removed in develop -- and brought
  back in during merge. Removed.
5296d05
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'feature/3844' into develop
Close #3844
6112463
@weierophinney weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#3844 from wryck7/fix-i…
…dentical-validator

Added new option to fix a little issue originated from last PR

Conflicts:
	tests/ZendTest/Validator/IdenticalTest.php
295f526
@weierophinney weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#3844] Remove tests no longer present in …
…develop

- Two tests were present on master but removed in develop -- and brought
  back in during merge. Removed.
7195a53
@weierophinney weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'feature/3844' into develop 963d2fe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment