Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tbd-rsm opened this issue May 14, 2018 · 3 comments
Open

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

tbd-rsm opened this issue May 14, 2018 · 3 comments
Assignees
Labels
Milestone

Comments

@tbd-rsm
Copy link

tbd-rsm commented May 14, 2018

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. (contao/core#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
Copy link
Author

tbd-rsm commented May 14, 2018

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
Copy link
Collaborator

zonky2 commented May 14, 2018

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

@tbd-rsm
Copy link
Author

tbd-rsm commented May 14, 2018

@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.

@zonky2 zonky2 added the Defect label Jul 10, 2018
@zonky2 zonky2 added this to the 3.3.17 milestone Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants