Open weierophinney opened 4 years ago
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 https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-137586919
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 https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147069548
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 https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147070015
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 https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147071318
IIRC every input is present in $context with the default value of null.
Originally posted by @Maks3w at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147071805
Also rely on $context makes the input dependant of InputFilter implementation and can't be used standalone.
Originally posted by @Maks3w at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147071844
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 Input::isValid()
method is already accepting a $context parameter. We can control the default value of that parameter inside the Input
class.
[]
.['inputName' => $val]
.Originally posted by @larsnystrom at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147073535
Even if looking in $context is needed to know what input name to search.
Originally posted by @Maks3w at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147074223
Yes, that's why the input's name must be injected into the validator. If you add a RequiredValidator::setName()
method, it will be compatible with the plugin manager.
Originally posted by @larsnystrom at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147075451
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 https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147076276
I just realized that my idea doesn't work. (The absence of a Required-validator wouldn't be equivalent to setRequired(false)
, since setRequired(false)
implies not running any validators, but an absent Required-validator wouldn't give any information concerning whether to run the other validators.)
Two validator chains might be a good idea.
Originally posted by @larsnystrom at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147079012
IMO, if we are going to have a Required
validator, then we really can't also have a required
flag without causing confusion.
Originally posted by @akrabat at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147179872
I agree with @akrabat
Originally posted by @stefanotorresi at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147453440
I also agree with that. Right now it seems like the only way to implement a Required
validator and not keep the required
flag is to implement two validator chains, like Maks3w suggested.
I'm not sure if that's a good idea or not.
Originally posted by @larsnystrom at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147469117
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 break_chain_on_success
and break_chain_on_failure
Originally posted by @Maks3w at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147642746
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 required
attribute altogether, make null
equivalent to a missing value (thus leaving all validation to the normal validator chain), and have an optional
attribute which defaults to false (optional
can never fail so no error message required, just skip filters and validation if it's optional and the value is missing). I'd also like to remove fallback
, continue_if_empty
and allow_empty
and instead have a default
value, which would be used instead of null
when a value is missing, but still be put through the whole filter and validation process.
But that's just a dream right now.
Originally posted by @larsnystrom at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147651894
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 https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147652527
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 https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147655051
I don't see this as a BC Break because the translation feature seems never has work.
that's not correct, it worked prior to 2.4.
Originally posted by @stefanotorresi at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147659542
@stefanotorresi Prior to 2.5.2/2.4.5 there was not a required message.
Originally posted by @Maks3w at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147662601
it was handled by the notEmpty validator, which was always injected for required inputs before continue_if_empty
and allow_empty
flags were deprecated.
Originally posted by @stefanotorresi at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-147666605
I really like this approach, but I share the same concern voiced by @akrabat — unless the required
flag is tied to the Required
validator, we're going to have a lot of confusion.
It seems like this is mostly the case right now, though; isValid()
auto-prepends the validator if the $required
flag is true. It's the other side of the equation we need, however: isRequired()
should return true if the Required
validator is present in the chain.
If you can implement that, I can merge this.
Originally posted by @weierophinney at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-185885909
Moved to 2.7.0 milestone; we're ready for 2.6.0 (zend-servicemanager forwards-compatibility).
Originally posted by @weierophinney at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-185886963
I keep thinking in new ways for make this better. Probably with something like #87
Originally posted by @Maks3w at https://github.com/zendframework/zend-inputfilter/pull/62#issuecomment-185970268
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 https://github.com/zendframework/zend-inputfilter/pull/62