From b4c8f699eb4dd75089100c1b180cd0df25146206 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Wed, 21 Nov 2018 16:55:57 +1300 Subject: [PATCH 1/5] FIX Provide alternatives to session for storing GridField_FormAction state --- _config/gridfield.yml | 6 +++ .../GridField/FormAction/AttributeStore.php | 51 +++++++++++++++++++ .../GridField/FormAction/SessionStore.php | 50 ++++++++++++++++++ src/Forms/GridField/FormAction/StateStore.php | 30 +++++++++++ src/Forms/GridField/GridField.php | 10 +++- src/Forms/GridField/GridField_FormAction.php | 43 +++++++++++----- 6 files changed, 176 insertions(+), 14 deletions(-) create mode 100644 _config/gridfield.yml create mode 100644 src/Forms/GridField/FormAction/AttributeStore.php create mode 100644 src/Forms/GridField/FormAction/SessionStore.php create mode 100644 src/Forms/GridField/FormAction/StateStore.php diff --git a/_config/gridfield.yml b/_config/gridfield.yml new file mode 100644 index 000000000..a47da229f --- /dev/null +++ b/_config/gridfield.yml @@ -0,0 +1,6 @@ +--- +Name: gridfieldconfig +--- +SilverStripe\Core\Injector\Injector: + SilverStripe\Forms\GridField\FormAction\StateStore: + class: SilverStripe\Forms\GridField\FormAction\SessionStore diff --git a/src/Forms/GridField/FormAction/AttributeStore.php b/src/Forms/GridField/FormAction/AttributeStore.php new file mode 100644 index 000000000..60f736d74 --- /dev/null +++ b/src/Forms/GridField/FormAction/AttributeStore.php @@ -0,0 +1,51 @@ +request = $request; + } + + /** + * Save the given state against the given ID returning an associative array to be added as attributes on the form + * action + * + * @param string $id + * @param array $state + * @return array + */ + public function save($id, array $state) + { + // Just save the state in the attributes of the action + return [ + 'data-action-state' => json_encode($state), + ]; + } + + /** + * Load state for a given ID + * + * @param string $id + * @return mixed + */ + public function load($id) + { + // Check the request + return json_decode($this->request->requestVar('ActionState'), true); + } +} diff --git a/src/Forms/GridField/FormAction/SessionStore.php b/src/Forms/GridField/FormAction/SessionStore.php new file mode 100644 index 000000000..35f2337b8 --- /dev/null +++ b/src/Forms/GridField/FormAction/SessionStore.php @@ -0,0 +1,50 @@ +request = $request; + } + + /** + * Save the given state against the given ID returning an associative array to be added as attributes on the form + * action + * + * @param string $id + * @param array $state + * @return array + */ + public function save($id, array $state) + { + $this->request->getSession()->set($id, $state); + + // This adapter does not require any additional attributes... + return []; + } + + /** + * Load state for a given ID + * + * @param string $id + * @return mixed + */ + public function load($id) + { + return $this->request->getSession()->get($id); + } +} diff --git a/src/Forms/GridField/FormAction/StateStore.php b/src/Forms/GridField/FormAction/StateStore.php new file mode 100644 index 000000000..e415a518b --- /dev/null +++ b/src/Forms/GridField/FormAction/StateStore.php @@ -0,0 +1,30 @@ +setValue($fieldData['GridState']); } + // Fetch the store for the "state" of actions (not the GridField) + /** @var StateStore $store */ + $store = Injector::inst()->create(StateStore::class . '.' . $this->getName(), $request); + foreach ($data as $dataKey => $dataValue) { if (preg_match('/^action_gridFieldAlterAction\?StateID=(.*)/', $dataKey, $matches)) { - $stateChange = $request->getSession()->get($matches[1]); + $stateChange = $store->load($matches[1]); + $actionName = $stateChange['actionName']; $arguments = array(); diff --git a/src/Forms/GridField/GridField_FormAction.php b/src/Forms/GridField/GridField_FormAction.php index 10dc772bc..a253d8b3b 100644 --- a/src/Forms/GridField/GridField_FormAction.php +++ b/src/Forms/GridField/GridField_FormAction.php @@ -3,8 +3,10 @@ namespace SilverStripe\Forms\GridField; use SilverStripe\Control\Controller; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\Form; use SilverStripe\Forms\FormAction; +use SilverStripe\Forms\GridField\FormAction\StateStore; /** * This class is the base class when you want to have an action that alters the state of the @@ -12,6 +14,11 @@ use SilverStripe\Forms\FormAction; */ class GridField_FormAction extends FormAction { + /** + * A common string prefix for keys generated to store form action "state" against + */ + const STATE_KEY_PREFIX = 'gf_'; + /** * @var GridField */ @@ -80,29 +87,39 @@ class GridField_FormAction extends FormAction */ public function getAttributes() { - // Store state in session, and pass ID to client side. + // Determine the state that goes with this action $state = array( 'grid' => $this->getNameFromParent(), 'actionName' => $this->actionName, 'args' => $this->args, ); - // Ensure $id doesn't contain only numeric characters - $id = 'gf_' . substr(md5(serialize($state)), 0, 8); + // Generate a key and attach it to the action name + $key = static::STATE_KEY_PREFIX . substr(md5(serialize($state)), 0, 8); + // Note: This field needs to be less than 65 chars, otherwise Suhosin security patch will strip it + $name = 'action_gridFieldAlterAction?StateID=' . $key; - $session = Controller::curr()->getRequest()->getSession(); - $session->set($id, $state); - $actionData['StateID'] = $id; + // Define attributes + $attributes = array( + 'name' => $name, + 'data-url' => $this->gridField->Link(), + 'type' => "button", + ); + // Create a "store" for the "state" of this action + /** @var StateStore $store */ + $store = Injector::inst()->create( + StateStore::class . '.' . $this->gridField->getName(), + // For some reason `getRequest` on GridField_FormAction does not return the correct request + Controller::curr()->getRequest() + ); + // Store the state and update attributes as required + $attributes += $store->save($key, $state); + + // Return attributes return array_merge( parent::getAttributes(), - array( - // Note: This field needs to be less than 65 chars, otherwise Suhosin security patch - // will strip it from the requests - 'name' => 'action_gridFieldAlterAction' . '?' . http_build_query($actionData), - 'data-url' => $this->gridField->Link(), - 'type' => "button", - ) + $attributes ); } From ddaa22986f86c634d5e0c849f727c6eb4c4f2509 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Thu, 22 Nov 2018 13:05:43 +1300 Subject: [PATCH 2/5] Updating StateStore interface not to define a constructor & fixing GridFieldFilterHeader to add required attributes --- .../FormAction/AbstractRequestAwareStore.php | 34 +++++++++++++++++++ .../GridField/FormAction/AttributeStore.php | 19 ++--------- .../GridField/FormAction/SessionStore.php | 21 +++--------- src/Forms/GridField/FormAction/StateStore.php | 9 +---- src/Forms/GridField/GridField.php | 2 +- src/Forms/GridField/GridFieldFilterHeader.php | 8 +++-- src/Forms/GridField/GridField_FormAction.php | 6 +--- 7 files changed, 50 insertions(+), 49 deletions(-) create mode 100644 src/Forms/GridField/FormAction/AbstractRequestAwareStore.php diff --git a/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php b/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php new file mode 100644 index 000000000..57c11e519 --- /dev/null +++ b/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php @@ -0,0 +1,34 @@ + '%$' . HTTPRequest::class, + ]; + + /** + * @var HTTPRequest + */ + protected $request; + + /** + * @return HTTPRequest + */ + public function getRequest() + { + return $this->request; + } + + /** + * @param HTTPRequest $request + * @return $this + */ + public function setRequest($request) + { + $this->request = $request; + return $this; + } +} diff --git a/src/Forms/GridField/FormAction/AttributeStore.php b/src/Forms/GridField/FormAction/AttributeStore.php index 60f736d74..b04d7c21e 100644 --- a/src/Forms/GridField/FormAction/AttributeStore.php +++ b/src/Forms/GridField/FormAction/AttributeStore.php @@ -6,21 +6,8 @@ use SilverStripe\Control\HTTPRequest; /** * Stores GridField action state on an attribute on the action and then analyses request parameters to load it back */ -class AttributeStore implements StateStore +class AttributeStore extends AbstractRequestAwareStore implements StateStore { - /** - * @var HTTPRequest - */ - protected $request; - - /** - * @param HTTPRequest $request - */ - public function __construct(HTTPRequest $request) - { - $this->request = $request; - } - /** * Save the given state against the given ID returning an associative array to be added as attributes on the form * action @@ -41,11 +28,11 @@ class AttributeStore implements StateStore * Load state for a given ID * * @param string $id - * @return mixed + * @return array */ public function load($id) { // Check the request - return json_decode($this->request->requestVar('ActionState'), true); + return (array) json_decode((string) $this->getRequest()->requestVar('ActionState'), true); } } diff --git a/src/Forms/GridField/FormAction/SessionStore.php b/src/Forms/GridField/FormAction/SessionStore.php index 35f2337b8..0798d0c6a 100644 --- a/src/Forms/GridField/FormAction/SessionStore.php +++ b/src/Forms/GridField/FormAction/SessionStore.php @@ -6,21 +6,8 @@ use SilverStripe\Control\HTTPRequest; /** * Stores GridField action state in the session in exactly the same way it has in the past */ -class SessionStore implements StateStore +class SessionStore extends AbstractRequestAwareStore implements StateStore { - /** - * @var HTTPRequest - */ - protected $request; - - /** - * @param HTTPRequest $request - */ - public function __construct(HTTPRequest $request) - { - $this->request = $request; - } - /** * Save the given state against the given ID returning an associative array to be added as attributes on the form * action @@ -31,7 +18,7 @@ class SessionStore implements StateStore */ public function save($id, array $state) { - $this->request->getSession()->set($id, $state); + $this->getRequest()->getSession()->set($id, $state); // This adapter does not require any additional attributes... return []; @@ -41,10 +28,10 @@ class SessionStore implements StateStore * Load state for a given ID * * @param string $id - * @return mixed + * @return array */ public function load($id) { - return $this->request->getSession()->get($id); + return (array) $this->getRequest()->getSession()->get($id); } } diff --git a/src/Forms/GridField/FormAction/StateStore.php b/src/Forms/GridField/FormAction/StateStore.php index e415a518b..3a0b89c70 100644 --- a/src/Forms/GridField/FormAction/StateStore.php +++ b/src/Forms/GridField/FormAction/StateStore.php @@ -1,15 +1,8 @@ create(StateStore::class . '.' . $this->getName(), $request); + $store = Injector::inst()->create(StateStore::class . '.' . $this->getName()); foreach ($data as $dataKey => $dataValue) { if (preg_match('/^action_gridFieldAlterAction\?StateID=(.*)/', $dataKey, $matches)) { diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index 2d2440877..d51ea5023 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -289,14 +289,18 @@ class GridFieldFilterHeader implements GridField_URLHandler, GridField_HTMLProvi }, array_keys($filters)), $filters); } + $searchAction = GridField_FormAction::create($gridField, 'filter', false, 'filter', null); + $clearAction = GridField_FormAction::create($gridField, 'reset', false, 'reset', null); $schema = [ 'formSchemaUrl' => $schemaUrl, 'name' => $searchField, 'placeholder' => _t(__CLASS__ . '.Search', 'Search "{name}"', ['name' => $name]), 'filters' => $filters ?: new \stdClass, // stdClass maps to empty json object '{}' 'gridfield' => $gridField->getName(), - 'searchAction' => GridField_FormAction::create($gridField, 'filter', false, 'filter', null)->getAttribute('name'), - 'clearAction' => GridField_FormAction::create($gridField, 'reset', false, 'reset', null)->getAttribute('name') + 'searchAction' => $searchAction->getAttribute('name'), + 'searchActionState' => $searchAction->getAttribute('data-action-state'), + 'clearAction' => $clearAction->getAttribute('name'), + 'clearActionState' => $clearAction->getAttribute('data-action-state'), ]; return Convert::raw2json($schema); diff --git a/src/Forms/GridField/GridField_FormAction.php b/src/Forms/GridField/GridField_FormAction.php index a253d8b3b..7e791410d 100644 --- a/src/Forms/GridField/GridField_FormAction.php +++ b/src/Forms/GridField/GridField_FormAction.php @@ -108,11 +108,7 @@ class GridField_FormAction extends FormAction // Create a "store" for the "state" of this action /** @var StateStore $store */ - $store = Injector::inst()->create( - StateStore::class . '.' . $this->gridField->getName(), - // For some reason `getRequest` on GridField_FormAction does not return the correct request - Controller::curr()->getRequest() - ); + $store = Injector::inst()->create(StateStore::class . '.' . $this->gridField->getName()); // Store the state and update attributes as required $attributes += $store->save($key, $state); From aace1da1f04af84b339eae4aeb89abe3c96d3257 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Thu, 22 Nov 2018 13:17:09 +1300 Subject: [PATCH 3/5] DOCS Adding notes on configuring the storage method for GridField_FormActions --- .../03_Forms/Field_types/04_GridField.md | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md b/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md index 3750c0761..43ce4ac95 100644 --- a/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md +++ b/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md @@ -472,11 +472,29 @@ functionality. See [How to Create a GridFieldComponent](../how_tos/create_a_grid ## Saving the GridField State -`GridState` is a class that is used to contain the current state and actions on the `GridField`. It's transfered +`GridState` is a class that is used to contain the current state and actions on the `GridField`. It's transferred between page requests by being inserted as a hidden field in the form. The `GridState_Component` sets and gets data from the `GridState`. +## Saving GridField_FormAction state + +By default state used for performing form actions is saved in the session and tagged with a key like `gf_abcd1234`. In +some cases session may not be an appropriate storage method. The storage method can be configured: + +```yaml +Name: mysitegridfieldconfig +After: gridfieldconfig +--- +SilverStripe\Core\Injector\Injector: + SilverStripe\Forms\GridField\FormAction\StateStore: + class: SilverStripe\Forms\GridField\FormAction\AttributeStore +``` + +The `AttributeStore` class configures action state to be stored in the DOM and sent back on the request that performs +the action. Custom storage methods can be created and used by implementing the `StateStore` interface and configuring +`Injector` in a similar fasion. + ## API Documentation * [GridField](api:SilverStripe\Forms\GridField\GridField) From 32d096d9e5daa221ae6a2d03a46c19d1c280989f Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Wed, 28 Nov 2018 14:58:51 +1300 Subject: [PATCH 4/5] DOCS Moving interface subscription to the abstract and adding a changelog entry --- .../02_Developer_Guides/03_Forms/Field_types/04_GridField.md | 2 +- docs/en/04_Changelogs/4.3.0.md | 1 + src/Forms/GridField/FormAction/AbstractRequestAwareStore.php | 2 +- src/Forms/GridField/FormAction/AttributeStore.php | 4 +--- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md b/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md index 43ce4ac95..9461ae801 100644 --- a/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md +++ b/docs/en/02_Developer_Guides/03_Forms/Field_types/04_GridField.md @@ -493,7 +493,7 @@ SilverStripe\Core\Injector\Injector: The `AttributeStore` class configures action state to be stored in the DOM and sent back on the request that performs the action. Custom storage methods can be created and used by implementing the `StateStore` interface and configuring -`Injector` in a similar fasion. +`Injector` in a similar fashion. ## API Documentation diff --git a/docs/en/04_Changelogs/4.3.0.md b/docs/en/04_Changelogs/4.3.0.md index ab31d003e..ffa49c558 100644 --- a/docs/en/04_Changelogs/4.3.0.md +++ b/docs/en/04_Changelogs/4.3.0.md @@ -8,6 +8,7 @@ - New React-based search UI for the CMS, Asset-Admin, GridFields and ModelAdmins. - A new `GridFieldLazyLoader` component can be added to `GridField`. This will delay the fetching of data until the user access the container Tab of the GridField. - `SilverStripe\VersionedAdmin\Controllers\CMSPageHistoryViewerController` is now the default CMS history controller and `SilverStripe\CMS\Controllers\CMSPageHistoryController` has been deprecated. + - It's now possible to avoid storing GridField data in session. See [the documentation on GridField](../developer_guides/forms/field_types/gridfield/#saving-the-gridfield-state). ## Upgrading {#upgrading} diff --git a/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php b/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php index 57c11e519..c1069aa3f 100644 --- a/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php +++ b/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php @@ -3,7 +3,7 @@ namespace SilverStripe\Forms\GridField\FormAction; use SilverStripe\Control\HTTPRequest; -abstract class AbstractRequestAwareStore +abstract class AbstractRequestAwareStore implements StateStore { private static $dependencies = [ 'request' => '%$' . HTTPRequest::class, diff --git a/src/Forms/GridField/FormAction/AttributeStore.php b/src/Forms/GridField/FormAction/AttributeStore.php index b04d7c21e..1f84fb768 100644 --- a/src/Forms/GridField/FormAction/AttributeStore.php +++ b/src/Forms/GridField/FormAction/AttributeStore.php @@ -1,12 +1,10 @@ Date: Wed, 28 Nov 2018 16:00:51 +1300 Subject: [PATCH 5/5] FIX Switching to use Controller::curr as it was using previously --- .../FormAction/AbstractRequestAwareStore.php | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php b/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php index c1069aa3f..84f683292 100644 --- a/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php +++ b/src/Forms/GridField/FormAction/AbstractRequestAwareStore.php @@ -1,34 +1,17 @@ '%$' . HTTPRequest::class, - ]; - - /** - * @var HTTPRequest - */ - protected $request; - /** * @return HTTPRequest */ public function getRequest() { - return $this->request; - } - - /** - * @param HTTPRequest $request - * @return $this - */ - public function setRequest($request) - { - $this->request = $request; - return $this; + // Replicating existing functionality from GridField_FormAction + return Controller::curr()->getRequest(); } }