Permalink
Browse files

BUG Fix CMSMain::getList to correctly respect filter result

Fixes #1064
CMSSiteTreeFilter refactored to allow SS_List of filtered pages to be returned
  • Loading branch information...
1 parent 84fcf02 commit 53dbbb7982429dfd75c5d7ae265087fc54dcc04b @tractorcow tractorcow committed Aug 6, 2014
Showing with 142 additions and 43 deletions.
  1. +25 −20 code/controllers/CMSMain.php
  2. +31 −23 code/controllers/CMSSiteTreeFilter.php
  3. +86 −0 tests/controller/CMSMainTest.php
View
45 code/controllers/CMSMain.php
@@ -698,34 +698,39 @@ public function listview($request) {
}
/**
+ * Safely reconstruct a selected filter from a given set of query parameters
+ *
+ * @param array $params Query parameters to use
+ * @return CMSSiteTreeFilter The filter class, or null if none present
+ * @throws InvalidArgumentException if invalid filter class is passed.
+ */
+ protected function getQueryFilter($params) {
+ if(empty($params['FilterClass'])) return null;
+ $filterClass = $params['FilterClass'];
+ if(!is_subclass_of($filterClass, 'CMSSiteTreeFilter')) {
+ throw new InvalidArgumentException("Invalid filter class passed: {$filterClass}");
+ }
+ return $filterClass::create($params);
+ }
+
+ /**
* Returns the pages meet a certain criteria as {@see CMSSiteTreeFilter} or the subpages of a parent page
* defaulting to no filter and show all pages in first level.
* Doubles as search results, if any search parameters are set through {@link SearchForm()}.
*
- * @param Array $params Search filter criteria
- * @param Int $parentID Optional parent node to filter on (can't be combined with other search criteria)
+ * @param array $params Search filter criteria
+ * @param int $parentID Optional parent node to filter on (can't be combined with other search criteria)
* @return SS_List
- * @throws Exception if invalid filter class is passed.
+ * @throws InvalidArgumentException if invalid filter class is passed.
*/
- public function getList($params, $parentID = 0) {
- $list = new DataList($this->stat('tree_class'));
- $filter = null;
- $ids = array();
- if(isset($params['FilterClass']) && $filterClass = $params['FilterClass']){
- if(!is_subclass_of($filterClass, 'CMSSiteTreeFilter')) {
- throw new Exception(sprintf('Invalid filter class passed: %s', $filterClass));
- }
- $filter = new $filterClass($params);
- $filterOn = true;
- foreach($pages=$filter->pagesIncluded() as $pageMap){
- $ids[] = $pageMap['ID'];
- }
- if(count($ids)) $list = $list->where('"'.$this->stat('tree_class').'"."ID" IN ('.implode(",", $ids).')');
+ public function getList($params = array(), $parentID = 0) {
+ if($filter = $this->getQueryFilter($params)) {
+ return $filter->getFilteredPages();
} else {
- $list = $list->filter("ParentID", is_numeric($parentID) ? $parentID : 0);
+ $list = DataList::create($this->stat('tree_class'));
+ $parentID = is_numeric($parentID) ? $parentID : 0;
+ return $list->filter("ParentID", $parentID);
}
-
- return $list;
}
public function ListViewForm() {
View
54 code/controllers/CMSSiteTreeFilter.php
@@ -92,9 +92,19 @@ public function getNumChildrenMethod() {
}
/**
- * @return Array Map of Page IDs to their respective ParentID values.
+ * Gets the list of filtered pages
+ *
+ * @see {@link SiteTree::getStatusFlags()}
+ * @return SS_List
+ */
+ abstract public function getFilteredPages();
+
+ /**
+ * @return array Map of Page IDs to their respective ParentID values.
*/
- public function pagesIncluded() {}
+ public function pagesIncluded() {
+ return $this->mapIDs($this->getFilteredPages());
+ }
/**
* Populate the IDs of the pages returned by pagesIncluded(), also including
@@ -231,15 +241,15 @@ static public function title() {
* Filters out all pages who's status who's status that doesn't exist on live
*
* @see {@link SiteTree::getStatusFlags()}
- * @return array
+ * @return SS_List
*/
- public function pagesIncluded() {
+ public function getFilteredPages() {
$pages = Versioned::get_including_deleted('SiteTree');
$pages = $this->applyDefaultFilters($pages);
$pages = $pages->filterByCallback(function($page) {
return $page->ExistsOnLive;
});
- return $this->mapIDs($pages);
+ return $pages;
}
}
@@ -267,10 +277,10 @@ static public function title() {
return _t('CMSSiteTreeFilter_DeletedPages.Title', "All pages, including deleted");
}
- public function pagesIncluded() {
+ public function getFilteredPages() {
$pages = Versioned::get_including_deleted('SiteTree');
$pages = $this->applyDefaultFilters($pages);
- return $this->mapIDs($pages);
+ return $pages;
}
}
@@ -286,12 +296,12 @@ static public function title() {
return _t('CMSSiteTreeFilter_ChangedPages.Title', "Changed pages");
}
- public function pagesIncluded() {
+ public function getFilteredPages() {
$pages = Versioned::get_by_stage('SiteTree', 'Stage');
$pages = $this->applyDefaultFilters($pages)
->leftJoin('SiteTree_Live', '"SiteTree_Live"."ID" = "SiteTree"."ID"')
->where('"SiteTree"."Version" > "SiteTree_Live"."Version"');
- return $this->mapIDs($pages);
+ return $pages;
}
}
@@ -310,17 +320,16 @@ static public function title() {
/**
* Filters out all pages who's status is set to "Removed from draft".
*
- * @see {@link SiteTree::getStatusFlags()}
- * @return array
+ * @return SS_List
*/
- public function pagesIncluded() {
+ public function getFilteredPages() {
$pages = Versioned::get_including_deleted('SiteTree');
$pages = $this->applyDefaultFilters($pages);
$pages = $pages->filterByCallback(function($page) {
// If page is removed from stage but not live
return $page->IsDeletedFromStage && $page->ExistsOnLive;
});
- return $this->mapIDs($pages);
+ return $pages;
}
}
@@ -340,16 +349,16 @@ static public function title() {
* Filters out all pages who's status is set to "Draft".
*
* @see {@link SiteTree::getStatusFlags()}
- * @return array
+ * @return SS_List
*/
- public function pagesIncluded() {
+ public function getFilteredPages() {
$pages = Versioned::get_by_stage('SiteTree', 'Stage');
$pages = $this->applyDefaultFilters($pages);
$pages = $pages->filterByCallback(function($page) {
// If page exists on stage but not on live
return (!$page->IsDeletedFromStage && $page->IsAddedToStage);
});
- return $this->mapIDs($pages);
+ return $pages;
}
}
@@ -379,17 +388,17 @@ static public function title() {
* Filters out all pages who's status is set to "Deleted".
*
* @see {@link SiteTree::getStatusFlags()}
- * @return array
+ * @return SS_List
*/
- public function pagesIncluded() {
+ public function getFilteredPages() {
$pages = Versioned::get_including_deleted('SiteTree');
$pages = $this->applyDefaultFilters($pages);
$pages = $pages->filterByCallback(function($page) {
// Doesn't exist on either stage or live
return $page->IsDeletedFromStage && !$page->ExistsOnLive;
});
- return $this->mapIDs($pages);
+ return $pages;
}
}
@@ -407,13 +416,12 @@ static public function title() {
* Retun an array of maps containing the keys, 'ID' and 'ParentID' for each page to be displayed
* in the search.
*
- * @return Array
+ * @return SS_List
*/
- public function pagesIncluded() {
-
+ public function getFilteredPages() {
// Filter default records
$pages = Versioned::get_by_stage('SiteTree', 'Stage');
$pages = $this->applyDefaultFilters($pages);
- return $this->mapIDs($pages);
+ return $pages;
}
}
View
86 tests/controller/CMSMainTest.php
@@ -349,6 +349,92 @@ public function testGetNewItem() {
$this->assertEquals($controller->getResponse()->getStatusCode(), 302);
}
}
+
+ /**
+ * Tests filtering in {@see CMSMain::getList()}
+ */
+ public function testGetList() {
+ $controller = new CMSMain();
+
+ // Test all pages (stage)
+ $pages = $controller->getList()->sort('Title');
+ $this->assertEquals(28, $pages->count());
+ $this->assertEquals(
+ array('Home', 'Page 1', 'Page 10', 'Page 11', 'Page 12'),
+ $pages->Limit(5)->column('Title')
+ );
+
+ // Change state of tree
+ $page1 = $this->objFromFixture('Page', 'page1');
+ $page3 = $this->objFromFixture('Page', 'page3');
+ $page11 = $this->objFromFixture('Page', 'page11');
+ $page12 = $this->objFromFixture('Page', 'page12');
+ // Deleted
+ $page1->doUnpublish();
+ $page1->delete();
+ // Live and draft
+ $page11->publish('Stage', 'Live');
+ // Live only
+ $page12->publish('Stage', 'Live');
+ $page12->delete();
+
+ // Re-test all pages (stage)
+ $pages = $controller->getList()->sort('Title');
+ $this->assertEquals(26, $pages->count());
+ $this->assertEquals(
+ array('Home', 'Page 10', 'Page 11', 'Page 13', 'Page 14'),
+ $pages->Limit(5)->column('Title')
+ );
+
+ // Test deleted page filter
+ $params = array(
+ 'FilterClass' => 'CMSSiteTreeFilter_StatusDeletedPages'
+ );
+ $pages = $controller->getList($params);
+ $this->assertEquals(1, $pages->count());
+ $this->assertEquals(
+ array('Page 1'),
+ $pages->column('Title')
+ );
+
+ // Test live, but not on draft filter
+ $params = array(
+ 'FilterClass' => 'CMSSiteTreeFilter_StatusRemovedFromDraftPages'
+ );
+ $pages = $controller->getList($params);
+ $this->assertEquals(1, $pages->count());
+ $this->assertEquals(
+ array('Page 12'),
+ $pages->column('Title')
+ );
+
+ // Test live pages filter
+ $params = array(
+ 'FilterClass' => 'CMSSIteTreeFilter_PublishedPages'
+ );
+ $pages = $controller->getList($params);
+ $this->assertEquals(2, $pages->count());
+ $this->assertEquals(
+ array('Page 11', 'Page 12'),
+ $pages->column('Title')
+ );
+
+ // Test that parentID is ignored when filtering
+ $pages = $controller->getList($params, $page3->ID);
+ $this->assertEquals(2, $pages->count());
+ $this->assertEquals(
+ array('Page 11', 'Page 12'),
+ $pages->column('Title')
+ );
+
+ // Test that parentID is respected when not filtering
+ $pages = $controller->getList(array(), $page3->ID);
+ $this->assertEquals(2, $pages->count());
+ $this->assertEquals(
+ array('Page 3.1', 'Page 3.2'),
+ $pages->column('Title')
+ );
+ }
}
class CMSMainTest_ClassA extends Page implements TestOnly {

0 comments on commit 53dbbb7

Please sign in to comment.