Feature Request #1864

ORM Validate() is backwards

Added by Ben Rogers about 5 years ago. Updated almost 5 years ago.

Status:ClosedStart date:07/23/2009
Priority:NormalDue date:
Assignee:Ben Rogers% Done:

100%

Category:Libraries:ORM
Target version:2.4
Resolution:fixed Points:

Description

the purpose of validation in the model is that it validates the model. The way ORM validate() currently works, is an array of savable data(generally through $_POST) is passed to it, which is then set to the model, and then checked for validation.

These should be two separate steps, though it probably couldn't hurt to have a ORM helper function to set all array key/value pairs to a model, something like a set_fields();

so ideally, a person wishing to validate $_POST through his ORM model would:

either loop through post and do $model->$key = $value; or something like $model->set_fields($_POST);

then do $model->validate(true);//passing true to save automatically if post validates.

i'd love to get opinions on this.


Related issues

Related to Kohana v2.x - Bug Report #2070: ORM find() marks all columns as changed. Closed 09/06/2009 09/06/2009
Related to Auth Module for v2.x - Bug Report #2071: validate() has changed in the 2.4 branch. Closed 09/05/2009
Duplicated by Kohana v2.x - Bug Report #1815: ORM Validate() shouldnt staticly type to Validation, caus... Closed 07/02/2009

Associated revisions

Revision 4536
Added by Ben Rogers almost 5 years ago

fixing #1864. also fixing files w/o an SVN keyword property set

Revision 4560
Added by Kiall Mac Innes almost 5 years ago

refs #2070 refs #1864 - __sleep() updated for the rename of loaded and saved to _loaded and _saved

History

#1 Updated by Jeremy Bush about 5 years ago

  • Status changed from New to Review

This is how it works in my Auto Modeler, and it make much more sense.

#2 Updated by Isaiah DeRose-Wilson about 5 years ago

We can already use ORM::load_values() for setting all the fields key/value pairs to a model. What if we made the first argument in the validate function optional, so you could still set the values, validate and save all in one easy function call if you wanted?

#3 Updated by John Heathco about 5 years ago

I like Isaiah's idea. Also, I think the validation object should be more tightly integrated with ORM, perhaps a property of each ORM. It already sounds like this is what you guys are thinking.

It would be nice if the validate() method in ORM would use the validate object that each ORM would possess, or an optional argument could be passed to override and specify your own validate object.

#4 Updated by Ben Rogers about 5 years ago

i'm not opposed to the optional validate() parameter which would be passed to load_values(by the way thanks for finding that isaiah, i was sure something existed like that, just couldnt find it), however i think that it doesnt make sense for the scope of the validate() function. If were changing this so validation occurs to the model, and not the posted data, it makes less readable sense to the programmer that this is whats happening if we are still passing $_POST or a similar array to the validate() function.

besides, it's not that hard to do a $model->load_values($_POST); $model->validate();

#5 Updated by Jeremy Bush about 5 years ago

John Heathco wrote:

I like Isaiah's idea. Also, I think the validation object should be more tightly integrated with ORM, perhaps a property of each ORM. It already sounds like this is what you guys are thinking.

I'd suggest looking at Auto_Modeler validation, as this is exactly how it works.

http://dev.kohanaphp.com/projects/automodeler/repository/entry/trunk/libraries/Auto_Modeler.php#L106

#6 Updated by John Heathco about 5 years ago

On an unrelated note, I think saved and loaded should become methods so that these can be used as database field names, no?

#7 Updated by Chris Bandy about 5 years ago

Another feature/functionality to keep in mind is filtering unwanted $_POST/input data. ORM::validate depends on defined rules and safe_array() while AM hardcodes column names.

#8 Updated by Ben Rogers about 5 years ago

@jheathco: agreed on the loaded/saved becoming methods

@cbandy: this is already solved if you use $model->load_values(), which is the point of this ticket, as that will ignore $_POST fields not found in the model.

#9 Updated by Chris Bandy about 5 years ago

That is not currently the case, which is why I bring it up.

source:branches/2.4/system/libraries/ORM.php@4474#L1314

#10 Updated by Martin Rampersad about 5 years ago

load_values doesn't set the $changed member in ORM, which I think save looks at to decide how to form the update query. perhaps that's a bug?

#11 Updated by Ben Rogers almost 5 years ago

  • Assignee changed from John Heathco to Ben Rogers

#12 Updated by Ben Rogers almost 5 years ago

  • Status changed from Review to Assigned

#13 Updated by Ben Rogers almost 5 years ago

just confirming the changes to be made in this ticket, much discussion occured in #kohana-dev between myself and zombor on the topic:

  • loaded/saved to become functions(allowing db fields to be named loaded/saved
  • validate() to have one optional parameter, a validation object to be passed instead of creating its own
  • load_values() needs to be fixed so it updates ORM::$changed
  • the appropriate way to pass $_POST to validate() is via load_values(). the appropriate way to save after validate() is save()
  • save will call validate if the unchanged record was not yet validated.
  • validate() will throw an exception(Database_Validation_Exception) if the model doesnt pass validation
  • user will no longer overload validate() but fill in validation rules to an array in their models

#14 Updated by Bruno Heridet - Delapouite almost 5 years ago

Really good news to port the Auto_Modeler validation philosophy to ORM.

#15 Updated by Ben Rogers almost 5 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

Applied in changeset r4536.

#16 Updated by Ben Rogers almost 5 years ago

  • Resolution set to fixed

Also available in: Atom PDF