Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

BUG: Delete parent page, child pages cannot be found anymore #1056

Merged
merged 1 commit into from Jul 25, 2014

Conversation

Projects
None yet
2 participants
Contributor

stojg commented Jul 21, 2014

This PR fixes a bug by making sure that CMSSiteTreeFilters passes the correct method for
counting the number of children into SiteTree::CMSTreeClasses() so the correct CSS / JS
classes is set so the they can be opened in the UI.

PR for framework silverstripe/silverstripe-framework#3309

Issue link: #1049

@stojg stojg referenced this pull request in silverstripe/silverstripe-framework Jul 21, 2014

Merged

Delete parent page, child pages cannot be found anymore #3309

@stojg stojg commented on the diff Jul 24, 2014

code/controllers/CMSSiteTreeFilter.php
@@ -101,17 +114,13 @@ protected function populateIDs() {
}
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');
@stojg

stojg Jul 24, 2014

Contributor

I'm unsure if this is a performance killer, or by casting it to map it actually doesn't create a new instance of sitetree.

@stojg stojg commented on the diff Jul 24, 2014

code/model/SiteTree.php
* @return string
*/
- public function CMSTreeClasses() {
+ public function CMSTreeClasses($numChildrenMethod="numChildren") {
@stojg

stojg Jul 24, 2014

Contributor

The only reason for passing this is so that it can pass it to markingClasses.. Not great, but I rather do this than relying on some internal state.

@stojg stojg Bug: CMS tree filters doesn't count the correct number of children fo…
…r deleted pages

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
45046f0
Contributor

tractorcow commented Jul 25, 2014

Merged framework PR. I've restarted the build to check this against the framework updates.

https://travis-ci.org/silverstripe/silverstripe-cms/builds/30811258

Contributor

tractorcow commented Jul 25, 2014

Too soon, it looks like the tests are running against the cached version of the framework (prior to the merge). Oh well.

@tractorcow tractorcow added a commit that referenced this pull request Jul 25, 2014

@tractorcow tractorcow Merge pull request #1056 from stojg/issue/cms-1049
BUG: Delete parent page, child pages cannot be found anymore
4967d3d

@tractorcow tractorcow merged commit 4967d3d into silverstripe:3.1 Jul 25, 2014

@stojg stojg deleted the stojg:issue/cms-1049 branch Jul 26, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment