Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEW Make CMSFields scaffolding configurable, plus new options #11328

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Forms/FieldList.php
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,9 @@ public function findTab($tabName)
$currentPointer = $this;

foreach ($parts as $k => $part) {
if ($currentPointer === null) {
return null;
}
Comment on lines +410 to +412
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't strictly needed, but it makes the test easier.
It seemed silly to be throwing an exception when $currentPointer is null, because in this scenario the tab doesn't exist so we can just return null from the method.

$currentPointer = $currentPointer->fieldByName($part);
}

Expand Down
54 changes: 52 additions & 2 deletions src/Forms/FormScaffolder.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,15 @@ class FormScaffolder
*/
public $tabbed = false;

/**
* Only set up the "Root.Main" tab, but skip scaffolding actual FormFields.
* If $tabbed is false, an empty FieldList will be returned.
*/
public bool $mainTabOnly = false;

/**
* @var boolean $ajaxSafe
* @deprecated 5.3.0 Will be removed without equivalent functionality.
*/
public $ajaxSafe = false;
Comment on lines 37 to 41
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually used at all


Expand All @@ -39,17 +46,33 @@ class FormScaffolder
*/
public $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}.
* By default the scaffolder will determine the field instance by {@link DBField::scaffoldFormField()}.
*/
public $fieldClasses;

/**
* @var boolean $includeRelations Include has_one, has_many and many_many relations
* @var boolean $includeRelations Include has_many and many_many relations
Comment on lines -49 to +61
Copy link
Member Author

@GuySartorelli GuySartorelli Aug 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentionally changed to not include has_one in 524d7a9

*/
public $includeRelations = false;

/**
* Array of relation names to use as an allow list.
* If left blank, all has_many and many_many relations will be scaffolded unless explicitly ignored.
*/
public array $restrictRelations = [];

/**
* Numeric array of has_many and many_many relations to explicitly not scaffold.
*/
public array $ignoreRelations = [];

/**
* @param DataObject $obj
*/
Expand All @@ -76,12 +99,20 @@ public function getFieldList()
$mainTab->setTitle(_t(__CLASS__ . '.TABMAIN', 'Main'));
}

if ($this->mainTabOnly) {
return $fields;
}

