[Form] Extend CollectionType to handle custom keys

[webmozart]

Right now, CollectionType is designed to work only with numerically and incrementally indexed collections. An interesting extension (see #7807) is to allow customly indexed collections as well by using the value of one of the fields in a row as key for the respective entry. The feature could be activated by setting an option such as index_by that configures the property path to the key in the data of the row.

Example:

$builder->add('employees', 'collection', array(
    'type' => 'employee',
    'index_by' => 'sin',
));

[kingcrunch]

Maybe I get this wrong, but as far as I have learned this a collection is "a finite set of unique, or non-unique ordered items", but it is not a map, or a struct(ure). How does string-keys fit into this?

[maciej-pyszynski]

Sometimes you need a map or structure. This feature doesn't have to be implemented directly in collection type. It can be put in new type e.g. "map" inheriting from collection.

Some use case:
You editing options for choice field representation. Choices are in format ['value' => 'label'].
In you model you have method getChoices() returning data in format above feeding form field (collection or map).

[webmozart]

@kingcrunch We could of course also implement this as another type, MapType.

[searbe]

It doesn't need to be implemented in the collection type, however it is quite unexpected that this can't be solved with a data transformer.

What I was expecting in my case was to be able to transform my model data:

[ 'key1' => 'value1', 'key2' => 'value2' ]

In to a normalised format which the collection can expect:

[ 0 => [ 'key' => 'key1', 'value' > 'value1' ], 1 => [ 'key' => 'key2', 'value' => 'value2' ] ]

(Obviously it is a collection of a form type with two child string types for key and value).

Because the ResizeListener ultimately takes "raw" model data from the PRE_SET_DATA event... well, it's model data, not normalised data, so we have no real way to transform the data. It feels strange in this instance that the form system cares about the precise format of my model's data, given that we have this whole normalisation system.

I was expecting model->norm transformation to happen almost instantly, so the whole form system would expect normalised data.

I'm probably not considering something, but if the model->norm transform did happen before PRE_SET_DATA, and that event shared normalised data instead of model data, would it solve this problem and discourage further dependencies on a specific model data format? Should the form system do anything with model data other than transform it to/from normalised data?

I currently have a hack in a custom "Map" type: listeners with higher and lower priority than the resize listener which convert the model data in to the format the resize listener expects, then transforms it back so that the 'regular' transformations still work!

[webmozart]

@searbe You are misinterpreting the collection type. This type expects a single-level array (collection), not a multi-leveled one, so even if you add your data transformer this can never work.

What you want instead is a collection of collection fields (one for the outer array, one for each inner one). Alternatively, if for example the outer keys are fixed, you can add the collection fields statically:

Example:

foreach (array('en', 'fr') as $key) {
    $builder->add('translations_' . $key, 'collection', array(
        'type' => '...',
        'property_path' => 'translations[' . $key . ']',
    ));
}

[searbe]

@bschussek Sorry, I probably wasn't clear, but yes - my MapType is a child of the CollectionType (with that listener hack). So it's a collection of form types which each contain two string types.

Anyway, my point is that it was unexpected that I can't map my model data to something the ResizeListener can work with - I have to fudge the model data to a specific format on the way in, using a separate mechanism to the data normalisation / transformers. It makes me wonder whether the resize listener (or anything else on the pre.set.data event) should be receiving model data at all - it feels like listeners should get normalised data, so that they aren't expecting any specific format from the models themselves..?

[webmozart]

@searbe I think I got your point now. Can you please open a new ticket? I will look into this later (and it is quite unrelated to MapType).

[searbe]

@bschussek Sure - I have added #7905 ... MapType is my custom type wrapping CollectionType ... I just bought my thoughts up here since it felt similar - the custom keys issue could be solved with a similar setup to mine and a data transformer, if the ResizeListener wasn't getting in the way. Thanks!

[wodka]

I'd say this is usefull. Consider the following case:

Collection of Items:

• Item A with Feature A
• Item B with Feature B
• Item C with Feature A

now to have the form itself more readable I'd like to group the entries by there features. I would like to do this in a way similar to this:

{% for feature in ["Feature A", "Feature B"] %}
  <h2>{{ feature }}</h2>
  {% for item in element.getItemsByFeature(feature) %}
    {{ form_row(form.items[...somehow...]) }}
  {% endfor %}
{% endfor %}

basically right now I can not easily find the right match because they are just numbered and the order does not represent the order of after the feature grouping.

[trsteel88]

+1 for an index_by option.

I have an issue where a collection has 1 row. If I delete that row and add a new row (all in 1 request), it now doesn't recognise the new row as a new row. It actually updates the existing row in the database (because it sees the first item as matching the first index). In some instances I don't want that same row to be updates because I store information such as a Created At datetime object.

[webmozart]

See #13116 (comment) which contains another detailed comment about this issue.

[sergeyfedotov]

@webmozart which indexes should be used for the new records? For example if an entity ID is used as index we can not simply use increment. What do you think about the introduction of the non mapped hidden field _id for the children forms? In this case the indexes can take any value.

[webmozart]

@sergeyfedotov That could be a good solution.

[kokers]

index_by would be awesome addition, as now I need to find a way around "reassigning" id as I have another relation inside the collection. Consider this entities with following relations:

City - onetomany -> Street - onetomany -> House - manytoone -> Person

Inside city form I have collectionType for streets. You can add/remove/reorder them.

Let say I have inside Street

id	city_id	street_name
1	2	Yellow
2	2	Blue
3	2	Green

and House

id	street_id	house_name
1	1	Big
2	1	Round
3	2	With loud neighbours

Now, I reorder streets inside City form and submit order: Green,Yellow,Blue. After submit I see this:

id	city_id	street_name
1	2	Green
2	2	Yellow
3	2	Blue

Before submit I had street Yellow that had [Big,Round] houses. Now it's the Green street that have this relation which is obviously incorrect.

Same happens if you remove something. The relation will not be removed as the id has changed and now will point to something else. Even if I make sure to remove Houses first from removed Street, then there still everything will be messed up. for example I remove Yellow along with [Big,Round]. And I will get:

id	city_id	street_name
1	2	Blue
2	2	Green

and House

id	street_id	house_name
3	2	With loud neighbours

House "With loud neighbours " is now assigned to "Green" instead of "Blue".

With that, CollectionType shouldn't be used if there is a relation inside that collection. If I could add another field "order" and use index_by => order, that would solve my issue. I think. Unless there is something I don't know or can't find in documentation for this use case to resolve this.

[pathmissing]

I still would consider this feature essential. Has there been any status update since 2016? Or a PR even?

[xabbuh]

@pathmissing There has been one attempt in #13116 which was closed as it did not fully cover everything. So you could still work on it if you would like to give it a try.

[oleg-andreyev]

My workaround for this was to pass Collection / array indexed by required key.

[robjensen82]

I agree with @sergeyfedotov's suggestion of adding a hidden _id field to child forms. I think that would make identifying objects for addition, update and removal pretty easy.

[nkanzari]

Any progress on this one?

[xabbuh]

@cre8izm I don't think anyone is working on this currently. So feel free to to start a pull request if you would like to give it a try.

[ThomasLandauer]

I'm a little confused about the status here. As @stof explained at #34453 (comment) it is already possible to index the collection by id. So @webmozart 's very first sentence here seems obsolete:

Right now, CollectionType is designed to work only with numerically and incrementally indexed collections.

Could it be that this issue is already resolved and they just forgot to close it?

However, I see a problem that is somewhat related: The members of the collection get persisted to the database as soon as you do $form->handleRequest($request); in the controller.

My scenario: I'm using a collection on a "live" table. Say, it displays the records with id 15, 87, 56. Now the database changes, and upon form submission, the database query returns 15, 93 (=new item), 87. But the form submission still contains the id's 15, 87, 56. Since 93 is not included in the form data, it gets emptied out in the database! And since 56 is not present in the new query result, it causes the error "This form should not contain extra fields". The problem is that by the time that error gets raised, the previous records 15 and 93 have already been persisted.

So I'm suggesting: Introduce a new option to prevent the automatic persisting/flushing of collection items. This would give the user the chance to manually iterate through the collection data in the controller, to check each item before persisting it.

[xabbuh]

So I'm suggesting: Introduce a new option to prevent the automatic persisting/flushing of collection items. This would give the user the chance to manually iterate through the collection data in the controller, to check each item before persisting it.

That's something related to Doctrine and is controlled by the developer when choosing whether or not to cascade persist operations (see https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/working-with-associations.html#transitive-persistence-cascade-operations). It's not something we can control from inside the Form component.

[ThomasLandauer]

@xabbuh No, I'm not talking about related entities. I'm talking about one single table. So there must be a persist/flush somewhere in Symfony.

[xabbuh]

If these entities are not associated with the "root entity" your form is bound to and if I correctly understand what you are doing in your example, the entities are not new and thus already managed by Doctrine which in turn means that their changes will be written to the database once flush() is called (persist() is not needed here). But that's still how Doctrine works and nothing we can nor want to control from the Form component.

[ThomasLandauer]

I don't have a root entity. My parent form contains just the collection and the submit button, and is therefore bound to nothing. However, when I do handleRequest() on this form, the collection entries get persisted automatically. I'm not doing any flush() or persist() in my controller.

[xabbuh]

If you create an example application, I can take a look into it and help you find out where the flush originates, but I assure you that it is not triggered from the Form component.

[ThomasLandauer]

@xabbuh Thanks for insisting! :-) I was using Codeception for easier reproducing, and this was in fact causing the (invalid) entities to be flushed, see https://github.com/Codeception/Codeception/issues/5439

