Skip to content

Form\Collection: allow remove all objects #4492

Closed
thestanislav opened this Issue May 16, 2013 · 23 comments
@thestanislav

There is no way to remove all objects from collection at this time. According to the logic I need to pass an empty array to Zend\Form\Element\Collection::populateValues. But empty arrays are ignored https://github.com/zendframework/zf2/blob/master/library/Zend/Form/Element/Collection.php#L185

The next possible upcoming issue is that there is not way to create the html element in view script that would generate an empty array. May be some type conversion should be implemented to handle this.

@thestanislav

Any comments?

@thestanislav

May be the second part of my issue is related with this but not the first part, "There is no way to remove all objects from collection at this time"

@thestanislav

Here I took an example from manual http://framework.zend.com/manual/2.0/en/modules/zend.form.collections.html#creating-fieldsets
I have only changed count parameter to 0 in Collection creation part, because in my case product can exists without any category. But I can create category fieldset via JavaScript in my view script.

namespace Application\Form;

use Application\Entity\Product;
use Zend\Form\Fieldset;
use Zend\InputFilter\InputFilterProviderInterface;
use Zend\Stdlib\Hydrator\ClassMethods as ClassMethodsHydrator;

class ProductFieldset extends Fieldset implements InputFilterProviderInterface
{
    public function __construct()
    {
        parent::__construct('product');
        $this->setHydrator(new ClassMethodsHydrator(false))
             ->setObject(new Product());

        $this->add(array(
            'name' => 'name',
            'options' => array(
                'label' => 'Name of the product'
            ),
            'attributes' => array(
                'required' => 'required'
            )
        ));

        $this->add(array(
            'name' => 'price',
            'options' => array(
                'label' => 'Price of the product'
            ),
            'attributes' => array(
                'required' => 'required'
            )
        ));

        $this->add(array(
            'type' => 'Application\Form\BrandFieldset',
            'name' => 'brand',
            'options' => array(
                'label' => 'Brand of the product'
            )
        ));

        $this->add(array(
            'type' => 'Zend\Form\Element\Collection',
            'name' => 'categories',
            'options' => array(
                'label' => 'Please choose categories for this product',
                'count' => 0,
                'should_create_template' => true,
                'allow_add' => true,
                'target_element' => array(
                    'type' => 'Application\Form\CategoryFieldset'
                )
            )
        ));
    }

    /**
     * Should return an array specification compatible with
     * {@link Zend\InputFilter\Factory::createInputFilter()}.
     *
     * @return array
     \*/
    public function getInputFilterSpecification()
    {
        return array(
            'name' => array(
                'required' => true,
            ),
            'price' => array(
                'required' => true,
                'validators' => array(
                    array(
                        'name' => 'Float'
                    )
                )
            )
        );
    }
}

Let's assume that I have Product object with some Categories, but I want to remove all categories from Product.
So here is my Controller from the manual. Lets assume that $_POST['categories'] is equal to empty array, $_POST['categories'] = array(), because I removed all category fieldset dynamically in view script, via JavaScript.
I expect categories to be null or an empty array in var_dump($product), but it's not

/**
  * @return array
  \*/
 public function indexAction()
 {
     $form = new CreateProduct();
     $product = new Product();
     $form->bind($product);

     if ($this->request->isPost()) {
         $form->setData($this->request->getPost());

         if ($form->isValid()) {
             var_dump($product);
         }
     }

     return array(
         'form' => $form
     );
 }
@thestanislav

Any like-minded people? :)

@ghost
ghost commented Jul 4, 2013

Got exactly same problem, its not possible to remove last element of Collection. Seems like bindValues is never triggered in Element\Collection if its null, so its not removed (count is always 1)

@thestanislav

Test for the issue #5010

@BreyndotEchse

I think the problem is Zend\Form\Form::prepareBindData(). This method removes all empty arrays returned by Zend\InputFilter\CollectionInputFilter::getValues().

My temporary fix for this is this (not the best way to do it):

use Zend\Form\Form as ZfForm;

class Form extends ZfForm
{
    /**
     * Parse filtered values and return only posted fields for binding
     *
     * @param array $values
     * @param array $match
     * @return array
     */
    protected function prepareBindData(array $values, array $match)
    {
        $data = array();
        foreach ($values as $name => $value) {
            if (!array_key_exists($name, $match)) {
                if (is_array($value) && empty($value)) {   // +
                    $data[$name] = array();                // +
                }                                          // +
                continue;
            }

            if (is_array($value) && is_array($match[$name])) {
                $data[$name] = $this->prepareBindData($value, $match[$name]);
            } else {
                $data[$name] = $value;
            }
        }
        return $data;
    }
}
@netiul
netiul commented Sep 27, 2013

If you need a temp fix, I would just transform the posted data.

F.e.

    if (!isset($postedData['fieldset']['collection'])) {
        $postedData['fieldset']['collection'] = array();
    }
@sebob
sebob commented Dec 10, 2013

Hello, I have the same problem, or the latest version of it may have been fixed?

For me, even if i'm trying to remove all items from the collection, and $datapost = array() /empty and count = 0
it is impossible to clear the collection, $form->isvalid() == false because collection need 1 element.

Maybe you have some other solution to this problem?

@sebob
sebob commented Dec 11, 2013

