From 7534807137fe8d4b41357b304a33b43570333f6b Mon Sep 17 00:00:00 2001 From: Jeremy Thomerson Date: Sun, 18 Aug 2013 01:46:46 +0000 Subject: [PATCH 1/2] Test case: class changes propagate to translations when published --- tests/unit/TranslatableTest.php | 66 ++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/tests/unit/TranslatableTest.php b/tests/unit/TranslatableTest.php index f039d64..1aa67a8 100755 --- a/tests/unit/TranslatableTest.php +++ b/tests/unit/TranslatableTest.php @@ -15,7 +15,7 @@ class TranslatableTest extends FunctionalTest { ); protected $requiredExtensions = array( - 'SiteTree' => array('Translatable'), + 'SiteTree' => array('Translatable', 'Versioned', 'EveryoneCanPublish'), 'SiteConfig' => array('Translatable'), 'TranslatableTest_DataObject' => array('Translatable'), 'TranslatableTest_OneByLocaleDataObject' => array('Translatable'), @@ -71,7 +71,7 @@ function testGetOneByLocale() { Translatable::enable_locale_filter(); $found = Translatable::get_one_by_locale('TranslatableTest_OneByLocaleDataObject', $obj->Locale); - $this->assertNotNull($found, 'should have found one for ' . $obj->Locale); + $this->assertNotNull($found, 'should have found one for ' . $obj->Locale); $this->assertEquals($obj->ID, $found->ID); $translated = $obj->createTranslation('de_DE'); @@ -226,6 +226,12 @@ function testTranslationGroups() { ); } + function assertClass($class, $node) { + $this->assertNotNull($node); + $this->assertEquals($class, $node->ClassName); + $this->assertEquals($class, get_class($node)); + } + function testChangingClassOfDefaultLocaleTranslationChangesOthers() { // see https://github.com/silverstripe/silverstripe-translatable/issues/97 // create an English SiteTree @@ -247,12 +253,9 @@ function testChangingClassOfDefaultLocaleTranslationChangesOthers() { $esPg = DataObject::get_by_id('Page', $esST->ID, $cache = false); // make sure they are all the right class - $this->assertEquals('Page', $enPg->ClassName); - $this->assertEquals('Page', get_class($enPg)); - $this->assertEquals('Page', $frPg->ClassName); - $this->assertEquals('Page', get_class($frPg)); - $this->assertEquals('Page', $esPg->ClassName); - $this->assertEquals('Page', get_class($esPg)); + $this->assertClass('Page', $enPg); + $this->assertClass('Page', $frPg); + $this->assertClass('Page', $esPg); // test that we get the right translations back from each instance $this->assertArrayEqualsAfterSort( @@ -268,6 +271,36 @@ function testChangingClassOfDefaultLocaleTranslationChangesOthers() { $esPg->getTranslations()->column('Locale') ); } + + function testChangingClassOfDefaultLocaleTranslationChangesOthersWhenPublished() { + // create an English SiteTree + $enST = new SiteTree(); + $enST->Locale = 'en_US'; + $enST->write(); + $enST->doPublish(); + + // create and publish French and Spanish translations + $frST = $enST->createTranslation('fr_FR'); + $this->assertTrue($frST->doPublish(), 'should have been able to publish French translation'); + $esST = $enST->createTranslation('es_ES'); + $this->assertTrue($esST->doPublish(), 'should have been able to publish Spanish translation'); + + // change the class name of the default locale's translation (as CMS admin would) + // and publish the change - we should see both versions of English change class + $enST->setClassName('Page'); + $enST->doPublish(); + $this->assertClass('Page', Versioned::get_one_by_stage('SiteTree', 'Stage', '"ID" = ' . $enST->ID)); + $this->assertClass('Page', Versioned::get_one_by_stage('SiteTree', 'Live', '"ID" = ' . $enST->ID)); + + // and all of the draft versions of translations: + $this->assertClass('Page', Versioned::get_one_by_stage('SiteTree', 'Stage', '"ID" = ' . $frST->ID)); + $this->assertClass('Page', Versioned::get_one_by_stage('SiteTree', 'Stage', '"ID" = ' . $esST->ID)); + + // and all of the live versions of translations as well: + $this->assertClass('Page', Versioned::get_one_by_stage('SiteTree', 'Live', '"ID" = ' . $frST->ID)); + $this->assertClass('Page', Versioned::get_one_by_stage('SiteTree', 'Live', '"ID" = ' . $esST->ID)); + + } function testTranslationGroupsWhenTranslationIsSubclass() { // create an English SiteTree @@ -303,16 +336,13 @@ function testTranslationGroupsWhenTranslationIsSubclass() { $esPg = DataObject::get_by_id('Page', $esST->ID, $cache = false); // make sure we successfully changed the class - $this->assertEquals('Page', $esPg->ClassName); - $this->assertEquals('Page', get_class($esPg)); + $this->assertClass('Page', $esPg); // and make sure that the class of the others did not change $frST = DataObject::get_by_id('SiteTree', $frST->ID, $cache = false); - $this->assertEquals('SiteTree', $frST->ClassName); - $this->assertEquals('SiteTree', get_class($frST)); + $this->assertClass('SiteTree', $frST); $enST = DataObject::get_by_id('SiteTree', $enST->ID, $cache = false); - $this->assertEquals('SiteTree', $enST->ClassName); - $this->assertEquals('SiteTree', get_class($enST)); + $this->assertClass('SiteTree', $enST); // now that we know our edge case is successfully configured, we need to // make sure that we get the right translations back from everything @@ -1213,4 +1243,12 @@ function getCMSFields() { } } + +class EveryoneCanPublish extends DataExtension { + + function canPublish($member = null) { + return true; + } +} + TranslatableTest_DataObject::add_extension('TranslatableTest_Extension'); From 9e37602ef700b5b2fe756d8afb8d9dbc60192d23 Mon Sep 17 00:00:00 2001 From: Jeremy Thomerson Date: Sun, 18 Aug 2013 00:36:37 +0000 Subject: [PATCH 2/2] Make publishing default locale node faster When you publish a node in the default locale it calls forceChange to make every field behave as if it has changed. The problem in Translatable is then it would think that you actually changed the class of the default locale node and it would force a change on every translation. This was unnecessary when you have not actually changed the class name. If you have a great deal of translations this was causing a significant lag when you publish anything in the default locale. --- code/model/Translatable.php | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/code/model/Translatable.php b/code/model/Translatable.php index 5e644d4..10eaf49 100755 --- a/code/model/Translatable.php +++ b/code/model/Translatable.php @@ -873,7 +873,33 @@ class_exists('SiteTree') // dropdown is readonly on all translations. if($this->owner->ID && $this->owner->Locale == Translatable::default_locale()) { $changedFields = $this->owner->getChangedFields(); - if(isset($changedFields['ClassName'])) { + $changed = isset($changedFields['ClassName']); + + if ($changed && $this->owner->hasExtension('Versioned')) { + // this is required because when publishing a node the before/after + // values of $changedFields['ClassName'] will be the same because + // the record was already written to the stage/draft table and thus + // the record was updated, and then publish('Stage', 'Live') is + // called, which uses forceChange, which will make all the fields + // act as though they are changed, although the before/after values + // will be the same + // So, we load one from the current stage and test against it + // This is to prevent the overhead of writing all translations when + // the class didn't actually change. + $baseDataClass = ClassInfo::baseDataClass($this->owner->class); + $currentStage = Versioned::current_stage(); + $fresh = Versioned::get_one_by_stage( + $baseDataClass, + Versioned::current_stage(), + '"ID" = ' . $this->owner->ID, + null + ); + if ($fresh) { + $changed = $changedFields['ClassName']['after'] != $fresh->ClassName; + } + } + + if($changed) { $this->owner->ClassName = $changedFields['ClassName']['before']; $translations = $this->owner->getTranslations(); $this->owner->ClassName = $changedFields['ClassName']['after'];