[ThomasLandauer]

Again, this issue looks resolved to me! What's been asked for in the title "Extend CollectionType to handle custom keys" is possible by passing an associative array; whatever is used as keys in this array will be used to "index" the collection. The suggested feature index_by isn't even necessary.

So I would say: This issue can be closed. The only thing that's missing is proper documentation at https://symfony.com/doc/current/form/form_collections.html

[Seb33300]

It looks like CollectionType properly works even if we manually set the collection index of each item.
This means we can manually set for example the object id as the collection index of each items and this can be a solution to prevent indices conflict.

public function buildForm(FormBuilderInterface $builder, array $options)
{
    $entity = $builder->getData();

    // Reindex collection using id
    $indexedCollection = new ArrayCollection();
    foreach ($entity->getCollectionField() as $collectionItem) {
        $indexedCollection->set($collectionItem->getId(), $collectionItem);
    }

    $builder
        ->add('collectionField', CollectionType::class, [
            'data' => $indexedCollection,
        ])
    ;
}

[xabbuh]

let's close here then

[patrickbussmann]

Hmm I have the same issue now but @Seb33300 is not working for me because I have deep form types.

i. e.

// Form Type 1

public function buildForm(FormBuilderInterface $builder, array $options)
{
    $builder
            ->add('subs', CollectionType::class, [
	            'entry_type' => SubType::class,
	            'allow_add' => true,
	            'allow_delete' => true,
	            'by_reference' => false
            ])
    ;
}