// Add logical fields directly specified in db config
foreach ($this->obj->config()->get('db') as $fieldName => $fieldType) {
// Skip restricted fields
// Skip fields that aren't in the allow list
if ($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])) {
$fieldClass = $this->fieldClasses[$fieldName];
Expand Down Expand Up @@ -110,6 +141,9 @@ public function getFieldList()
if ($this->restrictFields && !in_array($relationship, $this->restrictFields ?? [])) {
continue;
}
if (in_array($relationship, $this->ignoreFields)) {
continue;
}
$fieldName = $component === 'SilverStripe\\ORM\\DataObject'
? $relationship // Polymorphic has_one field is composite, so don't refer to ID subfield
: "{$relationship}ID";
Expand Down Expand Up @@ -138,6 +172,12 @@ public function getFieldList()
&& ($this->includeRelations === true || isset($this->includeRelations['has_many']))
) {
foreach ($this->obj->hasMany() as $relationship => $component) {
if (!empty($this->restrictRelations) && !in_array($relationship, $this->restrictRelations)) {
continue;
}
if (in_array($relationship, $this->ignoreRelations)) {
continue;
}
$includeInOwnTab = true;
$fieldLabel = $this->obj->fieldLabel($relationship);
$fieldClass = (isset($this->fieldClasses[$relationship]))
Expand Down Expand Up @@ -177,6 +217,12 @@ public function getFieldList()
&& ($this->includeRelations === true || isset($this->includeRelations['many_many']))
) {
foreach ($this->obj->manyMany() as $relationship => $component) {
if (!empty($this->restrictRelations) && !in_array($relationship, $this->restrictRelations)) {
continue;
}
if (in_array($relationship, $this->ignoreRelations)) {
continue;
}
static::addManyManyRelationshipFields(
$fields,
$relationship,
Expand Down Expand Up @@ -252,8 +298,12 @@ protected function getParamsArray()
{
return [
'tabbed' => $this->tabbed,
'mainTabOnly' => $this->mainTabOnly,
'includeRelations' => $this->includeRelations,
'restrictRelations' => $this->restrictRelations,
'ignoreRelations' => $this->ignoreRelations,
'restrictFields' => $this->restrictFields,
'ignoreFields' => $this->ignoreFields,
'fieldClasses' => $this->fieldClasses,
'ajaxSafe' => $this->ajaxSafe
];
Expand Down
29 changes: 23 additions & 6 deletions src/ORM/DataObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
*/
private static $table_name = null;

/**
* Settings used by the FormScaffolder that scaffolds fields for getCMSFields()
*/
private static array $scaffold_cms_fields_settings = [
'includeRelations' => true,
'tabbed' => true,
'ajaxSafe' => true,
];

/**
* Non-static relationship cache, indexed by component name.
*
Expand Down Expand Up @@ -2469,8 +2478,12 @@ public function scaffoldFormFields($_params = null)
$params = array_merge(
[
'tabbed' => false,
'mainTabOnly' => false,
'includeRelations' => false,
'restrictRelations' => [],
'ignoreRelations' => [],
'restrictFields' => false,
'ignoreFields' => [],
'fieldClasses' => false,
'ajaxSafe' => false
],
Expand All @@ -2479,8 +2492,12 @@ public function scaffoldFormFields($_params = null)

$fs = FormScaffolder::create($this);
$fs->tabbed = $params['tabbed'];
$fs->mainTabOnly = $params['mainTabOnly'];
$fs->includeRelations = $params['includeRelations'];
$fs->restrictRelations = $params['restrictRelations'];
$fs->ignoreRelations = $params['ignoreRelations'];
$fs->restrictFields = $params['restrictFields'];
$fs->ignoreFields = $params['ignoreFields'];
$fs->fieldClasses = $params['fieldClasses'];
$fs->ajaxSafe = $params['ajaxSafe'];

Expand Down Expand Up @@ -2600,12 +2617,12 @@ protected function afterUpdateCMSFields(callable $callback)
*/
public function getCMSFields()
{
$tabbedFields = $this->scaffoldFormFields([
// Don't allow has_many/many_many relationship editing before the record is first saved
'includeRelations' => ($this->ID > 0),
'tabbed' => true,
'ajaxSafe' => true
]);
$scaffoldOptions = static::config()->get('scaffold_cms_fields_settings');
// Don't allow has_many/many_many relationship editing before the record is first saved
if (!$this->isInDB()) {
$scaffoldOptions['includeRelations'] = false;
}
$tabbedFields = $this->scaffoldFormFields($scaffoldOptions);

$this->extend('updateCMSFields', $tabbedFields);

Expand Down
2 changes: 2 additions & 0 deletions tests/php/Forms/FieldListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ public function testFindTab()
$this->assertNull($fields->findTab('More'));
$this->assertEquals($fields->findTab('Root.More'), $more);
$this->assertEquals($fields->findTab('Root.More.Tab4'), $tab4);

GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
$this->assertNull($fields->findTab('This.Doesnt.Exist'));
}

/**
Expand Down
161 changes: 148 additions & 13 deletions tests/php/Forms/FormScaffolderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,40 @@ public function testGetFormFields()

public function provideScaffoldRelationFormFields()
{
return [
[true],
[false],
$scenarios = [
'ignore no relations' => [
'includeInOwnTab' => true,
'ignoreRelations' => [],
],
'ignore some relations' => [
'includeInOwnTab' => true,
'ignoreRelations' => [
'ChildrenHasMany',
'ChildrenManyManyThrough',
],
],
];
foreach ($scenarios as $name => $scenario) {
$scenario['includeInOwnTab'] = false;
$scenarios[$name . ' - not in own tab'] = $scenario;
}
return $scenarios;
}

/**
* @dataProvider provideScaffoldRelationFormFields
*/
public function testScaffoldRelationFormFields(bool $includeInOwnTab)
public function testScaffoldRelationFormFields(bool $includeInOwnTab, array $ignoreRelations)
{
$parent = $this->objFromFixture(ParentModel::class, 'parent1');
Child::$includeInOwnTab = $includeInOwnTab;
$fields = $parent->scaffoldFormFields(['includeRelations' => true, 'tabbed' => true]);
$fields = $parent->scaffoldFormFields([
'includeRelations' => true,
'tabbed' => true,
'ignoreRelations' => $ignoreRelations,
]);

// has_one
foreach (array_keys(ParentModel::config()->uninherited('has_one')) as $hasOneName) {
$scaffoldedFormField = $fields->dataFieldByName($hasOneName . 'ID');
if ($hasOneName === 'ChildPolymorphic') {
Expand All @@ -196,20 +215,136 @@ public function testScaffoldRelationFormFields(bool $includeInOwnTab)
$this->assertInstanceOf(DateField::class, $scaffoldedFormField, "$hasOneName should be a DateField");
}
}
// has_many
foreach (array_keys(ParentModel::config()->uninherited('has_many')) as $hasManyName) {
$this->assertInstanceOf(CurrencyField::class, $fields->dataFieldByName($hasManyName), "$hasManyName should be a CurrencyField");
if ($includeInOwnTab) {
$this->assertNotNull($fields->findTab("Root.$hasManyName"));
if (in_array($hasManyName, $ignoreRelations)) {
$this->assertNull($fields->dataFieldByName($hasManyName));
} else {
$this->assertNull($fields->findTab("Root.$hasManyName"));
$this->assertInstanceOf(CurrencyField::class, $fields->dataFieldByName($hasManyName), "$hasManyName should be a CurrencyField");
if ($includeInOwnTab) {
$this->assertNotNull($fields->findTab("Root.$hasManyName"));
} else {
$this->assertNull($fields->findTab("Root.$hasManyName"));
}
}
}
// many_many
foreach (array_keys(ParentModel::config()->uninherited('many_many')) as $manyManyName) {
$this->assertInstanceOf(TimeField::class, $fields->dataFieldByName($manyManyName), "$manyManyName should be a TimeField");
if ($includeInOwnTab) {
$this->assertNotNull($fields->findTab("Root.$manyManyName"));
if (in_array($hasManyName, $ignoreRelations)) {
$this->assertNull($fields->dataFieldByName($hasManyName));
} else {
$this->assertInstanceOf(TimeField::class, $fields->dataFieldByName($manyManyName), "$manyManyName should be a TimeField");
if ($includeInOwnTab) {
$this->assertNotNull($fields->findTab("Root.$manyManyName"));
} else {
$this->assertNull($fields->findTab("Root.$manyManyName"));
}
}
}
}

public function testScaffoldIgnoreFields(): void
{
$article1 = $this->objFromFixture(Article::class, 'article1');
$fields = $article1->scaffoldFormFields([
'ignoreFields' => [
'Content',
'Author',
],
]);
$this->assertSame(['ExtendedField', 'Title'], $fields->column('Name'));
}

public function testScaffoldRestrictRelations(): void
{
$article1 = $this->objFromFixture(Article::class, 'article1');
$fields = $article1->scaffoldFormFields([
'includeRelations' => true,
'restrictRelations' => [
'Tags',
],
// Ensure no db or has_one fields get scaffolded
'restrictFields' => [
'non-existent',
],
]);
$this->assertSame(['Tags'], $fields->column('Name'));
}

public function provideTabs(): array
{
return [
'only main tab' => [
'tabs' => true,
'mainTabOnly' => true,
],
'all tabs, all fields' => [
'tabs' => true,
'mainTabOnly' => false,
],
'no tabs, no fields' => [
'tabs' => false,
'mainTabOnly' => true,
],
'no tabs, all fields' => [
'tabs' => false,
'mainTabOnly' => false,
],
];
}

/**
* @dataProvider provideTabs
*/
public function testTabs(bool $tabbed, bool $mainTabOnly): void
{
$parent = $this->objFromFixture(ParentModel::class, 'parent1');
Child::$includeInOwnTab = true;
$fields = $parent->scaffoldFormFields([
'tabbed' => $tabbed,
'mainTabOnly' => $mainTabOnly,
'includeRelations' => true,
]);

$fieldsToExpect = [
['Name' => 'Title'],
['Name' => 'ChildID'],
['Name' => 'ChildrenHasMany'],
['Name' => 'ChildrenManyMany'],
['Name' => 'ChildrenManyManyThrough'],
];
$relationTabs = [
'Root.ChildrenHasMany',
'Root.ChildrenManyMany',
'Root.ChildrenManyManyThrough',
];

if ($tabbed) {
$this->assertNotNull($fields->findTab('Root.Main'));
if ($mainTabOnly) {
// Only Root.Main with no fields
$this->assertListNotContains($fieldsToExpect, $fields->flattenFields());
foreach ($relationTabs as $tabName) {
$this->assertNull($fields->findTab($tabName));
}
} else {
// All fields in all tabs
$this->assertListContains($fieldsToExpect, $fields->flattenFields());
foreach ($relationTabs as $tabName) {
$this->assertNotNull($fields->findTab($tabName));
}
}
} else {
if ($mainTabOnly) {
// Empty list
$this->assertEmpty($fields);
} else {
$this->assertNull($fields->findTab("Root.$manyManyName"));
// All fields, no tabs
$this->assertNull($fields->findTab('Root.Main'));
foreach ($relationTabs as $tabName) {
$this->assertNull($fields->findTab($tabName));
}
$this->assertListContains($fieldsToExpect, $fields->flattenFields());
}
}
}
Expand Down
Loading