From e3956c3b1678abdf9fac879452f6b6313a525b1d Mon Sep 17 00:00:00 2001 From: David Windell Date: Fri, 8 Jul 2016 16:04:57 +0100 Subject: [PATCH 1/4] Add failing test case --- test/FormTest.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/FormTest.php b/test/FormTest.php index f5a8ccca..884083d1 100644 --- a/test/FormTest.php +++ b/test/FormTest.php @@ -567,6 +567,35 @@ public function testSettingValidationGroupBindsOnlyThoseValuesToModel() $this->assertObjectNotHasAttribute('foobar', $model); } + public function testSettingValidationGroupWithoutCollectionBindsOnlyThoseValuesToModel() + { + $model = new stdClass; + $dataWithoutCollection = [ + 'foo' => 'abcde' + ]; + $this->populateForm(); + $this->form->add([ + 'type' => 'Zend\Form\Element\Collection', + 'name' => 'categories', + 'options' => [ + 'count' => 0, + 'target_element' => [ + 'type' => 'ZendTest\Form\TestAsset\CategoryFieldset' + ] + ] + ]); + $this->form->setHydrator(new Hydrator\ObjectProperty()); + $this->form->bind($model); + $this->form->setData($dataWithoutCollection); + $this->form->setValidationGroup(array('foo')); + $this->form->isValid(); + + $this->assertObjectHasAttribute('foo', $model); + $this->assertEquals('abcde', $model->foo); + $this->assertObjectNotHasAttribute('categories', $model); + $this->assertObjectNotHasAttribute('foobar', $model); + } + public function testCanBindModelsToArraySerializableObjects() { $model = new TestAsset\Model(); From 8991ddb1e9435a9a37240b138131bbf990951b9b Mon Sep 17 00:00:00 2001 From: Vytautas Stankus Date: Fri, 8 Jul 2016 19:03:51 +0300 Subject: [PATCH 2/4] fixed #102 do not ignore validation group for collections --- src/Element/Collection.php | 6 ++++-- src/Fieldset.php | 10 ++++++++-- src/Form.php | 5 +++-- test/FormTest.php | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Element/Collection.php b/src/Element/Collection.php index 2d58b67f..fbb793ed 100644 --- a/src/Element/Collection.php +++ b/src/Element/Collection.php @@ -261,16 +261,18 @@ public function allowValueBinding() * Bind values to the object * * @param array $values + * @param array $validationGroup + * * @return array|mixed|void */ - public function bindValues(array $values = []) + public function bindValues(array $values = [], array $validationGroup = null) { $collection = []; foreach ($values as $name => $value) { $element = $this->get($name); if ($element instanceof FieldsetInterface) { - $collection[] = $element->bindValues($value); + $collection[] = $element->bindValues($value, $validationGroup); } else { $collection[] = $value; } diff --git a/src/Fieldset.php b/src/Fieldset.php index 89086707..ea26ad5b 100644 --- a/src/Fieldset.php +++ b/src/Fieldset.php @@ -557,9 +557,11 @@ public function allowValueBinding() * Bind values to the bound object * * @param array $values + * @param array $validationGroup + * * @return mixed|void */ - public function bindValues(array $values = []) + public function bindValues(array $values = [], array $validationGroup = null) { $objectData = $this->extract(); $hydrator = $this->getHydrator(); @@ -568,6 +570,10 @@ public function bindValues(array $values = []) foreach ($this->iterator as $element) { $name = $element->getName(); + if ($validationGroup && (array_key_exists($name, $validationGroup) || !in_array($name, $validationGroup))) { + continue; + } + if (!array_key_exists($name, $values)) { if (!($element instanceof Collection)) { continue; @@ -579,7 +585,7 @@ public function bindValues(array $values = []) $value = $values[$name]; if ($element instanceof FieldsetInterface && $element->allowValueBinding()) { - $value = $element->bindValues($value); + $value = $element->bindValues($value, empty($validationGroup[$name]) ? null : $validationGroup[$name]); } // skip post values for disabled elements, get old value from object diff --git a/src/Form.php b/src/Form.php index 7cd274cc..87736634 100644 --- a/src/Form.php +++ b/src/Form.php @@ -357,13 +357,14 @@ public function bindValues(array $values = []) } $data = $this->prepareBindData($data, $this->data); + $validationGroup = $this->getValidationGroup(); // If there is a base fieldset, only hydrate beginning from the base fieldset if ($this->baseFieldset !== null) { $data = $data[$this->baseFieldset->getName()]; - $this->object = $this->baseFieldset->bindValues($data); + $this->object = $this->baseFieldset->bindValues($data, $validationGroup[$this->baseFieldset->getName()]); } else { - $this->object = parent::bindValues($data); + $this->object = parent::bindValues($data, $validationGroup); } } diff --git a/test/FormTest.php b/test/FormTest.php index 884083d1..7f2841d4 100644 --- a/test/FormTest.php +++ b/test/FormTest.php @@ -587,7 +587,7 @@ public function testSettingValidationGroupWithoutCollectionBindsOnlyThoseValuesT $this->form->setHydrator(new Hydrator\ObjectProperty()); $this->form->bind($model); $this->form->setData($dataWithoutCollection); - $this->form->setValidationGroup(array('foo')); + $this->form->setValidationGroup(['foo']); $this->form->isValid(); $this->assertObjectHasAttribute('foo', $model); From 4ea1aef94b31a1b4aa2b5560fb7928608daf3788 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 14 Sep 2016 12:33:30 -0500 Subject: [PATCH 3/4] Documented changes introduced by #106 --- doc/book/collections.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/doc/book/collections.md b/doc/book/collections.md index 2b7200e3..1ec7f4bd 100644 --- a/doc/book/collections.md +++ b/doc/book/collections.md @@ -816,3 +816,30 @@ cannot say, "validate the name input for the first element of the categories collection, but don't validate it for the second one." Now, the form validates (and the `url` is set to null as we didn't specify it). + +## Preventing validation from wiping out previous collection items + +In some cases, you may be representing collections within a model, but not +validating them; as an example, if you use a validation group that excludes the +collections from validation so that they remain untouched after binding. + +Starting in 2.8.4, behavior around collections changed in order to fix some +underlying bugs. One such change is that if a collection is found in a form, but +has no associated data, an empty array is assigned to it, even when not in the +validation group. This effectively wipes out the collection data when you bind +values. + +To prevent this behavior, starting in 2.9.1 you may pass an optional second +argument to `bindValues()` on either a fieldset or collection, +`$validationGroup`; when present, these instances will first check if the +collection is in the validation group before binding the value; if it is not, +the collection will not be represented. The `Form` class has been updated to +pass the validation group, if present, on to fieldset and collection instances +when performing `bindValues()` operations. + +For more details, refer to the following issues: + +- [zendframework/zend-form#19](https://github.com/zendframework/zend-form/pull/19) +- [zendframework/zend-form#102](https://github.com/zendframework/zend-form/pull/102) +- [zendframework/zend-form#103](https://github.com/zendframework/zend-form/pull/103) +- [zendframework/zend-form#106](https://github.com/zendframework/zend-form/pull/106) From cee7953bc10b78a12d6f110d0ea3bdb9b073e83e Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 14 Sep 2016 12:34:34 -0500 Subject: [PATCH 4/4] Added CHANGELOG for #106 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1db1e037..960924f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ All notable changes to this project will be documented in this file, in reverse ensuring that the initializer injecting a factory into a `FormFactoryAware` instance is triggered before the initializer that calls `init()`, and also that the initializer calling `init()` is always triggered last. +- [#106](https://github.com/zendframework/zend-form/pull/106) updates behavior + around binding collection values to a fieldset or form such that if the + collection is not part of the current validation group, its value will not be + overwritten with an empty set. ## 2.9.0 - 2016-06-07