From 4714a833332459bc29a6a8fd24aa6102bf2bca34 Mon Sep 17 00:00:00 2001 From: Niklas Forsdahl Date: Fri, 14 Jun 2024 11:50:59 +0300 Subject: [PATCH 1/5] NEW Initial SessionGridFieldStateManager implementation, for managing GridField states in session. --- .../GridFieldDetailForm_ItemRequest.php | 7 ++ .../GridFieldStateStoreInterface.php | 10 ++ src/Forms/GridField/GridState.php | 7 +- src/Forms/GridField/GridState_Data.php | 22 +++- .../SessionGridFieldStateManager.php | 101 ++++++++++++++++++ 5 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 src/Forms/GridField/GridFieldStateStoreInterface.php create mode 100644 src/Forms/GridField/SessionGridFieldStateManager.php diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index f4294dd94..f17d579b4 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -454,6 +454,13 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler $gridState = $this->gridField->getState(false); $actions->push(HiddenField::create($manager->getStateKey($this->gridField), null, $gridState)); + if ($manager instanceof GridFieldStateStoreInterface) { + $stateRequestVar = $manager->getStateRequestVar(); + $stateValue = $this->getRequest()->requestVar($stateRequestVar); + if ($stateValue) { + $actions->push(HiddenField::create($stateRequestVar, '', $stateValue)); + } + } $actions->push($this->getRightGroupField()); } else { // adding new record diff --git a/src/Forms/GridField/GridFieldStateStoreInterface.php b/src/Forms/GridField/GridFieldStateStoreInterface.php new file mode 100644 index 000000000..2037ad8ea --- /dev/null +++ b/src/Forms/GridField/GridFieldStateStoreInterface.php @@ -0,0 +1,10 @@ +data) { - $this->data = new GridState_Data(); + $this->data = new GridState_Data([], $this); } return $this->data; @@ -99,6 +99,11 @@ class GridState extends HiddenField return $this->grid->getList(); } + public function getGridField(): GridField + { + return $this->grid; + } + /** * Returns a json encoded string representation of this state. * diff --git a/src/Forms/GridField/GridState_Data.php b/src/Forms/GridField/GridState_Data.php index f74c60a23..e5f80840b 100644 --- a/src/Forms/GridField/GridState_Data.php +++ b/src/Forms/GridField/GridState_Data.php @@ -10,29 +10,33 @@ namespace SilverStripe\Forms\GridField; */ class GridState_Data { + use GridFieldStateAware; /** * @var array */ protected $data; + protected GridState $state; + protected $defaults = []; - public function __construct($data = []) + public function __construct($data = [], GridState $state = null) { $this->data = $data; + $this->state = $state; } public function __get($name) { - return $this->getData($name, new GridState_Data()); + return $this->getData($name, new GridState_Data([], $this->state)); } public function __call($name, $arguments) { // Assume first parameter is default value if (empty($arguments)) { - $default = new GridState_Data(); + $default = new GridState_Data([], $this->state); } else { $default = $arguments[0]; } @@ -72,16 +76,25 @@ class GridState_Data $this->data[$name] = $default; } else { if (is_array($this->data[$name])) { - $this->data[$name] = new GridState_Data($this->data[$name]); + $this->data[$name] = new GridState_Data($this->data[$name], $this->state); } } return $this->data[$name]; } + public function storeData() + { + $stateManager = $this->getStateManager(); + if ($stateManager instanceof GridFieldStateStoreInterface) { + $stateManager->storeState($this->state->getGridField(), $this->state->Value()); + } + } + public function __set($name, $value) { $this->data[$name] = $value; + $this->storeData(); } public function __isset($name) @@ -92,6 +105,7 @@ class GridState_Data public function __unset($name) { unset($this->data[$name]); + $this->storeData(); } public function __toString() diff --git a/src/Forms/GridField/SessionGridFieldStateManager.php b/src/Forms/GridField/SessionGridFieldStateManager.php new file mode 100644 index 000000000..10ade3bec --- /dev/null +++ b/src/Forms/GridField/SessionGridFieldStateManager.php @@ -0,0 +1,101 @@ +getStateRequestVar(); + $sessionStateID = $gridField->getForm()->getRequestHandler()->getRequest()->requestVar($requestVar); + if (!$sessionStateID) { + $sessionStateID = Controller::curr()->getRequest()->requestVar($requestVar); + } + if ($sessionStateID) { + return $sessionStateID; + } + $stateKey = $this->getStateKey($gridField); + if (isset(self::$state_ids[$stateKey])) { + $sessionStateID = self::$state_ids[$stateKey]; + } elseif ($create) { + $sessionStateID = substr(md5(time()), 0, 8); + // we don't want session state id to be strictly numeric, since this is used as a session key, + // and session keys in php has to be usable as variable names + if (is_numeric($sessionStateID)) { + $sessionStateID .= 'a'; + } + self::$state_ids[$stateKey] = $sessionStateID; + } + return $sessionStateID; + } + + public function storeState(GridField $gridField, $value = null) + { + $sessionStateID = $this->getStateID($gridField, true); + $sessionState = Controller::curr()->getRequest()->getSession()->get($sessionStateID); + if (!$sessionState) { + $sessionState = []; + } + $stateKey = $this->getStateKey($gridField); + $sessionState[$stateKey] = $value ?? $gridField->getState(false)->Value(); + Controller::curr()->getRequest()->getSession()->set($sessionStateID, $sessionState); + } + + public function getStateRequestVar(): string + { + return 'gridSessionState'; + } + + /** + * @param GridField $gridField + * @return string + */ + public function getStateKey(GridField $gridField): string + { + $record = $gridField->getForm()->getRecord(); + return $gridField->getName() . '-' . ($record ? $record->ID : 0); + } + + /** + * @param GridField $gridField + * @param string $url + * @return string + */ + public function addStateToURL(GridField $gridField, string $url): string + { + $sessionStateID = $this->getStateID($gridField); + if ($sessionStateID) { + return Controller::join_links($url, '?' . $this->getStateRequestVar() . '=' . $sessionStateID); + } + return $url; + } + + /** + * @param GridField $gridField + * @param HTTPRequest $request + * @return string|null + */ + public function getStateFromRequest(GridField $gridField, HTTPRequest $request): ?string + { + $gridSessionStateID = $request->requestVar($this->getStateRequestVar()); + if ($gridSessionStateID) { + $sessionState = $request->getSession()->get($gridSessionStateID); + $stateKey = $this->getStateKey($gridField); + if ($sessionState && isset($sessionState[$stateKey])) { + return $sessionState[$stateKey]; + } + } + return null; + } +} From d448708fb36a68a9cf8d306bf10358686c596314 Mon Sep 17 00:00:00 2001 From: Niklas Forsdahl Date: Fri, 14 Jun 2024 13:07:42 +0300 Subject: [PATCH 2/5] Added Unit Tests for SessionGridFieldStateManager --- .../SessionGridFieldStateManager.php | 4 +- .../SessionGridFieldStateManagerTest.php | 131 ++++++++++++++++++ 2 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php diff --git a/src/Forms/GridField/SessionGridFieldStateManager.php b/src/Forms/GridField/SessionGridFieldStateManager.php index 10ade3bec..750c1f336 100644 --- a/src/Forms/GridField/SessionGridFieldStateManager.php +++ b/src/Forms/GridField/SessionGridFieldStateManager.php @@ -18,7 +18,7 @@ class SessionGridFieldStateManager implements GridFieldStateManagerInterface, Gr protected function getStateID(GridField $gridField, $create = false): ?string { $requestVar = $this->getStateRequestVar(); - $sessionStateID = $gridField->getForm()->getRequestHandler()->getRequest()->requestVar($requestVar); + $sessionStateID = $gridField->getForm()?->getRequestHandler()->getRequest()->requestVar($requestVar); if (!$sessionStateID) { $sessionStateID = Controller::curr()->getRequest()->requestVar($requestVar); } @@ -63,7 +63,7 @@ class SessionGridFieldStateManager implements GridFieldStateManagerInterface, Gr */ public function getStateKey(GridField $gridField): string { - $record = $gridField->getForm()->getRecord(); + $record = $gridField->getForm()?->getRecord(); return $gridField->getName() . '-' . ($record ? $record->ID : 0); } diff --git a/tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php b/tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php new file mode 100644 index 000000000..a296260d3 --- /dev/null +++ b/tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php @@ -0,0 +1,131 @@ +ID = 1; + $form2 = new Form($controller, 'form2', new FieldList(), new FieldList()); + $form2->loadDataFrom($testObject); + + $grid1 = new GridField('A'); + $grid2 = new GridField('B'); + $grid1->setForm($form1); + $grid2->setForm($form2); + $this->assertEquals('A-0', $manager->getStateKey($grid1)); + $this->assertEquals('B-1', $manager->getStateKey($grid2)); + } + + public function testAddStateToURL() + { + $manager = new SessionGridFieldStateManager(); + $grid = new GridField('TestGrid'); + $grid->getState()->testValue = 'foo'; + $stateRequestVar = $manager->getStateRequestVar(); + $link = '/link-to/something'; + $this->assertTrue( + preg_match( + "|^$link\?{$stateRequestVar}=[a-zA-Z0-9]+$|", + $manager->addStateToURL($grid, $link) + ) == 1 + ); + + $link = '/link-to/something-else?someParam=somevalue'; + $this->assertTrue( + preg_match( + "|^/link-to/something-else\?someParam=somevalue&{$stateRequestVar}=[a-zA-Z0-9]+$|", + $manager->addStateToURL($grid, $link) + ) == 1 + ); + } + + public function testGetStateFromRequest() + { + $manager = new SessionGridFieldStateManager(); + + $session = new Session([]); + $request = new HTTPRequest( + 'GET', + '/link-to/something', + [ + $manager->getStateRequestVar() => 'testGetStateFromRequest' + ] + ); + $request->setSession($session); + + $controller = new Controller(); + $controller->setRequest($request); + $controller->pushCurrent(); + $form = new Form($controller, 'form1', new FieldList(), new FieldList()); + $grid = new GridField('TestGrid'); + $grid->setForm($form); + + $grid->getState()->testValue = 'foo'; + $state = $grid->getState(false)->Value() ?? '{}'; + $result = $manager->getStateFromRequest($grid, $request); + + $this->assertEquals($state, $result); + $controller->popCurrent(); + } + + public function testDefaultStateLeavesURLUnchanged() + { + $manager = new SessionGridFieldStateManager(); + $grid = new GridField('DefaultStateGrid'); + $grid->getState(false)->getData()->testValue->initDefaults(['foo' => 'bar']); + $link = '/link-to/something'; + + $this->assertEquals('{}', $grid->getState(false)->Value()); + + $this->assertEquals( + '/link-to/something', + $manager->addStateToURL($grid, $link) + ); + } + + public function testStoreState() + { + $manager = new SessionGridFieldStateManager(); + + $session = new Session([]); + $request = new HTTPRequest( + 'GET', + '/link-to/something', + [ + $manager->getStateRequestVar() => 'testStoreState' + ] + ); + $request->setSession($session); + + $controller = new Controller(); + $controller->setRequest($request); + $controller->pushCurrent(); + $form = new Form($controller, 'form1', new FieldList(), new FieldList()); + $grid = new GridField('TestGrid'); + $grid->setForm($form); + + $grid->getState()->testValue = 'foo'; + $state = $grid->getState(false)->Value() ?? '{}'; + + $manager->storeState($grid); + $this->assertEquals($state, $session->get('testStoreState')['TestGrid-0']); + + $controller->popCurrent(); + } +} From e9cd597a22bfe5a35ff107d2a0c3052b2fc00d6a Mon Sep 17 00:00:00 2001 From: Niklas Forsdahl Date: Thu, 20 Jun 2024 14:58:55 +0300 Subject: [PATCH 3/5] Fixed SessionGridFieldStateManager unit test to work regardless of current gridfield state manager configuration. --- .../Forms/GridField/SessionGridFieldStateManagerTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php b/tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php index a296260d3..e06319358 100644 --- a/tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php +++ b/tests/php/Forms/GridField/SessionGridFieldStateManagerTest.php @@ -5,15 +5,24 @@ namespace SilverStripe\Forms\Tests\GridField; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Session; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\Forms\GridField\GridFieldStateManagerInterface; use SilverStripe\Forms\GridField\SessionGridFieldStateManager; use SilverStripe\Forms\Tests\GridField\GridFieldPrintButtonTest\TestObject; class SessionGridFieldStateManagerTest extends SapphireTest { + protected function setUp(): void + { + parent::setUp(); + // configure the injector to use the session grid field state manager + Injector::inst()->registerService(new SessionGridFieldStateManager(), GridFieldStateManagerInterface::class); + } + public function testStateKey() { $manager = new SessionGridFieldStateManager(); From 898ef9ba31f9727a093d4c67bae5f726eed458b2 Mon Sep 17 00:00:00 2001 From: Niklas Forsdahl Date: Thu, 20 Jun 2024 15:06:56 +0300 Subject: [PATCH 4/5] Removed custom interface for SessionGridFieldStateManager, the extra methods will be incorporated into GridFieldStateManagerInterface in the future. --- .../GridField/GridFieldDetailForm_ItemRequest.php | 2 +- src/Forms/GridField/GridFieldStateStoreInterface.php | 10 ---------- src/Forms/GridField/GridState_Data.php | 4 +++- src/Forms/GridField/SessionGridFieldStateManager.php | 2 +- 4 files changed, 5 insertions(+), 13 deletions(-) delete mode 100644 src/Forms/GridField/GridFieldStateStoreInterface.php diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index f17d579b4..dacca37ed 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -454,7 +454,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler $gridState = $this->gridField->getState(false); $actions->push(HiddenField::create($manager->getStateKey($this->gridField), null, $gridState)); - if ($manager instanceof GridFieldStateStoreInterface) { + if (ClassInfo::hasMethod($manager, 'getStateRequestVar')) { $stateRequestVar = $manager->getStateRequestVar(); $stateValue = $this->getRequest()->requestVar($stateRequestVar); if ($stateValue) { diff --git a/src/Forms/GridField/GridFieldStateStoreInterface.php b/src/Forms/GridField/GridFieldStateStoreInterface.php deleted file mode 100644 index 2037ad8ea..000000000 --- a/src/Forms/GridField/GridFieldStateStoreInterface.php +++ /dev/null @@ -1,10 +0,0 @@ -getStateManager(); - if ($stateManager instanceof GridFieldStateStoreInterface) { + if (ClassInfo::hasMethod($stateManager, 'storeState')) { $stateManager->storeState($this->state->getGridField(), $this->state->Value()); } } diff --git a/src/Forms/GridField/SessionGridFieldStateManager.php b/src/Forms/GridField/SessionGridFieldStateManager.php index 750c1f336..4ed3133ea 100644 --- a/src/Forms/GridField/SessionGridFieldStateManager.php +++ b/src/Forms/GridField/SessionGridFieldStateManager.php @@ -11,7 +11,7 @@ use SilverStripe\Control\HTTPRequest; * (i.e. the state is changed from the default). * If a session state key is present in the request, it will always be used instead of generating a new one. */ -class SessionGridFieldStateManager implements GridFieldStateManagerInterface, GridFieldStateStoreInterface +class SessionGridFieldStateManager implements GridFieldStateManagerInterface { protected static $state_ids = []; From 6b69db1e3b16cc57c43cd0735ea62bb3b41247cc Mon Sep 17 00:00:00 2001 From: Niklas Forsdahl Date: Thu, 27 Jun 2024 16:06:32 +0300 Subject: [PATCH 5/5] Fix for UnitTest failure with new SessionGridFieldStateManager functionality --- src/Forms/GridField/GridState_Data.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Forms/GridField/GridState_Data.php b/src/Forms/GridField/GridState_Data.php index a0761a5d8..a8d16be3d 100644 --- a/src/Forms/GridField/GridState_Data.php +++ b/src/Forms/GridField/GridState_Data.php @@ -19,11 +19,11 @@ class GridState_Data */ protected $data; - protected GridState $state; + protected ?GridState $state; protected $defaults = []; - public function __construct($data = [], GridState $state = null) + public function __construct($data = [], ?GridState $state = null) { $this->data = $data; $this->state = $state; @@ -88,7 +88,7 @@ class GridState_Data public function storeData() { $stateManager = $this->getStateManager(); - if (ClassInfo::hasMethod($stateManager, 'storeState')) { + if (ClassInfo::hasMethod($stateManager, 'storeState') && $this->state) { $stateManager->storeState($this->state->getGridField(), $this->state->Value()); } }