mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 12:05:37 +00:00
FIX Respect tree node limits, fix search result node display
- Renamed $minNodeCount to more accurate $nodeCountThreshold - The $minNodeCount attribute wasn't properly respected during actual querying, so SilverStripe would always traverse the entire tree (and load all objects into memory), before then marking nodes as "unexpanded", which prevents them from actually being rendered. - Fixes nodes on search results to be expanded by default - Fixes nodes on search results to correctly ajax-expand
This commit is contained in:
parent
6931073956
commit
dd6f33ab37
@ -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.
|
||||
|
@ -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 = '"<li>" . $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) . "</li>\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 .= "</li>\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.
|
||||
|
@ -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(
|
||||
"",
|
||||
'"<li id=\"" . $child->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(
|
||||
"",
|
||||
'"<li id=\"" . $child->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(
|
||||
"",
|
||||
'"<li id=\"" . $child->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(
|
||||
"",
|
||||
'"<li id=\"" . $child->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(
|
||||
"",
|
||||
'"<li id=\"" . $child->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')",
|
||||
);
|
||||
}
|
||||
}
|
Loading…
x
Reference in New Issue
Block a user