-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
Feature/required validator #10
Comments
Inject a validator in the chain make difficult reset the input state for to validate different values or transititions set <=> not set Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
I haven't tried running this yet, but I like the idea. Why must "the flag setRequired" be set when adding the validator manually? Wouldn't it be better to just deprecate the flag? Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
Because the validator does not validate the input value (https://github.com/zendframework/zend-inputfilter/pull/62/files#diff-5e41c64803af5c0ad8ad84c3e25c834cR412) I didn't resolve how to make to replace the whole required funcionallity with the validator. This is needed too https://github.com/zendframework/zend-inputfilter/blob/master/src/BaseInputFilter.php#L253 Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
Hmm, yeah that's true. Do you think it would be possible to use the $data array as $context instead of the raw values? That would enable the Required-validator to check whether a key exists without having a reference to the input object. You would have to inject the input's name in the Required-validator's constructor as well, so the validator knew which key to look for. What do you think? Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
IIRC every input is present in $context with the default value of null. Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
Also rely on $context makes the input dependant of InputFilter implementation and can't be used standalone. Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
The problem is that they're initialized to null. Null is still a value, and would pass the required-validator. So we need the original input data, where the the input is not present. I don't think relying on $context makes the Input dependent on the InputFilter implementation. The
Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
Even if looking in $context is needed to know what input name to search. Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
Yes, that's why the input's name must be injected into the validator. If you add a Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
I start to think the best option could be to have 2 validator chains. One for validate the data and another one for validate the input state. Something like this
Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
I just realized that my idea doesn't work. (The absence of a Required-validator wouldn't be equivalent to Two validator chains might be a good idea. Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
IMO, if we are going to have a Originally posted by @akrabat at zendframework/zend-inputfilter#62 (comment) |
I agree with @akrabat Originally posted by @stefanotorresi at zendframework/zend-inputfilter#62 (comment) |
I also agree with that. Right now it seems like the only way to implement a I'm not sure if that's a good idea or not. Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
For to make this works seems It's needed a flag for "break the chain on success" for the ValidatorChain and create an OptionalValidator. Thoughts? I think its reasonable to have Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
I think we should merge #67 and #73 now since those won't cause any BC breaks. Then I think we should do a more thorough refactorization for version 3.0. In 3.0 I hope we can remove the But that's just a dream right now. Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
I don't see this as a BC Break because the translation feature seems never has work. Anyway the purpose of this PR is discuss about the required / optional field concept Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
I was serious in my last post where I suggested we completely get rid of the required flag and add an optional flag. That would be a BC break, but a lot simpler to maintain than two different validator chains. Originally posted by @larsnystrom at zendframework/zend-inputfilter#62 (comment) |
that's not correct, it worked prior to 2.4. Originally posted by @stefanotorresi at zendframework/zend-inputfilter#62 (comment) |
@stefanotorresi Prior to 2.5.2/2.4.5 there was not a required message. Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
it was handled by the notEmpty validator, which was always injected for required inputs before Originally posted by @stefanotorresi at zendframework/zend-inputfilter#62 (comment) |
I really like this approach, but I share the same concern voiced by @akrabat — unless the It seems like this is mostly the case right now, though; If you can implement that, I can merge this. Originally posted by @weierophinney at zendframework/zend-inputfilter#62 (comment) |
Moved to 2.7.0 milestone; we're ready for 2.6.0 (zend-servicemanager forwards-compatibility). Originally posted by @weierophinney at zendframework/zend-inputfilter#62 (comment) |
I keep thinking in new ways for make this better. Probably with something like #87 Originally posted by @Maks3w at zendframework/zend-inputfilter#62 (comment) |
These allows manage the required message like any other ValidatorInterface element.
Note: While you can add the validator manually the flag
setRequired
must be set tooNote:
setRequired(false)
may not be enough and you will need remove the validator from the chain.Originally posted by @Maks3w at zendframework/zend-inputfilter#62
The text was updated successfully, but these errors were encountered: