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 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
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
* 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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -2188,6 +2188,7 @@ public function saveAssociated($data = null, $options = array()) {
$db = $this->getDataSource();
$transactionBegun = $db->begin();
}

$associations = $this->getAssociated();
$return = array();
$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
* 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))) {
Expand All @@ -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'))) {
Expand Down
81 changes: 80 additions & 1 deletion lib/Cake/Test/Case/Model/ModelValidationTest.php
Expand Up @@ -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')));
}

}
97 changes: 48 additions & 49 deletions lib/Cake/Test/Case/Model/ModelWriteTest.php
Expand Up @@ -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(
Expand Down Expand Up @@ -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'
)
),
Expand Down Expand Up @@ -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'
)
),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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')),
Expand Down Expand Up @@ -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(
Expand Down
16 changes: 16 additions & 0 deletions lib/Cake/Test/Case/Model/models.php
Expand Up @@ -4948,6 +4948,13 @@ class CustomArticle extends AppModel {
*/
public $findMethods = array('unPublished' => true);

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

/**
* _findUnPublished custom find
*
Expand All @@ -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';
}

}

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.