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

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Aug 8, 2024

Doing this to support the CMS 6 changes - but there's no reason for this new feature to wait until April since it's not BC breaking.

  • Adds three new options for FormScaffolder:
    • tabsOnly: Only return the Root.Main tab in the FieldList (effectively don't scaffold anything).
      • This isn't strictly necessary, but it does make it easier to follow best practice (calling parent::getCMSFields()) even in scenarios where you want to build all your form fields from scratch.
    • ignoreFields: Array of field names in $db or relations in $has_one which should not be scaffolded.
    • ignoreRelations: Array of relations in $has_many or $many_many which should not be scaffolded.
    • restrictRelations: Array of relation names to use as an allow list. Similar to the existing restrictFields
  • Makes the options used to scaffold fields in getCMSFields() configurable.

Note

ignoreFields includes $has_one to match the behaviour of both restrictFields (includes $has_one) and includeRelations (for $has_many and $many_many only)

Issue

Comment on lines +410 to +412
if ($currentPointer === null) {
return null;
}
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.

Comment on lines -49 to +61
* @var boolean $includeRelations Include has_one, has_many and many_many relations
* @var boolean $includeRelations Include has_many and many_many relations
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

Comment on lines 37 to 41
/**
* @var boolean $ajaxSafe
* @deprecated 5.3.0 Will be removed without equivalent functionality.
*/
public $ajaxSafe = false;
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

@GuySartorelli GuySartorelli force-pushed the pulls/5/configurable-scaffolding-options branch from 6c64bf0 to c1a180b Compare August 11, 2024 23:24
@GuySartorelli
Copy link
Member Author

Added restrictRelations option which mirrors restrictFields option. This will facilitate easier upgrades for people who just want to use the old behaviour instead of updating the getCMSFields() and updateCMSFields() implementations in CMS 6, as they can just allow things to scaffold for SiteTree and other third-party pages, and leave their own pages as they are.

@GuySartorelli GuySartorelli force-pushed the pulls/5/configurable-scaffolding-options branch from c1a180b to 3a63d18 Compare August 11, 2024 23:27
tests/php/Forms/FieldListTest.php Show resolved Hide resolved
src/ORM/DataObject.php Outdated Show resolved Hide resolved
src/ORM/DataObject.php Outdated Show resolved Hide resolved
Note that includeRelations was intentionally changed to not include has_one in 524d7a9
@GuySartorelli GuySartorelli force-pushed the pulls/5/configurable-scaffolding-options branch from 3a63d18 to 2de54c8 Compare August 12, 2024 00:19
@emteknetnz emteknetnz merged commit dca62c7 into silverstripe:5 Aug 12, 2024
14 checks passed
@emteknetnz emteknetnz deleted the pulls/5/configurable-scaffolding-options branch August 12, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants