menatwork / MultiColumnWizard

Contao Extension :: Define unlimited dca fields dynamically
http://contao.org/en/extension-list/view/MultiColumnWizard.html
28 stars 30 forks source link

data loss with select column, multipe=>true #255

Open tbd-rsm opened 6 years ago

tbd-rsm commented 6 years ago

First of all: MCW is a great extension. Thank you for providing it!

Second: I'm not sure if this is a bug in MCW or should be addressed in contao/core. Please correct me if this is wrong here. Or maybe I just missed something.

Setup Contao 3.5.35 (clean installation) MultiColumnWizard 3.3.6/4 one custom test extension "mcw-multiselect-test" (see below)

Expected behaviour When editing a MCW field with a text column and a multi-select column i expect all filled in values to be saved into the Database.

Faulty behaviour The text field value stays empty while the selection is saved to the database.

Analysis There seems to be a bug when rendering the select field. The hidden input that is rendered for multiple select fields has the name rsm_mcw_multiselect_test[0][tag_select (note the missing closing bracket).

This seems to result in data loss while receiving data server side. When var_dumping $_REQUEST i can see the values for tag_select but no values (not even the field's name) for tag_name. Maybe the field order is important here, since the wrong field name is for the select while the affected field's data lies above the faulty one (as seen in the http request body).

The issue seems to lie in \Contao\SelectMenu::generate()'s use of rtrim($this->strName, '[]') in system/modules/core/widgets/SelectMenu.php. (This is why I'm not sure where to open post this issue.) The usage of rtrim does not seem to be a new thing in that place. (https://github.com/contao/core/issues/7760)

Extension mcw-multiselect-test This test extension basically only adds a new content element with a MCW field.

<?php
// config/config.php

$GLOBALS['TL_CTE']['includes']['mcw_multiselect_test'] = '';
<?php
// dca/tl_content.php

$GLOBALS['TL_DCA']['tl_content']['fields']['rsm_mcw_multiselect_test'] = array
(
    'label'  => &$GLOBALS['TL_LANG']['tl_content']['rsm_mcw_multiselect_test'],
    'exclude'       => true,
    'inputType'     => 'multiColumnWizard',
    'eval'          => array
    (
        'columnFields' => array
        (
            'tag_name' => array
            (
                'label'         => &$GLOBALS['TL_LANG']['tl_content']['categorytags_name'],
                'exclude'       => true,
                'inputType'     => 'text',
                'eval'          => array('style'=>'width:180px')
            ),
            'tag_select' => array
            (
                'label'         => &$GLOBALS['TL_LANG']['tl_content']['categorytags_select'],
                'exclude'       => true,
                'inputType'     => 'select',
                'eval'          => array
                (
                    'style'                     => 'width:250px',
                    'includeBlankOption'        => true,
                    'multiple'                  => true
                ),
                'options'        => array('A', 'B', 'C'),
            ),

        )
    ),
    'sql'           => "blob NULL"

);

$GLOBALS['TL_DCA']['tl_content']['palettes']['mcw_multiselect_test'] = '{type_legend},type,rsm_mcw_multiselect_test';
tbd-rsm commented 6 years ago

As a workaround that is update-proof i came up with this:

<?php
// config/config.php

/* [...] */

$GLOBALS['BE_FFL']['select'] = '\\MyNamespace\\SelectMenu';
<?php
// classes/SelectMenu.php

namespace MyNamespace;

class SelectMenu extends \Contao\SelectMenu
{
    public function generate()
    {
        //parent::generate() modifies this, so we have to use a tmp var here.
        $strName = $this->strName;

        $blnFixMultipleField = $this->multiple && substr($strName,-1) === ']';

        $strHtml = parent::generate();

        if ($blnFixMultipleField) {
            return str_replace(
                'name="' . substr($strName, 0, strlen($strName) - 1) . '"',
                'name="' . $strName . '"',
                $strHtml
            );
        }

        return $strHtml;
    }
}
zonky2 commented 6 years ago

@tbd-rsm pls test it with current version of MCW 3.3.16 - https://github.com/menatwork/MultiColumnWizard/releases

tbd-rsm commented 6 years ago

@zonky2 Thanks for the hint. I only checked the ER for updates. The issue still exists in 3.3.16.

I deleted the old version of multicolumnwizard, added 3.3.16 from the zip file in the release note, made sure it is uploaded correctly and cleared browser cache.

I also disabled my workaround and put an exit in my custom generate function, just to make sure that it is not active anymore.