[RFC] Improve compatibility with original Eloquent methods

[igorsantos07]

There are some changes in Ardent that made some methods incompatible or behaving differently than their original counterparts. Here are some examples that should be fixed in a new major version to make Ardent easier for newcomers:

• save receives custom rules and messages, plus custom hooks, besides the original options, but in a different order. This makes Eloquent code non-portable into Ardent.
• Although update operations are used in a similar manner by the end-user, this is the original Eloquent method. I'm not sure if it calls Ardent Validations, but I think it should work similarly as save does.
• Setting fields in different ways yield different behaviors:
  • setting fields directly in the $attributes param does not call mutators. Is this expected? Is this a bug in Eloquent?
  • setting a new field with $this->field = 'bla' inside a mutator does not validate it, but using $this->attributes['field'] = 'bla' apparently does.
  • setting a field as a new property in an object does not validate it upon update call ($user->data = 'bla'; $user->update();)
• there is updateUniques, that have the same signature as save but a different name pattern (it is prefixed). This also creates confusion since a user would expect update and save to call validations, but update don't and updateUniques looks related only to unique rules, what's not only the case.
• validation rules should be automatically fixed for update calls, removing validation for uniques. This is currently done inside the updateUniques method, by an auxiliary method.

Suggestions:

• save and update should follow Eloquent's signatures and always call validation on the final data before writing to the database. This should be accomplished using a decent test structure (Test Coverage #55), so we make sure no use case is forgotten.
• validate should verify if it's a new or existent entry (save/update call) and fix validation for uniques accordingly and transparently
• creation of other methods to handle custom options:
  1. customSave and customUpdate, that could have the same signature of custom rules, messages and hooks, OR
  2. setCustomValidation that would set rules and messages for the next validate call, and setCustomSaveHooks that would do the same for the hooks. This is also related to What's the usage of before and afterSave as arguments in save method? #74 since those hooks have a similar usage of plain code before and after the method calls.

[igorsantos07]

@nightowl77 I would be glad to have your opinions on this (:

Those are part of the ideas I had that I mentioned on #79, along with issue #83

[nightowl77]

Hey Igor.

I like what you say very very much. With the changes you suggest it would make plugging Ardent into an existing Eloquent codebase quite easy. So +1 from me for making it follow Eloquent's function declaration. It makes writing docs easier as well - just point people to the main Eloquent docs.

_Setting of Attributes_

Regarding the setting of attributes, I might not understand you correctly, but have you tried calling Eloquent's setAttribute? You mention something about calling $this->attribute inside a mutator. I see setAttribute (which will/should get called even if you call $this->attribute) calls hasMutator http://laravel.com/api/source-class-Illuminate.Database.Eloquent.Model.html#1930-1961 which could cause a loop. But I'd have to see more of your code to fully understand it.

_Eloquent->update()_

This might be quite useful. You would have to run validation once against the array passed to update and return the moment validation fails. You could then call it like this

$affectedRows = User::where('votes', '>', 100)->update(array('status' => 2));

if ($errors = $user->errors->all())
   return Redirect....with... errors

One suggestion is to break Ardent's validation function up into more functions. That one function is doing a bit much and parts of it cannot be re-used as it currently stands. With validate() split up a bit more it would be quite easy to bring auto validation magic to update() as well.

_Custom Rules_

I like setCustomValidation more because I like that $model->save is always your main save function, no matter what. For me that's part of the elegance of Eloquent. The other side of the coin is of course making rules non-static. There could be a reason for the statics (ie, you want to call your model without instantiating an object), but by they time you are mangling rules I'm pretty sure you'd have an object instantiated already. Maybe I missed a VERY important tutorial on why $rules must be static, and I'd love to see it. Personally, if I want to change the rules at runtime I'd just like to go

// Super protective password for admin
if ($user->isAdmin())
    $user->rules['password'] = 'required|min:30' ; 
}
$user->save();

I don't see a reason for an extra function. But yes, these are just comments in response to your RFC. I think it might break a lot of backwards compatibility and a lot of people using "dev-master" in their composer.json file would jump up and down. Sigh... if only they'd put that one semver.org link in the Laravel docs. But is seems nobody cares. Oh well. So be it.

[ronlobo]

Just want to throw in:
http://www.ange-agostini.com/blog/non-classe/laravel-ignore-current-record-when-using-unique-rule-with-ardent.html

As a quick fix in your update method modify the rules of your model, e.g.:

$project = Project::find($id);
Project::$rules['name'] = Project::$rules['name'] . ',' . $project->id;

[igorsantos07]

I think that's what the code does currently, @synlag, but in a generic way.

I'll review this soon (hopefully today if I can get my computer fixed)
On Aug 12, 2013 9:05 AM, "synlag" notifications@github.com wrote:

Just want to throw in:

http://www.ange-agostini.com/blog/non-classe/laravel-ignore-current-record-when-using-unique-rule-with-ardent.html

As a quick fix in your update method modify the rules of your model, e.g.:

$project = Project::find($id);
Project::$rules['name'] = Project::$rules['name'] . ',' . $project->id;

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/84#issuecomment-22488515
.

[Weeblewonder]

+1 for automatically sorting out rules for unique fields, having to do this manually a massive break in flow with how ardent should be used I think (in the "Ardent just take care of everything please" sense).

I have had to add a method to my models to call buildUniqueExclusionRules() as using repository designs for interacting with my models calling it like $this->model->buildUniqueExclusionRules() fails as it tries to call it on Builder for some reason (I dont know the anatomy behind the scenes well enough to figure out why, this is after the model has been populated from the db so its not a case of being a blank model).

Then needing to pass the, modified for uniques, rules to validation is a pain again just breaking the flow.

So all my +1's :)

[igorsantos07]

Hey! I'm working on this now. Should be ready in the following week (just a
guess).
While this, I think you can use the method updateUniques() (or something
similar, I don't remember exactly). It calls the method that clears unique
rules and then updates.
On Aug 15, 2013 7:33 PM, "Weeblewonder" notifications@github.com wrote:

+1 for automatically sorting out rules for unique fields, having to do
this manually a massive break in flow with how ardent should be used I
think (in the "Ardent just take care of everything please" sense).

I have had to add a method to my models to call
buildUniqueExclusionRules() as using repository designs for interacting
with my models calling it like $this->model->buildUniqueExclusionRules()
fails as it tries to call it on Builder for some reason (I dont know the
anatomy behind the scenes well enough to figure out why, this is after the
model has been populated from the db so its not a case of being a blank
model).

Then needing to pass the, modified for uniques, rules to validation is a
pain again just breaking the flow.

So all my +1's :)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/84#issuecomment-22735789
.

[Weeblewonder]

I call validation on its own a lot of the time before running any updates so for now I've just overriden the validate() method in my models and called buildUniqueExclusionRules() and passed that to parent::validate().

This way after its implemented to do it automatically in validate() all I should have to do is remove my override and the app shouldnt be disturbed :) hopefully :P

[andrew13]

Any word on the progress of this? And/or do you need another set of eyes on the changes?

[igorsantos07]

Needing time!
I've moved to my home country, took a month of vacations and now I'm
searching for a job + fixing SOs in phone and computer and whatnot.

If you're really willing to help, I need some hands with tests, but I'm not
sure about the issue number. They'll be essential for this issue to be
properly done.
On Oct 6, 2013 3:22 AM, "Andrew" notifications@github.com wrote:

Any word on the progress of this? And/or do you need another set of eyes
on the changes?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/84#issuecomment-25763085
.

[andrew13]

I can take a look at creating tests.

[tortuetorche]

@andrew13 Maybe you can start with this example: https://github.com/JeffreyWay/Laravel-Model-Validation/blob/master/tests/Database/ModelTest.php
Just my 2 cents...