I think it's good way @BreyndotEchse.

It is my temp fix for this bug, but is not the best way to do it (inelegant solution).
What do you think?

protected function prepareBindData(array $values, array $match)
{
    $data = array();

    foreach ($values as $name => $value) {

        if (!array_key_exists($name, $match)) {      
            if (is_array($value)) {
                $value = $this->remove_empty_array($value);
            }

            if (is_array($value) AND empty($value)) {   // +
                $data[$name] = array();                // +
            }                                          // +

            continue;
        }

        if (is_array($value) && is_array($match[$name])) {
            $data[$name] = $this->prepareBindData($value, $match[$name]);
        } else {
            $data[$name] = $value;
        }
    }

    return $data;
}

protected function remove_empty_array(&$array)
{
    foreach($array as $k => &$v) {
        if(is_array($v)) {
            $array = $this->remove_empty_array($v);
        }else {
            unset($array[$k]);
        }
    }
    return $array;
}
@thestanislav

May be it's better to extend Zend\Form\Element\Collection with custom element and handle empty values somewhere here https://github.com/zendframework/zf2/blob/master/library/Zend/Form/Element/Collection.php#L186

@fabiocarneiro

@thestanislav Please fix that code you wrote for Collection extract method.

When you have nested COLLECTIONS, collection extract method tries to set an array to fieldset setObject() method. You'll be only able to reproduce this bad behavior if you have a collection that has a collection inside it.

This PR is a starting point #5502. But that fix wasnt enough.

Also, there is a module with this testcase https://github.com/fabiocarneiro/FhcsFormTest. I got some extra help in that module and we've been working to try different things, but the same problem is still there.

@adamlundrigan

Any news or workarounds on this? I posted about it on SO a while back...crickets. I did get a suggestion to disable the validation completely, which unfortunately isn't practical (and didn't work anyway).

@sparrowek

I have the same problem and during debugging noticed that in FIledset::populateValues there is a case for this:
/* This ensures that collections with allow_remove don't re-create child
* elements if they all were removed */
$elementOrFieldset->populateValues(array());

but this does nothing since in Collection::populateValues there is I think an old check:
// Can't do anything with empty data
if (empty($data)) {
return;
}

@sparrowek

I have commented this check:
if (empty($data)) {
return;
}

and now the form validates itself and elements of collection gets removed :)
It would work fine but I ran into an issue with DoctrineObject hydrator because the data passed from input filter do not contain the emtpy collection (not even a key) and so hydrator does not fire removal of collection elements

@iprdp
iprdp commented Aug 13, 2014

+1. Same problem with Zend\Form\Form::prepareBindData() parsing for only submitted form params.

To the uninformed, Zend\Form\Form::prepareBindData() compares submitted form fields with validated form fields and allows only the submitted form fields to be set for hydration. This actually helps in modifying only the data that the user was intending to change, so that during hydration the unintended fields retain their original values (from the database/persistance).

While parsing and identifying only the submitted form params to be set to the hydrationData after validation is good (securing unexpected data removal), there should be a way to bypass/override it.

We need to bypass/override this because, if the last item in a collection is removed and submitted, there is no way to represent it through POST data. It will look like the field was not part of the POST data.

Even though bypassing/overriding seems to be the right logic to me, it is insecure to bypass it without making sure that the user has removed the last item. So, I have used a hidden element to indicate this through the POST data.

If the last item is removed, I will set the hidden element's flag and accordingly set an empty array to the POST data in the controller.

PS: I cannot do that directly because the same form sometimes will not have the collection shown to the user. In this case, it cannot remove the data from the database.

@davidomelettes

+1.

You cannot remove the last element of a collection if it has a bound object expressing input validators. Submitting the form causes it to fail validation if, for example, your collection element fieldset contains a Select element which does not allow an empty value.

@davidomelettes

This is the workaround I am currently using:

$data = $request->getPost();
$collectionNames = array('contactMethods', 'addresses');
$emptyCollections = array();
foreach ($collectionNames as $collectionName) {
    if (!isset($data->$collectionName)) {
        // Collection element expects an empty array if collection is empty
        $emptyCollections[] = $collectionName;
        $data->$collectionName = array();
    }
}
$form->setData($data);
foreach ($emptyCollections as $collectionName) {
    // Remove validation from the form for each empty collection 
    $formFilter = $form->getInputFilter();
    $collectionFilter = $formFilter->get($collectionName);
    foreach ($collectionFilter->getInputs() as $i => $collectionFieldsetFilter) {
        $collectionFilter->remove($i);
    }
}
@Qronicle

Having the same problem as @sparrowek and came to the same conclusion.
Guess we're going to override the Collection class for the time being.

@tomdrissen

I'm with @sparrowek and @Qronicle.

Isn't it as simple as removing the check for an empty array in populateValues in Form\Element\Collection? Or will this change break other functionality?

https://github.com/zendframework/zf2/blob/master/library/Zend/Form/Element/Collection.php#L199

I tested it myself and it looks like it's working as expected.

@sparrowek

I do not quite remember but I think it was not as simple as that - I also had to manually put an empty array in POST data because when a user removes the last element from collection, the collection does not appear in posted data and so the input filter does not initialize it. Otherwise I had a problem with doctrine hydrator not scheduling elements for removal.

@Maks3w Maks3w closed this Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.