From 0fb025f6dc7192f00e7af7db595b5b2cdb0fd93a Mon Sep 17 00:00:00 2001 From: Jose Lorenzo Rodriguez Date: Wed, 9 May 2012 23:51:27 -0430 Subject: [PATCH] Fixing error with validateMany and validateAssociated not saving values altered in beforeValidate callbacks --- lib/Cake/Model/Model.php | 12 ++- .../Test/Case/Model/ModelValidationTest.php | 81 +++++++++++++++- lib/Cake/Test/Case/Model/ModelWriteTest.php | 97 +++++++++---------- lib/Cake/Test/Case/Model/models.php | 16 +++ 4 files changed, 151 insertions(+), 55 deletions(-) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index 235ae7ff145..c527bbd2921 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -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 * 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); $this->validationErrors = $validationErrors = $return = array(); - foreach ($data as $key => $record) { + foreach ($data as $key => &$record) { if ($options['deep']) { $validates = $this->validateAssociated($record, $options); } else { $validates = $this->create($record) && $this->validates($options); + $data[$key] = $this->data; } if ($validates === false || (is_array($validates) && in_array(false, $validates, true))) { $validationErrors[$key] = $this->validationErrors; @@ -2178,7 +2179,6 @@ public function saveAssociated($data = null, $options = array()) { if ($options['validate'] === 'first') { $validates = $this->validateAssociated($data, $options); - $data = $this->data; if ((!$validates && $options['atomic']) || (!$options['atomic'] && in_array(false, $validates, true))) { return $validates; } @@ -2188,6 +2188,7 @@ public function saveAssociated($data = null, $options = array()) { $db = $this->getDataSource(); $transactionBegun = $db->begin(); } + $associations = $this->getAssociated(); $return = array(); $validates = true; @@ -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 * 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); $this->validationErrors = $validationErrors = $return = array(); if (!($this->create($data) && $this->validates($options))) { @@ -2315,8 +2316,9 @@ public function validateAssociated($data, $options = array()) { } else { $return[$this->alias] = true; } + $data[$this->alias] = $this->data[$this->alias]; $associations = $this->getAssociated(); - foreach ($data as $association => $values) { + foreach ($data as $association => &$values) { $validates = true; if (isset($associations[$association])) { if (in_array($associations[$association], array('belongsTo', 'hasOne'))) { diff --git a/lib/Cake/Test/Case/Model/ModelValidationTest.php b/lib/Cake/Test/Case/Model/ModelValidationTest.php index 451224e615c..6b6accf96d3 100644 --- a/lib/Cake/Test/Case/Model/ModelValidationTest.php +++ b/lib/Cake/Test/Case/Model/ModelValidationTest.php @@ -991,4 +991,83 @@ public function testOnRequiredConflictValidation() { $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'))); + } + +} \ No newline at end of file diff --git a/lib/Cake/Test/Case/Model/ModelWriteTest.php b/lib/Cake/Test/Case/Model/ModelWriteTest.php index f931b7ca391..f72b2a8bc6d 100644 --- a/lib/Cake/Test/Case/Model/ModelWriteTest.php +++ b/lib/Cake/Test/Case/Model/ModelWriteTest.php @@ -4277,7 +4277,7 @@ public function testSaveAllValidation() { 'author_id' => '3', 'title' => 'Just update the title', 'body' => 'Second Post Body', - 'published' => 'Y', + 'published' => 'N', 'created' => '2007-03-18 10:41:23' )), array( @@ -4366,7 +4366,7 @@ public function testSaveAllValidation() { 'author_id' => '3', 'title' => 'Just update the title', 'body' => 'Second Post Body', - 'published' => 'Y', + 'published' => 'N', 'created' => '2007-03-18 10:41:23' ) ), @@ -5631,7 +5631,7 @@ public function testSaveManyValidation() { 'author_id' => '3', 'title' => 'Just update the title', 'body' => 'Second Post Body', - 'published' => 'Y', + 'published' => 'N', 'created' => '2007-03-18 10:41:23' ) ), @@ -5727,7 +5727,7 @@ public function testSaveManyValidation() { 'author_id' => '3', 'title' => 'Just update the title', 'body' => 'Second Post Body', - 'published' => 'Y', + 'published' => 'N', )), array( 'Post' => array( @@ -5779,24 +5779,24 @@ public function testSaveManyValidation() { public function testValidateMany() { $TestModel = new Article(); $TestModel->validate = array('title' => 'notEmpty'); - $result = $TestModel->validateMany( - array( + $data = array( 0 => array('title' => ''), 1 => array('title' => 'title 1'), 2 => array('title' => 'title 2'), - )); + ); + $result = $TestModel->validateMany($data); $this->assertFalse($result); $expected = array( 0 => array('title' => array('This field cannot be left blank')), ); $this->assertEquals($expected, $TestModel->validationErrors); - $result = $TestModel->validateMany( - array( + $data = array( 0 => array('title' => 'title 0'), 1 => array('title' => ''), 2 => array('title' => 'title 2'), - )); + ); + $result = $TestModel->validateMany($data); $this->assertFalse($result); $expected = array( 1 => array('title' => array('This field cannot be left blank')), @@ -5966,47 +5966,46 @@ public function testValidateAssociated() { $TestModel->belongsTo = $TestModel->hasAndBelongsToMany = array(); $TestModel->Comment->validate = array('comment' => 'notEmpty'); - $result = $TestModel->validateAssociated( - array( - 'Article' => array('id' => 2), - 'Comment' => array( - array( - 'id' => 1, - 'comment' => '', - 'published' => 'Y', - 'user_id' => 1), - array( - 'id' => 2, - 'comment' => - 'comment', - 'published' => 'Y', - 'user_id' => 1 - )))); + $data = array( + 'Article' => array('id' => 2), + 'Comment' => array( + array( + 'id' => 1, + 'comment' => '', + 'published' => 'Y', + 'user_id' => 1), + array( + 'id' => 2, + 'comment' => + 'comment', + 'published' => 'Y', + 'user_id' => 1 + ))); + $result = $TestModel->validateAssociated($data); $this->assertFalse($result); - $result = $TestModel->validateAssociated( - array( - 'Article' => array('id' => 2), - 'Comment' => array( - array( - 'id' => 1, - 'comment' => '', - 'published' => 'Y', - 'user_id' => 1 - ), - array( - 'id' => 2, - 'comment' => 'comment', - 'published' => 'Y', - 'user_id' => 1 - ), - array( - 'id' => 3, - 'comment' => '', - 'published' => 'Y', - 'user_id' => 1 - ))), - array( + $data = array( + 'Article' => array('id' => 2), + 'Comment' => array( + array( + 'id' => 1, + 'comment' => '', + 'published' => 'Y', + 'user_id' => 1 + ), + array( + 'id' => 2, + 'comment' => 'comment', + 'published' => 'Y', + 'user_id' => 1 + ), + array( + 'id' => 3, + 'comment' => '', + 'published' => 'Y', + 'user_id' => 1 + ))); + $result = $TestModel->validateAssociated($data, array( 'atomic' => false )); $expected = array( diff --git a/lib/Cake/Test/Case/Model/models.php b/lib/Cake/Test/Case/Model/models.php index 43bad312327..41138e15544 100644 --- a/lib/Cake/Test/Case/Model/models.php +++ b/lib/Cake/Test/Case/Model/models.php @@ -4948,6 +4948,13 @@ class CustomArticle extends AppModel { */ public $findMethods = array('unPublished' => true); +/** + * belongsTo property + * + * @var array + */ + public $belongsTo = array('User'); + /** * _findUnPublished custom find * @@ -4961,4 +4968,13 @@ protected function _findUnPublished($state, $query, $results = array()) { return $results; } +/** + * Alters title data + * + * @return void + **/ + public function beforeValidate() { + $this->data[$this->alias]['title'] = 'foo'; + } + }