[Form] Collection Type with addXxx and removeXxx #3732

pocallaghan · 2012-03-30T09:21:03Z

I recently tried to set up a one-to-many relationship using the collection type to allow adding and removing elements. I had tremendous trouble getting it working, during the process of which I came across several issues, some of which are already reported, the last of which I don't believe is. I've include some information on all of it, for anybody attempting the same task.

The very first problem I had was that the addXxx and removeXxx functions never seemed to be called, I eventually tracked this down to a bug/ version incompatibility that had already been reported (Doctrines PersistentCollection didn't have __clone, meaning when the dataSnapshot is taken in MergeCollectionListener, the inner collection isn't cloned, thus changes are made to the dataSnapshot), I fixed this by updating composer.json to use doctrine/orm v2.2.1.

The second issue I had was that whilst addXxx was now being called, it was passing an array rather than an instance of my entity, I tracked this down to another bug report [https://github.com//issues/3354] , I used the suggest work around of adding the data_class to the options in the parent type.

At this point I am using symfony/symfony v2.1.* with doctrine/orm v2.2.1

After these steps the addXxx function seemed to work, but removeXxx was never getting triggered. After looking through the code I identified where I think the issue is as being MergeCollectionListener::onBindNormData. The logic for populating $itemsToDelete doesn't seem to make much sense to me. I have managed to get my test code up and running by changing the code....

          // Item not found, remember for deletion
            foreach ($originalData as $key => $item) {
                if ($item === $originalItem) {
                    $itemsToDelete[$key] = $item;
                    continue 2;
                }
            }

...to...

            foreach($originalData as $key => $item) {
                if ($item === $originalItem) {
                    continue 2;
                }
            }
            $itemsToDelete[] = $originalItem;

Here are the relevant Entities and Types, the controller / twig templates in use are the ones generated by the crud generator (with some JS added to add / remove fields).

<?php

namespace Acme\DemoBundle\Entity;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\Common\Collections\ArrayCollection;

/**
 * @ORM\Table(name="band")
 * @ORM\Entity
 */
class Band
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue
     */
    private $id;

    /**
     * @var string $number
     *
     * @ORM\Column(name="name", type="string", length=128)
     */
    private $name;

    /**
     * @var Doctrine\Common\Collections\ArrayCollection
     *
     * @ORM\OneToMany(targetEntity="BandDetail", mappedBy="band", cascade={"all"})
     */
    private $details;

    public function __construct()
    {
        $this->details = new \Doctrine\Common\Collections\ArrayCollection();
    }

    /**
     * Get id
     *
     * @return integer
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * Set name
     *
     * @param string $name
     * @return Band
     */
    public function setName($name)
    {
        $this->name = $name;
        return $this;
    }

    /**
     * Get name
     *
     * @return string
     */
    public function getName()
    {
        return $this->name;
    }

    /**
     * Add details
     *
     * @param Acme\DemoBundle\Entity\BandDetail $details
     */
    public function addDetail(\Acme\DemoBundle\Entity\BandDetail $detail)
    {
        $detail->setBand($this);
        $this->details->add($detail);
    }

    public function removeDetail(\Acme\DemoBundle\Entity\BandDetail $detail)
    {
        $detail->setBand(null);
        $this->details->removeElement($detail);
    }

    public function setDetails($details)
    {
        foreach($details as $detail) {
            $detail->setBand($this);
        }

        $this->details = $details;
    }

    /**
     * Get details
     *
     * @return Doctrine\Common\Collections\Collection
     */
    public function getDetails()
    {
        return $this->details;
    }
}

<?php

namespace Acme\DemoBundle\Entity;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\Common\Collections\ArrayCollection;

/**
 * @ORM\Table(name="band_details")
 * @ORM\Entity
 */
