API Move some code to framework to be reusable #3022
Conversation
GuySartorelli
force-pushed
the
pulls/6/move-to-framework
branch
from
4e32958
to
f7b0fbf
Compare
code/Model/SiteTree.php
Outdated
| 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 | ||
| */ | ||
| protected static $_allowedChildren = []; | ||
|
|
There was a problem hiding this comment.
Moved to Hierarchy in framework.
| private static $default_child = "Page"; | ||
| private static $default_child = Page::class; |
There was a problem hiding this comment.
Unrelated cleanup - this is a class name, not an arbitrary string.
GuySartorelli
force-pushed
the
pulls/6/move-to-framework
branch
2 times, most recently
from
c5b3124
to
0148368
Compare
| * @config | ||
| * @var string | ||
| */ | ||
| private static $default_parent = null; |
There was a problem hiding this comment.
default_parent moved to Hierarchy in framework.
| 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; |
There was a problem hiding this comment.
Unrelated cleanup - default_classname is a class name, not an arbitrary string.
| @@ -330,6 +296,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi | |||
|
|
|||
| private static $default_sort = "\"Sort\""; | |||
|
|
|||
| private static string $sort_field = 'Sort'; | |||
There was a problem hiding this comment.
New config from Hierarchy in framework. This allows CMSMain and Hierarchy both to reason about arbitrary models without requiring a hardcoded specific field name for sorting.
| * @config | ||
| * @var string | ||
| */ | ||
| private static $description = null; |
There was a problem hiding this comment.
Moved to DataObject in framework and renamed to a more specific class_description
| */ | ||
| private static $base_description = 'Generic content page'; | ||
| private static string $base_class_description = 'Generic content page'; |
There was a problem hiding this comment.
Renamed to match the description => class_description renaming
| * | ||
| * @return array | ||
| */ | ||
| public function provideI18nEntities() |
There was a problem hiding this comment.
Moved the description entity to DataObject in framework. This method is no longer needed in this class.
| /** | ||
| * Get the record to check against for allowed children check in validation. | ||
| */ | ||
| public function getRecordForAllowedChildrenValidation(): SiteTree |
There was a problem hiding this comment.
See Hierarchy::updateValidate() in framework
tests/php/Model/SiteTreeTest.php
Outdated
There was a problem hiding this comment.
Tests moved along with the logic they're testing
There was a problem hiding this comment.
This class was only ever used in a test that got moved
| // We need to flush the static cache so we don't have a stale reference to $page in $virtual->components | ||
| $virtual->flushCache(); |
There was a problem hiding this comment.
This test was really flakey - it relied on the fact that we hadn't fetched the CopyContentFrom has_one record yet. Changes in this and the framework PR resulted in it being fetched so we had a stale reference to the $page record that got originally saved (with the "published title" text).
We have to clear that out of the static cache here, so that we fetch the record with the "original" text so that can be copied through to our virtual page on write.
GuySartorelli
force-pushed
the
pulls/6/move-to-framework
branch
from
0148368
to
0e6a639
Compare
Some of this code needs to be usable on classes other than SiteTree with the CMSMain refactoring.
GuySartorelli
force-pushed
the
pulls/6/move-to-framework
branch
from
0e6a639
to
8b2df92
Compare
This logic is used by
CMSMainbut needs to be callable on any hierarchical model.See the corresponding framework PR to see where this code is going.
See the overall refactoring PR (or just look in CMSMain) to see where it's being used and why it needs to be moved. Basically, anything
CMSMaincalls needs to be available on aDataObjectrecord which has theHierarchyextension applied.Issue
SiteTreeinCMSMain#2947