Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

pass all the options of save also to validates #563

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

hans-d commented Jul 7, 2012

when having filters on validates it allows for options to be passed straight from entity->save()

Contributor

hans-d commented Jul 8, 2012

and allows for a validates filter to see if it was called from save or not eg by passing in some extra options

@nateabele nateabele commented on an outdated diff Jul 24, 2012

data/Model.php
@@ -816,7 +818,8 @@ public function save($entity, $data = null, array $options = array()) {
}
if ($rules = $options['validate']) {
$events = $options['events'];
- $validateOpts = is_array($rules) ? compact('rules','events') : compact('events');
+ $validateOpts = is_array($rules) ? compact('rules') : array();
+ $validateOpts += $options['validatesOptions'];
@nateabele

nateabele Jul 24, 2012

Owner

Okay, this looks good to me. Just a few notes:

  • I think 'validationOptions' works better as a key name, as it reads more fluently, even though it's more verbose
  • It looks like you removed events from the validation options — it should still get merged in (this is what lets you do 'on' => 'create' and 'on' => 'update' in validation rules)
  • When you're done, please squash this commit with the other two, and I'll get it merged

Thanks!

Contributor

hans-d commented Jul 24, 2012

adjusted events so that it could be passed the original way, or be passed via validateOptions.

@nateabele nateabele commented on the diff Jul 28, 2012

data/Model.php
@@ -816,7 +819,9 @@ public function save($entity, $data = null, array $options = array()) {
}
if ($rules = $options['validate']) {
$events = $options['events'];
- $validateOpts = is_array($rules) ? compact('rules','events') : compact('events');
+ $validateOpts = is_array($rules) ? compact('rules') : array();
+ $validateOpts += $events ? compact('events') : array();
@nateabele

nateabele Jul 28, 2012

Owner

Okay, last thing, I promise. :-) This is a really minor nitpick, but since $events will never be empty, it don't think it makes sense to have a ternary for it. Functionally, I don't see how these two lines of code are any different from the one line you're replacing.

Unless there's a reason for it which I'm not seeing, I'd say revert the two lines above. Then squash your commits and we'll get this merged in finally. :-P Thanks again for your work on this.

@nateabele nateabele closed this Jun 20, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment