Skip to content

Commit

Permalink
Fixing error with validateMany and validateAssociated not saving valu…
Browse files Browse the repository at this point in the history 
…es altered in beforeValidate callbacks
  • Loading branch information
@lorenzo
lorenzo committed May 10, 2012
1 parent a4bbfc0 commit 0fb025f
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 55 deletions.
12 changes: 7 additions & 5 deletions lib/Cake/Model/Model.php
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -2107,14 +2107,15 @@ public function saveMany($data = null, $options = array()) {
* Otherwise: array similar to the $data array passed, but values are set to true/false * Otherwise: array similar to the $data array passed, but values are set to true/false
* depending on whether each record validated successfully. * depending on whether each record validated successfully.
*/ */
public function validateMany($data, $options = array()) { public function validateMany(&$data, $options = array()) {
$options = array_merge(array('atomic' => true, 'deep' => false), $options); $options = array_merge(array('atomic' => true, 'deep' => false), $options);
$this->validationErrors = $validationErrors = $return = array(); $this->validationErrors = $validationErrors = $return = array();
foreach ($data as $key => $record) { foreach ($data as $key => &$record) {
if ($options['deep']) { if ($options['deep']) {
$validates = $this->validateAssociated($record, $options); $validates = $this->validateAssociated($record, $options);
} else { } else {
$validates = $this->create($record) && $this->validates($options); $validates = $this->create($record) && $this->validates($options);
$data[$key] = $this->data;
} }
if ($validates === false || (is_array($validates) && in_array(false, $validates, true))) { if ($validates === false || (is_array($validates) && in_array(false, $validates, true))) {
$validationErrors[$key] = $this->validationErrors; $validationErrors[$key] = $this->validationErrors;
Expand Down Expand Up @@ -2178,7 +2179,6 @@ public function saveAssociated($data = null, $options = array()) {


if ($options['validate'] === 'first') { if ($options['validate'] === 'first') {
$validates = $this->validateAssociated($data, $options); $validates = $this->validateAssociated($data, $options);
$data = $this->data;
if ((!$validates && $options['atomic']) || (!$options['atomic'] && in_array(false, $validates, true))) { if ((!$validates && $options['atomic']) || (!$options['atomic'] && in_array(false, $validates, true))) {
return $validates; return $validates;
} }
Expand All @@ -2188,6 +2188,7 @@ public function saveAssociated($data = null, $options = array()) {
$db = $this->getDataSource(); $db = $this->getDataSource();
$transactionBegun = $db->begin(); $transactionBegun = $db->begin();
} }

$associations = $this->getAssociated(); $associations = $this->getAssociated();
$return = array(); $return = array();
$validates = true; $validates = true;
Expand Down Expand Up @@ -2306,7 +2307,7 @@ public function saveAssociated($data = null, $options = array()) {
* Otherwise: array similar to the $data array passed, but values are set to true/false * Otherwise: array similar to the $data array passed, but values are set to true/false
* depending on whether each record validated successfully. * depending on whether each record validated successfully.
*/ */
public function validateAssociated($data, $options = array()) { public function validateAssociated(&$data, $options = array()) {
$options = array_merge(array('atomic' => true, 'deep' => false), $options); $options = array_merge(array('atomic' => true, 'deep' => false), $options);
$this->validationErrors = $validationErrors = $return = array(); $this->validationErrors = $validationErrors = $return = array();
if (!($this->create($data) && $this->validates($options))) { if (!($this->create($data) && $this->validates($options))) {
Expand All @@ -2315,8 +2316,9 @@ public function validateAssociated($data, $options = array()) {
} else { } else {
$return[$this->alias] = true; $return[$this->alias] = true;
} }
$data[$this->alias] = $this->data[$this->alias];
$associations = $this->getAssociated(); $associations = $this->getAssociated();
foreach ($data as $association => $values) { foreach ($data as $association => &$values) {
$validates = true; $validates = true;
if (isset($associations[$association])) { if (isset($associations[$association])) {
if (in_array($associations[$association], array('belongsTo', 'hasOne'))) { if (in_array($associations[$association], array('belongsTo', 'hasOne'))) {
Expand Down
81 changes: 80 additions & 1 deletion lib/Cake/Test/Case/Model/ModelValidationTest.php
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -991,4 +991,83 @@ public function testOnRequiredConflictValidation() {
$this->assertFalse($Article->validates()); $this->assertFalse($Article->validates());
} }


} /**
* Tests that altering data in a beforeValidate callback will lead to saving those
* values in database
*
* @return void
*/
public function testValidateFirstWithBeforeValidate() {
$this->loadFixtures('Article', 'User');
$model = new CustomArticle();
$model->validate = array(
'title' => array(
'notempty' => array(
'rule' => 'notEmpty',
'required' => true,
'allowEmpty' => false
)
)
);
$data = array(
'CustomArticle' => array(
'body' => 'foo0'
)
);
$result = $model->saveAll($data, array('validate' => 'first'));
$this->assertTrue($result);

$title = $model->field('title', array('body' => 'foo0'));
$this->assertEquals('foo', $title);

$data = array(
array('body' => 'foo1'),
array('body' => 'foo2'),
array('body' => 'foo3')
);

$result = $model->saveAll($data, array('validate' => 'first'));
$this->assertTrue($result);

$this->assertEquals('foo', $model->field('title', array('body' => 'foo1')));
$this->assertEquals('foo', $model->field('title', array('body' => 'foo2')));
$this->assertEquals('foo', $model->field('title', array('body' => 'foo3')));
}

/**
* Tests that altering data in a beforeValidate callback will lead to saving those
* values in database
*
* @return void
*/
public function testValidateAssociatedWithBeforeValidate() {
$this->loadFixtures('Article', 'User');
$model = new CustomArticle();
$model->validate = array(
'title' => array(
'notempty' => array(
'rule' => 'notEmpty',
'required' => true
)
)
);
$articles = array(
array('body' => 'foo1'),
array('body' => 'foo2'),
array('body' => 'foo3')
);
$user = new User();
$user->hasMany['CustomArticle'] = array('foreignKey' => 'user_id');
$data = array(
'User' => array('user' => 'foo', 'password' => 'bar'),
'CustomArticle' => $articles
);
$result = $user->saveAll($data, array('validate' => 'first'));
$this->assertTrue($result);

$this->assertEquals('foo', $model->field('title', array('body' => 'foo1')));
$this->assertEquals('foo', $model->field('title', array('body' => 'foo2')));
$this->assertEquals('foo', $model->field('title', array('body' => 'foo3')));
}

}
97 changes: 48 additions & 49 deletions lib/Cake/Test/Case/Model/ModelWriteTest.php
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -4277,7 +4277,7 @@ public function testSaveAllValidation() {
'author_id' => '3', 'author_id' => '3',
'title' => 'Just update the title', 'title' => 'Just update the title',
'body' => 'Second Post Body', 'body' => 'Second Post Body',
'published' => 'Y', 'published' => 'N',
'created' => '2007-03-18 10:41:23' 'created' => '2007-03-18 10:41:23'
)), )),
array( array(
Expand Down Expand Up @@ -4366,7 +4366,7 @@ public function testSaveAllValidation() {
'author_id' => '3', 'author_id' => '3',
'title' => 'Just update the title', 'title' => 'Just update the title',
'body' => 'Second Post Body', 'body' => 'Second Post Body',
'published' => 'Y', 'published' => 'N',
'created' => '2007-03-18 10:41:23' 'created' => '2007-03-18 10:41:23'
) )
), ),
Expand Down Expand Up @@ -5631,7 +5631,7 @@ public function testSaveManyValidation() {
'author_id' => '3', 'author_id' => '3',
'title' => 'Just update the title', 'title' => 'Just update the title',
'body' => 'Second Post Body', 'body' => 'Second Post Body',
'published' => 'Y', 'published' => 'N',
'created' => '2007-03-18 10:41:23' 'created' => '2007-03-18 10:41:23'
) )
), ),
Expand Down Expand Up @@ -5727,7 +5727,7 @@ public function testSaveManyValidation() {
'author_id' => '3', 'author_id' => '3',
'title' => 'Just update the title', 'title' => 'Just update the title',
'body' => 'Second Post Body', 'body' => 'Second Post Body',
'published' => 'Y', 'published' => 'N',
)), )),
array( array(
'Post' => array( 'Post' => array(
Expand Down Expand Up @@ -5779,24 +5779,24 @@ public function testSaveManyValidation() {
public function testValidateMany() { public function testValidateMany() {
$TestModel = new Article(); $TestModel = new Article();
$TestModel->validate = array('title' => 'notEmpty'); $TestModel->validate = array('title' => 'notEmpty');
$result = $TestModel->validateMany( $data = array(
array(
0 => array('title' => ''), 0 => array('title' => ''),
1 => array('title' => 'title 1'), 1 => array('title' => 'title 1'),
2 => array('title' => 'title 2'), 2 => array('title' => 'title 2'),
)); );
$result = $TestModel->validateMany($data);
$this->assertFalse($result); $this->assertFalse($result);
$expected = array( $expected = array(
0 => array('title' => array('This field cannot be left blank')), 0 => array('title' => array('This field cannot be left blank')),
); );
$this->assertEquals($expected, $TestModel->validationErrors); $this->assertEquals($expected, $TestModel->validationErrors);


$result = $TestModel->validateMany( $data = array(
array(
0 => array('title' => 'title 0'), 0 => array('title' => 'title 0'),
1 => array('title' => ''), 1 => array('title' => ''),
2 => array('title' => 'title 2'), 2 => array('title' => 'title 2'),
)); );
$result = $TestModel->validateMany($data);
$this->assertFalse($result); $this->assertFalse($result);
$expected = array( $expected = array(
1 => array('title' => array('This field cannot be left blank')), 1 => array('title' => array('This field cannot be left blank')),
Expand Down Expand Up @@ -5966,47 +5966,46 @@ public function testValidateAssociated() {
$TestModel->belongsTo = $TestModel->hasAndBelongsToMany = array(); $TestModel->belongsTo = $TestModel->hasAndBelongsToMany = array();
$TestModel->Comment->validate = array('comment' => 'notEmpty'); $TestModel->Comment->validate = array('comment' => 'notEmpty');


$result = $TestModel->validateAssociated( $data = array(
array( 'Article' => array('id' => 2),
'Article' => array('id' => 2), 'Comment' => array(
'Comment' => array( array(
array( 'id' => 1,
'id' => 1, 'comment' => '',
'comment' => '', 'published' => 'Y',
'published' => 'Y', 'user_id' => 1),
'user_id' => 1), array(
array( 'id' => 2,
'id' => 2, 'comment' =>
'comment' => 'comment',
'comment', 'published' => 'Y',
'published' => 'Y', 'user_id' => 1
'user_id' => 1 )));
)))); $result = $TestModel->validateAssociated($data);
$this->assertFalse($result); $this->assertFalse($result);


$result = $TestModel->validateAssociated( $data = array(
array( 'Article' => array('id' => 2),
'Article' => array('id' => 2), 'Comment' => array(
'Comment' => array( array(
array( 'id' => 1,
'id' => 1, 'comment' => '',
'comment' => '', 'published' => 'Y',
'published' => 'Y', 'user_id' => 1
'user_id' => 1 ),
), array(
array( 'id' => 2,
'id' => 2, 'comment' => 'comment',
'comment' => 'comment', 'published' => 'Y',
'published' => 'Y', 'user_id' => 1
'user_id' => 1 ),
), array(
array( 'id' => 3,
'id' => 3, 'comment' => '',
'comment' => '', 'published' => 'Y',
'published' => 'Y', 'user_id' => 1
'user_id' => 1 )));
))), $result = $TestModel->validateAssociated($data, array(
array(
'atomic' => false 'atomic' => false
)); ));
$expected = array( $expected = array(
Expand Down
16 changes: 16 additions & 0 deletions lib/Cake/Test/Case/Model/models.php
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -4948,6 +4948,13 @@ class CustomArticle extends AppModel {
*/ */
public $findMethods = array('unPublished' => true); public $findMethods = array('unPublished' => true);


/**
* belongsTo property
*
* @var array
*/
public $belongsTo = array('User');

/** /**
* _findUnPublished custom find * _findUnPublished custom find
* *
Expand All @@ -4961,4 +4968,13 @@ protected function _findUnPublished($state, $query, $results = array()) {
return $results; return $results;
} }


/**
* Alters title data
*
* @return void
**/
public function beforeValidate() {
$this->data[$this->alias]['title'] = 'foo';
}

} }

1 comment on commit 0fb025f

@ceeram
Copy link
Contributor

@ceeram ceeram commented on 0fb025f Oct 2, 2012

Choose a reason for hiding this comment

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

This commit introduced a new bug, the hasMany data in beforeSave is in different format, here is a test that shows this:
https://gist.github.com/3823928

Please sign in to comment.