Skip to content
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

Skip subsequent validation rules for a field when the one before it fails #10432

Conversation

sebastiaanluca
Copy link
Contributor

Proposal for issue #4789. Prevents fields validation rules from being executed when the input field doesn't meet the rule requirements of the rule before it. E.g. don't run all validation rules of a field when one fails.

@taylorotwell
Copy link
Member

This would be a major breaking change and I still feel it presents a bad UX situation where the user is informed of one rule at a time. For example, what if the password doesn't match and it also needs to be at least 8 character, but both of these rules fail? The user would only presented with the error messages one at a time, which doesn't make sense. We should be alerting them that both rules failed.

@sebastiaanluca
Copy link
Contributor Author

I know, but there are a lot of other use cases where for instance the 3rd validation rule for a field needs the first one to pass (being required or something). If you use the default flow, it'll try all validation rules regardless of input and previous rules. If you do database checks and the input provided passes all previous validation rules, it can break your database rule and throw unforeseen errors. In worst case scenarios, corrupt data.

@taylorotwell
Copy link
Member

That may be; however, we're not going to break the existing functionality for this new functionality. Any new functionality would have to be a new method or something.

@sebastiaanluca
Copy link
Contributor Author

Of course, that's why it's a proposal and it takes some searching to find the best solutions :) Something like $validator->passes(boolean $failOnFirst = false) or $validator->passesAll()? I'm bad at choosing method names haha…

Totally get your use case too btw. I have both situations across different projects.

@taylorotwell
Copy link
Member

I would prefer a new method name vs. adding a boolean parameter. I like to avoid adding parameters whenever possible. We’ll just have to settle on a good name.

On Wed, Sep 30, 2015 at 1:08 PM, Sebastiaan Luca notifications@github.com
wrote:

Of course, that's why it's a proposal and it takes some searching to find the best solutions :) Something like $validator->passes(boolean $failOnFirst = false) or $validator->passesAll()? I'm bad at choosing method names haha…

Totally get your use case too btw. I have both situations across different projects.

Reply to this email directly or view it on GitHub:
#10432 (comment)

@JosephSilber
Copy link
Member

Would it make sense to be an internal flag in the validator?

Something like this:

public function halt($halt = true)
{
    return $this->halt = $halt;
}

// use it on the validator instance
$validator->halt()->passes();

@JoostK
Copy link
Contributor

JoostK commented Oct 2, 2015

I'll propose this again: use a double pipe to indicate a halting point. Accomplished in a single line of code.

@JosephSilber
Copy link
Member

JosephSilber commented Oct 2, 2015 via email

@sebastiaanluca
Copy link
Contributor Author

@JoostK What do you mean exactly with "double pipe"? Would you place that in your validation rules or something? If so, I'd have to add double pipes after every rule.

@JoostK
Copy link
Contributor

JoostK commented Oct 4, 2015

@JosephSilber The array thing would require an empty rule in-between, not that nice indeed.
@sebastiaanluca Yes, in the validation rules. You may end up with using lots of double pipes, yes.

A method call to enable this like @JosephSilber's example avoids these downsides, but is also less configurable (I sometimes prefer seeing all validation errors for a field at once, instead of seeing new requirements after having updated the field to fix the first reported error. With double pipes, you could easily specify at which point validation must not proceed when an error occurred up to that point)

@sebastiaanluca
Copy link
Contributor Author

Maybe both? I only have use for a global halt method, but I can see how I could utilize the double pipes / empty rules feature in some cases (although it's not that nice to use).

$validator->passesAll();
$validator->passesOne();
$validator->passesOneByOne();
$validator->passesSingle();
…

I still prefer using the same method and just adding a parameter, but either way is fine for me. A new method would be more descriptive and wouldn't break anything in case they did override the method.

@kamazee
Copy link
Contributor

kamazee commented Oct 13, 2015

This would be a major breaking change and I still feel it presents a bad UX situation where the user is informed of one rule at a time. For example, what if the password doesn't match and it also needs to be at least 8 character, but both of these rules fail? The user would only presented with the error messages one at a time, which doesn't make sense. We should be alerting them that both rules failed.

I think it's just a matter of arranging validation rules.
The behavior @taylorotwell describes could be achieved by simply putting the rule that checks if a value matches another value first in validators chain for "Confirm Password" field.

And, well, I fail to come up with any use case where stopping after the first failed rule would not be useful. I think rules should extend each other and be arranged from the least to most specific one.

@lwtsn
Copy link

lwtsn commented Oct 21, 2015

@taylorotwell you did a great thing by creating Laravel and I respect you greatly for that. But this appears to be a very trivial thing -> there is no sense validating something which has already failed validation if this is going to cause further issues. Why should we go about work arounds when we can tackle the issue head on. The suggestion by @JoostK is, in my opinion, exactly what is required.

If we do not need to die on the first validation failure we single pipe. If we want something to not be validated after the first failure we die.

Where is the sense validating a required date field as a date format if it does not exist?

@taylorotwell
Copy link
Member

@Lukewatson12 since this is an open source project, anyone can contribute. I already gave my thoughts on the matter. Make a new method for this behavior and PR it.

@Joel-James
Copy link

Why don't we have a new validation rule? Something similar to sometimes rule?

I have created a pull request which adds a new rule failonfirst. Using this rule on a field, we can execute validation checks one by one. It is optional and can be used for single fields. So there won't be any bad UX situation like @taylorotwell said, as we can use it as a separate rule.

See #11986

@sebastiaanluca sebastiaanluca deleted the validation/fail_on_first_rule branch June 26, 2017 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants