Skip to content

Fix default values not being properly loaded for ArraySerialized fields #6231

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

Conversation

mimarcel
Copy link
Contributor

@mimarcel mimarcel commented Aug 18, 2016

Preconditions:

  • have latest M2.1 installed
  • install demo data (using bin/magento demodata installation options)
  • clean browser cookies for the domain you're at (to make sure we start from clean slate)
  • make sure that you start with clean cache

Bug:
Default values are not properly loaded for fields with backend model \Magento\Config\Model\Config\Backend\Serialized\ArraySerialized.

How to replicate:

  • Add system config field with backend Magento\Config\Model\Config\Backend\Serialized\ArraySerialized.

Example system.xml:

<?xml version="1.0"?>
<!--
 Copyright © 2009-2016 Vaimo AB. All rights reserved.
 See LICENSE.txt for license details.
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Config:etc/system_file.xsd">
    <system>
        <tab id="import_export" translate="label" sortOrder="1000">
            <label>Import Export</label>
        </tab>
        <section id="export" translate="label" type="text" sortOrder="1" showInDefault="1" showInWebsite="1" showInStore="1">
            <label>Export</label>
            <tab>import_export</tab>
            <resource>Vaimo_HellyHansenImportExport::config</resource>
            <group id="product_export" translate="label" type="text" sortOrder="1000" showInDefault="1" showInWebsite="1" showInStore="1">
                <label>Product Export</label>
                <field id="remove_export_columns" translate="label" type="text" sortOrder="1" showInDefault="1" showInWebsite="0" showInStore="0">
                    <label>Remove Export Columns</label>
                    <frontend_model>Vaimo\HellyHansenImportExport\Block\System\Config\Form\Field\Columns</frontend_model>
                    <backend_model>Magento\Config\Model\Config\Backend\Serialized\ArraySerialized</backend_model>
                    <comment>These columns will not be exported in Product Export.</comment>
                </field>
            </group>
        </section>
    </system>
</config>
  • In order to make this work, we need to add the resource for it.

Example acl.xml:

<?xml version="1.0"?>
<!--
 Copyright © 2009-2016 Vaimo AB. All rights reserved.
 See LICENSE.txt for license details.
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Acl/etc/acl.xsd">
    <acl>
        <resources>
            <resource id="Magento_Backend::admin">
                <resource id="Magento_Backend::stores">
                    <resource id="Magento_Backend::stores_settings">
                        <resource id="Magento_Config::config">
                            <resource id="Vaimo_HellyHansenImportExport::config" title="Helly Hansen Import Export" sortOrder="50" />
                        </resource>
                    </resource>
                </resource>
            </resource>
        </resources>
    </acl>
</config>
  • and also the frontend model:
<?php
/**
 * Copyright © 2009-2016 Vaimo AB. All rights reserved.
 * See LICENSE.txt for license details.
 */

namespace Vaimo\HellyHansenImportExport\Block\System\Config\Form\Field;

/**
 * Backend system config array field renderer
 *
 * @package Vaimo\HellyHansenItemExport\Block\System\Config\Form\Field
 */
class Columns extends \Magento\Config\Block\System\Config\Form\Field\FieldArray\AbstractFieldArray
{
    /**
     * Initialise form fields.
     *
     * @return void
     */
    protected function _construct()
    {
        $this->addColumn('column', ['label' => __('Column')]);
        $this->_addAfter = false;
        parent::_construct();
    }
}
  • Add default value in config.xml.

Example:

<?xml version="1.0"?>
<!--
 Copyright © 2009-2016 Vaimo AB. All rights reserved.
 See LICENSE.txt for license details.
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Store:etc/config.xsd">
    <default>
        <export>
            <product_export>
                <!--Remove image-related and bundle-related columns from export.-->
                <remove_export_columns><![CDATA[a:15:{i:0;a:1:{s:6:"column";s:10:"base_image";}i:1;a:1:{s:6:"column";s:16:"base_image_label";}i:2;a:1:{s:6:"column";s:11:"small_image";}i:3;a:1:{s:6:"column";s:17:"small_image_label";}i:4;a:1:{s:6:"column";s:15:"thumbnail_image";}i:5;a:1:{s:6:"column";s:21:"thumbnail_image_label";}i:6;a:1:{s:6:"column";s:12:"swatch_image";}i:7;a:1:{s:6:"column";s:18:"swatch_image_label";}i:8;a:1:{s:6:"column";s:17:"additional_images";}i:9;a:1:{s:6:"column";s:23:"additional_image_labels";}i:10;a:1:{s:6:"column";s:17:"bundle_price_type";}i:11;a:1:{s:6:"column";s:15:"bundle_sku_type";}i:12;a:1:{s:6:"column";s:17:"bundle_price_view";}i:13;a:1:{s:6:"column";s:18:"bundle_weight_type";}i:14;a:1:{s:6:"column";s:13:"bundle_values";}}]]></remove_export_columns>
            </product_export>
        </export>
    </default>
