? Pending

User tests: Successful: Unsuccessful:

avatar n3t
n3t
26 Sep 2017

Pull Request for Issue #18046 .

Summary of Changes

Added check to JAdminMenuCSS for disabled or not installed components.

Testing Instructions

Disable com_fields, and see the menu. Links to fields components are present, leading to 404 page.
Apply this patch, and see the menu again.

avatar joomla-cms-bot joomla-cms-bot - change - 26 Sep 2017
Category Modules Administration
avatar n3t n3t - open - 26 Sep 2017
avatar n3t n3t - change - 26 Sep 2017
Status New Pending
avatar n3t n3t - change - 26 Sep 2017
Title
Admin menu
Joomla 3.8.0: Disabled com_fields displayed in admin menu
avatar n3t n3t - edited - 26 Sep 2017
avatar waader waader - test_item - 26 Sep 2017 - Tested successfully
avatar waader waader - test_item - 26 Sep 2017 - Tested successfully
avatar waader
waader - comment - 26 Sep 2017

I have tested this item successfully on 0e0bdbf

Thanks n3t, works great!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18134.

avatar wojsmol
wojsmol - comment - 27 Sep 2017

@izharaazmi Please test this if you have a moment.

avatar izharaazmi izharaazmi - test_item - 27 Sep 2017 - Tested successfully
avatar izharaazmi
izharaazmi - comment - 27 Sep 2017

I have tested this item successfully on 0e0bdbf


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18134.

avatar franz-wohlkoenig franz-wohlkoenig - change - 27 Sep 2017
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 27 Sep 2017

RTC after two successful tests.

avatar mbabker mbabker - change - 30 Sep 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-09-30 14:44:26
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 30 Sep 2017
avatar mbabker mbabker - merge - 30 Sep 2017
avatar infograf768
infograf768 - comment - 7 Oct 2017

The problem with this code is that it increases drastically the queries.
Any better way? May be by specifically targeting com_fields ?

avatar izharaazmi
izharaazmi - comment - 7 Oct 2017

@infograf768 What do you mean?

avatar alikon
alikon - comment - 7 Oct 2017
avatar infograf768
infograf768 - comment - 7 Oct 2017

I guess we could have done

			// Exclude item if com_fields component is not installed or disabled
			if ($item->element == 'com_fields' && (!JComponentHelper::isInstalled($item->element) || !JComponentHelper::isEnabled($item->element)))
			{
				continue;
			}
avatar izharaazmi
izharaazmi - comment - 7 Oct 2017

Your suggestion only checks for com_fields, however the current code checks for all components and excludes a menu item if the related component is disabled/not installed.

avatar infograf768
infograf768 - comment - 7 Oct 2017

@izharaazmi
I tested disabling some components like com_associations, com_banners, etc., and it works fine with that code.
I suggest you test yourself.
com_fields is a specific case.

avatar izharaazmi
izharaazmi - comment - 7 Oct 2017

Ok, I'll test it

avatar izharaazmi
izharaazmi - comment - 9 Oct 2017

Okay. I tested it. The checks for items under component container are done internally so they are not needed, fine. For core (protected) components they cannot be disabled from UI, fine.

If we do have any other situation with any components we need this test for them. One such is com_fields. Since a custom menu can have any components link anywhere they will need this tests I think.

@infograf768 I think we must work to minimise the duplicate queries but restricting to com_fields does not seem right. What do you suggest?

avatar infograf768
infograf768 - comment - 9 Oct 2017

@izharaazmi
I suggest we discuss on #18264.
See my comment there #18264 (comment)
as I think that we may not really need to check if a comp is installed.

Add a Comment

Login with GitHub to post a comment