acl.xml definition

[tzyganu]

First of all, thank you for the new method of generating the admin menu from menu.xml files using add tags and parent attributes.
I think it would be easier to do the same for the acl.xml file. Here is an example from the acl.xml from the Magento_GoogleShopping module

        <resource id="Magento_Backend::admin">
            <resource id="Magento_Catalog::catalog">
                <resource id="Magento_GoogleShopping::googleshopping" title="Google Content" sortOrder="20">
                    <resource id="Magento_GoogleShopping::types" title="Attributes" sortOrder="10" />
                    <resource id="Magento_GoogleShopping::items" title="Items" sortOrder="20" />
                </resource>
            </resource>
        </resource>

it would be cleaner and consistent with the menu generation if I was able to do this

<resource id="Magento_GoogleShopping::googleshopping" title="Google Content" sortOrder="20" parent="Magento_Catalog::catalog"> <!-- notice the parent attribute -->
......
 </resource>

If there is a reason (other than lack of time) why is not implemented like this, let me know and I will shut up 😄

[antonkril]

Actually menu.xml is the ugly duckling of magento xml configuraiton because it has imperative format in declarative xml 😃

As for resources, we discussed these formats a lot and came to conclusion that flat trees are easier to write, but harder to read and understand by developer (you have to build inheritance relations yourself instead of just seeing them in your editor), especially when they are merged from all files of same config type. And since before writing some config developer will have to read and understand it, and write speed is only limited by developer's typing speed we decided to prefer explicit trees in config.

[tzyganu]

@antonkril I see your point, but I really like the "ugly duckling". 😄
I think the "parent" approach would be easier in some cases. Let's say you want to add in a module an acl resource on a lower level (3 or 4 or more) within the nodes from a different module. With the current approach you have to duplicate the upper nodes in the module. And if you decide to change anything in the original module I will have to redo my tree all over again.

It was worth a shot. If you don't plan to change anything regarding the acl structure, feel free to brutally close this issue without further notice.

[antonkril]

Modifications of resource tree should be stable. As for moving nodes, it should be implemented through extend/disable properties, so your code will not be broken.

But anyway community should dictate most convenient configuration format, so I will raise this question and report back to you next week.

[tzyganu]

Any news on this? I have a strong feeling that the acl will not get refactored. If this is true, feel free to close this issue.

[antonkril]

I did not report to you. Sorry. It was not refactored because it worked and we had so many higher priority things to do. For now it won't be modified to not introduce a breaking change on code that works. But you could fire a PR if your implementation will be backwards compatible.

[davidalger]

@tzyganu Sorry it didn't make it, but appreciate you taking the time to make the suggestion!