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

Validation using required_with behaves in a strange way #17211

Closed
stevevg opened this issue Jan 9, 2017 · 9 comments
Closed

Validation using required_with behaves in a strange way #17211

stevevg opened this issue Jan 9, 2017 · 9 comments

Comments

@stevevg
Copy link

stevevg commented Jan 9, 2017

  • Laravel Version: 5.3.28/5.3.29
  • PHP Version: 7.0.14
  • Database Driver & Version: Mysql 5.1.73

Description:

Using a validation rule of type: 'field1' => 'required_with:field2' fails even if field2 is present but empty. Is this the expected behavior? I'm quite sure this did not happen in 5.2

Steps To Reproduce:

I've tried with this rules in my CustomRequest file:

public function rules() {
    $rules = [
      'volume_min' => 'required_with:volume_max|integer|min:1',
      'volume_max' => 'required_with:volume_min|integer|greater_than_field:volume_min'
];
return rules;
}

but I receive the error volume_min must be an integer, volume_min must be at least 1 and volume_max must be an integer, volume_max must be greater than volume_min, even if both fields are POSTed (actually with _method: PUT) empty.

I've currently fixed using a method based on #17032 (comment) but I'm not sure if this is the desired behavior of the validator.

This is my fix:

public function rules() {
    $rules = [
      'volume_min' => 'integer|min:1',
      'volume_max' => 'integer|greater_than_field:volume_min'
];
$rules["volume_min"] .= $this->request->get("volume_max") ? "|required_with:volume_max" : "|nullable";
$rules["volume_max"] .= $this->request->get("volume_min") ? "|required_with:volume_min" : "|nullable";

return rules;
}
@themsaid
Copy link
Member

themsaid commented Jan 9, 2017

Please provide the data that fails to pass validation.

@stevevg
Copy link
Author

stevevg commented Jan 9, 2017

It's an ajax call in which I pass many fields, these are the ones of interest:

_method=PUT&[...]&volume_min=&volume_max=&other_var=[...]&_token=VsGtVG9OZNr1tb3bsloF2BcmDQJYHdt4SCA4p4Mq

@themsaid
Copy link
Member

themsaid commented Jan 9, 2017

The problem is that you assume the rest of the rules won't be applied if the field is empty but that's not true, the condition only falls on required_with, so for example:

"someFields" => "required_with:anotherField|min:1"

If anotherField is empty the required check will be skipped, but the min check will run regularly, you need to use the sometimes() to conditionally add rules based on the conditions you want.

$validator->sometimes('volume_min', 'integer', function(){ return request('volume_max') })

this will cause the integer check to run only if volume_max is not empty.

This is not an issue with the framework so i'll close the issue, but please feel free to ping me for further clarification.

@themsaid themsaid closed this as completed Jan 9, 2017
@stevevg
Copy link
Author

stevevg commented Jan 9, 2017

Ok, I see, and will look into it later to make it work with my app. But, as you said:

If anotherField is empty the required check will be skipped, but the min check will run regularly

isn't this behaviour in contradiction with this merged pull request? #15089

@themsaid
Copy link
Member

themsaid commented Jan 9, 2017

It'll skip validation if a required rule fails, not skipped. This is a confusing point yes, might worth mentioning in the docs, will take care of that :)

@Jameron
Copy link

Jameron commented Mar 17, 2017

Has this issue been updated in the documentation?

@Jameron
Copy link

Jameron commented Mar 17, 2017

I'm seeing this same issue using required_without_all, is there a fix?

'gnar'=>'required_without_all,foo,bar|min:1between:0.01,99999999.99',

foo and bar are both set, gnar is null, but gnar is failing as "gnar min must be at least 1"

@tpraxl
Copy link
Contributor

tpraxl commented Mar 23, 2018

This behavior might have good technical reasons, but it is IMHO counter-intuitive. Just spent at least an hour to debug, find this issue and workaround this issue.

@stevevg's example reads:

     // if volume_min is given, then this field is required. It should be an integer, greater than volume_min
      'volume_max' => 'required_with:volume_min|integer|greater_than_field:volume_min'

It does not read

     // if volume_min is given, then this field is required. 
     // It is nullable. And even if it's null then it should be an integer, greater than volume_min
      'volume_max' => 'required_with:volume_min|integer|greater_than_field:volume_min'

Hm.. thinking about it..

Would it be possible to specify it like (adding nullable) this to get the intended behavior?

      'volume_max' => 'required_with:volume_min|nullable|integer|greater_than_field:volume_min'

@HosseyNJF
Copy link

Any updates?

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

No branches or pull requests

5 participants