From f4e5137392d7654de90e1328574bb3fa6c9346e2 Mon Sep 17 00:00:00 2001 From: Jeremy Thomerson Date: Tue, 4 Jun 2013 15:05:00 +0000 Subject: [PATCH 1/2] FIX: make getTranslations work even when translation classes differ Even though it is an edge-case, some sites may allow translation groups to be composed of different classes. In that case, Translatable->getTranslations() should still work. Also, this commit adds a helper function for testing two array for equality where you don't care about the order of the elements. This cleaned up a lot of copy and paste boilerplate code that was sorting arrays to test. --- code/model/Translatable.php | 4 +- tests/unit/TranslatableTest.php | 154 +++++++++++++++++++++----------- 2 files changed, 103 insertions(+), 55 deletions(-) diff --git a/code/model/Translatable.php b/code/model/Translatable.php index e53580a..a8a6e97 100755 --- a/code/model/Translatable.php +++ b/code/model/Translatable.php @@ -1198,7 +1198,7 @@ class Translatable extends DataExtension implements PermissionProvider { if($this->owner->hasExtension("Versioned")) { if($stage) Versioned::reading_stage($stage); $translations = Versioned::get_by_stage( - $this->owner->class, + $baseDataClass, Versioned::current_stage(), $filter, null @@ -1206,7 +1206,7 @@ class Translatable extends DataExtension implements PermissionProvider { if($stage) Versioned::reading_stage($currentStage); } else { $class = $this->owner->class; - $translations = $class::get() + $translations = $baseDataClass::get() ->where($filter) ->leftJoin("{$baseDataClass}_translationgroups", $joinOnClause); } diff --git a/tests/unit/TranslatableTest.php b/tests/unit/TranslatableTest.php index b845e58..db45f52 100755 --- a/tests/unit/TranslatableTest.php +++ b/tests/unit/TranslatableTest.php @@ -41,6 +41,12 @@ class TranslatableTest extends FunctionalTest { parent::tearDown(); } + function assertArrayEqualsAfterSort($expected, $actual, $message = null) { + sort($expected); + sort($actual); + return $this->assertEquals($expected, $actual, $message); + } + function testLocaleFilteringEnabledAndDisabled() { $this->assertTrue(Translatable::locale_filter_enabled()); @@ -117,13 +123,9 @@ class TranslatableTest extends FunctionalTest { // test french - $array1=$frPage->getTranslations()->column('Locale'); - $array2=array('en_US','es_ES'); - sort($array1); - sort($array2); - $this->assertEquals( - $array1, - $array2 + $this->assertArrayEqualsAfterSort( + array('en_US','es_ES'), + $frPage->getTranslations()->column('Locale') ); $this->assertNotNull($frPage->getTranslation('en_US')); $this->assertEquals( @@ -137,13 +139,9 @@ class TranslatableTest extends FunctionalTest { ); // test english - $expected = array('es_ES', 'fr_FR'); - sort($expected); - $actual = $enPage->getTranslations()->column('Locale'); - sort($actual); - $this->assertEquals( - $expected, - $actual + $this->assertArrayEqualsAfterSort( + array('es_ES', 'fr_FR'), + $enPage->getTranslations()->column('Locale') ); $this->assertNotNull($frPage->getTranslation('fr_FR')); $this->assertEquals( @@ -157,13 +155,9 @@ class TranslatableTest extends FunctionalTest { ); // test spanish - $actual = $esPage->getTranslations()->column('Locale'); - sort($actual); - $expected = array('en_US', 'fr_FR'); - sort($expected); - $this->assertEquals( - $actual, - $expected + $this->assertArrayEqualsAfterSort( + array('en_US', 'fr_FR'), + $esPage->getTranslations()->column('Locale') ); $this->assertNotNull($esPage->getTranslation('fr_FR')); $this->assertEquals( @@ -177,6 +171,71 @@ class TranslatableTest extends FunctionalTest { ); } + function testTranslationGroupsWhenTranslationIsSubclass() { + // create an English SiteTree + $enST = new SiteTree(); + $enST->Locale = 'en_US'; + $enST->write(); + + // create French and Spanish translations + $frST = $enST->createTranslation('fr_FR'); + $esST = $enST->createTranslation('es_ES'); + + // test that we get the right translations back from each instance + $this->assertArrayEqualsAfterSort( + array('fr_FR', 'es_ES'), + $enST->getTranslations()->column('Locale') + ); + $this->assertArrayEqualsAfterSort( + array('en_US', 'es_ES'), + $frST->getTranslations()->column('Locale') + ); + $this->assertArrayEqualsAfterSort( + array('en_US', 'fr_FR'), + $esST->getTranslations()->column('Locale') + ); + + // this should be considered an edge-case, but on some sites translations + // may be allowed to be a subclass of the default locale's translation of + // the same page. In this case, we need to support getTranslations returning + // all of the translations, even if one of the translations is a different + // class from others + $esST->setClassName('Page'); + $esST->write(); + $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)); + + // 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)); + $enST = DataObject::get_by_id('SiteTree', $enST->ID, $cache = false); + $this->assertEquals('SiteTree', $enST->ClassName); + $this->assertEquals('SiteTree', get_class($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 + $this->assertArrayEqualsAfterSort( + array('fr_FR', 'es_ES'), + $enST->getTranslations()->column('Locale') + ); + $this->assertArrayEqualsAfterSort( + array('en_US', 'es_ES'), + $frST->getTranslations()->column('Locale') + ); + $this->assertArrayEqualsAfterSort( + array('en_US', 'fr_FR'), + $esPg->getTranslations()->column('Locale') + ); + $this->assertEquals($enST->ID, $esPg->getTranslation('en_US')->ID); + $this->assertEquals($frST->ID, $esPg->getTranslation('fr_FR')->ID); + $this->assertEquals($esPg->ID, $enST->getTranslation('es_ES')->ID); + $this->assertEquals($esPg->ID, $frST->getTranslation('es_ES')->ID); + } + function testTranslationGroupNotRemovedWhenSiteTreeUnpublished() { $enPage = new Page(); $enPage->Locale = 'en_US'; @@ -439,16 +498,13 @@ class TranslatableTest extends FunctionalTest { $child4PageTranslated->write(); Translatable::set_current_locale('en_US'); - $actual = $parentPage->Children()->column('ID'); - sort($actual); - $expected = array( - $child1Page->ID, - $child2Page->ID, - $child3Page->ID - ); - $this->assertEquals( - $actual, - $expected, + $this->assertArrayEqualsAfterSort( + array( + $child1Page->ID, + $child2Page->ID, + $child3Page->ID + ), + $parentPage->Children()->column('ID'), "Showing Children() in default language doesnt show children in other languages" ); @@ -495,17 +551,13 @@ class TranslatableTest extends FunctionalTest { "Showing liveChildren() in default language doesnt show children in other languages" ); $this->assertNotNull($parentPage->stageChildren()); - $actual = $parentPage->stageChildren()->column('ID'); - sort($actual); - $expected = array( - $child1Page->ID, - $child2Page->ID, - $child3Page->ID - ); - sort($expected); - $this->assertEquals( - $actual, - $expected, + $this->assertArrayEqualsAfterSort( + array( + $child1Page->ID, + $child2Page->ID, + $child3Page->ID + ), + $parentPage->stageChildren()->column('ID'), "Showing stageChildren() in default language doesnt show children in other languages" ); @@ -691,17 +743,13 @@ class TranslatableTest extends FunctionalTest { SiteTree::flush_and_destroy_cache(); $parentPage = $this->objFromFixture('Page', 'parent'); $children = $parentPage->AllChildrenIncludingDeleted(); - $expected = array( - $child2PageID, - $child3PageID, - $child1PageID // $child1Page was deleted from stage, so the original record doesn't have the ID set - ); - sort($expected); - $actual = $parentPage->AllChildrenIncludingDeleted()->column('ID'); - sort($actual); - $this->assertEquals( - $actual, - $expected, + $this->assertArrayEqualsAfterSort( + array( + $child2PageID, + $child3PageID, + $child1PageID // $child1Page was deleted from stage, so the original record doesn't have the ID set + ), + $parentPage->AllChildrenIncludingDeleted()->column('ID'), "Showing AllChildrenIncludingDeleted() in default language doesnt show deleted children in other languages" ); From f75a5fd5e3185341a91a82562ecc97e9dcd49f11 Mon Sep 17 00:00:00 2001 From: Jeremy Thomerson Date: Tue, 4 Jun 2013 15:45:16 +0000 Subject: [PATCH 2/2] TEST: adds test for changing class name of default locale translation Adds test case for silverstripe/silverstripe-translatable#97 --- tests/unit/TranslatableTest.php | 43 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/unit/TranslatableTest.php b/tests/unit/TranslatableTest.php index db45f52..0b05c1e 100755 --- a/tests/unit/TranslatableTest.php +++ b/tests/unit/TranslatableTest.php @@ -170,6 +170,49 @@ class TranslatableTest extends FunctionalTest { $enPage->ID ); } + + function testChangingClassOfDefaultLocaleTranslationChangesOthers() { + // see https://github.com/silverstripe/silverstripe-translatable/issues/97 + // create an English SiteTree + $enST = new SiteTree(); + $enST->Locale = 'en_US'; + $enST->write(); + + // create French and Spanish translations + $frST = $enST->createTranslation('fr_FR'); + $esST = $enST->createTranslation('es_ES'); + + // change the class name of the default locale's translation (as CMS admin would) + $enST->setClassName('Page'); + $enST->write(); + + // reload them all to get fresh instances + $enPg = DataObject::get_by_id('Page', $enST->ID, $cache = false); + $frPg = DataObject::get_by_id('Page', $frST->ID, $cache = false); + $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)); + + // test that we get the right translations back from each instance + $this->assertArrayEqualsAfterSort( + array('fr_FR', 'es_ES'), + $enPg->getTranslations()->column('Locale') + ); + $this->assertArrayEqualsAfterSort( + array('en_US', 'es_ES'), + $frPg->getTranslations()->column('Locale') + ); + $this->assertArrayEqualsAfterSort( + array('en_US', 'fr_FR'), + $esPg->getTranslations()->column('Locale') + ); + } function testTranslationGroupsWhenTranslationIsSubclass() { // create an English SiteTree