Sonataadminbundle: Unable to delete from sonata_type_collection (worked in 2.0 but not after upgrade)

[RobMasters]

I know that this used to be a common problem and the answer was always to put 'by_reference' => false. Here's my code:

protected function configureFormFields(FormMapper $form)
{
    $form
        ->add('title')
        ->add('collectionHasStories', 'sonata_type_collection', array('by_reference' => false, 'label' => 'Stories'), array(
            'edit' => 'inline',
            'inline' => 'table',
            'sortable'  => 'position'
        ))
    ;
}

I don't think that this is something I'm doing wrong as I've noticed that since upgrading I'm not even able to delete media items from a gallery's galleryHasMedia collection. This is a very big problem for me!

Has anything changed since that would cause this to break? Or is it something to do with the particular versions I'm using? Here is everything related to sonata in my composer.json:

    "sonata-project/user-bundle": "*",
    "sonata-project/cache-bundle": "dev-master",
    "sonata-project/doctrine-orm-admin-bundle": "*",
    "sonata-project/media-bundle": "*",

(I found I didn't need to add the admin-bundle as the user-bundle depends on the dev-master version of it)

[RobMasters]

After a little trial and error, I noticed that it now works when you specify 'by_reference' => true instead, or if you simply don't specify this at all.

Is this expected behavior?

[RobMasters]

Ok, after some more trial and error I've realized that this definitely is a problem. Without setting 'by_reference' to false, related entities aren't added to the collection so it isn't possible to create new collections. But if you set it to false to be able to add to collections then, as I mentioned previously, it isn't possible to remove anything from the collection.

Whichever way this is done, it needs to be consistent so it's possible to add and remove elements from a collection.

Any ideas where I should be looking to try and fix this? @rande ?

[RobMasters]

I've spent a while digging in to this and have come to the conclusion that the problem resides in the Form component somewhere. However, I've come up with a fairly generic solution (hack) that fixes the problem from within the Admin class:

# In a concrete Admin class

/**
 * @param mixed $object
 * @return mixed|void
 */
public function preUpdate($object)
{
    $form = $this->getForm();
    $children = $form->getChildren();

    foreach ($children as $childForm) {
        $data = $childForm->getData();
        if ($data instanceof Collection) {
            $proxies = $childForm->getChildren();
            foreach ($proxies as $proxy) {
                $entity = $proxy->getData();
                if (!$data->contains($entity)) {
                    $this->getModelManager()->delete($entity);
                }
            }
        }
    }
}

As you can see I'm currently doing this from a preUpdate hook in my Admin class, but do you think this could/should be moved up into the base Admin class? Possibly as a (temporary) $this->updateCollections($object) method that is called at the start of the update() method?

Shall I create a PR for this or something similar?

EDIT: see non-admin related solution I just posted to stackoverflow

[iamdey]

Hello,
I got the same problem on symfony 2.1 + sonata + Admin with many-to-many field; I set the doctrine's orphanRemoval property on my one-to-many field and then makes deletion working on type Collection, something likes :

/**
 * @var \Doctrine\Common\Collections\ArrayCollection
 * 
 * @ORM\OneToMany(targetEntity="CollectionHasStories", mappedBy="collection", cascade={"all"}, orphanRemoval=true)
 */
protected $collectionHasStories;

[kroshp]

Indeed, the orphanRemoval property resolve the problem.

[rmzamora]

Hi this problem still exist using symfony 2.1.4-DEV any solutions?

[toooni]

i do have this problem too..
but i also have collections for which this works.. i configured everything exactely the same (relations, getters, setters...) but in one case it works.. in the other it doesn't.. i am really frustrated...

[lineke]

@toooni I am having the exact same problem and it's driving me crazy. Did you find it?

[rande]

It is an issue with reference and the Form component. Please review: https://github.com/sonata-project/SonataMediaBundle/blob/master/Admin/GalleryAdmin.php#L125

[lineke]

@rande That doesn't fix my problem either. I have tried about everything I could find and I am really frustrated

[lineke]

Well, I finally got it working. I was overriding the getObject method in my
admin class because I needed to filter the displayed records. That was
causing the delete to not function.
Hope this helps someone someday.

2012/12/27 Thomas [email protected]

It is an issue with reference and the Form component. Please review:
https://github.com/sonata-project/SonataMediaBundle/blob/master/Admin/GalleryAdmin.php#L125

—
Reply to this email directly or view it on GitHubhttps://github.com/sonata-project/SonataAdminBundle/issues/998#issuecomment-11710763.

[toooni]

@rande My entries still aren't deleted.. even if i call the setters manually in the preUpdate and prePersist function of my admin class...

[rande]

Please review this documentation : http://symfony.com/doc/current/cookbook/form/form_collections.html

[bwhitty]

Also fixed this issue by adding orphanRemoval=true to my OneToMany associations in my OneToMany <-> ManyToMany <-> OneToMany relationship.

[adyballa]

I'm talking about the inverse side - like OneToMany.
That's (orphanRemoval) just a solution, if you also want to delete the children!
If it isnt an option, implement the remove-method and set the association to NULL or a corresponding value to your application-logic.

[fernandopg]

Hi @adyballa!

I have the problem that you said, I have a OneToMany collection and I want to be able to delete the associations but not to delete the children...

In your comment you said that we need to implement the remove-method and set the association to NULL...

I don't know if I understand well, I have the removeEntity function in my Entity class but never is called...

Ej:

User Entity:

/**
 * @ORM\OneToMany(targetEntity="MyBundle\Question", mappedBy="user", cascade={"all"})
 **/
protected $questions;

.....

public function __construct()
{
    $this->questions = new ArrayCollection();
}

public function removeQuestion($question)
{
    $question->setUser(null);
    $this->questions->removeElement($question);
}

Thanks!

[vikramparihar]

I did {
/*
* @ORMOneToMany(targetEntity="MandalAdminBundleEntityMember", mappedBy="family", cascade={"persist", "remove"},orphanRemoval=true)
*/
private $member;
}
and its worked for me.
Thanks esion

[laurentiustroia]

orphanRemoval=true remove all entities from collection, but I just want remove just checked entities, how can do this ?

[tamcy]

My Entity (simplified)

class News {
    /**
     * @ORM\OneToMany(targetEntity="NewsAttachment", mappedBy="owner", cascade={"all"}, orphanRemoval=true)
     * @ORM\OrderBy({"position" = "ASC"})
     *
     * @var NewsAttachment[]
     */
    private $attachments;

    public function setAttachments(iterable $attachments): void {
        $this->attachments = $attachments;
    }

    public function addAttachment(NewsAttachment $attachment): void {
        $attachment->setOwner($this);
        $this->attachments->add($attachment);
    }
}

Note that orphanRemoval=true is added to the OneToMany annotation. Also I left by_reference to be false in the form mapper.

The problem

Similar to above, I am seeing a problem that all items got deleted upon save.

The cause

This is due to the code in setAttachments, which replaces the existing PersistentCollection of the entity by a new array/ArrayCollection. This is WRONG. (See https://github.com/doctrine/doctrine2/issues/6344, quote: "Replacing collection instances is not really fully supported")

The solution

So I modified the setAttachments method like this:

    public function setAttachments(iterable $attachments): void
    {
        $itemsToAdd = [];
        $existingIds = [];

        /** @var NewsAttachment $attachment */
        foreach ($attachments as $attachment) {
            // New item's ID is null
            if (null === ($id = $attachment->getId())) {
                $itemsToAdd[] = $attachment;
            } else {
                $existingIds[$id] = true;
            }
        }

        /** @var AnnouncementFile $attachment */
        foreach ($this->attachments as $idx => $attachment) {
            if (!$attachment->getId()) {
                continue;
            }

            // Remove item from the collection if it isn't in the supplied
            // $attachments.
            if (!isset($existingIds[$attachment->getId()])) {
                $attachment->setOwner(null);
                unset($this->attachments[$idx]);
            }
        }

        // Add new items
        foreach ($itemsToAdd as $attachment) {
            $this->attachments[] = $attachment;
        }
    }

The local $attachments contains the new and existing items but not the deleted ones (they are removed by the ResizeFormListener). So the setAttachments method compares the entity's $attachments against the local one, adds new items to it and removes deleted items from it.

Now delete function should work correctly.

Alternative method

If I add the removeAttachment method to the News class it will get called by the PropertyAccessor before setAttachments():

    public function removeAttachment(NewsAttachment $attachment)
    {
        $this->attachment->removeElement($attachment);
    }

In this case, it's no longer neccessary to track the removed items in setAttachments(). Also, it turns out to be fine adding the same object to the PersistentCollection. This means that it's also not neccessary to track whether an item has already been added to the collection, we can just add everything to it and let Doctrine handles the rest. The setAttachments() method can then be simplified to:

    public function setAttachments(iterable $attachments): void
    {
        foreach ($attachments as $attachment) {
            $this->attachments[] = $attachment;
        }
    }

This works, but this would mean that the attachments array could be in an invalid state (Entity's $attachments field may contain duplicated item). Also setAttachments() now assumes that items not found in the $attachments argument be removed via removeFile in advance.