// Form Type 2
public function buildForm(FormBuilderInterface $builder, array $options)
{
    $entity = $builder->getData();
    // <- Problem: $entity is always NULL

    // Reindex collection using id
    $indexedCollection = new ArrayCollection();
    foreach ($entity->getCollectionField() as $collectionItem) {
        $indexedCollection->set($collectionItem->getId(), $collectionItem);
    }

    $builder
        ->add('collectionField', CollectionType::class, [
            'data' => $indexedCollection,
        ])
    ;
}

Some ideas?

Maybe a global form type listener which resolves the problem automatically.
When id field is found in array then use it as key and if not let the key as it is.

Would be a nice solution?

Thanks in Advance for ideas!

[arthurlauck]

Couldn't keep up with this and I can't find a solution, is there one? A definitive one?

[Seb33300]

@patrickbussmann @arthurlauck

In case $builder->getData() return NULL when trying my example you may be able to achieve it inside a form event (probably using PRE_SUBMIT or SUBMIT event)

[kevinpapst]

I am late to the party, but for the next one searching ... here is another example for certain circumstances:

        $builder->addEventListener(
            FormEvents::PRE_SET_DATA,
            function (FormEvent $event) {
                /** @var ArrayCollection<SomeInterface> $collection */
                $collection = $event->getData();
                foreach ($collection as $collectionItem) {
                    $collection->removeElement($collectionItem);
                    $collection->set($collectionItem->getName(), $collectionItem);
                }
            },
            100
        );

[Tofandel]

How would one go about saving the Collection Key to the entity

Say you submit

{"foo": {id: 1}, "bar": {id: 2}}

How to get the CollectionType to report the index to the entity so that you get

{"foo": {id: 1, name: "foo"}, "bar": {id: 2, name: "bar"}}

[Jalliuz]

Hmm I have the same issue now but @Seb33300 is not working for me because I have deep form types.

// Form Type 2

public function buildForm(FormBuilderInterface $builder, array $options)
{
    $entity = $builder->getData();
    // <- Problem: $entity is always NULL
}

Some ideas?

In a CollectionType ->getData() only works inside a PRE_SET_DATA Event and you get the entity from the event, not from the builder

// In a CollectionType you can't access the entity with $builder->getData()
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
    $form = $event->getForm();
    $yourEntity = $event->getData() ?: new YourEntity();

    // ... Do your thing here

    // ->add() the fields which rely on the entity here
   $form->add('someField');
});