From 6c5255ac73df0043262782d28ef1409c0aed67cc Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 6 May 2012 21:29:39 -0400 Subject: [PATCH] Fix saving translate records with saveAll() Also fixes issues saving translated data with validation disabled. By enabling data backup in beforeSave() and beforeValidate() the existing behavior is preserved, and the current issue is fixed. Fixes #2857 --- lib/Cake/Model/Behavior/TranslateBehavior.php | 30 ++- .../Model/Behavior/TranslateBehaviorTest.php | 179 ++++++++++++++---- lib/Cake/Test/Case/Model/models.php | 11 +- .../Test/Fixture/TranslatedItemFixture.php | 7 +- 4 files changed, 187 insertions(+), 40 deletions(-) diff --git a/lib/Cake/Model/Behavior/TranslateBehavior.php b/lib/Cake/Model/Behavior/TranslateBehavior.php index bce937ccab9..79036e01ac9 100644 --- a/lib/Cake/Model/Behavior/TranslateBehavior.php +++ b/lib/Cake/Model/Behavior/TranslateBehavior.php @@ -310,8 +310,35 @@ public function afterFind(Model $model, $results, $primary) { * @return boolean */ public function beforeValidate(Model $model) { + unset($this->runtime[$model->alias]['beforeSave']); + $this->_setRuntimeData($model); + return true; + } + +/** + * beforeSave callback. + * + * @param Model $model Model save was called on. + * @return boolean true. + */ + public function beforeSave(Model $model) { + $this->_setRuntimeData($model); + return true; + } + +/** + * Sets the runtime data. + * + * Used from beforeValidate() and beforeSave() for compatibility issues, + * and to allow translations to be persisted even when validation + * is disabled. + * + * @param Model $model + * @return void + */ + protected function _setRuntimeData(Model $model) { $locale = $this->_getLocale($model); - if (empty($locale)) { + if (empty($locale) || isset($this->runtime[$model->alias]['beforeSave'])) { return true; } $fields = array_merge($this->settings[$model->alias], $this->runtime[$model->alias]['fields']); @@ -333,7 +360,6 @@ public function beforeValidate(Model $model) { } } $this->runtime[$model->alias]['beforeSave'] = $tempData; - return true; } /** diff --git a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php index cc2cf20334d..ad2e3cef35e 100644 --- a/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php +++ b/lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php @@ -16,9 +16,6 @@ * @since CakePHP(tm) v 1.2.0.5669 * @license MIT License (http://www.opensource.org/licenses/mit-license.php) */ -if (!defined('CAKEPHP_UNIT_TEST_EXECUTION')) { - define('CAKEPHP_UNIT_TEST_EXECUTION', 1); -} App::uses('Model', 'Model'); App::uses('AppModel', 'Model'); @@ -119,7 +116,11 @@ public function testLocaleFalsePlain() { $TestModel->locale = false; $result = $TestModel->read(null, 1); - $expected = array('TranslatedItem' => array('id' => 1, 'slug' => 'first_translated')); + $expected = array('TranslatedItem' => array( + 'id' => 1, + 'slug' => 'first_translated', + 'translated_article_id' => 1, + )); $this->assertEquals($expected, $result); $result = $TestModel->find('all', array('fields' => array('slug'))); @@ -147,7 +148,7 @@ public function testLocaleFalseAssociations() { $result = $TestModel->read(null, 1); $expected = array( - 'TranslatedItem' => array('id' => 1, 'slug' => 'first_translated'), + 'TranslatedItem' => array('id' => 1, 'slug' => 'first_translated', 'translated_article_id' => 1), 'Title' => array( array('id' => 1, 'locale' => 'eng', 'model' => 'TranslatedItem', 'foreign_key' => 1, 'field' => 'title', 'content' => 'Title #1'), array('id' => 3, 'locale' => 'deu', 'model' => 'TranslatedItem', 'foreign_key' => 1, 'field' => 'title', 'content' => 'Titel #1'), @@ -203,7 +204,8 @@ public function testLocaleSingle() { 'slug' => 'first_translated', 'locale' => 'eng', 'title' => 'Title #1', - 'content' => 'Content #1' + 'content' => 'Content #1', + 'translated_article_id' => 1, ) ); $this->assertEquals($expected, $result); @@ -216,7 +218,8 @@ public function testLocaleSingle() { 'slug' => 'first_translated', 'locale' => 'eng', 'title' => 'Title #1', - 'content' => 'Content #1' + 'content' => 'Content #1', + 'translated_article_id' => 1, ) ), array( @@ -225,7 +228,8 @@ public function testLocaleSingle() { 'slug' => 'second_translated', 'locale' => 'eng', 'title' => 'Title #2', - 'content' => 'Content #2' + 'content' => 'Content #2', + 'translated_article_id' => 1, ) ), array( @@ -234,7 +238,8 @@ public function testLocaleSingle() { 'slug' => 'third_translated', 'locale' => 'eng', 'title' => 'Title #3', - 'content' => 'Content #3' + 'content' => 'Content #3', + 'translated_article_id' => 1, ) ) ); @@ -259,7 +264,8 @@ public function testLocaleSingleWithConditions() { 'slug' => 'first_translated', 'locale' => 'eng', 'title' => 'Title #1', - 'content' => 'Content #1' + 'content' => 'Content #1', + 'translated_article_id' => 1, ) ) ); @@ -273,7 +279,8 @@ public function testLocaleSingleWithConditions() { 'slug' => 'first_translated', 'locale' => 'eng', 'title' => 'Title #1', - 'content' => 'Content #1' + 'content' => 'Content #1', + 'translated_article_id' => 1, ) ) ); @@ -301,7 +308,8 @@ public function testLocaleSingleAssociations() { 'slug' => 'first_translated', 'locale' => 'eng', 'title' => 'Title #1', - 'content' => 'Content #1' + 'content' => 'Content #1', + 'translated_article_id' => 1, ), 'Title' => array( array('id' => 1, 'locale' => 'eng', 'model' => 'TranslatedItem', 'foreign_key' => 1, 'field' => 'title', 'content' => 'Title #1'), @@ -322,17 +330,35 @@ public function testLocaleSingleAssociations() { $result = $TestModel->find('all', array('fields' => array('TranslatedItem.title'))); $expected = array( array( - 'TranslatedItem' => array('id' => 1, 'locale' => 'eng', 'title' => 'Title #1', 'slug' => 'first_translated'), + 'TranslatedItem' => array( + 'id' => 1, + 'locale' => 'eng', + 'title' => 'Title #1', + 'slug' => 'first_translated', + 'translated_article_id' => 1, + ), 'Title' => array(array('foreign_key' => 1, 'content' => 'Title #1')), 'Content' => array(array('foreign_key' => 1, 'content' => 'Content #1')) ), array( - 'TranslatedItem' => array('id' => 2, 'locale' => 'eng', 'title' => 'Title #2', 'slug' => 'second_translated'), + 'TranslatedItem' => array( + 'id' => 2, + 'locale' => 'eng', + 'title' => 'Title #2', + 'slug' => 'second_translated', + 'translated_article_id' => 1, + ), 'Title' => array(array('foreign_key' => 2, 'content' => 'Title #2')), 'Content' => array(array('foreign_key' => 2, 'content' => 'Content #2')) ), array( - 'TranslatedItem' => array('id' => 3, 'locale' => 'eng', 'title' => 'Title #3','slug' => 'third_translated'), + 'TranslatedItem' => array( + 'id' => 3, + 'locale' => 'eng', + 'title' => 'Title #3', + 'slug' => 'third_translated', + 'translated_article_id' => 1, + ), 'Title' => array(array('foreign_key' => 3, 'content' => 'Title #3')), 'Content' => array(array('foreign_key' => 3, 'content' => 'Content #3')) ) @@ -358,7 +384,8 @@ public function testLocaleMultiple() { 'slug' => 'first_translated', 'locale' => 'deu', 'title' => 'Titel #1', - 'content' => 'Inhalt #1' + 'content' => 'Inhalt #1', + 'translated_article_id' => 1, ) ); $this->assertEquals($expected, $result); @@ -370,7 +397,7 @@ public function testLocaleMultiple() { 'slug' => 'first_translated', 'locale' => 'deu', 'content' => 'Inhalt #1', - 'title' => 'Titel #1' + 'title' => 'Titel #1', ) ), array( @@ -378,7 +405,7 @@ public function testLocaleMultiple() { 'slug' => 'second_translated', 'locale' => 'deu', 'title' => 'Titel #2', - 'content' => 'Inhalt #2' + 'content' => 'Inhalt #2', ) ), array( @@ -386,7 +413,7 @@ public function testLocaleMultiple() { 'slug' => 'third_translated', 'locale' => 'deu', 'title' => 'Titel #3', - 'content' => 'Inhalt #3' + 'content' => 'Inhalt #3', ) ) ); @@ -415,7 +442,8 @@ public function testMissingTranslation() { 'slug' => 'first_translated', 'locale' => 'rus', 'title' => '', - 'content' => '' + 'content' => '', + 'translated_article_id' => 1, ) ); $this->assertEquals($expected, $result); @@ -499,7 +527,12 @@ public function testSaveCreate() { $TestModel = new TranslatedItem(); $TestModel->locale = 'spa'; - $data = array('slug' => 'fourth_translated', 'title' => 'Leyenda #4', 'content' => 'Contenido #4'); + $data = array( + 'slug' => 'fourth_translated', + 'title' => 'Leyenda #4', + 'content' => 'Contenido #4', + 'translated_article_id' => null + ); $TestModel->create($data); $TestModel->save(); $result = $TestModel->read(); @@ -517,7 +550,7 @@ public function testSaveUpdate() { $TestModel = new TranslatedItem(); $TestModel->locale = 'spa'; - $oldData = array('slug' => 'fourth_translated', 'title' => 'Leyenda #4'); + $oldData = array('slug' => 'fourth_translated', 'title' => 'Leyenda #4', 'translated_article_id' => 1); $TestModel->create($oldData); $TestModel->save(); $id = $TestModel->id; @@ -554,7 +587,14 @@ public function testMultipleCreate() { $result = $TestModel->read(); $expected = array( - 'TranslatedItem' => array('id' => 4, 'slug' => 'new_translated', 'locale' => 'eng', 'title' => 'New title', 'content' => 'New content'), + 'TranslatedItem' => array( + 'id' => 4, + 'slug' => 'new_translated', + 'locale' => 'eng', + 'title' => 'New title', + 'content' => 'New content', + 'translated_article_id' => null, + ), 'Title' => array( array('id' => 21, 'locale' => 'eng', 'model' => 'TranslatedItem', 'foreign_key' => 4, 'field' => 'title', 'content' => 'New title'), array('id' => 22, 'locale' => 'spa', 'model' => 'TranslatedItem', 'foreign_key' => 4, 'field' => 'title', 'content' => 'Nuevo leyenda') @@ -591,7 +631,14 @@ public function testMultipleUpdate() { $TestModel->bindTranslation($translations, false); $result = $TestModel->read(null, 1); $expected = array( - 'TranslatedItem' => array('id' => '1', 'slug' => 'first_translated', 'locale' => 'eng', 'title' => 'New Title #1', 'content' => 'New Content #1'), + 'TranslatedItem' => array( + 'id' => '1', + 'slug' => 'first_translated', + 'locale' => 'eng', + 'title' => 'New Title #1', + 'content' => 'New Content #1', + 'translated_article_id' => 1, + ), 'Title' => array( array('id' => 1, 'locale' => 'eng', 'model' => 'TranslatedItem', 'foreign_key' => 1, 'field' => 'title', 'content' => 'New Title #1'), array('id' => 3, 'locale' => 'deu', 'model' => 'TranslatedItem', 'foreign_key' => 1, 'field' => 'title', 'content' => 'Neue Titel #1'), @@ -634,7 +681,14 @@ public function testMixedCreateUpdateWithArrayLocale() { $result['Title'] = Set::sort($result['Title'], '{n}.id', 'asc'); $result['Content'] = Set::sort($result['Content'], '{n}.id', 'asc'); $expected = array( - 'TranslatedItem' => array('id' => 1, 'slug' => 'first_translated', 'locale' => 'cze', 'title' => 'Titulek #1', 'content' => 'Upraveny obsah #1'), + 'TranslatedItem' => array( + 'id' => 1, + 'slug' => 'first_translated', + 'locale' => 'cze', + 'title' => 'Titulek #1', + 'content' => 'Upraveny obsah #1', + 'translated_article_id' => 1, + ), 'Title' => array( array('id' => 1, 'locale' => 'eng', 'model' => 'TranslatedItem', 'foreign_key' => 1, 'field' => 'title', 'content' => 'Updated Title #1'), array('id' => 3, 'locale' => 'deu', 'model' => 'TranslatedItem', 'foreign_key' => 1, 'field' => 'title', 'content' => 'Titel #1'), @@ -650,6 +704,44 @@ public function testMixedCreateUpdateWithArrayLocale() { $this->assertEquals($expected, $result); } +/** + * Test that saveAll() works with hasMany associations that contain + * translations. + * + * @return void + */ + public function testSaveAllTranslatedAssociations() { + $this->loadFixtures('Translate', 'TranslateArticle', 'TranslatedItem', 'TranslatedArticle', 'User'); + $Model = new TranslatedArticle(); + $Model->locale = 'eng'; + + $data = array( + 'TranslatedArticle' => array( + 'user_id' => 1, + 'published' => 'Y', + 'title' => 'Title (eng) #1', + 'body' => 'Body (eng) #1' + ), + 'TranslatedItem' => array( + array( + 'title' => 'Nuevo leyenda #1', + 'content' => 'Upraveny obsah #1' + ), + array( + 'title' => 'New Title #2', + 'content' => 'New Content #2' + ), + ) + ); + $result = $Model->saveAll($data); + $this->assertTrue($result); + + $result = $Model->TranslatedItem->find('all', array( + 'conditions' => array('translated_article_id' => $Model->id) + )); + $this->assertCount(2, $result); + } + /** * testValidation method * @@ -661,11 +753,13 @@ public function testValidation() { $TestModel = new TranslatedItem(); $TestModel->locale = 'eng'; $TestModel->validate['title'] = '/Only this title/'; - $data = array('TranslatedItem' => array( - 'id' => 1, - 'title' => array('eng' => 'New Title #1', 'deu' => 'Neue Titel #1', 'cze' => 'Novy Titulek #1'), - 'content' => array('eng' => 'New Content #1', 'deu' => 'Neue Inhalt #1', 'cze' => 'Novy Obsah #1') - )); + $data = array( + 'TranslatedItem' => array( + 'id' => 1, + 'title' => array('eng' => 'New Title #1', 'deu' => 'Neue Titel #1', 'cze' => 'Novy Titulek #1'), + 'content' => array('eng' => 'New Content #1', 'deu' => 'Neue Inhalt #1', 'cze' => 'Novy Obsah #1') + ) + ); $TestModel->create(); $this->assertFalse($TestModel->save($data)); $this->assertEquals(array('This field cannot be left blank'), $TestModel->validationErrors['title']); @@ -748,7 +842,8 @@ public function testAnotherTranslateTable() { 'slug' => 'first_translated', 'locale' => 'eng', 'title' => 'Another Title #1', - 'content' => 'Another Content #1' + 'content' => 'Another Content #1', + 'translated_article_id' => 1, ) ); $this->assertEquals($expected, $result); @@ -760,7 +855,7 @@ public function testAnotherTranslateTable() { * @return void */ public function testTranslateWithAssociations() { - $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User', 'Comment', 'ArticlesTag', 'Tag'); + $this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'TranslatedItem', 'User', 'Comment', 'ArticlesTag', 'Tag'); $TestModel = new TranslatedArticle(); $TestModel->locale = 'eng'; @@ -784,6 +879,23 @@ public function testTranslateWithAssociations() { 'password' => '5f4dcc3b5aa765d61d8327deb882cf99', 'created' => '2007-03-17 01:16:23', 'updated' => '2007-03-17 01:18:31' + ), + 'TranslatedItem' => array( + array( + 'id' => 1, + 'translated_article_id' => 1, + 'slug' => 'first_translated' + ), + array( + 'id' => 2, + 'translated_article_id' => 1, + 'slug' => 'second_translated' + ), + array( + 'id' => 3, + 'translated_article_id' => 1, + 'slug' => 'third_translated' + ), ) ); $this->assertEquals($expected, $result); @@ -863,7 +975,8 @@ public function testTranslateTableWithPrefix() { 'slug' => 'first_translated', 'locale' => 'eng', 'content' => 'Content #1', - 'title' => 'Title #1' + 'title' => 'Title #1', + 'translated_article_id' => 1, )); $this->assertEquals($expected, $result); } diff --git a/lib/Cake/Test/Case/Model/models.php b/lib/Cake/Test/Case/Model/models.php index e5c2856e834..43bad312327 100644 --- a/lib/Cake/Test/Case/Model/models.php +++ b/lib/Cake/Test/Case/Model/models.php @@ -3119,7 +3119,7 @@ class TranslatedItem2 extends CakeTestModel { /** * translateModel property * - * @var string 'TranslateTestModel' + * @var string */ public $translateModel = 'TranslateWithPrefix'; @@ -3163,7 +3163,7 @@ class TranslatedItemWithTable extends CakeTestModel { /** * translateModel property * - * @var string 'TranslateTestModel' + * @var string */ public $translateModel = 'TranslateTestModel'; @@ -3248,6 +3248,13 @@ class TranslatedArticle extends CakeTestModel { */ public $belongsTo = array('User'); +/** + * belongsTo property + * + * @var array + */ + public $hasMany = array('TranslatedItem'); + } class CounterCacheUser extends CakeTestModel { diff --git a/lib/Cake/Test/Fixture/TranslatedItemFixture.php b/lib/Cake/Test/Fixture/TranslatedItemFixture.php index 28485f26da6..3d5490371c3 100644 --- a/lib/Cake/Test/Fixture/TranslatedItemFixture.php +++ b/lib/Cake/Test/Fixture/TranslatedItemFixture.php @@ -38,6 +38,7 @@ class TranslatedItemFixture extends CakeTestFixture { */ public $fields = array( 'id' => array('type' => 'integer', 'key' => 'primary'), + 'translated_article_id' => array('type' => 'integer'), 'slug' => array('type' => 'string', 'null' => false) ); @@ -47,8 +48,8 @@ class TranslatedItemFixture extends CakeTestFixture { * @var array */ public $records = array( - array('slug' => 'first_translated'), - array('slug' => 'second_translated'), - array('slug' => 'third_translated') + array('translated_article_id' => 1, 'slug' => 'first_translated'), + array('translated_article_id' => 1, 'slug' => 'second_translated'), + array('translated_article_id' => 1, 'slug' => 'third_translated') ); }