Using Watson Validator with Form Requests
snipe opened this issue · 7 comments
Hey all! So, possibly a dumb question here, but I want to make sure I'm going about this the right way.
I have a User model with the following rules:
protected $rules = [
'first_name' => 'required|string|min:1',
'last_name' => 'required|string|min:1',
'username' => 'required|string|min:2|unique:users,username,NULL,deleted_at',
'email' => 'email',
'password' => 'required|min:6',
];
That works great for the user validation, but when I need to add some additional conditionals, things seem to go a little wonky.
Obviously, the way Form Requests work in Laravel 5.2 is that they fire on the form submit before any model validation fires. That seems fine, but if I want to combine the model level validation and the form validation into one message error response (versus having the form validator fail, then on a corrected submit, a new set of errors for the model validation) for a better user experience. I've been duplicating my model rules in the form validator, which is obviously not ideal, but wasn't sure exactly how to combine them (user and form validation), or if it's even possible.
For example:
and then once that's satisfied, the screen the admin sees is on re-submit:
The challenge I'm running into is on a new user versus saving an existing user. When I create a new user, I want the password (and password confirmation) to be required, but I don't need to require that on a User update. If the admin has entered a value for password, then and only then do I want the password confirmation validation to kick in.
Do I need to set up two different form requests, one for save new and one for save update? Or should I use the isValid('new)
style of validation versus save()
? I'd still have to duplicate those main model rules in the form request, right?
public function rules()
{
return [
'first_name' => 'required|string|min:1',
'last_name' => 'required|string|min:1',
'username' => 'required|string|min:2|unique:users,username,NULL,deleted_at',
'email' => 'email',
'password' => 'required|min:6|confirmed',
'password_confirm' => 'sometimes|required_if:password,NOT_NULL|same:password',
];
}
Again, I apologize if this is a dumb question. Might be a case of the Mondays, but I'm just not clear on the best way to approach this. Should I write a helper method to import the model level rules into the form request validator, so I don't have to duplicate the rules?
Thanks in advance
Not a dumb question at all, but I'm not sure I've got a reasonable solution for you. I've always used model validation as an alternative to form validation, and I fear using them together is causing more trouble than it's worth. Pick the approach that works best for you and stick with that.
One thing that is interesting, something we probably could look at for this library, is the use of a rules()
method on the model as a way to provide dynamic rulesets the same way form requests do. Would provide more flexibility than just the $rules
property on it's own.
You could use separate form requests for storing/updating, but there is a way to get around it too. On my blog I used separate form requests, but if you look at one of my update form requests you can see how you might be able to pickup the user from the request and adjust the rules that way.
public function rules()
{
$user = $this->route()->parameter('user');
$rules = [
'first_name' => 'required|string|min:1',
'last_name' => 'required|string|min:1',
'username' => 'required|string|min:2|unique:users,username,NULL,deleted_at',
'email' => 'email'
];
if ( ! $this->route()->parameter('user')) {
$rules = array_merge($rules, [
'password' => 'required|min:6|confirmed',
'password_confirm' => 'sometimes|required_if:password,NOT_NULL|same:password',
]);
}
return $rules;
}
I think the other alternative is to catch an invalid form request, chuck the parameters into the model, get any errors it has and then merge those errors with the form request errors, but that feels like more of a hack than a good solution to me. I could provide some pseudo-code of how that might look if that was the route you wanted to go down, however.
I hope this makes sense and is helpful at least. I'm running on little sleep at the moment but I've tried to understand your use-case and offer some useful solutions.
Maybe there could be a different kind of attribute to the 'password' rule - I don't know if this would go in the model or in the FormRequest
, or if it's even necessary any more if you already have model-level validation...but - something like:
'password' => 'required_if_new|min:6|confirmed',
'password_confirm' => 'sometimes|required_if:password,NOT_NULL',
Would that be helpful, somewhere?
@dwightwatson Thanks so much for such a quick and thoughtful reply. (I don't ask questions here often, but I'm always so impressed at your responses. As a fellow project maintainer, I hope I'm able be as fast and helpful as you are. Serious kudos and thanks to you.)
We wrote a janky method to import the existing model validation rules, but quickly realized that some of the model validation rules rely on database interactions that the form request can't handle - for example, uniqueness of a field using protected $injectUniqueIdentifier = true;
. My model level validator will validate 'username' => 'required|string|min:2|unique:users,username,NULL,deleted_at',
on editing an existing record, but the form request won't.
Right now I'm just accepting the double-post UX-ugliness, because I'm to tired to write custom validators for all this mess. It's frustrating that there isn't a cleaner way to accomplish something that seems pretty straightforward. (This isn't your fault, of course.) Fortunately, I think I'd only need to do this for Users (versus the 15 other models I have in my app), since it has that password conditional. But I'm too fried right now. If I come up with something clever, I'll definitely post it to this thread, in case it can help other folks.
I use a mix of form request and model validation in my app, largely because there are some things that model validation wouldn't cover very gracefully. For example, I have Users and Assets as models, and there is the notion of checking out an asset to a user. Unless I wanted to turn the checkout/checkin into a model (which feels weird), form request validation works really well there. But for importing users from CSV, for example, a form request wouldn't make sense, since there is no form.
@uberbrady That's an interesting idea. I could write a generic custom validator for require_if_new
(similar to this kind: https://gist.github.com/snipe/24aca75a3cdcd6b73932), and then extend that out whenever it's needed (which admittedly wouldn't be in a lot of places.) I'll play around with that a little maybe.
No worries - I'm guessing it could be a late Monday-itis for you but it's a chilled early afternoon down under. Thank you for the kind words though, I'm glad it's appreciated!
Yeah, because this library evolved from Laravel 4.x (pre-form requests) and was upgraded for 5.x it was considered an alternative to form requests. It can work alongside form requests but clearly the integration between the two is lacking... especially the unique identifier injector. That might actually be a cool extension to build for form requests.
If you do come up with a good solution please do let me know, it would be great even to pop it into the readme for anyone else dealing with a similar problem. Sounds like you've got some tricky requirements for your app but I'm happy to hear (for the most part) the library is working for you. Let me know if there is anything else I can do to help.
Hey @snipe great questions and stuff I deal with every day. @dwightwatson as per usual is spot on that this package is caught between two paradigms and there's not perfect solution. Here's my best stab at helping you out though:
Create vs. update
I'd suggest you look into http://github.com/esensi/model which builds on top of this package and ported the old Laravel 4.2 style of model validation rulesets to Laravel 5 style of form validation + model validation. So you can basically deal with different rules depending on creating or updating.
Duplicate Validations
Laravel 5 makes the form request validation amazingly simple since you just method inject and away you go with seamless validation errors. The issue you describe is quite common though where your form needs to validate input while your model needs to validate data integrity and therefore the rules are not quite the same and therefore the validation fails separately in two places. My general approach to this issue is to forgo the magic of Laravel and deal with validating your form request manually. You validate the request to ensure your input is valid and then you build up your models and just before you perform your save you validate your form request, catching any exceptions thrown. In the catch
you should have access to the errors so save those to a temporary variable and continue processing outside of the catch
. Now go ahead and do a model save()
. Then you can check to see if you have any errors and merge your form request errors with your model errors. Something like the following example code:
use Illuminate\Contracts\Validation\ValidationException;
use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Http\Exception\HttpResponseException;
use Illuminate\Http\Exception\HttpResponseException;
use Illuminate\Validation\Validator;
class FooRequest extends FormRequest {
// Revert back to base trait method for basic exceptions
protected function failedValidation(Validator $validator)
{
throw new ValidationException($validator);
}
// All your rules and customizations like normal
}
Router::post('/', function(){
// Do all your setup
$request = app(FooRequest::class);
$model = new Model;
$model->fill($request->all());
// Now start validating
// Might not even be needed as FormRequest validates when resolved
// and therefore may be needed around $request above.
try {
$request->validate();
} catch (ValidationException $exception) {
$errors = array_merge($errors ?: [], $exception->errors()->toArray());
}
// Now attempt to save
if( ! $model->save() ) {
$errors = array_merge($errors ?: [], $model->getErrors()->toArray());
}
// Check if validation failed
if( isset($errors) && ! empty($errors) ) {
throw new HttpResponseException($request->response($errors));
}
// Continue on a happy Snipe...
});
The challenge here really is that often if your input is not right you have to do all sorts of input checking just to get valid data to fill your model with. For basic form entry it often maps one to one but for more complex models and forms it rarely does and data massaging is needed: then your code will probably choke since the form data could not be validated. So there's that left unsolved.
UX Choices
Depending on your accessibility requirements you might do better with AJAX callbacks that validate your form in realtime as your user moves form input to input. You'd blow up your API limits on validation checks but you'd be giving more reactive realtime communication to your user about validation. You'd still have to perform the server-side validation when they actually submit as it's the equivalent of client-side validation but it would behave more like what you want anyway. The likelihood of the form request failing after they fill out the entire form without validation errors is very low and so the only errors they'll see are data integrity errors from the model. If API response time/latency is high consider using a websocket to reduce the connection overhead. I've done this solution before and it makes for a very responsive interface at the cost of no-JS browser incompatibility (though the duped validation can handle this fallback case).
The rules()
method
I'm totally on board with that and I know the Esensi team is planning to refactor everything thoroughly to make better use of the create vs. update ruleset logic and also make rules()
a first-class method. There should be a line between form request validation and model validation but the interface for both should be pretty close. In fact I think it would be very nice to have something of a FormRequest::mergeRules($model->rules())
or Model::mergeRules($request->rules())
sort of custom implemented method that allows you to make the best reuse of validation rules. That said, you could do that already if you set your rules on your model when it is booted or constructed, getting them from a rules class. Then you could overwrite the rules()
method on your form request class to return the same ones or merge with them. There's no silver bullet unfortunately and you have to take it on a form by form basis.
Validation Extensions
Your approach works and I've found myself having to create lots of custom validator rules between projects. I find using this provider as a drop-in replacement for Laravel's to be a good choice. You'll likely need to make some modifications to it if you're not using all of esensi/core
package but it basically just uses a config to define all the ones you want to load as class-based validation extensions. You'll also want to take a look at an example like ComparisonValidator
and the ValidatorTrait
hack to get it working. Once you understand it though its actually quite easy to create custom rules and often beats having to use ->sometimes()
callbacks.
I hope some of these ideas help you and others. Let me know if I missed anything or if you need more specific help. You can email me if you need some offline help!
One More Option
Almost forgot another work around: remember there are class observers you can use with Eloquent which would allow you to add your create vs. update rules conditionally when the model goes to be validated. For example you could hook into the validating
observable event that this package provides and check to see if the model exists or not, merging any base rules on top of your model. You'd of course want to ensure that in validated
you reset those back to their previous state. That's another reason a method like rules()
would be better as it should always return the same routes as defined on the model when constructed. This can make for a really clean separation of concern and there's some inspiration available in esensi/model
package code on how to go about it.