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

[12.x] Stops validation if a custom rule specifies it #54635

Draft
wants to merge 3 commits into
base: 12.x
Choose a base branch
from

Conversation

peterfox
Copy link
Contributor

Changes

  • Adds a Illuminate\Contracts\Validation\StopUponFailure contract.
  • Changes \Illuminate\Validation\Validator::shouldStopValidating() to accept two arguments, the new one being the rule
  • Changes \Illuminate\Validation\Validator::shouldStopValidating() to check if it's an invokable rule and if the custom rule it's using implements the Illuminate\Contracts\Validation\StopUponFailure contract.

Why

This would allow for more control over when validation rules fail and when they should continue. Instead of only relying on bail when one rule can trigger a failure to run all further attributes.

Example use:

class RunAntiVirusScan implements ValidationRule, StopUponFailure
{
    public function shouldStop(): bool
    {
        return true;
    }

    public function validate(string $attribute, mixed $value, Closure $fail): void
    {
        if ($scan->fails()) {
            $fail(':attribute is not safe');
        }
    }
}
$request->validate([
    'image' => ['image', new RunAntiVirusScan(), 'max:1024'],
]);

@taylorotwell
Copy link
Member

It feels a bit odd to have an interface called StopUponFailure but then have to implement a method which presumably will almost always return true. 🤔

@taylorotwell taylorotwell marked this pull request as draft February 17, 2025 17:11
@peterfox
Copy link
Contributor Author

@taylorotwell I did consider that It could just be an interface. The user might want to force that condition only in circumstances based on whether they felt it was risky or not to continue validation of that field was where my mind was at. I'm thinking where using AI/ML and you've got a score from a service and don't want to continue processing a file further.

Maybe a new name for the interface would help. MightStop as such for the interface and then a shouldStop method.

@shaedrich
Copy link
Contributor

Maybe a new name for the interface would help. MightStop as such for the interface and then a shouldStop method.

What about Stoppable?

@jmarcher
Copy link
Contributor

We use the term bail in other parts for rules, so maybe Bailable or IsBailable gives better context

@flavio-schoute
Copy link
Contributor

Why not name it: StopOnFailure and check if you implemented the empty interface, if so.. Stop on failure then.

@crynobone crynobone changed the base branch from master to 12.x February 18, 2025 22:02
@henzeb
Copy link
Contributor

henzeb commented Feb 22, 2025

I think @flavio-schoute is the best solution. We can later add Bailable interface that allows a method for conditional bail?

@peterfox peterfox force-pushed the feature/stop-on-failure-rule-contract branch from c9d0367 to 362f574 Compare February 25, 2025 01:55
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.

6 participants