-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Validate empty with context #4165
Validate empty with context #4165
Conversation
…ty instead of just being marked as valid if empty. Required to test for validity of an empty value based on context.
…ty instead of just being marked as valid if empty. Required to test for validity of an empty value based on context.
…ixing a failing unit test; test was not providing data for a required input.
…f empty, removing redundant tests that mysteriously turned up.
…eing commented out.
Validate empty with context Conflicts: library/Zend/InputFilter/Factory.php
Merged, and will release with 2.2.0. Thanks, Zach! |
Excellent. I promise that I'll update the documentation, really I will. |
- `InputInterface` had a BC break, as it added `setContinueIfEmpty()` and `continueIfEmpty()` -- which were technically already defined in `EmptyContextInterface`, which `Input` had been modified to implement. Removing the methods from `InputInterface` removes the BC break, and fixes an issue that appears in 5.3.3 with one class implementing two interfaces that define compatible methods.
@@ -187,8 +187,10 @@ protected function validateInputs(array $inputs) | |||
&& $input->isRequired() | |||
&& $input->allowEmpty() | |||
) { | |||
$this->validInputs[$name] = $input; | |||
continue; | |||
if (!$input->allowEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zburnham Should this be if (!$input->continueIfEmpty()) {
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, MWOP seemed to think it was ok, but I'll definitely take a look, more
eyes on the code and all that.
Z
On May 4, 2013, at 6:24 PM, Daniel Gimenes notifications@github.com wrote:
In library/Zend/InputFilter/BaseInputFilter.php:
@@ -187,8 +187,10 @@ protected function validateInputs(array $inputs)
&& $input->isRequired()
&& $input->allowEmpty()
) {
$this->validInputs[$name] = $input;
continue;
if (!$input->allowEmpty()) {
@zburnham https://github.com/zburnham Should this be if
(!$input->continueIfEmpty()) {?
—
Reply to this email directly or view it on
GitHubhttps://github.com//pull/4165/files#r4085890
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See line 188.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: #4426.
@zburnham You have not fulfilled your promise yet. 😄 |
I know :/ I'm going to be looking at it tonight anyway. On May 4, 2013, at 6:41 PM, Daniel Gimenes notifications@github.com wrote: @zburnham https://github.com/zburnham You have not fulfilled your promise — |
@zburnham Sorry if I'm wrong, but on second thought, this is useless since you can simply |
It stood a little tweaking, anyway. On Mon, May 6, 2013 at 8:40 AM, Daniel Gimenes notifications@github.comwrote:
|
@weierophinney Can you please review this before release RC2? |
@danizord Already did. :) |
@weierophinney It seems reasonable? As said |
@danizord they're in different contexts. So, it does seem reasonable to me; it provides more validation options, basically. |
I don't want to be against you, but this is going to be complicated. Now we have:
It is impossible to understand without looking at the code of the component. Since the documentation does not cover it all. |
The documentation is something I'm going to try to fix over the next few On Mon, May 6, 2013 at 1:02 PM, Daniel Gimenes notifications@github.comwrote:
|
…idate_empty_with_context Validate empty with context Conflicts: library/Zend/InputFilter/Factory.php
- per php-cs-fixer
…tInterface - `InputInterface` had a BC break, as it added `setContinueIfEmpty()` and `continueIfEmpty()` -- which were technically already defined in `EmptyContextInterface`, which `Input` had been modified to implement. Removing the methods from `InputInterface` removes the BC break, and fixes an issue that appears in 5.3.3 with one class implementing two interfaces that define compatible methods.
Trying this again, I messed up a rebase.
Currently there is no way for Zend\InputFilter to determine the validity of an empty field based on context; for example, an empty field might be valid if another field is 'Y', but not valid if that other field is 'N'. This adds a continueIfEmpty flag to the Input definition, via a separate interface.