ENHANCEMENT Redirecting to translated page when original is requested with a 'locale' GET parameter (e.g. 'about-us/?locale=de_DE' will redirect to 'ueber-uns' with a 301 HTTP response). Implemented in ContentController->handleRequest(). (see #5001)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@100937 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/control/ContentController.php

@@ -142,12 +142,11 @@ class ContentController extends Controller {
 		// control to a child controller. This allows for the creation of chains of controllers which correspond to a
 		// nested URL.
 		if($action && SiteTree::nested_urls() && !$this->hasAction($action)) {
+			// See ModelAdController->getNestedController() for similar logic
 			Translatable::disable_locale_filter();
-			
 			$child = DataObject::get_one('SiteTree', sprintf (
 				"\"ParentID\" = %s AND \"URLSegment\" = '%s'", $this->ID, Convert::raw2sql($action)
 			));
-			
 			Translatable::enable_locale_filter();
 		}
 		
@@ -157,6 +156,18 @@ class ContentController extends Controller {
 			
 			$response = ModelAsController::controller_for($child)->handleRequest($request);
 		} else {
+			// If a specific locale is requested, and it doesn't match the page found by URLSegment,
+			// look for a translation and redirect (see #5001). Only happens on the last child in
+			// a potentially nested URL chain.
+			if($request->getVar('locale') && $this->dataRecord && $this->dataRecord->Locale != $request->getVar('locale')) {
+				$translation = $this->dataRecord->getTranslation($request->getVar('locale'));
+				if($translation) {
+					$response = new SS_HTTPResponse();
+					$response->redirect($translation->Link(), 301);
+					throw new SS_HTTPResponse_Exception($response);
+				}
+			}
+			
 			Director::set_current_page($this->data());
 			$response = parent::handleRequest($request);
 			Director::set_current_page(null);

core/control/ModelAsController.php

@@ -87,12 +87,16 @@ class ModelAsController extends Controller implements NestedController {
 			throw new Exception('ModelAsController->getNestedController(): was not passed a URLSegment value.');
 		}
 		
+		// Find page by link, regardless of current locale settings
 		Translatable::disable_locale_filter();
-		
-		$sitetree = DataObject::get_one('SiteTree', sprintf (
-			'"URLSegment" = \'%s\' %s', Convert::raw2sql($URLSegment), (SiteTree::nested_urls() ? 'AND "ParentID" = 0' : null)
-		));
-		
+		$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) {
@@ -113,6 +117,7 @@ class ModelAsController extends Controller implements NestedController {
 			}
 		}
 		
+		// Enforce current locale setting to the loaded SiteTree object
 		if($sitetree->Locale) Translatable::set_current_locale($sitetree->Locale);
 		
 		if(isset($_REQUEST['debug'])) {

core/model/Translatable.php

@@ -245,7 +245,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider {
 		} else {
 			self::set_current_locale(self::default_locale());
 		}
-		
+
 		return self::$current_locale; 
 	}
 		
@@ -728,9 +728,12 @@ class Translatable extends DataObjectDecorator implements PermissionProvider {
 		return false;
 	}
 
+	/**
+	 * Note: The bulk of logic is in ModelAsController->getNestedController()
+	 * and ContentController->handleRequest()
+	 */
 	function contentcontrollerInit($controller) {
-		Translatable::choose_site_locale();
-		$controller->Locale = Translatable::get_current_locale();
+		$controller->Locale = Translatable::choose_site_locale();
 	}
 	
 	function modelascontrollerInit($controller) {

tests/model/TranslatableTest.php

@@ -21,6 +21,8 @@ class TranslatableTest extends FunctionalTest {
 	);
 	
 	private $origLocale;
+	
+	protected $autoFollowRedirection = false;
 
 	function setUp() {
 		parent::setUp();
@@ -39,6 +41,25 @@ class TranslatableTest extends FunctionalTest {
 
 		parent::tearDown();
 	}
+	
+	function testLocaleGetParamRedirectsToTranslation() {
+		$origPage = $this->objFromFixture('Page', 'testpage_en');
+		$origPage->publish('Stage', 'Live');
+		$translatedPage = $origPage->createTranslation('de_DE');
+		$translatedPage->URLSegment = 'ueber-uns';
+		$translatedPage->write();
+		$translatedPage->publish('Stage', 'Live');
+		
+		$response = $this->get($origPage->URLSegment);
+		$this->assertEquals(200, $response->getStatusCode(), 'Page request without Locale GET param doesnt redirect');
+		
+		$response = $this->get(Controller::join_links($origPage->URLSegment, '?locale=de_DE'));
+		$this->assertEquals(301, $response->getStatusCode(), 'Locale GET param causes redirect if it exists');
+		$this->assertContains($translatedPage->URLSegment, $response->getHeader('Location'));
+		
+		$response = $this->get(Controller::join_links($origPage->URLSegment, '?locale=xx_XX'));
+		$this->assertEquals(200, $response->getStatusCode(), 'Locale GET param without existing translation shows original page');
+	}
 
 	function testTranslationGroups() {
 		// first in french
@@ -174,7 +195,7 @@ class TranslatableTest extends FunctionalTest {
 		$origPage = $this->objFromFixture('Page', 'testpage_en');
 		$translatedPage = $origPage->createTranslation('de_DE');
 		$this->assertEquals($translatedPage->URLSegment, 'testpage-de-DE');
-
+	
 		$translatedPage->URLSegment = 'testpage';
 		$translatedPage->write();
 	
@@ -692,7 +713,7 @@ class TranslatableTest extends FunctionalTest {
 			$enPage = new $type();
 			$enPage->Locale = 'en_US';
 			$enPage->write();
-
+	
 			$dePage = $enPage->createTranslation('de_DE');
 			$dePage->write();
 			$this->assertEquals('de_DE', $dePage->Locale, "Page type $type retains Locale property");
@@ -875,7 +896,7 @@ class TranslatableTest extends FunctionalTest {
 			SiteTree::get_by_link($grandchildTranslation->Link())->ID,
 			'Grandchild pages can be found.'
 		);
-
+	
 		// TODO Re-enable test after clarifying with ajshort (see r88503).
 		// Its unclear if this is valid behaviour, and/or necessary for translated nested URLs
 		// to work properly