From 4c3a068859e94050797b01c16b57786dff4fa741 Mon Sep 17 00:00:00 2001 From: Simon Gow Date: Wed, 29 Aug 2018 14:34:21 +1200 Subject: [PATCH 1/2] Issue 3357 - Add GridField Readonly Transformation GridField doesn't have a valid readonly state if it's value is set to an Object without `forTemplate()`. The default behaviour is to render a ReadonlyField, but given GridField is a complex type this isn't suitable. This bugfix provides a transformation method to render only components that are whitelisted to provide a readonly state. @see #3357 - https://github.com/silverstripe/silverstripe-framework/issues/3357 --- src/Forms/GridField/GridField.php | 69 +++++++++++++++ .../Forms/GridField/GridFieldReadonlyTest.php | 87 +++++++++++++++++++ .../Forms/GridField/GridFieldReadonlyTest.yml | 27 ++++++ 3 files changed, 183 insertions(+) create mode 100644 tests/php/Forms/GridField/GridFieldReadonlyTest.php create mode 100644 tests/php/Forms/GridField/GridFieldReadonlyTest.yml diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index b66dc274e..5d959596d 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -105,6 +105,21 @@ class GridField extends FormField */ protected $name = ''; + /** + * A whitelist of readonly component classes allowed if performReadonlyTransform is called. + * + * @var array + */ + protected $readonlyComponents = array( + GridFieldDetailForm::class, + GridFieldDataColumns::class, + GridFieldConfig_RecordViewer::class, + GridFieldToolbarHeader::class, + GridFieldPageCount::class, + GridFieldPaginator::class, + GridState_Component::class + ); + /** * Pattern used for looking up */ @@ -193,6 +208,60 @@ class GridField extends FormField ); } + /** + * Overload the readonly components for this gridfield. + * + * @param array $components an array map of component class references to whitelist for a readonly version. + */ + public function setReadonlyComponents(array $components) + { + $this->readonlyComponents = $components; + } + + /** + * Return the readonly components + * + * @return array a map of component classes. + */ + public function getReadonlyComponents() + { + return $this->readonlyComponents; + } + + /** + * Custom Readonly transformation to remove actions which shouldn't be present for a readonly state. + * + * @return GridField + */ + public function performReadonlyTransformation() + { + $copy = clone $this; + $copy->setReadonly(true); + + // get the whitelist for allowable readonly components + $allowedComponents = $this->getReadonlyComponents(); + foreach ($this->getConfig()->getComponents() as $component) { + + // if a component doesn't exist, remove it from the readonly version. + if (!in_array(get_class($component), $allowedComponents)) { + $copy->getConfig()->removeComponent($component); + } + } + + return $copy; + } + + /** + * Disabling the gridfield should have the same affect as making it readonly (removing all action items). + * + * @return GridField + */ + public function performDisabledTransformation(){ + parent::performDisabledTransformation(); + + return $this->performReadonlyTransformation(); + } + /** * @return GridFieldConfig */ diff --git a/tests/php/Forms/GridField/GridFieldReadonlyTest.php b/tests/php/Forms/GridField/GridFieldReadonlyTest.php new file mode 100644 index 000000000..bb3d7136f --- /dev/null +++ b/tests/php/Forms/GridField/GridFieldReadonlyTest.php @@ -0,0 +1,87 @@ +getComponents('Cheerleaders'); + + $gridConfig = GridFieldConfig_RelationEditor::create(); + + // Build some commonly used components to make sure we're only allowing the correct components + $gridConfig->addComponent(new GridFieldButtonRow('before')); + $gridConfig->addComponent(new GridFieldAddNewButton('buttons-before-left')); + $gridConfig->addComponent(new GridFieldAddExistingAutocompleter('buttons-before-right')); + $gridConfig->addComponent(new GridFieldToolbarHeader()); + $gridConfig->addComponent($sort = new GridFieldSortableHeader()); + $gridConfig->addComponent($filter = new GridFieldFilterHeader()); + $gridConfig->addComponent(new GridFieldDataColumns()); + $gridConfig->addComponent(new GridFieldEditButton()); + $gridConfig->addComponent(new GridFieldDeleteAction(true)); + $gridConfig->addComponent(new GridField_ActionMenu()); + $gridConfig->addComponent(new GridFieldPageCount('toolbar-header-right')); + $gridConfig->addComponent($pagination = new GridFieldPaginator(2)); + $gridConfig->addComponent(new GridFieldDetailForm()); + $gridConfig->addComponent(new GridFieldDeleteAction()); + $gridConfig->addComponent(new VersionedGridFieldState()); + + $gridField = GridField::create( + 'Cheerleaders', + 'Cheerleaders', + $components, + $gridConfig + ); + + // Model Admin sets the value of the GridField directly to the relation, which doesn't have a forTemplate() + // function, if we rely on FormField to render into a ReadonlyField we'll get an error as HasManyRelation + // doesn't have a forTemplate() function. + $gridField->setValue($components); + $gridField->setModelClass(Cheerleader::class); + + // This function is called by $form->makeReadonly(). + $readonlyGridField = $gridField->performReadonlyTransformation(); + + // if we've made it this far, then the GridField is at least transforming correctly. + $readonlyComponents = $readonlyGridField->getReadonlyComponents(); + + // assert that all the components in the readonly version are present in the whitelist. + foreach($readonlyGridField->getConfig()->getComponents() as $component){ + $this->assertTrue(in_array(get_class($component), $readonlyComponents)); + } + } +} diff --git a/tests/php/Forms/GridField/GridFieldReadonlyTest.yml b/tests/php/Forms/GridField/GridFieldReadonlyTest.yml new file mode 100644 index 000000000..62855505e --- /dev/null +++ b/tests/php/Forms/GridField/GridFieldReadonlyTest.yml @@ -0,0 +1,27 @@ +SilverStripe\Forms\Tests\GridField\GridFieldTest\Team: + team1: + Name: Team 1 + City: Cologne + team2: + Name: Team 2 + City: Wellington + team3: + Name: Team 3 + City: Auckland + team4: + Name: Team 4 + City: Melbourne + +SilverStripe\Forms\Tests\GridField\GridFieldTest\Cheerleader: + cheerleader1: + Name: Heather + Team: =>SilverStripe\Forms\Tests\GridField\GridFieldTest\Team.team1 + cheerleader2: + Name: Bob + Team: =>SilverStripe\Forms\Tests\GridField\GridFieldTest\Team.team1 + cheerleader3: + Name: Jenny + Team: =>SilverStripe\Forms\Tests\GridField\GridFieldTest\Team.team1 + cheerleader4: + Name: Sam + Team: =>SilverStripe\Forms\Tests\GridField\GridFieldTest\Team.team1 From 3fc49dd4ce222b14881b76c57c4bde24a5d4a6cf Mon Sep 17 00:00:00 2001 From: Luke Edwards Date: Thu, 20 Sep 2018 13:32:52 +1200 Subject: [PATCH 2/2] Lint fixes and allow a few other components by default --- src/Forms/GridField/GridField.php | 13 ++++++++----- .../GridField/GridFieldConfig_RecordViewer.php | 1 + tests/php/Forms/GridField/GridFieldReadonlyTest.php | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index 5d959596d..629b1e681 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -111,13 +111,16 @@ class GridField extends FormField * @var array */ protected $readonlyComponents = array( + GridField_ActionMenu::class, + GridState_Component::class, + GridFieldConfig_RecordViewer::class, GridFieldDetailForm::class, GridFieldDataColumns::class, - GridFieldConfig_RecordViewer::class, - GridFieldToolbarHeader::class, GridFieldPageCount::class, GridFieldPaginator::class, - GridState_Component::class + GridFieldSortableHeader::class, + GridFieldToolbarHeader::class, + GridFieldViewButton::class, ); /** @@ -241,7 +244,6 @@ class GridField extends FormField // get the whitelist for allowable readonly components $allowedComponents = $this->getReadonlyComponents(); foreach ($this->getConfig()->getComponents() as $component) { - // if a component doesn't exist, remove it from the readonly version. if (!in_array(get_class($component), $allowedComponents)) { $copy->getConfig()->removeComponent($component); @@ -256,7 +258,8 @@ class GridField extends FormField * * @return GridField */ - public function performDisabledTransformation(){ + public function performDisabledTransformation() + { parent::performDisabledTransformation(); return $this->performReadonlyTransformation(); diff --git a/src/Forms/GridField/GridFieldConfig_RecordViewer.php b/src/Forms/GridField/GridFieldConfig_RecordViewer.php index 07ea8977a..fdaa91139 100644 --- a/src/Forms/GridField/GridFieldConfig_RecordViewer.php +++ b/src/Forms/GridField/GridFieldConfig_RecordViewer.php @@ -14,6 +14,7 @@ class GridFieldConfig_RecordViewer extends GridFieldConfig_Base $this->addComponent(new GridFieldViewButton()); $this->addComponent(new GridFieldDetailForm()); + $this->removeComponentsByType(GridFieldFilterHeader::class); $this->extend('updateConfig'); } diff --git a/tests/php/Forms/GridField/GridFieldReadonlyTest.php b/tests/php/Forms/GridField/GridFieldReadonlyTest.php index bb3d7136f..c4a1e1deb 100644 --- a/tests/php/Forms/GridField/GridFieldReadonlyTest.php +++ b/tests/php/Forms/GridField/GridFieldReadonlyTest.php @@ -80,7 +80,7 @@ class GridFieldReadonlyTest extends SapphireTest $readonlyComponents = $readonlyGridField->getReadonlyComponents(); // assert that all the components in the readonly version are present in the whitelist. - foreach($readonlyGridField->getConfig()->getComponents() as $component){ + foreach ($readonlyGridField->getConfig()->getComponents() as $component) { $this->assertTrue(in_array(get_class($component), $readonlyComponents)); } }