</config>
  • Go to admin and open STORES -> Configuration -> IMPORT EXPORT -> Export:

Expected: default value is loaded
configuration___settings___stores___magento_admin_-_correct

Actual: default value is missing because afterLoad was not called on backend model when configuration existed ONLY as default value in config.xml
configuration___settings___stores___magento_admin

Notes:

…model \Magento\Config\Model\Config\Backend\Serialized\ArraySerialized.
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Aug 18, 2016

CLA assistant check
All committers have signed the CLA.

@mimarcel mimarcel changed the title Fix default values not being properly loaded for fields with backend … Fix default values not being properly loaded for ArraySerialized fields Aug 18, 2016
@skovalenk
Copy link
Contributor

Hi @mimarcel

We watch on this code from another side:

  1. backend model processing cant be applied in all scenarios (because in some scenarios already unserialized data can come on backend model)
  2. We have one mechanism of getting configs. And this mechanism is \Magento\Framework\App\Config::class . This class allows to process backend models only one time
  3. We dont want to process backend models in blocks. Blocks shouldnt know anything about backend models

This amendments will allow to use serialized data in config.xml file or in another source.

Thanks, Sergii

@mimarcel
Copy link
Contributor Author

mimarcel commented Jan 2, 2017

@sereban in which Magento version will the new fix be included?

@maghamed maghamed self-requested a review February 14, 2017 14:56
@maghamed maghamed self-assigned this Feb 14, 2017
@maghamed
Copy link
Contributor

@mimarcel mimarcel closed this Feb 27, 2017
@mimarcel mimarcel deleted the fix/default_values_are_not_loaded_for_fields_with_backend_model_ArraySearialized branch February 27, 2017 12:35
@mimarcel mimarcel restored the fix/default_values_are_not_loaded_for_fields_with_backend_model_ArraySearialized branch February 27, 2017 12:35
@ghost
Copy link

ghost commented Mar 20, 2017

@maghamed the issue persists in 2.2, as described in Steps to Replicate.
The block of code

         if ($field->hasBackendModel()) {
             $backendModel = $field->getBackendModel();
             $backendModel->setPath($path)
                 ->setValue($data)
                 ->setWebsite($this->getWebsiteCode())
                 ->setStore($this->getStoreCode())
                 ->afterLoad();
             $data = $backendModel->getValue();
         }

is not called when configuration value is taken from config.xml directly.

@viktym
Copy link
Contributor

viktym commented Oct 10, 2017

@marcel-moldovan-vm @mimarcel you should define backend model in config.xml

<?xml version="1.0"?>
<!--
 Copyright © 2009-2016 Vaimo AB. All rights reserved.
 See LICENSE.txt for license details.
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Store:etc/config.xsd">
    <default>
        <export>
            <product_export>
                <!--Remove image-related and bundle-related columns from export.-->
                <remove_export_columns backend_model="Magento\Config\Model\Config\Backend\Serialized"><![CDATA[.....}}]]></remove_export_columns>
            </product_export>
        </export>
    </default>
</config>

Magento\Config\Model\Config\Backend\Serialized should implement \Magento\Framework\App\Config\Data\ProcessorInterface (will be in 2.2.1)

@mimarcel
Copy link
Contributor Author

@viktym setting the backend model in config.xml is a good hint, but it does not fix the issue.

@mimarcel mimarcel reopened this Oct 11, 2017
@magento-engcom-team magento-engcom-team changed the base branch from develop to 2.3-develop October 20, 2017 15:32
@vkublytskyi
Copy link

@mimarcel please check 86d46ce and 6c9a8b2. With these changes serialized config data should be properly displayed at Admin Panel. But to use persisted config value it should be deserialized as scope config will return serialized string.

We can not modify such behavior of serialized config data due to backward compatibility.
To resolve this issue nicely in your extension you may:

  1. Create Config class that will be a facade to config values declared by your module (like \Magento\Braintree\Gateway\Config\Config)
  2. Create own config backend model which will implement Magento\Framework\App\Config\Data\ProcessorInterface and use this class instead of Magento\Config\Model\Config\Backend\Serialized

Please note that now changes available only in 2.2, 2.2-develop, 2.2.2-develop branches and included only in Magento 2.2.1 release.

If this approach suitable for you we will be thankful for a backporting solution to 2.1-develop branch. If you still have an issue with serialized config data please provide us more information what exact issue still present in Magento 2.2.1

@okorshenko okorshenko assigned vkublytskyi and unassigned maghamed Nov 28, 2017
@vkublytskyi
Copy link

@mimarcel I close this PR as the issue should be already fixed in Magento 2.2 and code change from PR will lead to #3019. Feel free to reopen it or create new PR if issue is still not fixed

@vkublytskyi vkublytskyi closed this Dec 1, 2017
@mimarcel mimarcel deleted the fix/default_values_are_not_loaded_for_fields_with_backend_model_ArraySearialized branch January 2, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants