Skip to content

Menu item dependencies (dependsOnModule, dependsOnConfig) are broken #9720

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

Closed
mam08ixo opened this issue May 21, 2017 · 3 comments
Closed

Menu item dependencies (dependsOnModule, dependsOnConfig) are broken #9720

mam08ixo opened this issue May 21, 2017 · 3 comments
Assignees
Labels
bug report Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release

Comments

@mam08ixo
Copy link

Preconditions

  1. Magento 2.2.0-dev is installed (bdb0464 at the time of writing)
  2. Any menu.xml adds menu items with dependency to
  • module that is not installed
  • config value that evaluates to false
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Backend:etc/menu.xsd">
    <menu>
        <add id="Foo_MenuItemDependencies::dependencies"
             title="Menu Item Dependencies"
             translate="title"
             module="Foo_MenuItemDependencies"
             sortOrder="10"
             parent="Magento_Backend::system"
             resource="Foo_MenuItemDependencies::dependencies"
        />

        <add id="Foo_MenuItemDependencies::module_dependency"
             title="Depends On Module"
             module="Foo_MenuItemDependencies"
             sortOrder="10"
             resource="Foo_MenuItemDependencies::dependencies"
             parent="Foo_MenuItemDependencies::dependencies"
             action="foo/module"
             dependsOnModule="Foo_Bar"
        />

        <add id="Foo_MenuItemDependencies::config_dependency"
             title="Depends On Config"
             module="Foo_MenuItemDependencies"
             sortOrder="20"
             resource="Foo_MenuItemDependencies::dependencies"
             parent="Foo_MenuItemDependencies::dependencies"
             action="foo/config"
             dependsOnConfig="foo/config/enabled"
        />
    </menu>
</config>

Steps to reproduce

  1. Log in to admin panel
  2. Open up the top-level menu item that contains the menu items in question

Expected result

The menu items are not visible.

Actual result

The menu items are visible.

Additional information

The menu item handling was refactored to read item data from two different sources:

The issue is that the initial reading from the uncached menu.xml node fails because the array keys used in populateFromArray do not conform to the XSD. That in turn means that various attributes are never taken over to the transformed/cached menu entry and the dependency checks always return true.

The corresponding unit test does not reveal this issue because it makes wrong/incomplete assumptions on the incoming data. So yes, the code in question is covered but not the scenario where menu item data comes in as-is from the menu.xml file.

Another observation I made is that, although the same problem should apply to the sortOrder attribute, sorting actual works. That is because this attribute gets evaluated in another class where the original menu.xml contents are available.

Quick fix would probably be to align the array keys used during transformation with the attribute names in the XSD.

@magento-engcom-team magento-engcom-team added bug report develop Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team self-assigned this Sep 29, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Sep 29, 2017
@magento-engcom-team
Copy link
Contributor

@mam08ixo, thank you for your report.
The issue is already fixed in develop branch
But we will consider to backport the fix to patch releases

@magento-engcom-team magento-engcom-team added 2.2.x Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release labels Sep 29, 2017
@hannassy
Copy link

#mageconf

@okorshenko
Copy link
Contributor

Hi @mam08ixo
The issue has been fixed and delivered in 2.2-develop branch. Will be available with upcoming patch release.
Fixed in #12747 by @hannassy in 2.2-develop branch
Related commits:
1. ddf6c9f

@okorshenko okorshenko added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Dec 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release
Projects
None yet
Development

No branches or pull requests

5 participants