ENH Various changes to support SiteTree form field scaffolding (silve…

…rstripe#11327)
Cheddam · Aug 15, 2024 · 64a17e0 · 64a17e0
1 parent a8c3220
commit 64a17e0
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 30 deletions.
diff --git a/src/Forms/FormScaffolder.php b/src/Forms/FormScaffolder.php
@@ -17,16 +17,16 @@ class FormScaffolder
     use Injectable;
 
     /**
-     * @var DataObject $obj The object defining the fields to be scaffolded
+     * The object defining the fields to be scaffolded
      * through its metadata like $db, $searchable_fields, etc.
      */
-    protected $obj;
+    protected DataObject $obj;
 
     /**
-     * @var boolean $tabbed Return fields in a tabset, with all main fields in the path "Root.Main",
+     * Return fields in a tabset, with all main fields in the path "Root.Main",
      * relation fields in "Root.<relationname>" (if {@link $includeRelations} is enabled).
      */
-    public $tabbed = false;
+    public bool $tabbed = false;
 
     /**
      * Only set up the "Root.Main" tab, but skip scaffolding actual FormFields.
@@ -35,32 +35,26 @@ class FormScaffolder
     public bool $mainTabOnly = false;
 
     /**
-     * @var boolean $ajaxSafe
-     * @deprecated 5.3.0 Will be removed without equivalent functionality.
+     * Array of field names to use as an allow list.
+     * If left blank, all fields from {@link DataObject->db()} will be included unless explicitly ignored.
      */
-    public $ajaxSafe = false;
-
-    /**
-     * @var array $restrictFields Numeric array of a field name whitelist.
-     * If left blank, all fields from {@link DataObject->db()} will be included.
-     */
-    public $restrictFields;
+    public array $restrictFields = [];
 
     /**
      * Numeric array of field names and has_one relations to explicitly not scaffold.
      */
     public array $ignoreFields = [];
 
     /**
-     * @var array $fieldClasses Optional mapping of fieldnames to subclasses of {@link FormField}.
+     * Optional mapping of fieldnames to subclasses of {@link FormField}.
      * By default the scaffolder will determine the field instance by {@link DBField::scaffoldFormField()}.
      */
-    public $fieldClasses;
+    public array $fieldClasses = [];
 
     /**
-     * @var boolean $includeRelations Include has_many and many_many relations
+     * Include has_many and many_many relations
      */
-    public $includeRelations = false;
+    public bool|array $includeRelations = false;
 
     /**
      * Array of relation names to use as an allow list.
@@ -106,15 +100,15 @@ public function getFieldList()
         // Add logical fields directly specified in db config
         foreach ($this->obj->config()->get('db') as $fieldName => $fieldType) {
             // Skip fields that aren't in the allow list
-            if ($this->restrictFields && !in_array($fieldName, $this->restrictFields ?? [])) {
+            if (!empty($this->restrictFields) && !in_array($fieldName, $this->restrictFields)) {
                 continue;
             }
             // Skip ignored fields
             if (in_array($fieldName, $this->ignoreFields)) {
                 continue;
             }
 
-            if ($this->fieldClasses && isset($this->fieldClasses[$fieldName])) {
+            if (isset($this->fieldClasses[$fieldName])) {
                 $fieldClass = $this->fieldClasses[$fieldName];
                 $fieldObject = new $fieldClass($fieldName);
             } else {
@@ -138,7 +132,7 @@ public function getFieldList()
         // add has_one relation fields
         if ($this->obj->hasOne()) {
             foreach ($this->obj->hasOne() as $relationship => $component) {
-                if ($this->restrictFields && !in_array($relationship, $this->restrictFields ?? [])) {
+                if (!empty($this->restrictFields) && !in_array($relationship, $this->restrictFields)) {
                     continue;
                 }
                 if (in_array($relationship, $this->ignoreFields)) {
@@ -147,7 +141,7 @@ public function getFieldList()
                 $fieldName = $component === 'SilverStripe\\ORM\\DataObject'
                     ? $relationship // Polymorphic has_one field is composite, so don't refer to ID subfield
                     : "{$relationship}ID";
-                if ($this->fieldClasses && isset($this->fieldClasses[$fieldName])) {
+                if (isset($this->fieldClasses[$fieldName])) {
                     $fieldClass = $this->fieldClasses[$fieldName];
                     $hasOneField = new $fieldClass($fieldName);
                 } else {
@@ -305,7 +299,6 @@ protected function getParamsArray()
             'restrictFields' => $this->restrictFields,
             'ignoreFields' => $this->ignoreFields,
             'fieldClasses' => $this->fieldClasses,
-            'ajaxSafe' => $this->ajaxSafe
         ];
     }
 }
diff --git a/src/Forms/Tab.php b/src/Forms/Tab.php
@@ -75,7 +75,6 @@ public function __construct($name, $titleOrField = null, $fields = null)
         // Assign name and title (not assigned by parent constructor)
         $this->setName($name);
         $this->setTitle($title);
-        $this->setID(Convert::raw2htmlid($name));
     }
 
     public function ID()
@@ -99,6 +98,16 @@ public function setID($id)
         return $this;
     }
 
+    public function setName($name)
+    {
+        // Use raw properties instead of ID() and getName() here because we
+        // only want to check the actual raw values of those properties.
+        if (($this->id ?? '') === Convert::raw2htmlid($this->name)) {
+            $this->setID(Convert::raw2htmlid($name));
+        }
+        return parent::setName($name);
+    }
+
     /**
      * Get child fields
      *

diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php
@@ -295,7 +295,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
     private static array $scaffold_cms_fields_settings = [
         'includeRelations' => true,
         'tabbed' => true,
-        'ajaxSafe' => true,
     ];
 
     /**
@@ -2396,8 +2395,8 @@ public function scaffoldSearchFields($_params = null)
     {
         $params = array_merge(
             [
-                'fieldClasses' => false,
-                'restrictFields' => false
+                'fieldClasses' => [],
+                'restrictFields' => []
             ],
             (array)$_params
         );
@@ -2487,10 +2486,9 @@ public function scaffoldFormFields($_params = null)
                 'includeRelations' => false,
                 'restrictRelations' => [],
                 'ignoreRelations' => [],
-                'restrictFields' => false,
+                'restrictFields' => [],
                 'ignoreFields' => [],
-                'fieldClasses' => false,
-                'ajaxSafe' => false
+                'fieldClasses' => [],
             ],
             (array)$_params
         );
@@ -2504,7 +2502,6 @@ public function scaffoldFormFields($_params = null)
         $fs->restrictFields = $params['restrictFields'];
         $fs->ignoreFields = $params['ignoreFields'];
         $fs->fieldClasses = $params['fieldClasses'];
-        $fs->ajaxSafe = $params['ajaxSafe'];
 
         $this->extend('updateFormScaffolder', $fs, $this);
 

diff --git a/src/Security/InheritedPermissionsExtension.php b/src/Security/InheritedPermissionsExtension.php
@@ -42,4 +42,21 @@ class InheritedPermissionsExtension extends DataExtension
         'ViewerMembers',
         'EditorMembers',
     ];
+
+    /**
+     * These fields will need to be added manually, since SiteTree wants it in the special settings tab
+     * and nothing else in code that uses these fields is scaffolded.
+     */
+    private static array $scaffold_cms_fields_settings = [
+        'ignoreFields' => [
+            'CanViewType',
+            'CanEditType',
+        ],
+        'ignoreRelations' => [
+            'ViewerGroups',
+            'EditorGroups',
+            'ViewerMembers',
+            'EditorMembers',
+        ],
+    ];
 }
diff --git a/tests/php/Forms/TabTest.php b/tests/php/Forms/TabTest.php
@@ -0,0 +1,30 @@
+<?php
+
+namespace SilverStripe\Forms\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Forms\Tab;
+
+class TabTest extends SapphireTest
+{
+    protected $usesDatabase = false;
+
+    public function testNameAndID(): void
+    {
+        // ID is set on instantiation based on the name
+        $tab = new Tab('MyName _-()!@#$');
+        $this->assertSame('MyName _-()!@#$', $tab->getName());
+        $this->assertSame('MyName_-', $tab->ID());
+
+        // Changing the name changes the ID
+        $tab->setName('NewName');
+        $this->assertSame('NewName', $tab->getName());
+        $this->assertSame('NewName', $tab->ID());
+
+        // If ID is explicitly set, changing the name doesn't override it
+        $tab->setID('Custom-ID');
+        $tab->setName('AnotherName');
+        $this->assertSame('AnotherName', $tab->getName());
+        $this->assertSame('Custom-ID', $tab->ID());
+    }
+}