API Move some code to framework to be reusable #3022

GuySartorelli · 2024-11-10T22:44:11Z

This logic is used by CMSMain but 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 CMSMain calls needs to be available on a DataObject record which has the Hierarchy extension applied.

Issue

Don't hardcode SiteTree in CMSMain #2947

GuySartorelli · 2024-11-11T02:42:58Z

code/Model/SiteTree.php

-    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 = [];
-


Moved to Hierarchy in framework.

GuySartorelli · 2024-11-11T02:43:21Z

code/Model/SiteTree.php

-    private static $default_child = "Page";
+    private static $default_child = Page::class;


Unrelated cleanup - this is a class name, not an arbitrary string.

GuySartorelli · 2024-11-11T02:43:39Z

code/Model/SiteTree.php

-     * @config
-     * @var string
-     */
-    private static $default_parent = null;


default_parent moved to Hierarchy in framework.

GuySartorelli · 2024-11-11T02:43:51Z

code/Model/SiteTree.php

-    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;


Unrelated cleanup - default_classname is a class name, not an arbitrary string.

GuySartorelli · 2024-11-11T02:44:34Z

code/Model/SiteTree.php

@@ -330,6 +296,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi

    private static $default_sort = "\"Sort\"";

+    private static string $sort_field = 'Sort';


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.

GuySartorelli · 2024-11-11T02:44:54Z

code/Model/SiteTree.php

-     * @config
-     * @var string
-     */
-    private static $description = null;


Moved to DataObject in framework and renamed to a more specific class_description

GuySartorelli · 2024-11-11T02:45:09Z

code/Model/SiteTree.php

     */
-    private static $base_description = 'Generic content page';
+    private static string $base_class_description = 'Generic content page';


Renamed to match the description => class_description renaming

GuySartorelli · 2024-11-11T02:49:55Z

code/Model/SiteTree.php

-     *
-     * @return array
-     */
-    public function provideI18nEntities()


Moved the description entity to DataObject in framework. This method is no longer needed in this class.

GuySartorelli · 2024-11-11T02:50:23Z

code/Model/VirtualPage.php

+    /**
+     * Get the record to check against for allowed children check in validation.
+     */
+    public function getRecordForAllowedChildrenValidation(): SiteTree


See Hierarchy::updateValidate() in framework

GuySartorelli · 2024-11-11T02:50:36Z

tests/php/Model/SiteTreeTest.php

Tests moved along with the logic they're testing

GuySartorelli · 2024-11-11T02:50:47Z

tests/php/Model/SiteTreeTest_ClassCext.php

This class was only ever used in a test that got moved

GuySartorelli · 2024-11-11T04:19:43Z

tests/php/Model/VirtualPageTest.php

+        // We need to flush the static cache so we don't have a stale reference to $page in $virtual->components
+        $virtual->flushCache();


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 · 2024-11-20T02:36:22Z

code/Model/SiteTree.php

@@ -3008,6 +2709,7 @@ public function getTreeTitle()
    {
        $children = $this->creatableChildPages();
        $flags = $this->getStatusFlags();
+        $this->extend('updateStatusFlagsForTreeTitle', $flags);


This new extension hook allows fluent to exclude its flags from the site tree list, because they look awful there.
In the next set of PRs after this one, it'll be on CMSMain instead, but for now SiteTree directly handles how things look in that tree.

Some of this code needs to be usable on classes other than SiteTree with the CMSMain refactoring.

GuySartorelli marked this pull request as draft November 10, 2024 22:44

GuySartorelli force-pushed the pulls/6/move-to-framework branch from 4e32958 to f7b0fbf Compare November 11, 2024 02:17

GuySartorelli commented Nov 11, 2024

View reviewed changes

GuySartorelli force-pushed the pulls/6/move-to-framework branch 2 times, most recently from c5b3124 to 0148368 Compare November 11, 2024 20:39

GuySartorelli mentioned this pull request Nov 11, 2024

API Deprecate and update code in preparation for CMS 6 #3023

Merged

GuySartorelli commented Nov 11, 2024

View reviewed changes

GuySartorelli force-pushed the pulls/6/move-to-framework branch 2 times, most recently from 0e6a639 to 8b2df92 Compare November 14, 2024 04:37

GuySartorelli marked this pull request as ready for review November 14, 2024 04:40

GuySartorelli force-pushed the pulls/6/move-to-framework branch from 8b2df92 to 2f9e750 Compare November 20, 2024 02:34

GuySartorelli commented Nov 20, 2024

View reviewed changes

GuySartorelli force-pushed the pulls/6/move-to-framework branch 2 times, most recently from 2b2fcea to 52a3e72 Compare November 21, 2024 21:19

API Move some code to framework to be reusable

34a10aa

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 52a3e72 to 34a10aa Compare November 21, 2024 21:19

emteknetnz approved these changes Nov 26, 2024

View reviewed changes

emteknetnz merged commit 0778d7b into silverstripe:6 Nov 26, 2024
6 of 12 checks passed

emteknetnz deleted the pulls/6/move-to-framework branch November 26, 2024 03:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API Move some code to framework to be reusable #3022

API Move some code to framework to be reusable #3022

GuySartorelli commented Nov 10, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 11, 2024

GuySartorelli Nov 20, 2024

		private static $default_child = "Page";
		private static $default_child = Page::class;

		@@ -330,6 +296,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi

		private static $default_sort = "\"Sort\"";

		private static string $sort_field = 'Sort';

		// We need to flush the static cache so we don't have a stale reference to $page in $virtual->components
		$virtual->flushCache();

API Move some code to framework to be reusable #3022

API Move some code to framework to be reusable #3022

Conversation

GuySartorelli commented Nov 10, 2024

Issue

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment