diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index ac148af5e..a3a3871b3 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -691,7 +691,7 @@ class LeftAndMain extends Controller implements PermissionProvider { * @return String Nested unordered list with links to each page */ public function getSiteTreeFor($className, $rootID = null, $childrenMethod = null, $numChildrenMethod = null, - $filterFunction = null, $minNodeCount = 30) { + $filterFunction = null, $nodeCountThreshold = 30) { // Filter criteria $params = $this->request->getVar('q'); @@ -719,7 +719,7 @@ class LeftAndMain extends Controller implements PermissionProvider { // Mark the nodes of the tree to return if ($filterFunction) $obj->setMarkingFilterFunction($filterFunction); - $obj->markPartialTree($minNodeCount, $this, $childrenMethod, $numChildrenMethod); + $obj->markPartialTree($nodeCountThreshold, $this, $childrenMethod, $numChildrenMethod); // Ensure current page is exposed if($p = $this->currentPage()) $obj->markToExpose($p); @@ -744,7 +744,7 @@ class LeftAndMain extends Controller implements PermissionProvider { true, $childrenMethod, $numChildrenMethod, - $minNodeCount + $nodeCountThreshold ); // Wrap the root if needs be. diff --git a/model/Hierarchy.php b/model/Hierarchy.php index 4fa8f98d9..4374d5960 100644 --- a/model/Hierarchy.php +++ b/model/Hierarchy.php @@ -74,12 +74,17 @@ class Hierarchy extends DataExtension { * @param string $childrenMethod The name of the method used to get children from each object * @param boolean $rootCall Set to true for this first call, and then to false for calls inside the recursion. You * should not change this. - * @param int $minNodeCount + * @param int $nodeCountThreshold The lower bounds for the amount of nodes to mark. If set, the logic will expand + * nodes until it eaches at least this number, and then stops. Root nodes will always + * show regardless of this settting. Further nodes can be lazy-loaded via ajax. + * This isn't a hard limit. Example: On a value of 10, with 20 root nodes, each having + * 30 children, the actual node count will be 50 (all root nodes plus first expanded child). + * * @return string */ public function getChildrenAsUL($attributes = "", $titleEval = '"
  • " . $child->Title', $extraArg = null, $limitToMarked = false, $childrenMethod = "AllChildrenIncludingDeleted", - $numChildrenMethod = "numChildren", $rootCall = true, $minNodeCount = 30) { + $numChildrenMethod = "numChildren", $rootCall = true, $nodeCountThreshold = 30) { if($limitToMarked && $rootCall) { $this->markingFinished($numChildrenMethod); @@ -103,9 +108,25 @@ class Hierarchy extends DataExtension { if(!$limitToMarked || $child->isMarked()) { $foundAChild = true; $output .= (is_callable($titleEval)) ? $titleEval($child) : eval("return $titleEval;"); - $output .= "\n" . - $child->getChildrenAsUL("", $titleEval, $extraArg, $limitToMarked, $childrenMethod, - $numChildrenMethod, false, $minNodeCount) . "
  • \n"; + $output .= "\n"; + + + $numChildren = $child->$numChildrenMethod(); + if( + // Always traverse into opened nodes (they might be exposed as parents of search results) + $child->isExpanded() + // Only traverse into children if we haven't reached the maximum node count already. + // Otherwise, the remaining nodes are lazy loaded via ajax. + && $child->isMarked() + ) { + $output .= $child->getChildrenAsUL("", $titleEval, $extraArg, $limitToMarked, $childrenMethod, + $numChildrenMethod, false, $nodeCountThreshold); + } + elseif($child->isTreeOpened()) { + // Since we're not loading children, don't mark it as open either + $child->markClosed(); + } + $output .= "\n"; } } @@ -125,21 +146,23 @@ class Hierarchy extends DataExtension { * This method returns the number of nodes marked. After this method is called other methods * can check isExpanded() and isMarked() on individual nodes. * - * @param int $minNodeCount The minimum amount of nodes to mark. + * @param int $nodeCountThreshold See {@link getChildrenAsUL()} * @return int The actual number of nodes marked. */ - public function markPartialTree($minNodeCount = 30, $context = null, + public function markPartialTree($nodeCountThreshold = 30, $context = null, $childrenMethod = "AllChildrenIncludingDeleted", $numChildrenMethod = "numChildren") { - if(!is_numeric($minNodeCount)) $minNodeCount = 30; + if(!is_numeric($nodeCountThreshold)) $nodeCountThreshold = 30; $this->markedNodes = array($this->owner->ID => $this->owner); $this->owner->markUnexpanded(); // foreach can't handle an ever-growing $nodes list while(list($id, $node) = each($this->markedNodes)) { - $this->markChildren($node, $context, $childrenMethod, $numChildrenMethod); - if($minNodeCount && sizeof($this->markedNodes) >= $minNodeCount) { + $children = $this->markChildren($node, $context, $childrenMethod, $numChildrenMethod); + if($nodeCountThreshold && sizeof($this->markedNodes) > $nodeCountThreshold) { + // Undo marking children as opened since they're lazy loaded + if($children) foreach($children as $child) $child->markClosed(); break; } } @@ -200,6 +223,7 @@ class Hierarchy extends DataExtension { /** * Mark all children of the given node that match the marking filter. * @param DataObject $node Parent node. + * @return DataList */ public function markChildren($node, $context = null, $childrenMethod = "AllChildrenIncludingDeleted", $numChildrenMethod = "numChildren") { @@ -213,7 +237,13 @@ class Hierarchy extends DataExtension { $node->markExpanded(); if($children) { foreach($children as $child) { - if(!$this->markingFilter || $this->markingFilterMatches($child)) { + $markingMatches = $this->markingFilterMatches($child); + // Filtered results should always show opened, since actual matches + // might be hidden by non-matching parent nodes. + if($this->markingFilter && $markingMatches) { + $child->markOpened(); + } + if(!$this->markingFilter || $markingMatches) { if($child->$numChildrenMethod()) { $child->markUnexpanded(); } else { @@ -223,6 +253,8 @@ class Hierarchy extends DataExtension { } } } + + return $children; } /** @@ -350,6 +382,15 @@ class Hierarchy extends DataExtension { self::$marked[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = true; self::$treeOpened[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = true; } + + /** + * Mark this DataObject's tree as closed. + */ + public function markClosed() { + if(isset(self::$treeOpened[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID])) { + unset(self::$treeOpened[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID]); + } + } /** * Check if this DataObject is marked. diff --git a/tests/model/HierarchyTest.php b/tests/model/HierarchyTest.php index 7d4088a62..881b3a247 100644 --- a/tests/model/HierarchyTest.php +++ b/tests/model/HierarchyTest.php @@ -180,6 +180,238 @@ class HierarchyTest extends SapphireTest { $this->assertEquals('Obj 2 » Obj 2a » Obj 2aa', $obj2aa->getBreadcrumbs()); } + public function testGetChildrenAsUL() { + $obj1 = $this->objFromFixture('HierarchyTest_Object', 'obj1'); + $obj2 = $this->objFromFixture('HierarchyTest_Object', 'obj2'); + $obj2a = $this->objFromFixture('HierarchyTest_Object', 'obj2a'); + $obj2aa = $this->objFromFixture('HierarchyTest_Object', 'obj2aa'); + + $nodeCountThreshold = 30; + + $root = new HierarchyTest_Object(); + $root->markPartialTree($nodeCountThreshold); + $html = $root->getChildrenAsUL( + "", + '"
  • ID . "\">" . $child->Title', + null, + false, + "AllChildrenIncludingDeleted", + "numChildren", + true, // rootCall + $nodeCountThreshold + ); + $parser = new CSSContentParser($html); + $node2 = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' + ); + $this->assertTrue( + (bool)$node2, + 'Contains root elements' + ); + $node2a = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' . + '/ul/li[@id="' . $obj2a->ID . '"]' + ); + $this->assertTrue( + (bool)$node2a, + 'Contains child elements (in correct nesting)' + ); + $node2aa = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' . + '/ul/li[@id="' . $obj2a->ID . '"]' . + '/ul/li[@id="' . $obj2aa->ID . '"]' + ); + $this->assertTrue( + (bool)$node2aa, + 'Contains grandchild elements (in correct nesting)' + ); + } + + public function testGetChildrenAsULMinNodeCount() { + $obj1 = $this->objFromFixture('HierarchyTest_Object', 'obj1'); + $obj2 = $this->objFromFixture('HierarchyTest_Object', 'obj2'); + $obj2a = $this->objFromFixture('HierarchyTest_Object', 'obj2a'); + + // Set low enough that it should be fulfilled by root only elements + $nodeCountThreshold = 3; + + $root = new HierarchyTest_Object(); + $root->markPartialTree($nodeCountThreshold); + $html = $root->getChildrenAsUL( + "", + '"
  • ID . "\">" . $child->Title', + null, + false, + "AllChildrenIncludingDeleted", + "numChildren", + true, + $nodeCountThreshold + ); + $parser = new CSSContentParser($html); + $node1 = $parser->getByXpath( + '//ul/li[@id="' . $obj1->ID . '"]' + ); + $this->assertTrue( + (bool)$node1, + 'Contains root elements' + ); + $node2 = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' + ); + $this->assertTrue( + (bool)$node2, + 'Contains root elements' + ); + $node2a = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' . + '/ul/li[@id="' . $obj2a->ID . '"]' + ); + $this->assertFalse( + (bool)$node2a, + 'Does not contains child elements because they exceed minNodeCount' + ); + } + + public function testGetChildrenAsULMinNodeCountWithMarkToExpose() { + $obj2 = $this->objFromFixture('HierarchyTest_Object', 'obj2'); + $obj2a = $this->objFromFixture('HierarchyTest_Object', 'obj2a'); + $obj2aa = $this->objFromFixture('HierarchyTest_Object', 'obj2aa'); + + // Set low enough that it should be fulfilled by root only elements + $nodeCountThreshold = 3; + + $root = new HierarchyTest_Object(); + $root->markPartialTree($nodeCountThreshold); + + // Mark certain node which should be included regardless of minNodeCount restrictions + $root->markToExpose($obj2aa); + + $html = $root->getChildrenAsUL( + "", + '"
  • ID . "\">" . $child->Title', + null, + false, + "AllChildrenIncludingDeleted", + "numChildren", + true, + $nodeCountThreshold + ); + $parser = new CSSContentParser($html); + $node2 = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' + ); + $this->assertTrue( + (bool)$node2, + 'Contains root elements' + ); + $node2aa = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' . + '/ul/li[@id="' . $obj2a->ID . '"]' . + '/ul/li[@id="' . $obj2aa->ID . '"]' + ); + $this->assertTrue((bool)$node2aa); + } + + public function testGetChildrenAsULMinNodeCountWithFilters() { + $obj1 = $this->objFromFixture('HierarchyTest_Object', 'obj1'); + $obj2 = $this->objFromFixture('HierarchyTest_Object', 'obj2'); + $obj2a = $this->objFromFixture('HierarchyTest_Object', 'obj2a'); + $obj2aa = $this->objFromFixture('HierarchyTest_Object', 'obj2aa'); + + // Set low enough that it should fit all search matches without lazy loading + $nodeCountThreshold = 3; + + $root = new HierarchyTest_Object(); + + // Includes nodes by filter regardless of minNodeCount restrictions + $root->setMarkingFilterFunction(function($record) use($obj2, $obj2a, $obj2aa) { + // Results need to include parent hierarchy, even if we just want to + // match the innermost node. + // var_dump($record->Title); + // var_dump(in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID))); + return in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID)); + }); + $root->markPartialTree($nodeCountThreshold); + + $html = $root->getChildrenAsUL( + "", + '"
  • ID . "\">" . $child->Title', + null, + true, // limit to marked + "AllChildrenIncludingDeleted", + "numChildren", + true, + $nodeCountThreshold + ); + $parser = new CSSContentParser($html); + $node1 = $parser->getByXpath( + '//ul/li[@id="' . $obj1->ID . '"]' + ); + $this->assertFalse( + (bool)$node1, + 'Does not contain root elements which dont match the filter' + ); + $node2aa = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' . + '/ul/li[@id="' . $obj2a->ID . '"]' . + '/ul/li[@id="' . $obj2aa->ID . '"]' + ); + $this->assertTrue( + (bool)$node2aa, + 'Contains non-root elements which match the filter' + ); + } + + public function testGetChildrenAsULHardLimitsNodes() { + $obj1 = $this->objFromFixture('HierarchyTest_Object', 'obj1'); + $obj2 = $this->objFromFixture('HierarchyTest_Object', 'obj2'); + $obj2a = $this->objFromFixture('HierarchyTest_Object', 'obj2a'); + $obj2aa = $this->objFromFixture('HierarchyTest_Object', 'obj2aa'); + + // Set low enough that it should fit all search matches without lazy loading + $nodeCountThreshold = 3; + + $root = new HierarchyTest_Object(); + + // Includes nodes by filter regardless of minNodeCount restrictions + $root->setMarkingFilterFunction(function($record) use($obj2, $obj2a, $obj2aa) { + // Results need to include parent hierarchy, even if we just want to + // match the innermost node. + // var_dump($record->Title); + // var_dump(in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID))); + return in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID)); + }); + $root->markPartialTree($nodeCountThreshold); + + $html = $root->getChildrenAsUL( + "", + '"
  • ID . "\">" . $child->Title', + null, + true, // limit to marked + "AllChildrenIncludingDeleted", + "numChildren", + true, + $nodeCountThreshold + ); + $parser = new CSSContentParser($html); + $node1 = $parser->getByXpath( + '//ul/li[@id="' . $obj1->ID . '"]' + ); + $this->assertFalse( + (bool)$node1, + 'Does not contain root elements which dont match the filter' + ); + $node2aa = $parser->getByXpath( + '//ul/li[@id="' . $obj2->ID . '"]' . + '/ul/li[@id="' . $obj2a->ID . '"]' . + '/ul/li[@id="' . $obj2aa->ID . '"]' + ); + $this->assertTrue( + (bool)$node2aa, + 'Contains non-root elements which match the filter' + ); + } + } class HierarchyTest_Object extends DataObject implements TestOnly { @@ -191,4 +423,4 @@ class HierarchyTest_Object extends DataObject implements TestOnly { 'Hierarchy', "Versioned('Stage', 'Live')", ); -} +} \ No newline at end of file