laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.32k stars 10.95k forks source link

Validation rules on a single attribute do not $stopOnFirstFailure #43839

Closed newtonjob closed 2 years ago

newtonjob commented 2 years ago

Description:

It looks like, for some reason, the Validator::stopOnFirstFailure() doesn't apply to closure-based rules.

Steps To Reproduce:

For example, consider this rules definition from a FormRequest;

/**
* Indicates if the validator should stop on the first rule failure.
*
* @var bool
*/
protected $stopOnFirstFailure = true;

/**
 * Get the validation rules that apply to the request.
 *
 * @return array
 */
public function rules(Remita $remita)
{
    return [
        'bill_id' => [
            'required', Rule::unique('payments')
                ->where('user_id', $this->user->id)
                ->whereNotNull('transaction_id')
        ],

        'reference' => [
            //'bail', Looks like we need this, since the rules aren't obeying $this->$stopOnFirstFailure
            function ($attribute, $value, $fail) {
                if (Payment::whereRelation('transaction', 'reference', $value)->exists()) {
                    $fail('Transaction has already been used.');
                }
            },
            function ($attribute, $value, $fail) use ($remita) {
                if (! $remita->verify($value)->valid()) {
                    $fail($remita->message);
                }
            },
            function ($attribute, $value, $fail) use ($remita) {
                if ($remita->amount < $this->bills->sum('fee.amount')) {
                    $fail('Transaction Declined! Unmatched Amount');
                }
            },
        ]
    ];
}

When the first closure rule for the reference parameter fails, the validator still invokes the other two closure rules. Keep in mind that I have $stopOnFirstFailure set to true on the FormRequest class;

However, if I pop the bail rule into the array (or uncomment as in example above), it works!

I know that $stopOnFirstFailure works for regular validation rules, but not sure why it doesn't work for closure based rules. There is also no disclaimer on the docs, at least, that I have seen.

Is this behaviour normal or expected?

driesvints commented 2 years ago

Heya, thanks for reporting.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as separate commits on the main/master branch and share the repository here? Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Please do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

driesvints commented 2 years ago

Will reopen when a repo is provided.

newtonjob commented 2 years ago

Thank you for your response @driesvints

So, after digging further, I realize the issue has nothing to do with closure rules particularly.

Consider this simple validation example;

validator(['email' => 'me'], [
    'email' => 'required|email|date|array'
])->stopOnFirstFailure()->errors()->messages();

Obviously, all the other rules except required will fail, and this is the output;

[
   "email" => [
     "The email must be a valid email address.",
     "The email is not a valid date.",
     "The email must be an array.",
   ],
 ]

I had thought that validation will stop right after the email rule.

But looking at the Validator class on line 410, it appears, $stopOnFirstFailure only stops validation when there is any error at all from the previously validated attribute, and not necessarily for the rules of a particular attribute. https://github.com/laravel/framework/blob/e07e00507e67c710798eb98cdb09845f65026429/src/Illuminate/Validation/Validator.php#L410

I just need to know that this is the expected behavior, and if it is, they may be no need to create a whole new repo to reproduce it, and the issue may remain closed.

Many thanks.