From 0778d7b960e703e7a57ca050556ecd238a22602c Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:41:20 +1300 Subject: [PATCH] API Move some code to framework to be reusable (#3022) Some of this code needs to be usable on classes other than SiteTree with the CMSMain refactoring. --- code/Controllers/CMSMain.php | 2 + code/Model/RedirectorPage.php | 5 - code/Model/SiteTree.php | 364 +----------------- code/Model/VirtualPage.php | 17 +- tests/php/Controllers/CMSMainTest.php | 68 +++- .../CMSMainTest/TestStatusFlagsPage.php | 16 + tests/php/Model/SiteTreeTest.php | 122 ------ tests/php/Model/SiteTreeTest_ClassCext.php | 15 - .../Model/SiteTreeTest_StageStatusInherit.php | 2 +- tests/php/Model/VirtualPageTest.php | 4 +- 10 files changed, 113 insertions(+), 502 deletions(-) create mode 100644 tests/php/Controllers/CMSMainTest/TestStatusFlagsPage.php delete mode 100644 tests/php/Model/SiteTreeTest_ClassCext.php diff --git a/code/Controllers/CMSMain.php b/code/Controllers/CMSMain.php index 553418b2d3..644b3ec05c 100644 --- a/code/Controllers/CMSMain.php +++ b/code/Controllers/CMSMain.php @@ -1064,6 +1064,8 @@ public function Breadcrumbs($unlinked = false) if ($page = SiteTree::get()->byID($parentID)) { $this->buildListViewBreadcrumb($items, $page); } + // At this stage we also know the parent ID is the "current" ID for this view + $this->setCurrentPageID($parentID); } } diff --git a/code/Model/RedirectorPage.php b/code/Model/RedirectorPage.php index c121282014..250d602a48 100644 --- a/code/Model/RedirectorPage.php +++ b/code/Model/RedirectorPage.php @@ -21,11 +21,6 @@ */ class RedirectorPage extends Page { - /** - * @deprecated 5.4.0 use class_description instead. - */ - private static $description = 'Redirects requests to another location'; - private static string $class_description = 'Redirects requests to another location'; private static $icon_class = 'font-icon-p-redirect'; diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 5dc572ad9e..2cf04fcb05 100755 --- a/code/Model/SiteTree.php +++ b/code/Model/SiteTree.php @@ -114,32 +114,6 @@ */ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvider, CMSPreviewable, Resettable, Flushable, MemberCacheFlusher { - /** - * Indicates what kind of children this page type can have. - * This can be an array of allowed child classes, or the string "none" - - * indicating that this page type can't have children. - * If a classname is prefixed by "*", such as "*Page", then only that - * class is allowed - no subclasses. Otherwise, the class and all its - * subclasses are allowed. - * To control allowed children on root level (no parent), use {@link $can_be_root}. - * - * Note that this setting is cached when used in the CMS, use the "flush" query parameter to clear it. - * - * @config - * @var array - */ - private static $allowed_children = [ - SiteTree::class - ]; - - /** - * Used as a cache for `SiteTree::allowedChildren()` - * Drastically reduces admin page load when there are a lot of page types - * @var array - * @deprecated 5.4.0 will be moved to SilverStripe\ORM\Hierarchy\Hierarchy->cache_allowedChildren - */ - protected static $_allowedChildren = []; - /** * Determines if the Draft Preview panel will appear when in the CMS admin * @var bool @@ -159,7 +133,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi * @config * @var string */ - private static $default_child = "Page"; + private static $default_child = Page::class; /** * Default value for SiteTree.ClassName enum @@ -168,16 +142,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi * @config * @var string */ - private static $default_classname = "Page"; - - /** - * The default parent class for this page. - * Note: Value might be cached, see {@link $allowed_chilren}. - * - * @config - * @var string - */ - private static $default_parent = null; + private static $default_classname = Page::class; /** * Controls whether a page can be in the root of the site tree. @@ -331,6 +296,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi private static $default_sort = "\"Sort\""; + private static string $sort_field = 'Sort'; + /** * If this is false, the class cannot be created in the CMS by regular content authors, only by ADMINs. * @var boolean @@ -411,8 +378,6 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi */ private static $show_meta_generator_version = true; - protected $_cache_statusFlags = null; - /** * Plural form for SiteTree / Page classes. Not inherited by subclasses. * @@ -429,34 +394,9 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi */ private static $base_singular_name = 'Page'; - /** - * Description of the class functionality, typically shown to a user - * when selecting which page type to create. Translated through {@link provideI18nEntities()}. - * - * @see SiteTree::classDescription() - * @see SiteTree::i18n_classDescription() - * - * @config - * @var string - * @deprecated 5.4.0 use class_description instead. - */ - private static $description = null; - - /** - * Description for Page and SiteTree classes, but not inherited by subclasses. - * override SiteTree::$description in subclasses instead. - * - * @see SiteTree::classDescription() - * @see SiteTree::i18n_classDescription() - * - * @config - * @var string - * @deprecated 5.4.0 use base_class_description instead. - */ - private static $base_description = 'Generic content page'; - /** * Description for Page and SiteTree classes, but not inherited by subclasses. + * override class_description in subclasses instead. */ private static string $base_class_description = 'Generic content page'; @@ -899,32 +839,6 @@ protected function onBeforeDuplicate($original, $doWrite) $this->Sort = 0; } - /** - * Duplicates each child of this node recursively and returns the top-level duplicate node. - * - * @return static The duplicated object - */ - public function duplicateWithChildren() - { - $clone = $this->duplicate(); - $children = $this->AllChildren(); - - if ($children) { - $sort = 0; - foreach ($children as $child) { - $childClone = method_exists($child, 'duplicateWithChildren') - ? $child->duplicateWithChildren() - : $child->duplicate(); - $childClone->ParentID = $clone->ID; - //retain sort order by manually setting sort values - $childClone->Sort = ++$sort; - $childClone->write(); - } - } - - return $clone; - } - /** * Duplicate this node and its children as a child of the node with the given ID * @@ -1103,47 +1017,6 @@ public function can($perm, $member = null, $context = []) return ($member && Permission::checkMember($member, $perm)); } - /** - * This function should return true if the current user can add children to this page. It can be overloaded to - * customise the security model for an application. - * - * Denies permission if any of the following conditions is true: - * - alternateCanAddChildren() on a extension returns false - * - canEdit() is not granted - * - There are no classes defined in {@link $allowed_children} - * - * @uses SiteTreeExtension->canAddChildren() - * @uses canEdit() - * @uses $allowed_children - * - * @param Member|int $member - * @return bool True if the current user can add children - */ - public function canAddChildren($member = null) - { - // Disable adding children to archived pages - if (!$this->isOnDraft()) { - return false; - } - - if (!$member) { - $member = Security::getCurrentUser(); - } - - // Standard mechanism for accepting permission changes from extensions - $extended = $this->extendedCan('canAddChildren', $member); - if ($extended !== null) { - return $extended; - } - - // Default permissions - if ($member && Permission::checkMember($member, "ADMIN")) { - return true; - } - - return $this->canEdit($member) && $this->config()->get('allowed_children') !== 'none'; - } - /** * This function should return true if the current user can view this page. It can be overloaded to customise the * security model for an application. @@ -1313,14 +1186,6 @@ public function canCreate($member = null, $context = []) $member = Security::getCurrentUser(); } - // Check parent (custom canCreate option for SiteTree) - // Block children not allowed for this parent type - $parent = isset($context['Parent']) ? $context['Parent'] : null; - $strictParentInstance = ($parent && $parent instanceof SiteTree); - if ($strictParentInstance && !in_array(static::class, $parent->allowedChildren() ?? [])) { - return false; - } - // Standard mechanism for accepting permission changes from extensions $extended = $this->extendedCan(__FUNCTION__, $member, $context); if ($extended !== null) { @@ -1332,14 +1197,16 @@ public function canCreate($member = null, $context = []) return true; } - // Fall over to inherited permissions - if ($strictParentInstance && $parent->exists()) { - return $parent->canAddChildren($member); - } else { + // Check parent (added to context through CMSMain) + $parent = isset($context['Parent']) ? $context['Parent'] : null; + if (!$parent?->exists() || !($parent instanceof SiteTree)) { // This doesn't necessarily mean we are creating a root page, but that // we don't know if there is a parent, so default to this permission return SiteConfig::current_site_config()->canCreateTopLevel($member); } + + // If we haven't returned by now, resort to edit permissions + return $this->canEdit($member); } /** @@ -1820,12 +1687,6 @@ protected function onAfterDelete() parent::onAfterDelete(); } - public function flushCache($persistent = true) - { - parent::flushCache($persistent); - $this->_cache_statusFlags = null; - } - /** * Flushes the member specific cache for creatable children * @@ -1850,41 +1711,6 @@ public function validate() { $result = parent::validate(); - // Allowed children validation - $parent = $this->getParent(); - if ($parent && $parent->exists()) { - // No need to check for subclasses or instanceof, as allowedChildren() already - // deconstructs any inheritance trees already. - $allowed = $parent->allowedChildren(); - $subject = ($this instanceof VirtualPage && $this->CopyContentFromID) - ? $this->CopyContentFrom() - : $this; - if (!in_array($subject->ClassName, $allowed ?? [])) { - $result->addError( - _t( - 'SilverStripe\\CMS\\Model\\SiteTree.PageTypeNotAllowed', - 'Page type "{type}" not allowed as child of this parent page', - ['type' => $subject->i18n_singular_name()] - ), - ValidationResult::TYPE_ERROR, - 'ALLOWED_CHILDREN' - ); - } - } - - // "Can be root" validation - if (!$this->config()->get('can_be_root') && !$this->ParentID) { - $result->addError( - _t( - 'SilverStripe\\CMS\\Model\\SiteTree.PageTypNotAllowedOnRoot', - 'Page type "{type}" is not allowed on the root level', - ['type' => $this->i18n_singular_name()] - ), - ValidationResult::TYPE_ERROR, - 'CAN_BE_ROOT' - ); - } - // Ensure ExtraMeta can be turned into valid HTML if ($this->ExtraMeta && !HTMLValue::create($this->ExtraMeta)->getContent()) { $result->addError( @@ -2794,54 +2620,6 @@ protected function getClassDropdown() return $result; } - /** - * Returns an array of the class names of classes that are allowed to be children of this class. - * - * @return string[] - */ - public function allowedChildren() - { - if (isset(static::$_allowedChildren[$this->ClassName])) { - $allowedChildren = static::$_allowedChildren[$this->ClassName]; - } else { - // Get config based on old FIRST_SET rules - $candidates = null; - $class = get_class($this); - while ($class) { - if (Config::inst()->exists($class, 'allowed_children', Config::UNINHERITED)) { - $candidates = Config::inst()->get($class, 'allowed_children', Config::UNINHERITED); - break; - } - $class = get_parent_class($class ?? ''); - } - if (!$candidates || $candidates === 'none' || $candidates === 'SiteTree_root') { - return []; - } - - // Parse candidate list - $allowedChildren = []; - foreach ((array)$candidates as $candidate) { - // If a classname is prefixed by "*", such as "*Page", then only that class is allowed - no subclasses. - // Otherwise, the class and all its subclasses are allowed. - if (substr($candidate ?? '', 0, 1) == '*') { - $allowedChildren[] = substr($candidate ?? '', 1); - } elseif (($candidate !== 'SiteTree_root') - && ($subclasses = ClassInfo::subclassesFor($candidate)) - ) { - foreach ($subclasses as $subclass) { - if (!is_a($subclass, HiddenClass::class, true)) { - $allowedChildren[] = $subclass; - } - } - } - static::$_allowedChildren[get_class($this)] = $allowedChildren; - } - } - $this->extend('updateAllowedChildren', $allowedChildren); - - return $allowedChildren; - } - /** * * Gets a list of the page types that can be created under this specific page, including font icons @@ -2877,34 +2655,6 @@ public function creatableChildPages() return $children[$this->ID]; } - /** - * Returns the class name of the default class for children of this page. - * - * @return string - */ - public function defaultChild() - { - $default = $this->config()->get('default_child'); - $allowed = $this->allowedChildren(); - if ($allowed) { - if (!$default || !in_array($default, $allowed ?? [])) { - $default = reset($allowed); - } - return $default; - } - return null; - } - - /** - * Returns the class name of the default class for the parent of this page. - * - * @return string - */ - public function defaultParent() - { - return $this->config()->get('default_parent'); - } - /** * Get the title for use in menus for this page. If the MenuTitle field is set it returns that, else it returns the * Title field. @@ -2935,55 +2685,6 @@ public function setMenuTitle($value) } } - /** - * A flag provides the user with additional data about the current page status, for example a "removed from draft" - * status. Each page can have more than one status flag. Returns a map of a unique key to a (localized) title for - * the flag. The unique key can be reused as a CSS class. Use the 'updateStatusFlags' extension point to customize - * the flags. - * - * Example (simple): - * "deletedonlive" => "Deleted" - * - * Example (with optional title attribute): - * "deletedonlive" => ['text' => "Deleted", 'title' => 'This page has been deleted'] - * - * @param bool $cached Whether to serve the fields from cache; false regenerate them - * @return array - */ - public function getStatusFlags($cached = true) - { - if (!$this->_cache_statusFlags || !$cached) { - $flags = []; - if ($this->isOnLiveOnly()) { - $flags['removedfromdraft'] = [ - 'text' => _t(__CLASS__.'.ONLIVEONLYSHORT', 'On live only'), - 'title' => _t(__CLASS__.'.ONLIVEONLYSHORTHELP', 'Page is published, but has been deleted from draft'), - ]; - } elseif ($this->isArchived()) { - $flags['archived'] = [ - 'text' => _t(__CLASS__.'.ARCHIVEDPAGESHORT', 'Archived'), - 'title' => _t(__CLASS__.'.ARCHIVEDPAGEHELP', 'Page is removed from draft and live'), - ]; - } elseif ($this->isOnDraftOnly()) { - $flags['addedtodraft'] = [ - 'text' => _t(__CLASS__.'.ADDEDTODRAFTSHORT', 'Draft'), - 'title' => _t(__CLASS__.'.ADDEDTODRAFTHELP', "Page has not been published yet") - ]; - } elseif ($this->isModifiedOnDraft()) { - $flags['modified'] = [ - 'text' => _t(__CLASS__.'.MODIFIEDONDRAFTSHORT', 'Modified'), - 'title' => _t(__CLASS__.'.MODIFIEDONDRAFTHELP', 'Page has unpublished changes'), - ]; - } - - $this->extend('updateStatusFlags', $flags); - - $this->_cache_statusFlags = $flags; - } - - return $this->_cache_statusFlags; - } - /** * Returns the CSS class used for the page icon in the site tree. * @@ -3008,6 +2709,7 @@ public function getTreeTitle() { $children = $this->creatableChildPages(); $flags = $this->getStatusFlags(); + $this->extend('updateStatusFlagsForTreeTitle', $flags); $treeTitle = sprintf( '%s', $this->getIconClass(), @@ -3252,49 +2954,15 @@ public function getPageIconURL() return null; } - /** - * Get description for this page type - * - * @return string|null - */ - public function classDescription() + public function classDescription(): ?string { // Ensure base class has an appropriate description if not explicitly set, // since we can't set that config for projects. $base = in_array(static::class, [Page::class, SiteTree::class]); - if ($base) { - $baseDescription = static::config()->get('base_class_description'); - // Fall back to the deprecated config - if (!$baseDescription) { - $baseDescription = static::config('base_description'); - } - return $baseDescription; - } - // For all other classes, use the direct class_description config - $description = parent::classDescription(); - if (!$description) { - // Fall back to the deprecated config - $description = static::config()->get('description'); - } - return $description; - } - - /** - * Overloaded to also provide entities for 'Page' class which is usually located in custom code, hence textcollector - * picks it up for the wrong folder. - * - * @return array - */ - public function provideI18nEntities() - { - $entities = parent::provideI18nEntities(); - - // Add optional description - $description = $this->classDescription(); - if ($description) { - $entities[static::class . '.DESCRIPTION'] = $description; + if ($base && !static::config()->get('class_description', Config::UNINHERITED)) { + return $this->config()->get('base_class_description'); } - return $entities; + return parent::classDescription(); } /** diff --git a/code/Model/VirtualPage.php b/code/Model/VirtualPage.php index 4f2188279a..cfbc2c6a82 100644 --- a/code/Model/VirtualPage.php +++ b/code/Model/VirtualPage.php @@ -28,11 +28,6 @@ */ class VirtualPage extends Page { - /** - * @deprecated 5.4.0 use class_description instead. - */ - private static $description = 'Displays the content of another page'; - private static string $class_description = 'Displays the content of another page'; private static $icon_class = 'font-icon-p-virtual'; @@ -184,6 +179,18 @@ public function allowedChildren() return []; } + /** + * Get the record to check against for allowed children check in validation. + */ + public function getRecordForAllowedChildrenValidation(): SiteTree + { + $copyFrom = $this->CopyContentFrom(); + if ($copyFrom && $copyFrom->exists()) { + return $copyFrom; + } + return $this; + } + public function syncLinkTracking() { if ($this->CopyContentFromID) { diff --git a/tests/php/Controllers/CMSMainTest.php b/tests/php/Controllers/CMSMainTest.php index 205166b404..0a9d0c8543 100644 --- a/tests/php/Controllers/CMSMainTest.php +++ b/tests/php/Controllers/CMSMainTest.php @@ -7,6 +7,7 @@ use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\CMS\Model\RedirectorPage; use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\CMS\Tests\Controllers\CMSMainTest\TestStatusFlagsPage; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse_Exception; @@ -23,6 +24,7 @@ use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\SiteConfig\SiteConfig; +use SilverStripe\Versioned\RecursivePublishable; use SilverStripe\Versioned\Versioned; class CMSMainTest extends FunctionalTest @@ -31,6 +33,10 @@ class CMSMainTest extends FunctionalTest protected static $orig = []; + protected static $extraDataObjects = [ + TestStatusFlagsPage::class, + ]; + protected function setUp(): void { parent::setUp(); @@ -358,10 +364,13 @@ public function testCreationOfRestrictedPage() public function testBreadcrumbs() { $page31 = $this->objFromFixture(SiteTree::class, 'page31'); + // Ensure there are no versioned badges populating the breadcrumbs + if ($page31->hasExtension(Versioned::class)) { + $page31->publishSingle(); + } $this->logInAs('admin'); $response = $this->get('admin/pages/edit/show/' . $page31->ID); - $parser = new CSSContentParser($response->getBody()); $this->assertCrumbs( ['Page 3', 'Page 3.1'], $response, @@ -369,9 +378,31 @@ public function testBreadcrumbs() ); } + public function testBreadcrumbsHaveStatusFlags() + { + $page = new TestStatusFlagsPage(); + $page->write(); + $this->logInAs('admin'); + + $response = $this->get('admin/pages/edit/show/' . $page->ID); + $parser = new CSSContentParser($response->getBody()); + $badges = $parser->getBySelector('.breadcrumbs-wrapper .crumb .badge'); + $badgesMarkup = ''; + foreach ($badges as $badge) { + $badgesMarkup .= $badge->asXML(); + } + $flagsMarkup = $page->getStatusFlagMarkup('badge--breadcrumbs'); + + $this->assertSame($flagsMarkup, $badgesMarkup); + } + public function testBreadcrumbsListView() { $page311 = $this->objFromFixture(SiteTree::class, 'page311'); + // Ensure there are no versioned badges populating the breadcrumbs + if ($page311->hasExtension(Versioned::class)) { + $page311->publishSingle(); + } $this->logInAs('admin'); $response = $this->get('admin/pages?ParentID=' . $page311->ID); @@ -382,9 +413,26 @@ public function testBreadcrumbsListView() ); } + public function testBreadcrumbsListViewHasStatusFlags() + { + $page = new TestStatusFlagsPage(); + $page->write(); + $this->logInAs('admin'); + + $response = $this->get('admin/pages?ParentID=' . $page->ID); + $parser = new CSSContentParser($response->getBody()); + $badges = $parser->getBySelector('.breadcrumbs-wrapper .crumb .badge'); + $badgesMarkup = ''; + foreach ($badges as $badge) { + $badgesMarkup .= $badge->asXML(); + } + $flagsMarkup = $page->getStatusFlagMarkup('badge--breadcrumbs'); + + $this->assertSame($flagsMarkup, $badgesMarkup); + } + public function testBreadcrumbsListViewTopLevel() { - $page311 = $this->objFromFixture(SiteTree::class, 'page311'); $this->logInAs('admin'); $response = $this->get('admin/pages'); @@ -398,6 +446,10 @@ public function testBreadcrumbsListViewTopLevel() public function testBreadcrumbsListViewWithPjax() { $page311 = $this->objFromFixture(SiteTree::class, 'page311'); + // Ensure there are no versioned badges populating the breadcrumbs + if ($page311->hasExtension(Versioned::class)) { + $page311->publishSingle(); + } $this->logInAs('admin'); $response = $this->get('admin/pages?ParentID=' . $page311->ID); @@ -411,6 +463,10 @@ public function testBreadcrumbsListViewWithPjax() public function testBreadcrumbsSearchView() { $page311 = $this->objFromFixture(SiteTree::class, 'page311'); + // Ensure there are no versioned badges populating the breadcrumbs + if ($page311->hasExtension(Versioned::class)) { + $page311->publishSingle(); + } $this->logInAs('admin'); $response = $this->get( @@ -423,12 +479,13 @@ public function testBreadcrumbsSearchView() ); $jsonStr = $response->getBody(); $data = json_decode($jsonStr, true); - + $parser = new CSSContentParser($data['Breadcrumbs']); $crumbs = $parser->getBySelector('.breadcrumbs-wrapper .crumb'); $crumbs = array_map(function ($crumb) { - return (string)$crumb; + // Whitespace doesn't matter, just the actual text + return trim((string)$crumb); }, $crumbs); $this->assertNotNull($crumbs, 'Should have found some crumbs'); @@ -445,7 +502,8 @@ private function assertCrumbs(array $expectedCrumbs, $response, string $message) $crumbs = $parser->getBySelector('.breadcrumbs-wrapper .crumb'); $crumbs = array_map(function ($crumb) { - return (string)$crumb; + // Whitespace doesn't matter, just the actual text + return trim((string)$crumb); }, $crumbs); $this->assertNotNull($crumbs, $message); diff --git a/tests/php/Controllers/CMSMainTest/TestStatusFlagsPage.php b/tests/php/Controllers/CMSMainTest/TestStatusFlagsPage.php new file mode 100644 index 0000000000..dd152051a9 --- /dev/null +++ b/tests/php/Controllers/CMSMainTest/TestStatusFlagsPage.php @@ -0,0 +1,16 @@ +assertStringEndsWith('changed-on-live/my-staff?stage=Live', $child->getAbsoluteLiveLink()); } - public function testDuplicateChildrenRetainSort() - { - $parent = new SiteTree(); - $parent->Title = 'Parent'; - $parent->write(); - - $child1 = new SiteTree(); - $child1->ParentID = $parent->ID; - $child1->Title = 'Child 1'; - $child1->Sort = 2; - $child1->write(); - - $child2 = new SiteTree(); - $child2->ParentID = $parent->ID; - $child2->Title = 'Child 2'; - $child2->Sort = 1; - $child2->write(); - - $duplicateParent = $parent->duplicateWithChildren(); - $duplicateChildren = $duplicateParent->AllChildren()->toArray(); - $this->assertCount(2, $duplicateChildren); - - $duplicateChild2 = array_shift($duplicateChildren); - $duplicateChild1 = array_shift($duplicateChildren); - - - $this->assertEquals('Child 1', $duplicateChild1->Title); - $this->assertEquals('Child 2', $duplicateChild2->Title); - - // assertGreaterThan works by having the LOWER value first - $this->assertGreaterThan($duplicateChild2->Sort, $duplicateChild1->Sort); - } - public function testDeleteFromStageOperatesRecursively() { Config::modify()->set(SiteTree::class, 'enforce_strict_hierarchy', false); @@ -1263,94 +1229,6 @@ public function testAllowedChildrenContainsCoreSubclassesButNotHiddenClass() ); } - /** - * Tests that various types of SiteTree classes will or will not be returned from the allowedChildren method - * @param string $className - * @param array $expected - * @param string $assertionMessage - */ - #[DataProvider('allowedChildrenProvider')] - public function testAllowedChildren($className, $expected, $assertionMessage) - { - $class = new $className(); - $this->assertEquals($expected, $class->allowedChildren(), $assertionMessage); - } - - /** - * @return array - */ - public static function allowedChildrenProvider() - { - return [ - [ - // Class name - SiteTreeTest_ClassA::class, - // Expected - [ SiteTreeTest_ClassB::class ], - // Assertion message - 'Direct setting of allowed children', - ], - [ - SiteTreeTest_ClassB::class, - [ SiteTreeTest_ClassC::class, SiteTreeTest_ClassCext::class ], - 'Includes subclasses', - ], - [ - SiteTreeTest_ClassC::class, - [], - 'Null setting', - ], - [ - SiteTreeTest_ClassD::class, - [SiteTreeTest_ClassC::class], - 'Excludes subclasses if class is prefixed by an asterisk', - ], - ]; - } - - public function testAllowedChildrenValidation() - { - $page = new SiteTree(); - $page->write(); - $classA = new SiteTreeTest_ClassA(); - $classA->write(); - $classB = new SiteTreeTest_ClassB(); - $classB->write(); - $classC = new SiteTreeTest_ClassC(); - $classC->write(); - $classD = new SiteTreeTest_ClassD(); - $classD->write(); - $classCext = new SiteTreeTest_ClassCext(); - $classCext->write(); - - $classB->ParentID = $page->ID; - $valid = $classB->validate(); - $this->assertTrue($valid->isValid(), "Does allow children on unrestricted parent"); - - $classB->ParentID = $classA->ID; - $valid = $classB->validate(); - $this->assertTrue($valid->isValid(), "Does allow child specifically allowed by parent"); - - $classC->ParentID = $classA->ID; - $valid = $classC->validate(); - $this->assertFalse($valid->isValid(), "Doesnt allow child on parents specifically restricting children"); - - $classB->ParentID = $classC->ID; - $valid = $classB->validate(); - $this->assertFalse($valid->isValid(), "Doesnt allow child on parents disallowing all children"); - - $classB->ParentID = $classCext->ID; - $valid = $classB->validate(); - $this->assertTrue($valid->isValid(), "Extensions of allowed classes are incorrectly reported as invalid"); - - $classCext->ParentID = $classD->ID; - $valid = $classCext->validate(); - $this->assertFalse( - $valid->isValid(), - "Doesnt allow child where only parent class is allowed on parent node, and asterisk prefixing is used" - ); - } - /** * @return void */ diff --git a/tests/php/Model/SiteTreeTest_ClassCext.php b/tests/php/Model/SiteTreeTest_ClassCext.php deleted file mode 100644 index 8ad10d84e2..0000000000 --- a/tests/php/Model/SiteTreeTest_ClassCext.php +++ /dev/null @@ -1,15 +0,0 @@ -write(); + // We need to flush the static cache so we don't have a stale reference to $page in $virtual->components + $virtual->flushCache(); $nonVirtual = $virtual; $nonVirtual->ClassName = VirtualPageTest_ClassA::class; @@ -618,7 +620,7 @@ public function testVirtualPageAsAnAllowedChild() try { $childVirtualPage->write(); } catch (ValidationException $e) { - $this->assertStringContainsString('not allowed as child of this parent page', $e->getMessage()); + $this->assertStringContainsString('not allowed as child of this parent record', $e->getMessage()); $isDetected = true; }