Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

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

Closed

Conversation

iquabius
Copy link
Contributor

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._

@@ -43,7 +43,8 @@ class Identical extends AbstractValidator
*/
protected $tokenString;
protected $token;
protected $strict = true;
protected $strict = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why you add a space ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To align with the line below!

@iquabius
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link
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
Copy link
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.

@ghost ghost assigned weierophinney Mar 25, 2013
weierophinney added a commit that referenced this pull request Mar 25, 2013
Added new option to fix a little issue originated from last PR

Conflicts:
	tests/ZendTest/Validator/IdenticalTest.php
weierophinney added a commit that referenced this pull request Mar 25, 2013
- Two tests were present on master but removed in develop -- and brought
  back in during merge. Removed.
weierophinney added a commit that referenced this pull request Mar 25, 2013
@iquabius
Copy link
Contributor Author

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
Copy link
Member

Forgot to close when I merged!

@iquabius iquabius deleted the fix-identical-validator branch March 27, 2013 21:57
weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
…dentical-validator

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

Conflicts:
	tests/ZendTest/Validator/IdenticalTest.php
weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
…develop

- Two tests were present on master but removed in develop -- and brought
  back in during merge. Removed.
weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants