Sonataadminbundle: New items to 1:n collections overwrite existing ones due to broken counter

Created on 8 Aug 2018  ·  5Comments  ·  Source: sonata-project/SonataAdminBundle

Environment

Sonata packages

$ composer show --latest 'sonata-project/*'
sonata-project/admin-bundle              3.37.0 3.37.0 The missing Symfony Admin Generator
sonata-project/block-bundle              3.12.1 3.12.1 Symfony SonataBlockBundle
sonata-project/cache                     2.0.1  2.0.1  Cache library
sonata-project/core-bundle               3.11.2 3.11.2 Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.3.1  2.3.1  Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2  1.0.2  Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.6.1  3.6.1  Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/easy-extends-bundle       2.5.0  2.5.0  Symfony SonataEasyExtendsBundle
sonata-project/exporter                  1.9.1  1.9.1  Lightweight Exporter library
sonata-project/user-bundle               4.2.3  4.2.3  Symfony SonataUserBundle

Symfony packages

$ composer show --latest 'symfony/*'
symfony/monolog-bundle     v3.3.0  v3.3.0 Symfony MonologBundle
symfony/phpunit-bridge     v3.4.14 v4.1.3 Symfony PHPUnit Bridge
symfony/polyfill-apcu      v1.9.0  v1.9.0 Symfony polyfill backporting apcu_* functions to lower PHP versions
symfony/polyfill-ctype     v1.9.0  v1.9.0 Symfony polyfill for ctype functions
symfony/polyfill-intl-icu  v1.9.0  v1.9.0 Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring  v1.9.0  v1.9.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php56     v1.9.0  v1.9.0 Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70     v1.9.0  v1.9.0 Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-util      v1.9.0  v1.9.0 Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.1  v3.0.1 Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v2.6.7  v3.2.2 Symfony SwiftmailerBundle
symfony/symfony            v3.4.14 v4.1.3 The Symfony PHP framework

PHP version

$ php -v
PHP 7.1.20-1+0~20180725103315.2+stretch~1.gbpd5b650 (cli) (built: Jul 25 2018 10:33:20) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.1.20-1+0~20180725103315.2+stretch~1.gbpd5b650, Copyright (c) 1999-2018, by Zend Technologies

Subject

To manage a 1:n child entity, we have created our own implementation of Symfony\Component\Form\AbstractType and have defined a form field, which works pretty nice in general.

Here are the relevant parts of our code (slightly abstracted):

OurType.php

class OurType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder
             ->add(/*...*/)
             ->add(/*...*/)
        ;
    }
}

ParentAdmin.php

$formMapper
    /* ... */
    ->add('ourField', CollectionType::class, [
        "by_reference" => false,
        "entry_type" => OurType::class,
        "allow_add" => true,
        "allow_delete" => true,
        'label' => 'Foobar'
    ])
<div class="child-elem">
    <table>
        <tr>
            <td class="lbl">Foo</td>
            <td class="store">{{ form_widget(form.children.foo) }}</td>
            <td class="lbl">Bar</td>
            <td class="preference">{{ form_widget(form.children.bar, {attr: { min: 0, max: 10 }}) }}</td>
        </tr>
    </table>
</div>

There is just one problem: When saving the entity, then trying to add new child elements, then saving again, the new child elements overwrite the old ones.

This is due to the following code in vendor/sonata-project/admin-bundle/src/Resources/public/Admin.js:

var counter = 0;
collection.children().each(function() {
    var matches = highestCounterRegexp.exec(jQuery('[id^="sonata-ba-field-container"]', this).attr('id'));
    if (matches && matches[1] && matches[1] > counter) {
        counter = parseInt(matches[1], 10);
    }
});

This code seems to expect some sort of “magic” ID, which is however not set. Therefore, matches is empty and the counter is set to zero. (By the way, why not set counter = collection.children().length? Why the magic ID stuff?)

This has the effect that when adding new elements, they get the same index as existing ones. When submitting the form to PHP, the backend received an array with duplicate keys and discards the existing ones.

pending author stale

Most helpful comment

Hello,

I also can confirm this bug because I ran into the same issue here. I've got a collectionType of a custom form field and the counter for that collection is broken. The suggestion of counter = collection.children().length seems to fix it (in my case anyway). I will try to make a PR for it to get this fixed.

All 5 comments

@rande I think this was done long time ago, can you maybe give us a bit of detail here?

@OskarStark @rande Any update on this? This is really a blocker for us.

From my side not 😕

Hello,

I also can confirm this bug because I ran into the same issue here. I've got a collectionType of a custom form field and the counter for that collection is broken. The suggestion of counter = collection.children().length seems to fix it (in my case anyway). I will try to make a PR for it to get this fixed.

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Was this page helpful?
0 / 5 - 0 ratings