From 535cb46b0e7ce733e0d61609daebeccf59c620f4 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 8 Aug 2024 12:48:29 +1200 Subject: [PATCH] ENH Various changes to support SiteTree form field scaffolding --- src/Forms/FormScaffolder.php | 37 ++++++++----------- src/Forms/Tab.php | 11 +++++- src/ORM/DataObject.php | 11 ++---- .../InheritedPermissionsExtension.php | 17 +++++++++ tests/php/Forms/TabTest.php | 30 +++++++++++++++ 5 files changed, 76 insertions(+), 30 deletions(-) create mode 100644 tests/php/Forms/TabTest.php diff --git a/src/Forms/FormScaffolder.php b/src/Forms/FormScaffolder.php index 87f59b85933..099dabf5d6e 100644 --- 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." (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,16 +35,10 @@ 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. @@ -52,15 +46,15 @@ class FormScaffolder 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,7 +100,7 @@ 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 @@ -114,7 +108,7 @@ public function getFieldList() 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 index 3f3fe538d28..0133010611f 100644 --- 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 index aa7f82c5177..3852d640c0c 100644 --- 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 index 2b2432d3d1c..e9c1975c018 100644 --- 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 new file mode 100644 index 00000000000..8b1ed439a96 --- /dev/null +++ b/tests/php/Forms/TabTest.php @@ -0,0 +1,30 @@ +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()); + } +}