tonyair/silverstripe-reports
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-reports synced 2024-10-22 11:05:53 +02:00
Code Issues Projects Releases Wiki Activity

Bug: CMS tree filters doesn't count the correct number of children for deleted pages

Browse Source 
This is a bug that combines Hierarchy, Versioned and LeftAndMain admins and CMSSiteTreeFilters.

This bug can be reproduced by having a large site tree with enough deleted pages in it so it doesn't
pre load all the children pages when initially opening an admin. Filter by either 'All pages including deleted'
or 'Deleted pages'. For CMS users it will look like deleted pages are gone.

The solution involves a couple of smaller fixes in both CMS and framework modules.

1) Ensure that 'numHistoricalChildren' are used instead of 'numChildren' when dealing with deleted pages
2) LeftAndMain::currentPage() deletes all the 'marking' cache previously built up by Hierarchy::markPartialTree()
3) Use Versioned::get_included_deleted() instead of raw DB queries against the DataObject tables when calculating parents in CMSSiteTreeFilter
This commit is contained in:
Stig Lindqvist 2014-07-20 11:46:15 +12:00
parent 6a0366fb84
commit 45046f08e8
3 changed files with 53 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
code/controllers/CMSSiteTreeFilter.php
View File

@ -36,6 +36,11 @@ abstract class CMSSiteTreeFilter extends Object {
*/
protected $childrenMethod = null;
/**
* @var string
*/
protected $numChildrenMethod = 'numChildren';
/**
* Returns a sorted array of all implementators of CMSSiteTreeFilter, suitable for use in a dropdown.
*
@ -71,13 +76,21 @@ abstract class CMSSiteTreeFilter extends Object {
}
/**
* @return String Method on {@link Hierarchy} objects
* which is used to traverse into children relationships.
* Method on {@link Hierarchy} objects which is used to traverse into children relationships.
*
* @return String
*/
public function getChildrenMethod() {
return $this->childrenMethod;
}
/**
* Method on {@link Hierarchy} objects which is used find the number of children for a parent page
*/
public function getNumChildrenMethod() {
return $this->numChildrenMethod;
}
/**
* @return Array Map of Page IDs to their respective ParentID values.
*/
@ -101,17 +114,13 @@ abstract class CMSSiteTreeFilter extends Object {
}
while(!empty($parents)) {
$q = new SQLQuery();
$q->setSelect(array('"ID"','"ParentID"'))
->setFrom('"SiteTree"')
->setWhere('"ID" in ('.implode(',',array_keys($parents)).')');
$q = Versioned::get_including_deleted('SiteTree', '"RecordID" in ('.implode(',',array_keys($parents)).')');
$list = $q->map('ID', 'ParentID');
$parents = array();
foreach($q->execute() as $row) {
if ($row['ParentID']) $parents[$row['ParentID']] = true;
$this->_cache_ids[$row['ID']] = true;
$this->_cache_expanded[$row['ID']] = true;
foreach($list as $id => $parentID) {
if ($parentID) $parents[$parentID] = true;
$this->_cache_ids[$id] = true;
$this->_cache_expanded[$id] = true;
}
}
}
@ -203,8 +212,16 @@ abstract class CMSSiteTreeFilter extends Object {
*/
class CMSSiteTreeFilter_DeletedPages extends CMSSiteTreeFilter {
/**
* @var string
*/
protected $childrenMethod = "AllHistoricalChildren";
/**
* @var string
*/
protected $numChildrenMethod = 'numHistoricalChildren';
static public function title() {
return _t('CMSSiteTreeFilter_DeletedPages.Title', "All pages, including deleted");
}
@ -303,8 +320,16 @@ class CMSSiteTreeFilter_StatusDraftPages extends CMSSiteTreeFilter {
*/
class CMSSiteTreeFilter_StatusDeletedPages extends CMSSiteTreeFilter {
/**
* @var string
*/
protected $childrenMethod = "AllHistoricalChildren";
/**
* @var string
*/
protected $numChildrenMethod = 'numHistoricalChildren';
static public function title() {
return _t('CMSSiteTreeFilter_StatusDeletedPages.Title', 'Deleted pages');
}
@ -318,6 +343,7 @@ class CMSSiteTreeFilter_StatusDeletedPages extends CMSSiteTreeFilter {
public function pagesIncluded() {
$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;

5
code/model/SiteTree.php
View File

@ -2736,9 +2736,10 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
/**
* Return the CSS classes to apply to this node in the CMS tree
*
* @param string $numChildrenMethod
* @return string
*/
public function CMSTreeClasses() {
public function CMSTreeClasses($numChildrenMethod="numChildren") {
$classes = sprintf('class-%s', $this->class);
if($this->HasBrokenFile || $this->HasBrokenLink) {
$classes .= " BrokenLink";
@ -2765,7 +2766,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
if($this->hasExtension('Translatable') && $controller->Locale != Translatable::default_locale() && !$this->isTranslation())
$classes .= " untranslated ";
*/
$classes .= $this->markingClasses();
$classes .= $this->markingClasses($numChildrenMethod);
return $classes;
}

8
code/model/VirtualPage.php
View File

@ -387,8 +387,12 @@ class VirtualPage extends Page {
$this->ImageTracking()->setByIdList($this->CopyContentFrom()->ImageTracking()->column('ID'));
}
public function CMSTreeClasses() {
return parent::CMSTreeClasses() . ' VirtualPage-' . $this->CopyContentFrom()->ClassName;
/**
* @param string $numChildrenMethod
* @return string
*/
public function CMSTreeClasses($numChildrenMethod="numChildren") {
return parent::CMSTreeClasses($numChildrenMethod) . ' VirtualPage-' . $this->CopyContentFrom()->ClassName;
}
/**