validating icon indicating copy to clipboard operation
validating copied to clipboard

MessageBag emtpy when using "Unique" rule on transaction

Open giulioprovasi opened this issue 3 years ago • 2 comments

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 ?

giulioprovasi avatar May 18 '21 09:05 giulioprovasi

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!

dwightwatson avatar May 18 '21 12:05 dwightwatson

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 ;)

giulioprovasi avatar May 19 '21 13:05 giulioprovasi