From ea906a50a8a82bd40ee26764cfc3d44addbf0acf Mon Sep 17 00:00:00 2001 From: Andrew Short Date: Sun, 11 Oct 2009 00:07:22 +0000 Subject: [PATCH] FEATURE: Implemented Translatable::get_homepage_link_by_locale() to enable translated home pages to function. FEATURE: Updated SiteTree::get_by_link() to integrate with translatable, and allow it to work across languages by implementing Translatable->alternateGetByLink(). API CHANGE: Moved lang_filter enabling & disabling into static methods on Translatable, and renamed to locale_filter. BUGFIX: Fixed viewing a translatable page by URL without explicitly setting a Locale in ContentController->handleRequest(). From: Andrew Short git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@88503 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- core/control/ContentController.php | 20 +++--- core/control/ModelAsController.php | 4 ++ core/model/SiteTree.php | 16 +++-- core/model/Translatable.php | 110 +++++++++++++++++++++-------- tests/model/TranslatableTest.php | 52 ++++++++++++-- 5 files changed, 153 insertions(+), 49 deletions(-) diff --git a/core/control/ContentController.php b/core/control/ContentController.php index 6a8e7e103..25a79abb0 100755 --- a/core/control/ContentController.php +++ b/core/control/ContentController.php @@ -169,25 +169,29 @@ class ContentController extends Controller { */ public function handleRequest(HTTPRequest $request) { Director::set_current_page($this->data()); - $response = parent::handleRequest($request); // If the default handler returns an error, due to the action not existing, attempt to fall over to a child // SiteTree object, then use its corresponding ContentController to handle the request. This allows for the // building of nested chains of controllers corresponding to a nested URL. if(SiteTree::nested_urls() && $response instanceof HTTPResponse && $response->isError()) { - $SQL_URLParam = Convert::raw2sql($request->param('Action')); + Translatable::disable_locale_filter(); - if($SQL_URLParam && $nextPage = DataObject::get_one('SiteTree', "\"ParentID\" = $this->ID AND \"URLSegment\" = '$SQL_URLParam'")) { - if($nextPage->canView()) { - $request->shiftAllParams(); - return ModelAsController::controller_for($nextPage)->handleRequest($request); - } + $child = DataObject::get_one('SiteTree', sprintf ( + "\"ParentID\" = %s AND \"URLSegment\" = '%s'", + $this->ID, + Convert::raw2sql($request->param('Action')) + )); + + Translatable::enable_locale_filter(); + + if($child && $child->canView()) { + $request->shiftAllParams(); + return ModelAsController::controller_for($child)->handleRequest($request); } } Director::set_current_page(null); - return $response; } diff --git a/core/control/ModelAsController.php b/core/control/ModelAsController.php index 244b35497..415e04bf5 100755 --- a/core/control/ModelAsController.php +++ b/core/control/ModelAsController.php @@ -69,10 +69,14 @@ class ModelAsController extends Controller implements NestedController { throw new Exception('ModelAsController->getNestedController(): was not passed a URLSegment value.'); } + Translatable::disable_locale_filter(); + $sitetree = DataObject::get_one('SiteTree', sprintf ( '"URLSegment" = \'%s\' %s', Convert::raw2sql($URLSegment), (SiteTree::nested_urls() ? 'AND "ParentID" = 0' : null) )); + Translatable::enable_locale_filter(); + if(!$sitetree) { // If a root page has been renamed, redirect to the new location. if($redirect = $this->findOldPage($URLSegment)) { diff --git a/core/model/SiteTree.php b/core/model/SiteTree.php index 4348a8283..4db028cd3 100755 --- a/core/model/SiteTree.php +++ b/core/model/SiteTree.php @@ -262,11 +262,13 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid // Attempt to grab an alternative page from decorators. if(!$sitetree) { - if($alternatives = singleton('SiteTree')->extend('alternateGetByLink', $link, $filter, $cache, $order)) { - foreach($alternatives as $alternative) if($alternative) return $alternative; + $parentID = self::nested_urls() ? 0 : null; + + if($alternatives = singleton('SiteTree')->extend('alternateGetByLink', $URLSegment, $parentID)) { + foreach($alternatives as $alternative) if($alternative) $sitetree = $alternative; } - return false; + if(!$sitetree) return false; } // Check if we have any more URL parts to parse. @@ -279,11 +281,13 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid ); if(!$next) { - if($alternatives = singleton('SiteTree')->extend('alternateGetByLink', $link, $filter, $cache, $order)) { - foreach($alternatives as $alternative) if($alternative) return $alternative; + $parentID = (int) $sitetree->ID; + + if($alternatives = singleton('SiteTree')->extend('alternateGetByLink', $segment, $parentID)) { + foreach($alternatives as $alternative) if($alternative) $next = $alternative; } - return false; + if(!$next) return false; } $sitetree->destroy(); diff --git a/core/model/Translatable.php b/core/model/Translatable.php index f3dbaa7ef..ede74a267 100755 --- a/core/model/Translatable.php +++ b/core/model/Translatable.php @@ -189,11 +189,13 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { protected $original_values = null; /** - * @var boolean Temporarily override the "auto-filter" for {@link get_current_locale()} - * in {@link augmentSQL()}. IMPORTANT: You must set this value back to TRUE - * after the temporary usage. + * If this is set to TRUE then {@link augmentSQL()} will automatically add a filter + * clause to limit queries to the current {@link get_current_locale()}. This camn be + * disabled using {@link disable_locale_filter()} + * + * @var bool */ - protected static $enable_lang_filter = true; + protected static $locale_filter_enabled = true; /** * @var array All locales in which a translation can be created. @@ -208,9 +210,9 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { * Reset static configuration variables to their default values */ static function reset() { + self::enable_locale_filter(); self::$default_locale = 'en_US'; self::$current_locale = null; - self::$enable_lang_filter = true; self::$allowed_locales = null; } @@ -281,7 +283,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { * Set the reading language, either namespaced to 'site' (website content) * or 'cms' (management backend). This value is used in {@link augmentSQL()} * to "auto-filter" all SELECT queries by this language. - * See {@link $enable_lang_filter} on how to override this behaviour temporarily. + * See {@link disable_locale_filter()} on how to override this behaviour temporarily. * * @param string $lang New reading language. */ @@ -327,6 +329,29 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { return $result; } + /** + * @return bool + */ + public static function locale_filter_enabled() { + return self::$locale_filter_enabled; + } + + /** + * Enables automatic filtering by locale. This is normally called after is has been + * disabled using {@link disable_locale_filter()}. + */ + public static function enable_locale_filter() { + self::$locale_filter_enabled = true; + } + + /** + * Disables automatic locale filtering in {@link augmentSQL()}. This can be re-enabled + * using {@link enable_locale_filter()}. + */ + public static function disable_locale_filter() { + self::$locale_filter_enabled = false; + } + /** * Gets all translations for this specific page. * Doesn't include the language of the current record. @@ -491,7 +516,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { * It falls back to "Locale='' OR Lang IS NULL" and assumes that * this implies querying for the default language. * - * Use {@link $enable_lang_filter} to temporarily disable this "auto-filtering". + * Use {@link disable_locale_filter()} to temporarily disable this "auto-filtering". */ function augmentSQL(SQLQuery &$query) { // If the record is saved (and not a singleton), and has a locale, @@ -503,7 +528,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { if( $locale // unless the filter has been temporarily disabled - && self::$enable_lang_filter + && self::locale_filter_enabled() // DataObject::get_by_id() should work independently of language && !$query->filtersOnID() // the query contains this table @@ -786,27 +811,40 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { } /** - * Getter specifically for {@link SiteTree} subclasses - * which is hooked in to {@link SiteTree::get_by_url()}. - * Disables translatable to get the page independently - * of the current language setting. - * - * @param string $urlSegment - * @param string $extraFilter - * @param boolean $cache - * @param string|array $orderby - * @return DataObject + * Attempt to get the page for a link in the default language that has been translated. + * + * @param string $URLSegment + * @param int|null $parentID + * @return SiteTree */ - function alternateGetByUrl($urlSegment, $extraFilter, $cache = null, $orderby = null) { - $filter = sprintf("\"SiteTree\".\"URLSegment\" = '%s'", Convert::raw2sql($urlSegment)); - if($extraFilter) $filter .= " AND $extraFilter"; - self::$enable_lang_filter = false; - $record = DataObject::get_one('SiteTree', $filter); - self::$enable_lang_filter = true; - - return $record; + public function alternateGetByLink($URLSegment, $parentID) { + // If the parentID value has come from a translated page, then we need to find the corresponding parentID value + // in the default Locale. + if ( + is_int($parentID) + && $parentID > 0 + && ($parent = DataObject::get_by_id('SiteTree', $parentID)) + && ($parent->isTranslation()) + ) { + $parentID = $parent->getTranslationGroup(); + } + + // Find the locale language-independent of the page + self::disable_locale_filter(); + $default = DataObject::get_one ( + 'SiteTree', + sprintf ( + '"URLSegment" = \'%s\'%s', + Convert::raw2sql($URLSegment), + (is_int($parentID) ? " AND \"ParentID\" = $parentID" : null) + ), + false + ); + self::enable_locale_filter(); + + return $default; } - + //-----------------------------------------------------------------------------------------------// /** @@ -985,7 +1023,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { if($this->owner->exists()) { // HACK need to disable language filtering in augmentSQL(), // as we purposely want to get different language - self::$enable_lang_filter = false; + self::disable_locale_filter(); $translationGroupID = $this->getTranslationGroup(); @@ -1017,7 +1055,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { $translations = DataObject::get($this->owner->class, $filter, null, $join); } - self::$enable_lang_filter = true; + self::enable_locale_filter(); return $translations; } @@ -1219,9 +1257,21 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { } /** - * @todo + * Get the RelativeLink value for a home page in another locale. This is found by searching for the default home + * page in the default language, then returning the link to the translated version (if one exists). + * + * @return string */ public static function get_homepage_link_by_locale($locale) { + $originalLocale = self::get_current_locale(); + + self::set_current_locale(self::default_locale()); + $original = SiteTree::get_by_link(RootURLController::get_default_homepage_link()); + self::set_current_locale($originalLocale); + + if($original) { + if($translation = $original->getTranslation($locale)) return trim($translation->RelativeLink(true), '/'); + } } /** diff --git a/tests/model/TranslatableTest.php b/tests/model/TranslatableTest.php index 34a3a503c..1e73bae20 100755 --- a/tests/model/TranslatableTest.php +++ b/tests/model/TranslatableTest.php @@ -641,7 +641,7 @@ class TranslatableTest extends FunctionalTest { Translatable::set_current_locale('en_US'); } - function testRootUrlDefaultsToTranslatedUrlSegment() { + function testRootUrlDefaultsToTranslatedLink() { $origPage = $this->objFromFixture('Page', 'homepage_en'); $origPage->publish('Stage', 'Live'); $translationDe = $origPage->createTranslation('de_DE'); @@ -844,22 +844,64 @@ class TranslatableTest extends FunctionalTest { Translatable::set_current_locale($origLocale); } - public function testSiteTreeGetByUrlFindsTranslationWithoutLocale() { + public function testAlternateGetByLink() { + $parent = $this->objFromFixture('Page', 'parent'); + $child = $this->objFromFixture('Page', 'child1'); + $grandchild = $this->objFromFixture('Page', 'grandchild1'); + + $parentTranslation = $parent->createTranslation('en_AU'); + $parentTranslation->write(); + + $childTranslation = $child->createTranslation('en_AU'); + $childTranslation->write(); + + $grandchildTranslation = $grandchild->createTranslation('en_AU'); + $grandchildTranslation->write(); + + SiteTree::enable_nested_urls(); + Translatable::set_current_locale('en_AU'); + + $this->assertEquals ( + $parentTranslation->ID, + Sitetree::get_by_link($parentTranslation->Link())->ID, + 'Top level pages can be found.' + ); + + $this->assertEquals ( + $childTranslation->ID, + SiteTree::get_by_link($childTranslation->Link())->ID, + 'Child pages can be found.' + ); + + $this->assertEquals ( + $grandchildTranslation->ID, + SiteTree::get_by_link($grandchildTranslation->Link())->ID, + 'Grandchild pages can be found.' + ); + + $this->assertEquals ( + $childTranslation->ID, + SiteTree::get_by_link($parentTranslation->Link($child->URLSegment))->ID, + 'Links can be made up of multiple languages' + ); + } + + public function testSiteTreeGetByLinkFindsTranslationWithoutLocale() { $parent = $this->objFromFixture('Page', 'parent'); $parentTranslation = $parent->createTranslation('en_AU'); $parentTranslation->URLSegment = 'parent-en-AU'; $parentTranslation->write(); - $match = Sitetree::get_by_url($parentTranslation->URLSegment); + $match = Sitetree::get_by_link($parentTranslation->URLSegment); $this->assertNotNull( $match, - 'SiteTree::get_by_url() doesnt need a locale setting to find translated pages' + 'SiteTree::get_by_link() doesnt need a locale setting to find translated pages' ); $this->assertEquals( $parentTranslation->ID, $match->ID, - 'SiteTree::get_by_url() doesnt need a locale setting to find translated pages' + 'SiteTree::get_by_link() doesnt need a locale setting to find translated pages' ); } }