diff --git a/code/controllers/OldPageRedirector.php b/code/controllers/OldPageRedirector.php index bac62316..6ab6e997 100644 --- a/code/controllers/OldPageRedirector.php +++ b/code/controllers/OldPageRedirector.php @@ -38,43 +38,38 @@ class OldPageRedirector extends Extension { * Attempt to find an old/renamed page from some given the URL as an array * * @param array $params The array of URL, e.g. /foo/bar as array('foo', 'bar') - * @param SiteTree $parent The current parent in the recursive flow + * @param SiteTree|null $parent The current parent in the recursive flow * @param boolean $redirect Whether we've found an old page worthy of a redirect * * @return string|boolean False, or the new URL */ static public function find_old_page($params, $parent = null, $redirect = false) { - $parent = is_numeric($parent) ? SiteTree::get()->byId($parent) : $parent; + $parent = is_numeric($parent) ? SiteTree::get()->byID($parent) : $parent; $params = (array)$params; $URL = rawurlencode(array_shift($params)); if (empty($URL)) { return false; } + $pages = SiteTree::get()->filter(array( + 'URLSegment' => $URL, + )); if ($parent) { - $page = SiteTree::get()->filter(array('ParentID' => $parent->ID, 'URLSegment' => $URL))->First(); - } else { - $page = SiteTree::get()->filter(array('URLSegment' => $URL))->First(); + $pages = $pages->filter(array( + 'ParentID' => $parent->ID, + )); } + $page = $pages->first(); if (!$page) { // If we haven't found a candidate, lets resort to finding an old page with this URL segment - $oldFilter = array( - '"SiteTree_versions"."URLSegment"' => $URL, - '"SiteTree_versions"."WasPublished"' => true - ); - if($parent) { - $oldFilter[] = array('"SiteTree_versions"."ParentID"' => $parent->ID); - } - $query = new SQLSelect( - '"RecordID"', - '"SiteTree_versions"', - $oldFilter, - '"LastEdited" DESC', - null, - null, - 1 - ); - $record = $query->execute()->first(); + $pages = $pages + ->filter(array( + 'WasPublished' => true, + )) + ->sort('LastEdited', 'DESC') + ->setDataQueryParam("Versioned.mode", 'all_versions'); + + $record = $pages->first(); if ($record) { - $page = SiteTree::get()->byID($record['RecordID']); + $page = SiteTree::get()->byID($record->RecordID); $redirect = true; } } diff --git a/tests/controller/ModelAsControllerTest.php b/tests/controller/ModelAsControllerTest.php index 593044b7..1da33ffb 100644 --- a/tests/controller/ModelAsControllerTest.php +++ b/tests/controller/ModelAsControllerTest.php @@ -4,14 +4,12 @@ * @subpackage tests */ class ModelAsControllerTest extends FunctionalTest { - - protected $usesDatabase = true; - - protected static $fixture_file = 'ModelAsControllerTest.yml'; - - protected $autoFollowRedirection = false; - protected $orig = array(); + protected $usesDatabase = true; + + protected static $fixture_file = 'ModelAsControllerTest.yml'; + + protected $autoFollowRedirection = false; /** * New tests require nested urls to be enabled, but the site might not @@ -22,24 +20,9 @@ class ModelAsControllerTest extends FunctionalTest { public function setUp() { parent::setUp(); - $this->orig['nested_urls'] = SiteTree::config()->nested_urls; Config::inst()->update('SiteTree', 'nested_urls', true); } - /** - * New tests require nested urls to be enabled, but the site might not - * support nested URLs. - * This setup will enable nested-urls for this test and resets the state - * after the tests have been performed. - */ - public function tearDown() { - - if (isset($this->orig['nested_urls']) && !$this->orig['nested_urls']) { - SiteTree::config()->nested_urls = false; - } - parent::tearDown(); - } - protected function generateNestedPagesFixture() { $level1 = new Page(); @@ -47,34 +30,34 @@ class ModelAsControllerTest extends FunctionalTest { $level1->URLSegment = 'level1'; $level1->write(); $level1->publish('Stage', 'Live'); - + $level1->URLSegment = 'newlevel1'; $level1->write(); $level1->publish('Stage', 'Live'); - + $level2 = new Page(); $level2->Title = 'Second Level'; $level2->URLSegment = 'level2'; $level2->ParentID = $level1->ID; $level2->write(); $level2->publish('Stage', 'Live'); - + $level2->URLSegment = 'newlevel2'; $level2->write(); $level2->publish('Stage', 'Live'); - + $level3 = New Page(); $level3->Title = "Level 3"; $level3->URLSegment = 'level3'; $level3->ParentID = $level2->ID; $level3->write(); $level3->publish('Stage','Live'); - + $level3->URLSegment = 'newlevel3'; $level3->write(); $level3->publish('Stage','Live'); } - + /** * We're building up a page hierarchy ("nested URLs") and rename * all the individual pages afterwards. The assumption is that @@ -87,7 +70,7 @@ class ModelAsControllerTest extends FunctionalTest { */ public function testRedirectsNestedRenamedPages(){ $this->generateNestedPagesFixture(); - + // check a first level URLSegment $response = $this->get('level1/action'); $this->assertEquals($response->getStatusCode(),301); @@ -95,7 +78,7 @@ class ModelAsControllerTest extends FunctionalTest { Controller::join_links(Director::baseURL() . 'newlevel1/action'), $response->getHeader('Location') ); - + // check second level URLSegment $response = $this->get('newlevel1/level2'); $this->assertEquals($response->getStatusCode(),301 ); @@ -103,7 +86,7 @@ class ModelAsControllerTest extends FunctionalTest { Controller::join_links(Director::baseURL() . 'newlevel1/newlevel2/'), $response->getHeader('Location') ); - + // check third level URLSegment $response = $this->get('newlevel1/newlevel2/level3'); $this->assertEquals($response->getStatusCode(), 301); @@ -111,7 +94,7 @@ class ModelAsControllerTest extends FunctionalTest { Controller::join_links(Director::baseURL() . 'newlevel1/newlevel2/newlevel3/'), $response->getHeader('Location') ); - + $response = $this->get('newlevel1/newlevel2/level3'); } @@ -126,7 +109,7 @@ class ModelAsControllerTest extends FunctionalTest { $page->URLSegment = 'oldurl'; $page->write(); $page->publish('Stage', 'Live'); - + $page->URLSegment = 'newurl'; $page->write(); $page->publish('Stage', 'Live'); @@ -137,21 +120,21 @@ class ModelAsControllerTest extends FunctionalTest { $page2->ParentID = $page->ID; $page2->write(); $page2->publish('Stage', 'Live'); - + $page3 = new Page(); $page3->Title = 'Third Level Page'; $page3->URLSegment = 'level3'; $page3->ParentID = $page2->ID; $page3->write(); $page3->publish('Stage', 'Live'); - + $page4 = new Page(); $page4->Title = 'Fourth Level Page'; $page4->URLSegment = 'level4'; $page4->ParentID = $page3->ID; $page4->write(); $page4->publish('Stage', 'Live'); - + $page5 = new Page(); $page5->Title = 'Fifth Level Page'; $page5->URLSegment = 'level5'; @@ -187,13 +170,13 @@ class ModelAsControllerTest extends FunctionalTest { public function testDoesntRedirectToNestedChildrenOutsideOfOwnHierarchy() { $this->generateNestedPagesFixture(); - + $otherParent = new Page(array( 'URLSegment' => 'otherparent' )); $otherParent->write(); $otherParent->publish('Stage', 'Live'); - + $response = $this->get('level1/otherparent'); $this->assertEquals($response->getStatusCode(), 301); @@ -204,7 +187,7 @@ class ModelAsControllerTest extends FunctionalTest { 'Requesting an unrelated page on a renamed parent should be interpreted as a missing action, not a redirect' ); } - + /** * * NOTE: This test requires nested_urls @@ -212,7 +195,7 @@ class ModelAsControllerTest extends FunctionalTest { */ public function testRedirectsNestedRenamedPagesWithGetParameters() { $this->generateNestedPagesFixture(); - + // check third level URLSegment $response = $this->get('newlevel1/newlevel2/level3/?foo=bar&test=test'); $this->assertEquals($response->getStatusCode(), 301); @@ -221,7 +204,7 @@ class ModelAsControllerTest extends FunctionalTest { $response->getHeader('Location') ); } - + /** * * NOTE: This test requires nested_urls @@ -229,20 +212,20 @@ class ModelAsControllerTest extends FunctionalTest { */ public function testDoesntRedirectToNestedRenamedPageWhenNewExists() { $this->generateNestedPagesFixture(); - + $otherLevel1 = new Page(array( 'Title' => "Other Level 1", 'URLSegment' => 'level1' )); $otherLevel1->write(); $otherLevel1->publish('Stage', 'Live'); - + $response = $this->get('level1'); $this->assertEquals( $response->getStatusCode(), 200 ); - + $response = $this->get('level1/newlevel2'); $this->assertEquals( $response->getStatusCode(), @@ -250,7 +233,7 @@ class ModelAsControllerTest extends FunctionalTest { 'The old newlevel2/ URLSegment is checked as an action on the new page, which shouldnt exist.' ); } - + /** * * NOTE: This test requires nested_urls @@ -262,30 +245,30 @@ class ModelAsControllerTest extends FunctionalTest { $page->URLSegment = 'oldurl'; $page->write(); $page->publish('Stage', 'Live'); - + $page->URLSegment = 'newurl'; $page->write(); $page->publish('Stage', 'Live'); - + $url = OldPageRedirector::find_old_page('oldurl'); $matchedPage = SiteTree::get_by_link($url); $this->assertEquals('First Level',$matchedPage->Title); - + $page2 = new Page(); $page2->Title = 'Second Level Page'; $page2->URLSegment = 'oldpage2'; $page2->ParentID = $page->ID; $page2->write(); $page2->publish('Stage', 'Live'); - + $page2->URLSegment = 'newpage2'; $page2->write(); $page2->publish('Stage', 'Live'); - + $url = OldPageRedirector::find_old_page('oldpage2',$page2->ParentID); $matchedPage = SiteTree::get_by_link($url); $this->assertEquals('Second Level Page',$matchedPage->Title); - + $url = OldPageRedirector::find_old_page('oldpage2',$page2->ID); $matchedPage = SiteTree::get_by_link($url); $this->assertEquals(false, $matchedPage);