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

[Request] Model self-validating trait #3751

Closed
dwightwatson opened this issue Mar 2, 2014 · 8 comments

Comments

Projects
None yet
9 participants
@dwightwatson
Copy link
Contributor

commented Mar 2, 2014

I saw on Twitter that for 4.2 the soft delete functionality would be implemented as a trait. As we now have the option of traits to enable selective use of Eloquent functionality, I'd like to request the reconsideration of self-validating models (see #1169 back before the release of 4.0).

I think self validating models will help clean up a lot of validation code from controllers and help ensure the validity of model data being saved in the database. I believe having it available as core functionality in Laravel will allow for more consistent use, much like model validation in Rails.

In essence, prior to saving a model (whether it is being created or updated), validation rules would be run over it and return either a boolean or raise an exception indicating that the save did not complete. Model rules could be available on a protected static variable, like Model::$rules, which is just an array like the Validator class takes.

The model validation should also consider use of the unique rule, ensuring to pass in the model's own ID to the rule if the model has already been persisted.

The following methods available on an Eloquent model would allow additional functionality on top of just ensuring invalid models aren't saved. Access boolean states of whether the model is valid or not, and access the errors/messages of when a model is invalid.

  • isValid()
  • isInvalid()
  • errors()
  • messages()
@taylorotwell

This comment has been minimized.

Copy link
Member

commented Mar 2, 2014

Yeah I think we can have some discusson on this. A few initial thoughts that come to mind, a simple valid usually won't work, since many times you are validating different states, etc. like "is this valid for update? is this valid for X?, etc."...

Would be interested to hear what people think on if this should be a part of the core or if we should let the community manage it through packages.

@Anahkiasen

This comment has been minimized.

Copy link
Contributor

commented Mar 2, 2014

I'd love this to be part of the core, I think you could possibly pass the context like $user->isValid('signup') and it would validate against that set of rules.

@Kindari

This comment has been minimized.

Copy link

commented Mar 3, 2014

Another possibility is a simple set of rules, and then a callback that receives the validation object, for use with conditional validation rules. The basic set of rules would be applied to all states, and then you can conditionally add rules based on whatever logic you wish.

@dalabarge

This comment has been minimized.

Copy link
Contributor

commented May 23, 2014

@taylorotwell I use Ardent and Magniloquent and would be willing to create a uniform interface and trait set that implements the key features from both. I would think that they need to be implemented in a way like your SoftDeletingTraits in that they should be opt-in only so as to not affect any previous code usage.

Having found both of these community packages either lacking features, unit tests, or otherwise abandoned, I'm actually trying to figure out the direction to take some of our own software going forward. Right now it seems the best thing to do is to fork Magniloquent and implement it as a set of traits in a way that's more compatible with Eloquent's model changes. Another alternative is to fork Illuminate\Database and implement it directly and then provide you the opportunity to review and revise to your liking.

It would be better all around for Laravel if it were in the core vs. a separate package which leads to fragmentation as fewer people use each package and therefore submit issues. Can you share whether or not you'd actually consider it as a core pull request (maybe share the interface you'd like to see implemented) before I get started on a 3rd-party package.

@franzliedke

This comment has been minimized.

Copy link
Contributor

commented May 24, 2014

I agree it would be benefitial to have this as an optional trait.

@GrahamCampbell

This comment has been minimized.

Copy link
Member

commented May 24, 2014

👍

@laurencei

This comment has been minimized.

Copy link
Member

commented May 24, 2014

The only thing is where the rules themselves are saved. i.e. I dont want to store my validation rules inside my model. There are times I need to validate something (i.e. a form) but not always in the process of 'saving' to a model (such as an ajax request to check validation without actually saving it).

So the way I've extended Laravel is I have a Validator Service that is 'abstract'. It has a validate($rules) input. Then I just extend the validator service for each set of domain logic rules (i.e. UserValidator, PostValidator). In the extended class I just do something like this

    public $register  = [
        'email' => 'required|email|max:255|unique:users,email',
        'name' => 'required|max:60|min:2',
        'password' => 'required|max:60|min:6'
        ];

    public $change_password = [
        'password' => 'required|max:60|min:6|confirmed',
        'current_password' => 'required'
    ];

And just define as many sets of rules as I like. Then all my Model does is on the "saving()" event, calls its own validator service, and validates against the rule provided.

    $user->rules('change_password')->save();

But best of all - I can inject my UserValidator elsewhere - like on a form unrelated to saving to the model.

@leemason

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2014

I'm working on a ardent esque model class that validates, however I've built in "scopes" for validation.

so the $rules array contains 'global', 'create', and 'update' rules. as implied the global and current scope rules are merged during validation, and just like ardent model->save() runs validation and returns true/false. then there is also the model->validates() function which does the same without save.

its not just validation, it adds custom messages per scope to the validation errors too, and allows you to define data types for the model (allowing array/object values to be automatically serialised on save and unserialise on access), and auto hashes password values via data type if set.

this basically creates setAttribute and get*Attribute functions through the __call method, so not nesassery, but would be really nice as a trait.

it currently "works" but isn't pretty and needs a bit more dev to be production ready.

maybe it would help with the dev of adding this to the core?

it was build with php5.3 in mind, but as this would be a laravel 4.2 thing, it may make more sense to implement as a trait as suggested above:

class SomeModel extends Eloquent{
   use EloquentValidation;
   use EloquentPasswordHashing;
   use EloquentSerializer;
   $rules = [
      'global' => [],
      'create' => [],
      'update' => [],
   ];
   $hashing = ['password','another_columns'];
   $serialize = ['column_value_is_array','column_value_is_object'];
}

one thing to note with validation is the rules confirmed and unique.

unique is pretty easy as you can just modify the rules to include the id if exists (my model and ardent cover this pretty easy).

confirmed however adds attributes to the model, which unless removed before inserting to the db causes pro exceptions.

maybe globally eloquent should remove all **_confirmation attributes before save, regardless of if it uses the validation trait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.