Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix Collection form element replacing bound objects with dummies upon form validation #4077

Closed
wants to merge 3 commits into from

3 participants

Samu Juvonen David Windell Matthew Weier O'Phinney
Samu Juvonen

Currently the Zend\Form\Element\Collection will discard bound objects and replace them with clones of its dummy template when the form is validated. This will result in loss of all properties that do not have a field in the Collection's target element (fieldset).

Also the returned object instances have to be the same that were passed to the form originally or it will cause corruption.

David Windell

Does this have any conflicts with #4077?

Samu Juvonen

Do you mean #4075? Yes, these two PRs do conflict, though my patch would be easy to fix with a single-line check to assert createNewObjects() == false. I don't, though, thoroughly understand why a bug should be restored in the functionality albeit optional.

I noticed there is an attempt to fix this very same bug in commit b0c03a3 but it doesn't test the case extensively. My test case tests for when allow_add == true (in the Collection element) AND initial count is set to less than the actual count of objects contained by the parent object that is bound into the form.

The part of the bug that my PR would fix but the original commit doesn't is to preserve all those entities that increase the amount of Collection's rows from the initially specified value. For example, if count in Collection is initially set to 2 but and object with three sub-objects was bound, the third one would be replaced with a dummy.

Matthew Weier O'Phinney weierophinney referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#4077] CS fixes
- braces
dea34f1
Filippo Tessarotto Slamdunk referenced this pull request from a commit in Slamdunk/zf2
Filippo Tessarotto Slamdunk Added compatibility with #4077 03e7fd7
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#4077] CS fixes
- braces
58a5393
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney Merge branch 'hotfix/4077'
Close #4077
0461dd5
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney Merge branch 'hotfix/4077' into develop
Forward port #4077
3dfb349
Deleted user Unknown referenced this pull request from a commit
Filippo Tessarotto Slamdunk Added compatibility with #4077 4389801
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 19, 2013
  1. Samu Juvonen

    Fix bug with not preserving the original entity objects in a Collecti…

    sjuvonen authored
    …on upon validating the form
  2. Samu Juvonen

    Add test case for verifying that a Collection preserves original enti…

    sjuvonen authored
    …ty objects upon form validation
  3. Samu Juvonen

    Add fieldset class needed by Zend\Form\FormTest::testPreserveEntities…

    sjuvonen authored
    …BoundToCollectionAfterValidation
This page is out of date. Refresh to see the latest.
24 library/Zend/Form/Element/Collection.php
View
@@ -233,6 +233,8 @@ public function populateValues($data)
)
);
}
+
+ $this->replaceTemplateObjects();
}
/**
@@ -536,4 +538,26 @@ protected function createTemplateElement()
return $elementOrFieldset;
}
+
+ /**
+ * Replaces the default template object of a sub element with the corresponding
+ * real entity so that all properties are preserved.
+ *
+ * @return void
+ */
+ protected function replaceTemplateObjects()
+ {
+ $fieldsets = $this->getFieldsets();
+
+ if (!count($fieldsets) || !$this->object) {
+ return;
+ }
+
+ foreach ($fieldsets as $fieldset) {
+ $i = $fieldset->getName();
+ if (isset($this->object[$i])) {
+ $fieldset->setObject($this->object[$i]);
+ }
+ }
+ }
}
43 tests/ZendTest/Form/FormTest.php
View
@@ -1357,4 +1357,47 @@ public function testGetValidationGroupReturnsNullWhenNoneSet()
{
$this->assertNull($this->form->getValidationGroup());
}
+
+ public function testPreserveEntitiesBoundToCollectionAfterValidation() {
+ $this->form->setInputFilter(new \Zend\InputFilter\InputFilter());
+ $fieldset = new TestAsset\ProductCategoriesFieldset();
+ $fieldset->setUseAsBaseFieldset(true);
+
+ $product = new Entity\Product();
+ $product->setName('Foobar');
+ $product->setPrice(100);
+
+ $c1 = new Entity\Category();
+ $c1->setId(1);
+ $c1->setName('First Category');
+
+ $c2 = new Entity\Category();
+ $c2->setId(2);
+ $c2->setName('Second Category');
+
+ $product->setCategories(array($c1, $c2));
+
+ $this->form->add($fieldset);
+ $this->form->bind($product);
+
+ $data = array(
+ 'product' => array(
+ 'name' => 'Barbar',
+ 'price' => 200,
+ 'categories' => array(
+ array('name' => 'Something else'),
+ array('name' => 'Totally different'),
+ ),
+ ),
+ );
+
+ $hash1 = spl_object_hash($this->form->getObject()->getCategory(0));
+ $this->form->setData($data);
+ $this->form->isValid();
+ $hash2 = spl_object_hash($this->form->getObject()->getCategory(0));
+
+ // Returned object has to be the same as when binding or properties
+ // will be lost. (For example entity IDs.)
+ $this->assertTrue($hash1 == $hash2);
+ }
}
22 tests/ZendTest/Form/TestAsset/Entity/Category.php
View
@@ -13,6 +13,11 @@
class Category
{
/**
+ * @var int
+ */
+ protected $id;
+
+ /**
* @var string
*/
protected $name;
@@ -34,4 +39,21 @@ public function getName()
{
return $this->name;
}
+
+ /**
+ * @param int $id
+ * @return Product
+ */
+ public function setId($id)
+ {
+ $this->id = $id;
+ return $this;
+ }
+
+ /**
+ * @return int
+ */
+ public function getId() {
+ return $this->id;
+ }
}
20 tests/ZendTest/Form/TestAsset/Entity/Product.php
View
@@ -80,4 +80,24 @@ public function getPrice()
{
return $this->price;
}
+
+ /**
+ * Return category from index
+ *
+ * @param int $i
+ */
+ public function getCategory($i)
+ {
+ return $this->categories[$i];
+ }
+
+ /**
+ * Required when binding to a form
+ *
+ * @return array
+ */
+ public function getArrayCopy()
+ {
+ return get_object_vars($this);
+ }
}
24 tests/ZendTest/Form/TestAsset/ProductCategoriesFieldset.php
View
@@ -0,0 +1,24 @@
+<?php
+
+namespace ZendTest\Form\TestAsset;
+
+class ProductCategoriesFieldset extends ProductFieldset
+{
+ public function __construct() {
+ parent::__construct();
+
+ $template = new CategoryFieldset();
+
+ $this->add(array(
+ 'name' => 'categories',
+ 'type' => 'collection',
+ 'options' => array(
+ 'label' => 'Categories',
+ 'should_create_template' => true,
+ 'allow_add' => true,
+ 'count' => 0,
+ 'target_element' => $template,
+ ),
+ ));
+ }
+}
Something went wrong with that request. Please try again.