diff --git a/src/Forms/TreeDropdownField.php b/src/Forms/TreeDropdownField.php index b27dae2e5..a6f21ec14 100644 --- a/src/Forms/TreeDropdownField.php +++ b/src/Forms/TreeDropdownField.php @@ -501,7 +501,7 @@ class TreeDropdownField extends FormField // Begin marking $markingSet->markPartialTree(); - // Explicitely mark our search results if necessary + // Explicitly mark our search results if necessary foreach ($this->searchIds as $id => $marked) { if ($marked) { $object = $this->objectForKey($id); diff --git a/src/ORM/Hierarchy/Hierarchy.php b/src/ORM/Hierarchy/Hierarchy.php index 9760155f0..6f6d2e84c 100644 --- a/src/ORM/Hierarchy/Hierarchy.php +++ b/src/ORM/Hierarchy/Hierarchy.php @@ -4,6 +4,8 @@ namespace SilverStripe\ORM\Hierarchy; use SilverStripe\Admin\LeftAndMain; use SilverStripe\Control\Controller; +use SilverStripe\Core\ClassInfo; +use SilverStripe\Core\Extensible; use SilverStripe\ORM\DataList; use SilverStripe\ORM\SS_List; use SilverStripe\ORM\ValidationResult; @@ -28,7 +30,7 @@ class Hierarchy extends DataExtension { /** * The lower bounds for the amount of nodes to mark. If set, the logic will expand nodes until it reaches at least - * this number, and then stops. Root nodes will always show regardless of this settting. Further nodes can be + * this number, and then stops. Root nodes will always show regardless of this setting. 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). * @@ -284,7 +286,7 @@ class Hierarchy extends DataExtension /** * Return the number of direct children. By default, values are cached after the first invocation. Can be - * augumented by {@link augmentNumChildrenCountQuery()}. + * augmented by {@link augmentNumChildrenCountQuery()}. * * @param bool $cache Whether to retrieve values from cache * @return int @@ -320,7 +322,7 @@ class Hierarchy extends DataExtension /** * Pre-populate any appropriate caches prior to rendering a tree. * This is used to allow for the efficient rendering of tree views, notably in the CMS. - * In the cace of Hierarchy, it caches numChildren values. Other extensions can provide an + * In the case of Hierarchy, it caches numChildren values. Other extensions can provide an * onPrepopulateTreeDataCache(DataList $recordList = null, array $options) methods to hook * into this event as well. * @@ -333,7 +335,7 @@ class Hierarchy extends DataExtension if (empty($options['numChildrenMethod']) || $options['numChildrenMethod'] === 'numChildren') { $idList = is_array($recordList) ? $recordList : ($recordList instanceof DataList ? $recordList->column('ID') : null); - self::prepopulate_numchildren_cache($this->owner->baseClass(), $idList); + self::prepopulate_numchildren_cache($this->getHierarchyBaseClass(), $idList); } $this->owner->extend('onPrepopulateTreeDataCache', $recordList, $options); @@ -407,25 +409,47 @@ class Hierarchy extends DataExtension && in_array($controller->getAction(), ["treeview", "listview", "getsubtree"]); } + /** + * Find the first class in the inheritance chain that has Hierarchy extension applied + * + * @return string + */ + private function getHierarchyBaseClass(): string + { + $ancestry = ClassInfo::ancestry($this->owner); + $ancestorClass = array_shift($ancestry); + while ($ancestorClass && !Extensible::has_extension($ancestorClass, self::class)) { + $ancestorClass = array_shift($ancestry); + } + + return $ancestorClass; + } + /** * Return children in the stage site. * * @param bool $showAll Include all of the elements, even those not shown in the menus. Only applicable when * extension is applied to {@link SiteTree}. - * @param bool $skipParentIDFilter Set to true to supress the ParentID and ID where statements. + * @param bool $skipParentIDFilter Set to true to suppress the ParentID and ID where statements. * @return DataList */ public function stageChildren($showAll = false, $skipParentIDFilter = false) { - $hideFromHierarchy = $this->owner->config()->hide_from_hierarchy; - $hideFromCMSTree = $this->owner->config()->hide_from_cms_tree; - $baseClass = $this->owner->baseClass(); - $baseTable = $this->owner->baseTable(); - $staged = DataObject::get($baseClass)->where(sprintf( + /** @var DataObject|Hierarchy $owner */ + $owner = $this->owner; + $hideFromHierarchy = $owner->config()->hide_from_hierarchy; + $hideFromCMSTree = $owner->config()->hide_from_cms_tree; + $class = $this->getHierarchyBaseClass(); + + $schema = DataObject::getSchema(); + $tableForParentID = $schema->tableForField($class, 'ParentID'); + $tableForID = $schema->tableForField($class, 'ID'); + + $staged = DataObject::get($class)->where(sprintf( '%s.%s <> %s.%s', - Convert::symbol2sql($baseTable), + Convert::symbol2sql($tableForParentID), Convert::symbol2sql("ParentID"), - Convert::symbol2sql($baseTable), + Convert::symbol2sql($tableForID), Convert::symbol2sql("ID") )); @@ -466,7 +490,7 @@ class Hierarchy extends DataExtension $hideFromHierarchy = $owner->config()->hide_from_hierarchy; $hideFromCMSTree = $owner->config()->hide_from_cms_tree; - $children = DataObject::get($owner->baseClass()) + $children = DataObject::get($this->getHierarchyBaseClass()) ->filter('ParentID', (int)$owner->ID) ->exclude('ID', (int)$owner->ID) ->setDataQueryParam([ diff --git a/tests/php/Forms/TreeDropdownFieldTest.php b/tests/php/Forms/TreeDropdownFieldTest.php index ec349fa0b..3d78c7ab3 100644 --- a/tests/php/Forms/TreeDropdownFieldTest.php +++ b/tests/php/Forms/TreeDropdownFieldTest.php @@ -12,6 +12,8 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\TreeDropdownField; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestObject; +use SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject; use SilverStripe\ORM\Tests\HierarchyTest\TestObject; class TreeDropdownFieldTest extends SapphireTest @@ -20,7 +22,9 @@ class TreeDropdownFieldTest extends SapphireTest protected static $fixture_file = 'TreeDropdownFieldTest.yml'; protected static $extra_dataobjects = [ - TestObject::class + TestObject::class, + HierarchyOnSubclassTestObject::class, + HierarchyOnSubclassTestSubObject::class, ]; public function testSchemaStateDefaults() @@ -52,7 +56,7 @@ class TreeDropdownFieldTest extends SapphireTest { $field = new TreeDropdownField('TestTree', 'Test tree', Folder::class); - // case insensitive search against keyword 'sub' for folders + // case-insensitive search against keyword 'sub' for folders $request = new HTTPRequest('GET', 'url', ['search'=>'sub', 'format' => 'json']); $request->setSession(new Session([])); $response = $field->tree($request); @@ -83,7 +87,7 @@ class TreeDropdownFieldTest extends SapphireTest { $field = new TreeDropdownField('TestTree', 'Test tree', Folder::class); - // case insensitive search against keyword 'sub' for folders + // case-insensitive search against keyword 'sub' for folders $request = new HTTPRequest('GET', 'url', ['search'=>'sub', 'format' => 'json', 'flatList' => '1']); $request->setSession(new Session([])); $response = $field->tree($request); @@ -137,11 +141,42 @@ class TreeDropdownFieldTest extends SapphireTest $this->assertEquals($expectedNodeIDs, $actualNodeIDs); } + public function testTreeSearchJsonFlatlistWithLowNodeThresholdUsingSubObject() + { + // Initialise our TreeDropDownField + $field = new TreeDropdownField('TestTree', 'Test tree - Hierarchy on subclass', HierarchyOnSubclassTestSubObject::class); + $field->config()->set('node_threshold_total', 2); + + // Search for all Test object matching our criteria + $request = new HTTPRequest( + 'GET', + 'url', + ['search' => 'SubObject', 'format' => 'json', 'flatList' => '1'] + ); + $request->setSession(new Session([])); + $response = $field->tree($request); + $tree = json_decode($response->getBody(), true); + $actualNodeIDs = array_column($tree['children'], 'id'); + + // Get the list of expected node IDs from the YML Fixture + $expectedNodeIDs = array_map( + function ($key) { + return $this->objFromFixture(HierarchyOnSubclassTestSubObject::class, $key)->ID; + }, + ['four', 'fourB', 'fourA2'] // Those are the identifiers of the object we expect our search to find + ); + + sort($actualNodeIDs); + sort($expectedNodeIDs); + + $this->assertEquals($expectedNodeIDs, $actualNodeIDs); + } + public function testTreeSearch() { $field = new TreeDropdownField('TestTree', 'Test tree', Folder::class); - // case insensitive search against keyword 'sub' for folders + // case-insensitive search against keyword 'sub' for folders $request = new HTTPRequest('GET', 'url', ['search'=>'sub']); $request->setSession(new Session([])); $response = $field->tree($request); @@ -179,7 +214,7 @@ class TreeDropdownFieldTest extends SapphireTest $field = new TreeDropdownField('TestTree', 'Test tree', File::class); - // case insensitive search against keyword 'sub' for files + // case-insensitive search against keyword 'sub' for files $request = new HTTPRequest('GET', 'url', ['search'=>'sub']); $request->setSession(new Session([])); $response = $field->tree($request); @@ -233,6 +268,39 @@ class TreeDropdownFieldTest extends SapphireTest ); } + public function testTreeSearchUsingSubObject() + { + $field = new TreeDropdownField('TestTree', 'Test tree', HierarchyOnSubclassTestSubObject::class); + + // case-insensitive search against keyword 'SubObject' for objects that have Hierarchy extension + // applied to a class that doesn't directly inherit from DataObject + $request = new HTTPRequest('GET', 'url', ['search' => 'SubObject']); + $request->setSession(new Session([])); + $response = $field->tree($request); + $tree = $response->getBody(); + + $subObject1 = $this->objFromFixture(HierarchyOnSubclassTestSubObject::class, 'four'); + $subObject1ChildB = $this->objFromFixture(HierarchyOnSubclassTestSubObject::class, 'fourB'); + + $parser = new CSSContentParser($tree); + $cssPath = 'ul.tree li#selector-TestTree-' . $subObject1->ID . ' li#selector-TestTree-' . $subObject1ChildB->ID . ' a'; + $firstResult = $parser->getBySelector($cssPath); + $this->assertEquals( + $subObject1ChildB->Name, + (string)$firstResult[0], + $subObject1ChildB->Name . ' is found, nested under ' . $subObject1->Name + ); + + // other objects which don't contain the keyword 'SubObject' are not returned in search results + $subObject2 = $this->objFromFixture(HierarchyOnSubclassTestSubObject::class, 'five'); + $cssPath = 'ul.tree li#selector-TestTree-' . $subObject2->ID . ' a'; + $noResult = $parser->getBySelector($cssPath); + $this->assertEmpty( + $noResult, + $subObject2 . ' is not found' + ); + } + public function testReadonly() { $field = new TreeDropdownField('TestTree', 'Test tree', File::class); @@ -272,7 +340,7 @@ class TreeDropdownFieldTest extends SapphireTest $treeBaseID = $this->idFromFixture(Folder::class, 'folder1'); $field = new TreeDropdownField('TestTree', 'Test tree', Folder::class); - // getSchemaDataDefaults needs the field to be attach to a form + // getSchemaDataDefaults needs the field to be attached to a form new Form( null, 'mock', diff --git a/tests/php/Forms/TreeDropdownFieldTest.yml b/tests/php/Forms/TreeDropdownFieldTest.yml index 2207cc300..66ced1fc9 100644 --- a/tests/php/Forms/TreeDropdownFieldTest.yml +++ b/tests/php/Forms/TreeDropdownFieldTest.yml @@ -62,3 +62,29 @@ SilverStripe\ORM\Tests\HierarchyTest\TestObject: ParentID: =>SilverStripe\ORM\Tests\HierarchyTest\TestObject.twoA three: Title: Three MatchSearchCriteria +SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject: + four: + Title: Four SubObject + fourA: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.four + Title: Child A of Four + fourB: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.four + Title: Child B of Four SubObject + fourA1: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.fourA + Title: Child 1 of Child A of Four + fourA2: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.fourA + Title: Child 2 of Child A of Four SubObject + fourB1: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.fourB + Title: Child 1 of Child B of Four + fourB2: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.fourB + Title: Child 2 of Child B of Four + fourB3: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.fourB + Title: Child 3 of Child B of Four + five: + Title: Five diff --git a/tests/php/ORM/HierarchyCachingTest.php b/tests/php/ORM/HierarchyCachingTest.php index 7f101fb50..2d3de8dfb 100644 --- a/tests/php/ORM/HierarchyCachingTest.php +++ b/tests/php/ORM/HierarchyCachingTest.php @@ -48,10 +48,10 @@ class HierachyCacheTest extends SapphireTest [TestObject::class, 'obj2', true, 2, 'Root object numChildren should count direct children when cache'], [TestObject::class, 'obj3a', false, 2, 'Sub object numChildren should count direct children'], [TestObject::class, 'obj3a', true, 2, 'Sub object numChildren should count direct children when cache'], - [TestObject::class, 'obj3d', false, 0, 'Childess Sub object numChildren should be 0'], - [TestObject::class, 'obj3d', true, 0, 'Childess Sub object numChildren should be 0 when cache'], + [TestObject::class, 'obj3d', false, 0, 'Childless Sub object numChildren should be 0'], + [TestObject::class, 'obj3d', true, 0, 'Childless Sub object numChildren should be 0 when cache'], [HideTestObject::class, 'obj4', false, 1, 'Hidden object should not be included in count'], - [HideTestObject::class, 'obj4', true, 1, 'Hidden object should not be included in couunt when cache'] + [HideTestObject::class, 'obj4', true, 1, 'Hidden object should not be included in count when cache'] ]; } @@ -68,7 +68,7 @@ class HierachyCacheTest extends SapphireTest $this->assertEquals($expected, $actual, $message); if ($cache) { - // When caching is eanbled, try re-accessing the numChildren value to make sure it doesn't change. + // When caching is enabled, try re-accessing the numChildren value to make sure it doesn't change. $actual = $node->numChildren($cache); $this->assertEquals($expected, $actual, $message); } diff --git a/tests/php/ORM/HierarchyTest.php b/tests/php/ORM/HierarchyTest.php index f4b859d99..6d4676377 100644 --- a/tests/php/ORM/HierarchyTest.php +++ b/tests/php/ORM/HierarchyTest.php @@ -14,6 +14,8 @@ class HierarchyTest extends SapphireTest HierarchyTest\TestObject::class, HierarchyTest\HideTestObject::class, HierarchyTest\HideTestSubObject::class, + HierarchyTest\HierarchyOnSubclassTestObject::class, + HierarchyTest\HierarchyOnSubclassTestSubObject::class, ]; public static function getExtraDataObjects() @@ -145,6 +147,43 @@ class HierarchyTest extends SapphireTest ); } + public function testNumChildrenHierarchyOnSubclass() + { + /** @var HierarchyTest\HierarchyOnSubclassTestObject $obj5 */ + $obj5 = $this->objFromFixture(HierarchyTest\HierarchyOnSubclassTestObject::class, 'obj5'); + + $this->assertFalse( + $obj5->hasMethod('numChildren'), + 'numChildren() cannot be called on object without Hierarchy extension' + ); + + /** @var HierarchyTest\HierarchyOnSubclassTestSubObject $obj5a */ + $obj5a = $this->objFromFixture(HierarchyTest\HierarchyOnSubclassTestSubObject::class, 'obj5a'); + /** @var HierarchyTest\HierarchyOnSubclassTestSubObject $obj5b */ + $obj5b = $this->objFromFixture(HierarchyTest\HierarchyOnSubclassTestSubObject::class, 'obj5b'); + + $this->assertEquals(2, $obj5a->numChildren()); + $this->assertEquals(1, $obj5b->numChildren()); + + $obj5bChild2 = new HierarchyTest\HierarchyOnSubclassTestSubObject(); + $obj5bChild2->ParentID = $obj5b->ID; + $obj5bChild2->write(); + $this->assertEquals( + $obj5b->numChildren(false), + 2, + 'numChildren() caching can be disabled through method parameter' + ); + $obj5bChild3 = new HierarchyTest\HierarchyOnSubclassTestSubObject(); + $obj5bChild3->ParentID = $obj5b->ID; + $obj5bChild3->write(); + $obj5b->flushCache(); + $this->assertEquals( + $obj5b->numChildren(), + 3, + 'numChildren() caching can be disabled by flushCache()' + ); + } + public function testLoadDescendantIDListIntoArray() { /** @var HierarchyTest\TestObject $obj2 */ @@ -219,7 +258,7 @@ class HierarchyTest extends SapphireTest $this->assertEquals('Obj 2 » Obj 2a » Obj 2aa', $obj2aa->getBreadcrumbs()); } - public function testNoHideFromHeirarchy() + public function testNoHideFromHierarchy() { /** @var HierarchyTest\HideTestObject $obj4 */ $obj4 = $this->objFromFixture(HierarchyTest\HideTestObject::class, 'obj4'); @@ -232,7 +271,7 @@ class HierarchyTest extends SapphireTest $this->assertEquals($obj4->liveChildren()->Count(), 2); } - public function testHideFromHeirarchy() + public function testHideFromHierarchy() { HierarchyTest\HideTestObject::config()->update( 'hide_from_hierarchy', diff --git a/tests/php/ORM/HierarchyTest.yml b/tests/php/ORM/HierarchyTest.yml index cd2e66576..a8a9d8c3b 100644 --- a/tests/php/ORM/HierarchyTest.yml +++ b/tests/php/ORM/HierarchyTest.yml @@ -53,3 +53,20 @@ SilverStripe\ORM\Tests\HierarchyTest\HideTestObject: SilverStripe\ORM\Tests\HierarchyTest\HideTestSubObject: obj4b: Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HideTestObject.obj4 +SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestObject: + obj5: + Title: Obj 5 +SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject: + obj5a: + Title: Obj 5a + obj5b: + Title: Obj 5b + obj5aa: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.obj5a + Title: Obj 5aa + obj5ab: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.obj5a + Title: Obj 5ab + obj5ba: + Parent: =>SilverStripe\ORM\Tests\HierarchyTest\HierarchyOnSubclassTestSubObject.obj5b + Title: Obj 5ba diff --git a/tests/php/ORM/HierarchyTest/HierarchyOnSubclassTestObject.php b/tests/php/ORM/HierarchyTest/HierarchyOnSubclassTestObject.php new file mode 100644 index 000000000..48dfdd9a1 --- /dev/null +++ b/tests/php/ORM/HierarchyTest/HierarchyOnSubclassTestObject.php @@ -0,0 +1,30 @@ + 'Varchar' + ]; + + private static $extensions = [ + Versioned::class, + ]; + + private static $default_sort = 'Title ASC'; + + public function cmstreeclasses() + { + return $this->markingClasses(); + } +} diff --git a/tests/php/ORM/HierarchyTest/HierarchyOnSubclassTestSubObject.php b/tests/php/ORM/HierarchyTest/HierarchyOnSubclassTestSubObject.php new file mode 100644 index 000000000..427474942 --- /dev/null +++ b/tests/php/ORM/HierarchyTest/HierarchyOnSubclassTestSubObject.php @@ -0,0 +1,18 @@ +