Skip to content

Commit

Permalink
Avoid running validation again, when saveAll() is called with 'valida…
Browse files Browse the repository at this point in the history 
…te' => 'first'
  • Loading branch information
@ceeram
ceeram committed Apr 16, 2012
1 parent d136fac commit 9556ff7
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/Cake/Model/Model.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2044,7 +2044,7 @@ public function saveMany($data = null, $options = array()) {
if ((!$validates && $options['atomic']) || (!$options['atomic'] && in_array(false, $validates, true))) {
return $validates;
}
$options['validate'] = true;
$options['validate'] = false;
}

if ($options['atomic']) {
Expand Down Expand Up @@ -2175,7 +2175,7 @@ public function saveAssociated($data = null, $options = array()) {
if ((!$validates && $options['atomic']) || (!$options['atomic'] && in_array(false, $validates, true))) {
return $validates;
}
$options['validate'] = true;
$options['validate'] = false;
}
if ($options['atomic']) {
$db = $this->getDataSource();
Expand Down

5 comments on commit 9556ff7

@teddyzeenny
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reduces redundancy, but causes the save to skip all beforeValidate callbacks... any data changed during these callbacks no longer applies to the actual save.

@ceeram
Copy link
Contributor Author

@ceeram ceeram commented on 9556ff7 May 4, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo you shouldnt use 'validate' first option then, but true instead
I feel that it only used to work because it incorrectly ran validation twice

@teddyzeenny
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, but this will break many applications though, as it did mine :)
How about we make it call beforeValidate but skip the validation? You think that would be a good idea ?

Another option would be to fetch the new data from the model (after the initial validation) and save that... I don't know if i just made sense.

@cvandeplas
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change broke my application because the behavior is not as expected from the documentation. (before validation there is always the beforeValidate executed)

I reported a bug here: http://cakephp.lighthouseapp.com/projects/42648-cakephp/tickets/2893

@ceeram
Copy link
Contributor Author

@ceeram ceeram commented on 9556ff7 May 21, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been fixed in 2.1 dev branch and will be in next release 2.1.3

[Edit:clarification]
Before this change, validation was run twice, now just once. beforeValidate is still being executed, but if data is altered in the callback those changes are lost. The commit(s) that fixes the loss of changed data will be in the 2.1.3 release.

Please sign in to comment.