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

isValid() does not throw ValidationException #44

Closed
mikebronner opened this issue Jul 1, 2014 · 6 comments
Closed

isValid() does not throw ValidationException #44

mikebronner opened this issue Jul 1, 2014 · 6 comments
Milestone

Comments

@mikebronner
Copy link

Is the above description correct in 0.9.0? I run the isValid function on models that don't meet the criteria, and no exception is thrown, instead it returns a boolean, in this case false, which is expected.

What call can I make that does trigger validationexception without saving?

Sorry I'm having so many issues getting things in place, for some reason I keep bumping into edge cases. :)

@arthurkirkosa
Copy link

/**
 * Whether the model should throw a ValidationException if it
 * fails validation. If not set, it will default to false.
 *
 * @var boolean
 */
protected $throwValidationExceptions = true;

All in the readme

@mikebronner
Copy link
Author

Got that all in there. It throws exceptions when calling $model->save(), just not when calling $model->isValid('saving');

I was able to work around the issue by adding the following code:

        if (!$user->isValid('saving')) {
            $exception = new ValidationException(get_class($user) . ' model could not be persisted as it failed validation.');
            $exception->setModel($user);
            $exception->setErrors($user->getErrors());
            throw $exception;
        }

Looking at the source for the validation trait, is appears that the trait method performValidation() is overriding the observer method of the same name. The trait method does not throw an exception, while the observer method does.

@dwightwatson
Copy link
Owner

Haha, yeah it is. My reasoning was that asking a model if it isValid() isn't necessarily an exceptional event. It just tells you whether it is or not. On the other hand, attempting to save a model that isInvalid() should be considered an exceptional event.

I suppose one thing I could do is move the throwing of an exception into a method on the model, say throwValidationException(). That it would be slightly easier for you to accomplish what you're after (but still not exactly what you're going for...):

if ( ! $user->isValid('saving')) $user->throwValidationException()

Actually, another alternative is (I'm thinking as I type!) is another method, like isValidOrFail('saving') which would achieve what you're after without being totally weird about it. What do you think?

@mikebronner
Copy link
Author

Yes, I agree with the semantics :). Asking if something IS or ISNOT should return true or false. Is isInvalid() simply the inverse of isValid(), or will it actually throw an exception? If so, I'll be happy to use that.
I like the isValidOrFail() idea, it's semantically clear and conforms to Laravel conventions. I could also see validate() throwing an exception, but not sure if that has naming conflicts.

@dwightwatson
Copy link
Owner

isInvalid() simply returns the inverse of isValid(). I'll take a look at implementing isValidOrFail() and isInvalidOrFail() methods tonight. Can't have a validate() method as it seems to conflict with a few other packages, one of which being Sentry I believe, would have to make it work with our performValidation method.

@dwightwatson dwightwatson added this to the 0.9.1 milestone Jul 1, 2014
@dwightwatson
Copy link
Owner

Tagged in 0.9.1.

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

3 participants