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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants