Many-To-Many relations in symfony forms do not work as described

[MaksSlesarenko]

Hi, at page http://propelorm.org/cookbook/symfony2/mastering-symfony2-forms-with-propel.html section 'Many-To-Many relations' there is a warning box

The parameter by_reference has to be defined and set to false. This is required to tell the Form Component to call the setter method (setBooks() in this example).

Actually method setBooks will not be called since Symfony\Component\Form\Util\PropertyPath::writeProperty (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Util/PropertyPath.php#L465) first looks for addBook and removeBook, and ONLY if it can not find them both it will look for setBooks method. But this is not gonna to happen since propel already generated addBook, removeBook and setBooks methods

[havvg]

I didn't check the code in details, but it's working - is it just an outdated documentation?

This works fine with Symfony 2.1.

<?php

// ...

class ProductType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        // ...

        $builder
            // ...
            ->add('industry_sectors', 'collection', array(
                'label' => 'product.industry_sectors',

                'type' => 'model',

                'allow_add' => true,
                'allow_delete' => true,
                'by_reference' => false,

                'prototype' => true,
                'prototype_name' => 'industry_sector___name__',

                'options' => array(
                    'class' => 'Ormigo\Model\Product\IndustrySector',
                    'empty_value' => 'empty.choice.industry_sector',
                    'property' => 'name',
                    'query' => IndustrySectorQuery::create()
                        ->useI18nQuery($options['locale'])
                            ->orderByName()
                        ->endUse()
                    ,
                ),

                'constraints' => array(
                    new Count(array(
                        'min' => 1,
                        'minMessage' => 'product.industry_sector.min',
                    )),
                ),
            ))
        ;
    }

    // ...

}

[MaksSlesarenko]

So you have addCollection, removeCollection and setCollection methods, but in your case setCollection is used instead of addCollection and removeCollection?

Can you show how you object looks like in writeProperty method?

[havvg]

Hm.., no it's calling addIndustrySector and removeIndustrySector instead of setIndustrySectors. Just tried it with both actions on the same request, works fine.

When I set by_reference to true, it's not working. That's fine, as the PropelObjectCollection is changed, but the changes need to be forwarded to the Product instance and the form component isn't calling either of those three methods.

Can you show how you object looks like in writeProperty method?

What do you mean? It's getting called with indexed entries for the collection:

Output when adding var_dump('writeProperty', $property, $objectOrArray);

An existing relation being removed:

string 'writeProperty' (length=13)
string 'industry_sector_2' (length=17)
object(PropelObjectCollection)[640]

    object(Ormigo\Model\Product\IndustrySector)[628]

A new relation being added:

string 'writeProperty' (length=13)
string '0' (length=1)
object(PropelObjectCollection)[640]

    object(Ormigo\Model\Product\IndustrySector)[628]

and finally on the product to save the changes:

string 'writeProperty' (length=13)
string 'industry_sectors' (length=16)
object(Ormigo\Model\Product\Product)[840]

[MaksSlesarenko]

I have same result, it is working but not as described.

And before #549 changes in collections items were not tracked if no item was deleted.

[havvg]

Oh, I don't have that patch, seems like I got an outdated Propel version, am I missing something here, or what's actually broken?

[jaugustin]

@MaksSlesarenko the warning in the doc was to prevent the issue you fixed ;)

@havvg your setup is fined ;)

[MaksSlesarenko]

@jaugustin by_reference only defines whether addItem, removeItem methods will be called. What I've done is to make propel to save changes in sub entities that were not neither added nor removed, because setItemCollection method is not called anyway

[bmeynell]

Confused. Based on @MaksSlesarenko's last comment, by_reference=false causes addItem and removeItem to be called. This is contrary to the propel and symfony documentation that state that by_reference=false causes setItems to be called. Will someone please clarify?

[carlwitt]

I guess my problem is related. I have a schema like this:

<table name="publication">

    <!-- [ more attributes and foreign keys... ] -->
    <column name="title_id" type="INTEGER" required="true"/>

</table>

<table name="title">
    <column name="id" type="INTEGER" primaryKey="true" autoIncrement="true" required="true"/>
</table>

<table name="titlefragment">

    <!-- [ more attributes and foreign keys... ] -->
    <column name="title_id" type="INTEGER" required="true"/>

</table>

I.e. publication : title = 1 : 1 title : titleFragment = 1 : n When modifying a publications title fragment(s) via a symfony form and saving, all changes are lost, because $title->isModified() returns false.

Is that for performance reasons? I think it's highly inconsistent and therefore dangerous. Because if I edit the $title instance directly in a symfony form and modify its title fragments and then call $title->save() everything works as expected. If on the other hand I call $publication->save(), $title->save() is executed but the title entity doesn't notice it has changed. (And it conceptually has, since its parts have changed).

I know I can work around by adding custom code to Title.php.

 public function isModified()
 {
     $modified = !empty($this->modifiedColumns);
     if( ! $modified ){
         foreach (parent::getTitlefragments() as $tf) {
             if($tf->isModified()) return true;
         }
     }
     return false;
 }

But it goes without saying, that silently discarded changes are hard to notice (yet inacceptable). But if I forget to modify an entities custom code like this, it would happen. Let alone explaining this to my team mates maintaining the code somewhen in the future.

[bmeynell]

When modifying a publications title fragment(s) via a symfony form and saving, all changes are lost, because $title->isModified() returns false.

This epitomizes my experience as well. In a nutshell when I modify elements in child form(s) but invoke save on the parent object the changes don't take affect because the parent object is not seen as being modified. Unfortunately, I've had to resort to numerous hacks similar to above. I agree this is a big problem.

[bmeynell]

@havvg Thoughts? Symfony docs state, _"So, all that byreference=false really does is force the framework to call the setter on the parent object." But in the case of propel, if the parent object is not modified, the setter is never called. When the parent object is modified, then the setters are called.