class BandDetail
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue
     */
    private $id;

    /**
     * @ORM\ManyToOne(targetEntity="Band", inversedBy="details")
     * @ORM\JoinColumn(name="band_id", referencedColumnName="id")
     */
    private $band;

    /**
     * @var string $number
     *
     * @ORM\Column(name="text", type="string", length=128)
     */
    private $text;

    /**
     * Get id
     *
     * @return integer
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * Set text
     *
     * @param string $text
     * @return BandDetail
     */
    public function setText($text)
    {
        $this->text = $text;
        return $this;
    }

    /**
     * Get text
     *
     * @return string
     */
    public function getText()
    {
        return $this->text;
    }

    /**
     * Set band
     *
     * @param Acme\DemoBundle\Entity\Band $band
     * @return BandDetail
     */
    public function setBand(\Acme\DemoBundle\Entity\Band $band = null)
    {
        $this->band = $band;
        return $this;
    }

    /**
     * Get band
     *
     * @return Acme\DemoBundle\Entity\Band
     */
    public function getBand()
    {
        return $this->band;
    }
}

<?php

namespace Acme\DemoBundle\Form;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\FormBuilder;

class BandDetailType extends AbstractType
{
    public function buildForm(FormBuilder $builder, array $options)
    {
        $builder
            ->add('text')
        ;
    }

    public function getDefaultOptions(array $options)
    {
        return array(
            'data_class' => 'Acme\DemoBundle\Entity\BandDetail',
        );
    }

    public function getName()
    {
        return 'banddetail';
    }
}

<?php

namespace Acme\DemoBundle\Form;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\FormBuilder;

class BandType extends AbstractType
{
    public function buildForm(FormBuilder $builder, array $options)
    {
        $builder
            ->add('name')
            ->add('details', 'collection', array(
                'type' => new BandDetailType(),
                'allow_add' => true,
                'allow_delete' => true,
                'prototype' => true,
                'by_reference' => true,
                'options' => array('data_class' => 'Acme\DemoBundle\Entity\BandDetail'),
            ))
        ;
    }

    public function getDefaultOptions(array $options)
    {
        return array(
            'data_class' => 'Acme\DemoBundle\Entity\Band',
        );
    }

    public function getName()
    {
        return 'acme_demobundle_bandtype';
    }
}

Apologies for the length of the post, I was just trying to be thorough. I am a newcomer to both Symfony and Doctrine, but I'm relatively confident there is a bug here. Can anybody shed some light on to whether this is a valid fix to the issue, or whether I'm just doing something wrong?

Cheers.

The text was updated successfully, but these errors were encountered:

stof · 2012-03-30T11:43:35Z

@bschussek ping

webmozart · 2012-04-06T21:52:32Z

Confirmed. Working on this.

Commits ------- c4e68a3 [Form] Moved logic of addXxx()/removeXxx() methods to the PropertyPath class Discussion ---------- [Form] Moved logic of addXxx()/removeXxx() methods to the PropertyPath class Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #3732 Todo: - ![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3732) The addXxx()/removeXxx() methods should now be called correctly in ChoiceType and CollectionType. PropertyPath now favors addXxx()/removeXxx() over setXxx() for collections. For example: ``` $propertyPath = new PropertyPath('article.tags'); // Tries to use addTag()/removeTag() and only uses setTags() (et al.) // if not found $propertyPath->setValue($article, $tags); ``` For other languages than English or very irregular plurals, a custom singular can be set by separating it with a pipe: ``` $propertyPath = new PropertyPath('article.genera|genus'); ``` --------------------------------------------------------------------------- by bschussek at 2012-04-07T12:40:39Z Again, the failing build is not my fault.

This was referenced Apr 7, 2012

[Form] Moved logic of addXxx()/removeXxx() methods to the PropertyPath class #3819

Merged

[Form] Configuration of custom adders/removers symfony/symfony-docs#1227

Closed

fabpot closed this as completed Apr 10, 2012

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[Form] Collection Type with addXxx and removeXxx #3732

[Form] Collection Type with addXxx and removeXxx #3732

Uh oh!

Uh oh!

Uh oh!

[Form] Collection Type with addXxx and removeXxx #3732

[Form] Collection Type with addXxx and removeXxx #3732

Comments

Uh oh!

Uh oh!