From 70ffb3297acd1e281d8246bb26878a2e35d530fe Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Fri, 23 Aug 2019 17:15:29 +1200 Subject: [PATCH] API Only include gridfield state value that differ from the expected default --- src/Forms/GridField/GridField.php | 20 ++++- src/Forms/GridField/GridFieldFilterHeader.php | 29 +++++-- src/Forms/GridField/GridFieldPaginator.php | 16 ++-- .../GridField/GridFieldSortableHeader.php | 23 ++++- src/Forms/GridField/GridFieldStateManager.php | 5 ++ .../GridField/GridField_StateProvider.php | 21 +++++ src/Forms/GridField/GridState.php | 20 ++++- src/Forms/GridField/GridState_Data.php | 62 ++++++++++++-- .../GridField/GridFieldStateManagerTest.php | 15 ++++ .../php/Forms/GridField/GridStateDataTest.php | 83 +++++++++++++++++++ 10 files changed, 264 insertions(+), 30 deletions(-) create mode 100644 src/Forms/GridField/GridField_StateProvider.php create mode 100644 tests/php/Forms/GridField/GridStateDataTest.php diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index dc9ddec8d..f5b76cd07 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -156,8 +156,6 @@ class GridField extends FormField $this->setConfig($config); - $this->state = new GridState($this); - $this->addExtraClass('grid-field'); } @@ -407,6 +405,11 @@ class GridField extends FormField */ public function getState($getData = true) { + // Initialise state on first call. This ensures it's evaluated after components have been added + if (!$this->state) { + $this->initState(); + } + if ($getData) { return $this->state->getData(); } @@ -414,6 +417,19 @@ class GridField extends FormField return $this->state; } + private function initState(): void + { + $this->state = new GridState($this); + + $data = $this->state->getData(); + + foreach ($this->getComponents() as $item) { + if ($item instanceof GridField_StateProvider) { + $item->initDefaultState($data); + } + } + } + /** * Returns the whole gridfield rendered with all the attached components. * diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index 32f713ed3..1076744c4 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -26,7 +26,7 @@ use SilverStripe\View\SSViewer; * * @see GridField */ -class GridFieldFilterHeader implements GridField_URLHandler, GridField_HTMLProvider, GridField_DataManipulator, GridField_ActionProvider +class GridFieldFilterHeader implements GridField_URLHandler, GridField_HTMLProvider, GridField_DataManipulator, GridField_ActionProvider, GridField_StateProvider { /** * See {@link setThrowExceptionOnBadDataType()} @@ -173,8 +173,8 @@ class GridFieldFilterHeader implements GridField_URLHandler, GridField_HTMLProvi return; } - $state = $gridField->State->GridFieldFilterHeader; - $state->Columns = null; + $state = $this->getState($gridField); + if ($actionName === 'filter') { if (isset($data['filter'][$gridField->getName()])) { foreach ($data['filter'][$gridField->getName()] as $key => $filter) { @@ -184,6 +184,20 @@ class GridFieldFilterHeader implements GridField_URLHandler, GridField_HTMLProvi } } + /** + * Extract state data from the parent gridfield + * @param GridField $gridField + * @return GridState_Data + */ + private function getState(GridField $gridField): GridState_Data + { + return $gridField->State->GridFieldFilterHeader; + } + + public function initDefaultState(GridState_Data $data): void + { + $data->GridFieldFilterHeader->initDefaults(['Columns' => []]); + } /** * @inheritDoc @@ -195,13 +209,12 @@ class GridFieldFilterHeader implements GridField_URLHandler, GridField_HTMLProvi } /** @var Filterable $dataList */ - /** @var GridState_Data $columns */ - $columns = $gridField->State->GridFieldFilterHeader->Columns(null); - if (empty($columns)) { + /** @var array $filterArguments */ + $filterArguments = $this->getState($gridField)->Columns->toArray(); + if (empty($filterArguments)) { return $dataList; } - $filterArguments = $columns->toArray(); $dataListClone = clone($dataList); $results = $this->getSearchContext($gridField) ->getQuery($filterArguments, false, false, $dataListClone); @@ -413,7 +426,7 @@ class GridFieldFilterHeader implements GridField_URLHandler, GridField_HTMLProvi } $columns = $gridField->getColumns(); - $filterArguments = $gridField->State->GridFieldFilterHeader->Columns->toArray(); + $filterArguments = $this->getState($gridField)->Columns->toArray(); $currentColumn = 0; $canFilter = false; $fieldsList = new ArrayList(); diff --git a/src/Forms/GridField/GridFieldPaginator.php b/src/Forms/GridField/GridFieldPaginator.php index 76ece4f44..8573c0efd 100755 --- a/src/Forms/GridField/GridFieldPaginator.php +++ b/src/Forms/GridField/GridFieldPaginator.php @@ -14,7 +14,7 @@ use LogicException; * GridFieldPaginator paginates the {@link GridField} list and adds controls * to the bottom of the {@link GridField}. */ -class GridFieldPaginator implements GridField_HTMLProvider, GridField_DataManipulator, GridField_ActionProvider +class GridFieldPaginator implements GridField_HTMLProvider, GridField_DataManipulator, GridField_ActionProvider, GridField_StateProvider { use Configurable; @@ -140,13 +140,15 @@ class GridFieldPaginator implements GridField_HTMLProvider, GridField_DataManipu */ protected function getGridPagerState(GridField $gridField) { - $state = $gridField->State->GridFieldPaginator; + return $gridField->State->GridFieldPaginator; + } - // Force the state to the initial page if none is set - $state->currentPage(1); - $state->itemsPerPage($this->getItemsPerPage()); - - return $state; + public function initDefaultState(GridState_Data $data): void + { + $data->GridFieldPaginator->initDefaults([ + 'currentPage' => 1, + 'itemsPerPage' => $this->getItemsPerPage() + ]); } /** diff --git a/src/Forms/GridField/GridFieldSortableHeader.php b/src/Forms/GridField/GridFieldSortableHeader.php index d0c3d8c31..1cabd8b5e 100644 --- a/src/Forms/GridField/GridFieldSortableHeader.php +++ b/src/Forms/GridField/GridFieldSortableHeader.php @@ -18,7 +18,7 @@ use LogicException; * * @see GridField */ -class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataManipulator, GridField_ActionProvider +class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataManipulator, GridField_ActionProvider, GridField_StateProvider { /** @@ -119,7 +119,7 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM $forTemplate = new ArrayData([]); $forTemplate->Fields = new ArrayList; - $state = $gridField->State->GridFieldSortableHeader; + $state = $this->getState($gridField); $columns = $gridField->getColumns(); $currentColumn = 0; @@ -236,7 +236,7 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM return; } - $state = $gridField->State->GridFieldSortableHeader; + $state = $this->getState($gridField); switch ($actionName) { case 'sortasc': $state->SortColumn = $arguments['SortColumn']; @@ -266,11 +266,26 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM } /** @var Sortable $dataList */ - $state = $gridField->State->GridFieldSortableHeader; + $state = $this->getState($gridField); if ($state->SortColumn == "") { return $dataList; } return $dataList->sort($state->SortColumn, $state->SortDirection('asc')); } + + /** + * Extract state data from the parent gridfield + * @param GridField $gridField + * @return GridState_Data + */ + private function getState(GridField $gridField): GridState_Data + { + return $gridField->State->GridFieldSortableHeader; + } + + public function initDefaultState(GridState_Data $data): void + { + $data->GridFieldSortableHeader->initDefaults(['SortColumn' => null, 'SortDirection' => 'asc']); + } } diff --git a/src/Forms/GridField/GridFieldStateManager.php b/src/Forms/GridField/GridFieldStateManager.php index 69a149a46..645ed26fb 100644 --- a/src/Forms/GridField/GridFieldStateManager.php +++ b/src/Forms/GridField/GridFieldStateManager.php @@ -41,6 +41,11 @@ class GridFieldStateManager implements GridFieldStateManagerInterface $key = $this->getStateKey($gridField); $value = $gridField->getState(false)->Value(); + // Using a JSON-encoded empty array as the blank value, to avoid changing Value() semantics in a minor release + if ($value === '[]') { + return $url; + } + return HTTP::setGetVar($key, $value, $url); } diff --git a/src/Forms/GridField/GridField_StateProvider.php b/src/Forms/GridField/GridField_StateProvider.php new file mode 100644 index 000000000..74cf03bf1 --- /dev/null +++ b/src/Forms/GridField/GridField_StateProvider.php @@ -0,0 +1,21 @@ +initDefaults() to do this. + * + * @param $data The top-level sate object + */ + public function initDefaultState(GridState_Data $data): void; +} diff --git a/src/Forms/GridField/GridState.php b/src/Forms/GridField/GridState.php index e19588c65..72d88f0db 100644 --- a/src/Forms/GridField/GridState.php +++ b/src/Forms/GridField/GridState.php @@ -59,14 +59,26 @@ class GridState extends HiddenField public function setValue($value, $data = null) { - if (is_string($value)) { - $this->data = new GridState_Data(json_decode($value, true)); + // Apply the value on top of the existing defaults + $data = json_decode($value, true); + if ($data) { + $this->mergeValues($this->getData(), $data); } - parent::setValue($value); return $this; } + private function mergeValues(GridState_Data $data, array $array): void + { + foreach ($array as $k => $v) { + if (is_array($v)) { + $this->mergeValues($data->$k, $v); + } else { + $data->$k = $v; + } + } + } + /** * @return GridState_Data */ @@ -98,7 +110,7 @@ class GridState extends HiddenField return json_encode([]); } - return json_encode($this->data->toArray()); + return json_encode($this->data->getChangesArray()); } /** diff --git a/src/Forms/GridField/GridState_Data.php b/src/Forms/GridField/GridState_Data.php index 8dd22e715..e73da7a70 100644 --- a/src/Forms/GridField/GridState_Data.php +++ b/src/Forms/GridField/GridState_Data.php @@ -16,6 +16,8 @@ class GridState_Data */ protected $data; + protected $defaults = []; + public function __construct($data = []) { $this->data = $data; @@ -23,30 +25,49 @@ class GridState_Data public function __get($name) { - return $this->getData($name, new GridState_Data()); + return $this->getData($name, new self()); } public function __call($name, $arguments) { // Assume first parameter is default value - $default = empty($arguments) ? new GridState_Data() : $arguments[0]; + if (empty($arguments)) { + $default = new self(); + } else { + $default = $arguments[0]; + } + return $this->getData($name, $default); } + /** + * Initialise the defaults values for the grid field state + * These values won't be included in getChangesArray() + * + * @param array $defaults + */ + public function initDefaults(array $defaults): void + { + foreach ($defaults as $key => $value) { + $this->defaults[$key] = $value; + $this->getData($key, $value); + } + } + /** * Retrieve the value for the given key * * @param string $name The name of the value to retrieve - * @param mixed $default Default value to assign if not set + * @param mixed $default Default value to assign if not set. Note that this *will* be included in getChangesArray() * @return mixed The value associated with this key, or the value specified by $default if not set */ public function getData($name, $default = null) { - if (!isset($this->data[$name])) { + if (!array_key_exists($name, $this->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 self($this->data[$name]); } } @@ -77,6 +98,9 @@ class GridState_Data return json_encode($this->toArray()); } + /** + * Return all data, including defaults, as array + */ public function toArray() { $output = []; @@ -85,6 +109,34 @@ class GridState_Data $output[$k] = (is_object($v) && method_exists($v, 'toArray')) ? $v->toArray() : $v; } + return $output; + } + /** + * Convert the state to an array including only value that differ from the default state defined by initDefaults() + * @return array + */ + public function getChangesArray(): array + { + $output = []; + + foreach ($this->data as $k => $v) { + if (is_object($v) && method_exists($v, 'getChangesArray')) { + $value = $v->getChangesArray(); + // Empty arrays represent pristine data, so we do not include them + if (empty($value)) { + continue; + } + } else { + $value = $v; + // Check if we have a default value for this key and if it matches our current value + if (array_key_exists($k, $this->defaults) && $this->defaults[$k] === $value) { + continue; + } + } + + $output[$k] = $value; + } + return $output; } } diff --git a/tests/php/Forms/GridField/GridFieldStateManagerTest.php b/tests/php/Forms/GridField/GridFieldStateManagerTest.php index 856438fea..c7ab6738c 100644 --- a/tests/php/Forms/GridField/GridFieldStateManagerTest.php +++ b/tests/php/Forms/GridField/GridFieldStateManagerTest.php @@ -79,4 +79,19 @@ class GridFieldStateManagerTest extends SapphireTest $this->assertEquals($state, $result); } + + public function testDefaultStateLeavesURLUnchanged() + { + $manager = new GridFieldStateManager(); + $grid = new GridField('TestGrid'); + $grid->getState()->initDefaults(['testValue' => 'foo']); + $link = '/link-to/something'; + + $this->assertEquals('[]', $grid->getState(false)->Value()); + + $this->assertEquals( + '/link-to/something', + $manager->addStateToURL($grid, $link) + ); + } } diff --git a/tests/php/Forms/GridField/GridStateDataTest.php b/tests/php/Forms/GridField/GridStateDataTest.php new file mode 100644 index 000000000..552b58311 --- /dev/null +++ b/tests/php/Forms/GridField/GridStateDataTest.php @@ -0,0 +1,83 @@ +assertEquals('Bar', $state->getData('Foo', 'Bar')); + $this->assertEquals('Bar', $state->Foo); + $this->assertEquals('Bar', $state->getData('Foo', 'Hello World')); + } + + public function testCall() + { + $state = new GridState_Data(); + + $foo = $state->Foo(); + $this->assertInstanceOf(GridState_Data::class, $foo); + + $bar = $state->Bar(123456); + $this->assertEquals(123456, $bar); + + $zone = $state->Zone(null); + $this->assertEquals(null, $zone); + } + + public function testInitDefaults() + { + $state = new GridState_Data(); + $state->initDefaults(['Foo' => 'Bar', 'Hello' => 'World']); + + $this->assertEquals('Bar', $state->Foo); + $this->assertEquals('World', $state->Hello); + } + + public function testToArray() + { + $state = new GridState_Data(); + + $this->assertEquals([], $state->toArray()); + + $state->Foo = 'Bar'; + $this->assertEquals(['Foo' => 'Bar'], $state->toArray()); + + $state->initDefaults(['Foo' => 'Bar', 'Hello' => 'World']); + + $this->assertEquals(['Foo' => 'Bar', 'Hello' => 'World'], $state->toArray()); + $this->assertEquals([], $state->getChangesArray()); + + $boom = $state->Boom(); + $boom->Pow = 'Kaboom'; + + $state->Boom(null); + + $this->assertEquals(['Foo' => 'Bar', 'Hello' => 'World', 'Boom' => ['Pow' => 'Kaboom']], $state->toArray()); + $this->assertEquals(['Boom' => ['Pow' => 'Kaboom']], $state->getChangesArray()); + } + + public function testInitDefaultsAfterSetValue() + { + $state = new GridState(new GridField('x')); + $state->setValue('{"Foo":{"Bar":"Baz","Wee":null}}'); + $data = $state->getData(); + + $data->Foo->initDefaults([ + 'Bar' => 'Bing', + 'Zoop' => 'Zog', + 'Wee' => 'Wing', + ]); + + $this->assertEquals(['Bar' => 'Baz', 'Zoop' => 'Zog', 'Wee' => null], $data->Foo->toArray()); + $this->assertEquals(['Bar' => 'Baz', 'Wee' => null], $data->Foo->getChangesArray()); + } +}