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

MessageBag emtpy when using "Unique" rule on transaction #229

Open
giulioprovasi opened this issue May 18, 2021 · 2 comments
Open

MessageBag emtpy when using "Unique" rule on transaction #229

giulioprovasi opened this issue May 18, 2021 · 2 comments

Comments

@giulioprovasi
Copy link

Hey,
after digging a while on a weird issue, here what I get :

Model rule:

'name' => [
                'bail',
                'required',
                'string',
                'max:255',
                (new Rules\Unique('folders', 'name'))
                    ->ignore($this->id)
                    ->where('parent_id', $this->parent_id)
                    ->whereNull($this->getQualifiedDiscardedAtColumn())
                    ->whereNull($this->getQualifiedDeletedAtColumn())
                ,
            ],
 DB::transaction(static function() {
            $x = new Test();
            $x->name = 'test'; # name is not unique, so it will trigger Unicity rule
            $x->saveOrFail(); # fails correctly
        });

The issue is that in the following method

    /**
     * Throw a validation exception.
     *
     * @throws \Watson\Validating\ValidationException
     */
    public function throwValidationException()
    {
        $validator = $this->makeValidator($this->getRules());

        throw new ValidationException($validator, $this);
    }

The validator is not "triggered" with a ->fail(), so message bag is empty right now.
Then, my transaction rollback and the unique rule does not fails anymore, so the validator has no messages :/

Won't it be better to do something like this :


    public function throwValidationException()
    {
        $validator = $this->makeValidator($this->getRules());

        throw new ValidationException(tap($validator)->fails(), $this);
    }

in order to populate the validator with errors before anything else in the app happens ?

@dwightwatson
Copy link
Owner

I see where you're coming from, however a concern here is that we would now run the rules twice - and this is especially an issue when talking about the Unique rule which would mean duplicate database queries. It's almost like you'd want to persist the last validator that was used (for an isValid or isInvalid call as an instance variable so you could pass it into the exception.

I'm not really sure that I'm going to tackle that any time soon, or that I'm keen to make what could be a substantial breaking change right now. In the meantime, overriding throwValidationException in your app would be the best bet if it solves your use-case. Hope this is helpful!

@giulioprovasi
Copy link
Author

It's almost like you'd want to persist the last validator

Indeed this should be the proper behavior.

I will indeed override the throwValidationException method right now ;)

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

2 participants