User tests: Successful: Unsuccessful:
Pull Request for Issue #32456 .
The changes introduced hide menu items for disabled components from the cpanel.
The menu item "Content Security Policy" is still available in the cpanel.
The menu item "Content Security Policy" is not available in the cpanel anymore.
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Administration |
Labels |
Added:
?
|
I don't know if this is the right approach or not, but it contains a few mistakes:
joomla-cms/administrator/modules/mod_submenu/src/Menu/Menu.php
Lines 44 to 46 in 47b2cff
As far as I understand the purpose of this PR, only extensions of type "component" should be fetched with that query, and they should be restricted also to the right client_id here (1 for admin).
For lucky circumstances, the "name" and "element" values are the same for (core) components, but that might not always be granted, so it can happen we compare apples and pears here. Therefore at least from a formal point of view the above check is wrong.
P.S. to my above comment's items 1 and 2: For the reasons stated there, I would expect the query to be:
$query->select($db->quoteName('e.element'))
->from($db->quoteName('#__extensions', 'e'))
->where($db->quoteName('e.type') . ' = ' . $db->quote('component'))
->where($db->quoteName('e.client_id') . ' = 1')
->where($db->quoteName('e.enabled') . ' = 0');
And later below it should be:
$names[] = $item->element;
@richard67 Thank you so much for the comments! I updated the PR. Let's see what @SharkyKZ's feedback is.
This would be my first contribution to joomla despite having used it for the past couple of years, so I would not be surprised if I misunderstood something and my approach is totally wrong.
The idea behind my approach is basically:
Since the menu is rendered from the preset-xml files (in the error case) my only information are the element
and type
of the menu item, so I know that its a component of "type" com_csp. To link that to the state of the component (enabled, disabled) I have to query the database beforehand and pass that information to the precheck function, which already appears to be hiding menu items under certain conditions.
Agreed! Did not know about the ComponentHelper
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-03-02 12:32:32 |
Closed_By | ⇒ | sawmurai |
Yep, I will close it :)
@SharkyKZ could you please elaborate why you gave a thumb down? Probably there is room for improvement in this PR based on you feedback. Thanks.