search

symfony-cmf / menu-bundle

Extends the KnpMenuBundle to work with PHPCR ODM
https://cmf.symfony.com
32 stars 48 forks source link

Method removeChild is missing #181

Closed uwej711 closed 10 years ago

uwej711 commented 10 years ago

I made an admin that uses sonata_type_collection to display the first level of children of a menu (node). The Symfony Form Component complains about a missing removeChild method. Adding that looks easy at first but then I realized that there is a little mess with what $this->children really is: when you create a MenuNode it is initialized with an array. I think it should be an ArrayCollection so you can use the Collections methods to add or remove children.

PR is no problem once we agree ...

lsmith77 commented 10 years ago

i think this is a past mistake .. we should use ArrayCollections .. since this is what we get after deserializing and so it would be iffy to have an array after creation and ArrayCollection after retrieving from the DB.

dbu commented 10 years ago

We do that in most places and its the right thing for docrine collection fields. People might even relay on getting a collection and be surprised with a new document. +1 to standardize and have removeChild method. Are there other default docs with this problem?

uwej711 commented 10 years ago

Update: just came across another issue here: when I try to reorder children of an uninitialized proxy the following line fails (and it would fail with an ArrayCollection too): https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/UnitOfWork.php#L2508

uwej711 commented 10 years ago

Checking for PersistentCollection there helps.

lsmith77 commented 10 years ago

can you also work on a PR for that then?

uwej711 commented 10 years ago

sure

uwej711 commented 10 years ago

see PR