search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.5k stars 9.31k forks source link

Cannot restore config value from store view scope using "Use website" checkbox. #26732

Closed brian-ebizmarts closed 3 years ago

brian-ebizmarts commented 4 years ago

Preconditions (*)

  1. Magento >=2.2

Steps to reproduce (*)

  1. Uncheck "Use website" and set specific setting at store view level, save config.
  2. Check "Use website" and save config.

Expected result (*)

  1. delete value from table core_config_data

Actual result (*)

  1. Cannot inherit values from parent scopes.

Additional information

I found in xdebug that the function getFieldPath on the module-config is called with wrong parameters.

Function getFieldPath is declared as below Model/Config.php#L294

https://github.com/magento/magento2/blob/5f3b86ab4bd3e3b94e65429fed33f748d29c1bbe/app/code/Magento/Config/Model/Config.php#L285-L294

In function _processGroup the parameter $extraOldGroups is passed instead of $oldConfig. Config/Model/Config.php#L456

https://github.com/magento/magento2/blob/5f3b86ab4bd3e3b94e65429fed33f748d29c1bbe/app/code/Magento/Config/Model/Config.php#L456

As these two parameters are passed by reference $extraOldGroups overwrite the $oldConfig consequently the checkbox status won't validate and the value is not deleted.

https://github.com/magento/magento2/blob/5f3b86ab4bd3e3b94e65429fed33f748d29c1bbe/app/code/Magento/Config/Model/Config.php#L460-L471

I've noticed inheritance problems between scopes when trying to use multiples sections in system.xml.

magento-deployment-service[bot] commented 4 years ago

Thanks for opening this issue!

m2-assistant[bot] commented 4 years ago

Hi @brian-ebizmarts. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

@brian-ebizmarts do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

magento-deployment-service[bot] commented 4 years ago

Thanks for opening this issue!

gaurav-473 commented 4 years ago

@magento give me 2.4-develop instance

magento-engcom-team commented 4 years ago

Hi @gauravagarwal1001. Thank you for your request. I'm working on Magento 2.4-develop instance for you

magento-engcom-team commented 4 years ago

Hi @gauravagarwal1001, here is your Magento instance. Admin access: https://i-26732-2-4-develop.instances.magento-community.engineering/admin_e554 Login: 5c1a94c8 Password: 01f7f3143cb0 Instance will be terminated in up to 3 hours.

m2-assistant[bot] commented 4 years ago

Hi @shikhamis11. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

shikhamis11 commented 4 years ago

Hi @brian-ebizmarts I checked this issue is not reproducible at 2.4-develop instance .. let me know if it requires any additional steps to reproduce this issue .. also please share video or screenshots if you are able to reproduce this issue .

brian-ebizmarts commented 4 years ago

Hi @shikhamis11, here is a video in Magento EE 2.3.2 instance: https://drive.google.com/file/d/1UUa1HlARvJ83NOH1FgLY_AlcsMZ3XCvG/view?usp=sharing

Let me know if you need anything else.

brian-ebizmarts commented 4 years ago

Hi @shikhamis11, this issue is reproducible with multiples sections in the System.xml:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Magento/Config/etc/system_file.xsd">
    <system>
        <tab id="pos" translate="label" sortOrder="550">
            <label>POS by Ebizmarts</label>
        </tab>
        <section id="pos_general" translate="label" type="text" sortOrder="100" showInDefault="1" showInWebsite="1" showInStore="1">
           .....
        </section>
        <section id="pos_payment" translate="label" type="text" sortOrder="200" showInDefault="1" showInWebsite="1" showInStore="1">
            <class>separator-top</class>
            <label>Payment Methods</label>
            <tab>pos</tab>
            <resource>Ebizmarts_Pos::pos_config</resource>
            .....
        </section>
        <section id="pos_shipping" translate="label" type="text" sortOrder="300" showInDefault="1" showInWebsite="1" showInStore="1">
            <label>Shipping methods</label>
            <tab>pos</tab>
            <resource>Ebizmarts_Pos::pos_config</resource>
            .....
        </section>
        <section id="pos_cron" translate="label" type="text" sortOrder="400" showInDefault="1" showInWebsite="1" showInStore="1">
            <label>Cron</label>
            <tab>pos</tab>
            <resource>Ebizmarts_Pos::cron_pos</resource>
            .....
        </section>
        <section id="pos_backup" translate="label" type="text" sortOrder="500" showInDefault="1" showInWebsite="1" showInStore="1">
            <label>Backups</label>
            <tab>pos</tab>
            <resource>Ebizmarts_Pos::backup_pos</resource>
            .....
        </section>
    </system>
</config>

I've to mention that only in the first section id(pos_general) the checkbox "Use website" works as expected and delete the entry in core_config_data table. In the other sections pos_payment, pos_shipping, pos_cron, pos_backup is not working.

Replacing this line: https://github.com/magento/magento2/blob/5f3b86ab4bd3e3b94e65429fed33f748d29c1bbe/app/code/Magento/Config/Model/Config.php#L456

To pass the parameters as in the function declaration it works correctly: $path = $this->getFieldPath($field, $fieldId, $oldConfig, $extraOldGroups);

m2-assistant[bot] commented 4 years ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Bravo commented 4 years ago

Hello @brian-ebizmarts

Thank you for your report.

It looks like you have third-party extensions installed on your Magento instance. Unfortunately, we have to close this issue. The GitHub issue tracker is intended for Magento Core technical issues only. Please refer to the Community Forums or the Magento Stack Exchange site for advice or general discussion about this issue.

Thank you

mam08ixo commented 4 years ago

The bug is revealed by a third-party extension, yes. But it is still caused by a Magento Core technical issue.

I can replicate it by setting a field's config_path property to a different section. Simplified example:

<section id="foo">
    <group id="b">
        <field id="c" showInDefault="1" showInWebsite="1" showInStore="1">
            <label>Foo</label>
            <config_path>bar/b/c</config_path>
        </field>
    </group>
</section>

Fixing the argument order as detected by @brian-ebizmarts solves the issue.

@shikhamis11 @engcom-Bravo This needs to be reopened.

sdzhepa commented 4 years ago

Based on feedback it seems issue should be reopened and re-verified one more time on the 2.4-develop

m2-assistant[bot] commented 4 years ago

Hi @engcom-Alfa. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Alfa commented 3 years ago

HI @brian-ebizmarts . Thank you for your report! Unfortunately, we are not able to reproduce this issue on fresh 2.4-develop. Everything works as expected. Looks like the problem has already been fixed by commit 005b10d7cd.

@brian-ebizmarts Could you take a look? Thanks!

brian-ebizmarts commented 3 years ago

Hi @engcom-Alfa,

The problem has been solved by the commit 005b10d. Would it be possible to add this fix on Magento 2.3?

Thanks in advance.

engcom-Alfa commented 3 years ago

Hi @brian-ebizmarts .

Unfortunately, there is no such possibility to add this fix on Magento 2.3. According to the DevBlog:

The latest Magento version as of now is 2.4.1 and moving forward we will be focusing on the 2.4 and 2.5 release lines. As a result of such an approach, we will be parking the delivery of bug fixes or features for the 2.3 line and only delivering security fixes to it.

Thanks!

engcom-Alfa commented 3 years ago

Will close this issue as the problem has already been fixed by commit 005b10d.