FIXED: Issue where translation of siteconfigs would create additional objects.

ADDED: Test case assertion
FIXED: Coding convention
This commit is contained in:
Damian Mooyman 2012-08-24 10:40:09 +12:00 committed by Ingo Schommer
parent 1120f2974e
commit 91596197ee
2 changed files with 28 additions and 18 deletions

View File

@ -1149,35 +1149,42 @@ class Translatable extends DataExtension implements PermissionProvider {
// Singleton objects should not call populateDefault() but a bug seems to be allowing it // Singleton objects should not call populateDefault() but a bug seems to be allowing it
// to happen anyway. Looking up existing SiteConfig objects without a database crashes horribly // to happen anyway. Looking up existing SiteConfig objects without a database crashes horribly
// @todo Remove this hack once this bug is fixed // @todo Remove this hack once this bug is fixed
if(!DB::getConn()->hasTable($this->owner->class)) if(!DB::getConn()->hasTable($this->owner->class)) return;
return;
// Find the best base translation for site config // Find the best base translation for site config
Translatable::disable_locale_filter(); Translatable::disable_locale_filter();
$existingConfig = SiteConfig::get()->filter(array('Locale' => Translatable::default_locale()))->first(); $existingConfig = SiteConfig::get()->filter(array('Locale' => Translatable::default_locale()))->first();
if(!$existingConfig) if(!$existingConfig) $existingConfig = SiteConfig::get()->first();
$existingConfig = SiteConfig::get()->first();
Translatable::enable_locale_filter(); Translatable::enable_locale_filter();
// Base case; creation of the first site config // Base case; creation of the first site config
if(!$existingConfig) if(!$existingConfig) {
{
$this->owner->Locale = Translatable::get_current_locale(); $this->owner->Locale = Translatable::get_current_locale();
return; return;
} }
// Edge case: creating new translation in same locale as an existing object probably // Edge case: creating new translation in same locale as an existing object probably
// should not have the same translation group (@todo find examples) // should not have the same translation group (@todo find examples)
if(Translatable::get_current_locale() == $existingConfig->Locale) if(Translatable::get_current_locale() == $existingConfig->Locale) return;
return;
// Create a third "staging" translated object using the correct createTranslation mechanism // Create a third unsaved "staging" translated object using the correct createTranslation mechanism
$stagingConfig = $existingConfig->createTranslation(Translatable::get_current_locale()); $stagingConfig = $existingConfig->createTranslation(Translatable::get_current_locale(), false);
// Copy fields from translated object (including ID) to the generated object // Copy fields from translated object to the generated object.
// This is similar to the creation of objects via createTranslation, but we need to // This is similar to the creation of objects via createTranslation,
// preserve the ID in order to prevent unnecessary object creation // although by default this object should not be saved
$this->owner->update($stagingConfig->toMap()); $this->owner->update($stagingConfig->toMap());
// Copy translation group
// @todo Candidate for a refactor, along with other parts of this code
// that reference _TranslationGroupID / getTranslationGroup
if($group = $stagingConfig->getTranslationGroup()) {
$this->owner->_TranslationGroupID = $group;
} elseif(!empty($stagingConfig->_TranslationGroupID)) {
$this->owner->_TranslationGroupID = $stagingConfig->_TranslationGroupID;
} else {
$this->owner->_TranslationGroupID = $stagingConfig->ID;
}
} }
/** /**
@ -1193,7 +1200,7 @@ class Translatable extends DataExtension implements PermissionProvider {
public function populateDefaults() { public function populateDefaults() {
if (empty($this->owner->ID) && ($this->owner instanceof SiteConfig) && self::$enable_siteconfig_generation) if (empty($this->owner->ID) && ($this->owner instanceof SiteConfig) && self::$enable_siteconfig_generation)
{ {
// Use enable_siteconfig_generation to prevent infinite loop during // Use enable_siteconfig_generation to prevent infinite loop during object creation
self::$enable_siteconfig_generation = false; self::$enable_siteconfig_generation = false;
$this->populateSiteConfig(); $this->populateSiteConfig();
self::$enable_siteconfig_generation = true; self::$enable_siteconfig_generation = true;
@ -1209,10 +1216,12 @@ class Translatable extends DataExtension implements PermissionProvider {
* it belongs to. For "original records" which are not created through this * it belongs to. For "original records" which are not created through this
* method, the "translation group" is set in {@link onAfterWrite()}. * method, the "translation group" is set in {@link onAfterWrite()}.
* *
* @param string $locale * @param string $locale Target locale to translate this object into
* @param boolean $saveTranslation Flag indicating whether the new record
* should be saved to the database.
* @return DataObject The translated object * @return DataObject The translated object
*/ */
function createTranslation($locale) { function createTranslation($locale, $saveTranslation = true) {
if($locale && !i18n::validate_locale($locale)) throw new InvalidArgumentException(sprintf('Invalid locale "%s"', $locale)); if($locale && !i18n::validate_locale($locale)) throw new InvalidArgumentException(sprintf('Invalid locale "%s"', $locale));
if(!$this->owner->exists()) { if(!$this->owner->exists()) {
@ -1261,7 +1270,7 @@ class Translatable extends DataExtension implements PermissionProvider {
// hacky way to set an existing translation group in onAfterWrite() // hacky way to set an existing translation group in onAfterWrite()
$translationGroupID = $this->getTranslationGroup(); $translationGroupID = $this->getTranslationGroup();
$newTranslation->_TranslationGroupID = $translationGroupID ? $translationGroupID : $this->owner->ID; $newTranslation->_TranslationGroupID = $translationGroupID ? $translationGroupID : $this->owner->ID;
$newTranslation->write(); if($saveTranslation) $newTranslation->write();
return $newTranslation; return $newTranslation;
} }

View File

@ -41,6 +41,7 @@ class TranslatableSiteConfigTest extends SapphireTest {
$this->assertInstanceOf('SiteConfig', $configFr); $this->assertInstanceOf('SiteConfig', $configFr);
$this->assertEquals($configFr->Locale, 'fr_FR'); $this->assertEquals($configFr->Locale, 'fr_FR');
$this->assertEquals($configFr->Title, $configEn->Title, 'Copies title from existing config'); $this->assertEquals($configFr->Title, $configEn->Title, 'Copies title from existing config');
$this->assertEquals($configFr->getTranslationGroup(), $configEn->getTranslationGroup(), 'Created in the same translation group');
} }
function testCanEditTranslatedRootPages() { function testCanEditTranslatedRootPages() {