? ? ? ? Pending

User tests: Successful: Unsuccessful:

avatar infograf768
infograf768
23 Jul 2017

Pull Request for Issue #17155

Summary of Changes

Allow administrator menu modules to be tagged and displayed per available administrator languages.
Added Warning when no menu modules are tagged to All languages and some administrator menu modules are missing for an administrator language in order to not suddenly lose all admin menu modules when switching backend language.
This warning will only display when the new Menus parameter option is set to yes.
Added a new adminfilter file for modules.

Testing Instructions

Patch staging.
Set the new parameter in Menus Options.

screen shot 2017-07-21 at 15 05 30

I have created 3 custom admin menus. French, English and one for ALL

screen shot 2017-07-21 at 15 07 12

I have assigned each of these menus to an admin menu module and tagged that module to an administrator language.

screen shot 2017-07-23 at 20 20 44

Tagging to a language is only possible for admin menu modules.
screen shot 2017-07-23 at 20 22 44

screen shot 2017-07-23 at 20 24 21

If there are no Published menu module tagged to ALL languages and some menu modules tagged to an administrator language are missing, we get a warning.
On my site I have 6 administrator languages and only 2 have a custom admin menu (French and English).

In the modules manager page
screen shot 2017-07-23 at 20 26 45

and also in the menus/menu items manager pages
screen shot 2017-07-23 at 20 28 12

As formerly, if any of the custom admin menus has a compulsory item missing , we get the Warning.
Both Warnings may be combined.
screen shot 2017-07-23 at 20 30 33

If the Menus Options Filtering parameter is set to No, the language field and columns are not displayed in the modules manager.

Notes

To be sure I get the Warning, I added the code to both menus and modules controller display() method. A better solution is welcome.

The purpose of this patch is to deal ONLY with administrator menu modules.
It can easily be upgraded to deal with all admin modules by modifying the conditional which is now set as a parameter in com_menus.
If set as a Global parameter, or Modules parameter, or system plugin in the future (or right now) the method JLanguageMultilang::isAdminEnabled() will just have to be modified.

Documentation Changes Required

Complete custom admin menus docs.

@izharaazmi
@RCheesley
All improvements welcome.

avatar joomla-cms-bot joomla-cms-bot - change - 23 Jul 2017
Category Administration com_menus com_modules Language & Strings Libraries
avatar infograf768 infograf768 - open - 23 Jul 2017
avatar infograf768 infograf768 - change - 23 Jul 2017
Status New Pending
avatar infograf768 infograf768 - change - 24 Jul 2017
Labels Added: ? ?
avatar infograf768
infograf768 - comment - 24 Jul 2017

After some thought, I think we should create the parameter in the Modules Options and not the Menus options. In that case, we may not limit it to mod_menu and make it a totally new feature.

What do you think?

avatar izharaazmi
izharaazmi - comment - 24 Jul 2017

@infograf768 This should work for the check part.

if (JLanguageMultilang::isAdminEnabled())
{
	$languages = JLanguageHelper::getInstalledLanguages(1, true);
	$langCodes = array();

	foreach ($languages as $language)
	{
		$langCodes[$language->metadata['tag']] = $language->name;
	}

	$db    = JFactory::getDbo();
	$query = $db->getQuery(true);

	$query->select($db->qn('m.language'))
		->from($db->qn('#__modules', 'm'))
		->where($db->qn('m.module') . ' = ' . $db->quote('mod_menu'))
		->where($db->qn('m.published') . ' = 1')
		->where($db->qn('m.client_id') . ' = 1')
		->group($db->qn('m.language'));

	$mLanguages = $db->setQuery($query)->loadColumn();

	// Check if we have a mod_menu module set to All languages or a mod_menu module for each admin language.
	if (!in_array('*', $mLanguages) && count($langMissing = array_diff(array_keys($langCodes), $mLanguages)))
	{
		$app         = JFactory::getApplication();
		$langMissing = array_intersect_key($langCodes, array_flip($langMissing));

		$app->enqueueMessage(JText::sprintf('JMENU_MULTILANG_WARNING_MISSING_MODULES', implode(', ', $langMissing)), 'warning');
	}
}
JMENU_MULTILANG_WARNING_MISSING_MODULES="The administrator menu module for <strong>%s</strong> does not exist. Create a custom administrator menu and module for each administrator language or publish a menu module set to All languages."
avatar infograf768
infograf768 - comment - 24 Jul 2017

@izharaazmi
Thanks. Committed after just changing name to nativeName.
This gives:
screen shot 2017-07-24 at 11 37 01

avatar infograf768
infograf768 - comment - 24 Jul 2017

@mbabker @Bakual

Shall we set the parameter in the Modules Options instead of Menus Options?
In that case, shall we implement language filtering for all admin modules instead of only mod_menu?

avatar brianteeman
brianteeman - comment - 24 Jul 2017

Language filtering for admin modules would be good. This has come up before as a feature request

avatar brianteeman
brianteeman - comment - 24 Jul 2017

Just wondering if we even need an option. Would I r not be enough to check if the admin menu exists for the selected admin language

avatar RCheesley
RCheesley - comment - 24 Jul 2017

I'd say it would be the next logical step to be able to deliver different
language versions of any module on the administrator interface if we can
get the menu modules working, so if both can be accomplished at the same
time it would be great!

On 24 July 2017 at 15:48, Brian Teeman notifications@github.com wrote:

Just wondering if we even need an option. Would I r not be enough to check
if the admin menu exists for the selected admin language


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17215 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACy3oaSON0vyZ6Q8HBsP_jh8CG8mwpY7ks5sRK7CgaJpZM4OgiMn
.

--

Be like me, be Carbon free - don't print this and save a tree
IMPORTANT: The contents of this email and any attachments are confidential.
They are intended for the named recipient(s) only. If you have received
this email by mistake, please notify the sender immediately and do not
disclose the contents to anyone or make copies thereof.

avatar infograf768
infograf768 - comment - 24 Jul 2017

I will modify the PR to get the multilingual option for all modules.
Tomorrow or after tomorrow. Target 3.8 anyway . 😄

avatar joomla-cms-bot joomla-cms-bot - change - 25 Jul 2017
Category Administration com_menus com_modules Language & Strings Libraries Administration com_menus com_modules Language & Strings Layout Libraries
avatar infograf768 infograf768 - change - 25 Jul 2017
Labels Added: ?
avatar infograf768 infograf768 - change - 25 Jul 2017
Title
Implementing Multilang custom admin menus
[New Feature] Implementing filtering admin modules per admin languages (including custom menu modules)
avatar infograf768 infograf768 - edited - 25 Jul 2017
avatar infograf768
infograf768 - comment - 25 Jul 2017

Ok, folks. We should get there.
To implement filtering one has now to set the option in the Modules Options instead of menus
I have preferred to keep the Option to let users chose.

screen shot 2017-07-25 at 15 11 06

When the site has only got one admin language, no filter or language field will be displayed.
Batch language for admin modules will now be done depending on admin languages and not content languages.

Please test with one language (en-GB) and 2 or more. Filter enabled and disabled.
Test with mod_menus as explained above and also with other admin modules (custom modules or feed modules are great to test with)

When fully tested, and if no issue please set test as OK on issues.joomla.org.
Michael left us until Friday to get this RTC and merged into 3.8.0

@izharaazmi
@RCheesley Please use patch tester.

avatar brianteeman
brianteeman - comment - 25 Jul 2017

I still dont see the need for the option - to me its enough to check if a menu module exists in the admin language being used. We have too many options. Would you ever have multiple menu modules in multiple languages if you didnt want this option?

avatar infograf768
infograf768 - comment - 25 Jul 2017

I would advise any one commenting to first test the PR. It will be more useful.

Specially as we have a PR in 4.0 taking off the language field/columns, etc, when language filter is disabled, it is rather weird to read that a simple Option allowing or not filtering by admin language in the Modules Option could be a problem for backend modules.

avatar izharaazmi
izharaazmi - comment - 26 Jul 2017

@infograf768 Please resolve the conflicts and I'll test this.

avatar infograf768
infograf768 - comment - 26 Jul 2017

Normally, conflicts are solved and both files have been modified by adding \ when necessary.
BUT we have an error when editing modules here:

Catchable fatal error: Argument 1 passed to Joomla\CMS\Form\Field\ChromeStyleField::setup() must be an instance of Joomla\CMS\Form\Field\SimpleXMLElement, instance of SimpleXMLElement given, called in /Applications/MAMP/htdocs/testnew/infograf768/libraries/src/Joomla/CMS/Form/Form.php on line 1925 and defined in /Applications/MAMP/htdocs/testnew/infograf768/libraries/src/Joomla/CMS/Form/Field/ChromestyleField.php on line 98

which means the PR cant be tested until this is solved as my PR does not touch these files.

@mbabker

avatar izharaazmi
izharaazmi - comment - 26 Jul 2017

That is caused by namespacing classes without updating namespace references
used inside those files. Many files are affected by this. I am trying to
find them but it isn't easy at this stage.

avatar infograf768
infograf768 - comment - 26 Jul 2017

that error has now been fixed in staging. i guess you can now test

avatar infograf768
infograf768 - comment - 26 Jul 2017

@izharaazmi
I have no idea why, but a custom admin menu tagged to a language (not All) is not any more sending a warning if some compulsory menu items are missing. That was working before the 3.8.0 merge.

avatar infograf768
infograf768 - comment - 26 Jul 2017

To be more precise, it justs show the warning when the mod_menu loaded is tagged to the same language as the UI.
If I am in en-GB, it will not show the warning for an admin menu module tagged to fr-FR.

avatar izharaazmi
izharaazmi - comment - 26 Jul 2017

This should not be true as I wrote the code, not sure if it was modified
later. Will check it in an hour or so.

avatar infograf768
infograf768 - comment - 26 Jul 2017

@izharaazmi
I think I found the issue.
As we only load mod_menus tagged to ALL and the language in use,

$menu = new JAdminCssMenu;
$menu->load($params, $enabled);

the check is only done on loaded custom menus.

avatar infograf768
infograf768 - comment - 27 Jul 2017

@izharaazmi
Any way to still get the check to be done on all mod_menus, even if not loaded?

avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

Currently I am working on a system check feature view in com_admin which would allow plugins as well as other extensions to inject their check methods. That would be helpful in this case as well. But seeing the feature freeze tomorrow, it looks like I could not make it into 3.8.0

Now at current scenario, we don't need the check for non loaded menu. That check offers the active user a recovery_mode option and that would be sufficient for the situation where the loaded menu does not have the required items.

avatar wojsmol
wojsmol - comment - 27 Jul 2017

Currently I am working on a system check feature view

@izharaazmi Happy to read that. Ping me if you nead any help.

avatar infograf768
infograf768 - comment - 27 Jul 2017

@izharaazmi
In that case, and if your tests are fine, can you mark this PR tested OK on issues.joomla.org?

@wojsmol
Please also test so that we get this in 3.8.0

avatar wojsmol
wojsmol - comment - 27 Jul 2017

@infograf768 I will test tonight (Polish time).

avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

Sure I'll test this evening :)

avatar AlexRed AlexRed - test_item - 27 Jul 2017 - Tested successfully
avatar AlexRed
AlexRed - comment - 27 Jul 2017

I have tested this item successfully on c9536f3

Test ok for me


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

avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

@infograf768 I have two things in my mind:

  1. If frontend language filtering is enabled by default unconditionally why would we want a setting for administrator modules?
  2. If we really should have this option then it should be called JModuleHelper::isAdminMultilang() instead of JLanguageMultilang::isAdminEnabled() because the latter sound like If multilang is enabled in backend, which is not intended here.
avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

@infograf768 After you put your opinion about the above I will submit my test result.

avatar infograf768
infograf768 - comment - 27 Jul 2017

@izharaazmi
concerning 1. there is already a pr for 4.0 where all language fields/columns/ etc. would not be proposed/displayed if the language filter plugin is disabled, therefore also for frontend modules.

concerning 2. I just thought it would be simpler and more consistent to add the check in the same file as frontend. This as the JLanguageMulltilang class already exists. We must not forget that this new feature will also work for monolingual sites where multiple languages are installed (choice by backend user). Your idea would also work. Just a matter of choice I guess. I may have though that the new method may find a use (once modified) for other stuff than modules.

avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

the new method may find a use (once modified) for other stuff than modules.

Correct. Therefore this check should not be bound to com_modules params anyway, I guess?

avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

@infograf768 I'd rather try getting this into 3.8 first, and we can discuss these later 😄 👍

avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

@infograf768 My tests are ok, just added few comments please reply to them.

avatar infograf768
infograf768 - comment - 27 Jul 2017

@izharaazmi

the new method may find a use (once modified) for other stuff than modules.

Correct. Therefore this check should not be bound to com_modules params anyway, I guess?

"Should not be bound ONLY to..." .
This is why I wrote above "once modified":

I may have though that the new method may find a use (once modified) for other stuff than modules.

The code for example for specific enabled plugins or user groups or whatever would be "added" to the one checking the admin modules params.

if "something" || "somethingelse" || "anothercondition" || "the modules params"
{
     $enabled = true;
}

if judged necessary, we may then add a new method in JModuleHelper and use it in JLanguageMultilang::isAdminEnabled() to keep things in a handy place.

We could do that right now, but we are in a hurry to get this in 3.8.0
😃

If this is satisfying, please add test in issues.joomla.org

avatar infograf768
infograf768 - comment - 27 Jul 2017

did not get these, weird.
Looking

avatar infograf768
infograf768 - comment - 27 Jul 2017

Done, if I understood correctly

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

I have tested this item successfully on 31abf2d

Filters working for backend modules and new configuration parameter enables/disables filtering feature correctly.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17215.
avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

I guess this counts as 2 tests as the changes were not affecting code. Right?

avatar infograf768 infograf768 - alter_testresult - 27 Jul 2017 - AlexRed: Tested successfully
avatar izharaazmi
izharaazmi - comment - 27 Jul 2017

Is this RTC?

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

RTC after two successful tests.

avatar wojsmol
wojsmol - comment - 27 Jul 2017

@infograf768 I see RTC but if enything is neadet plese ping me.

avatar mbabker mbabker - change - 27 Jul 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-07-27 22:52:27
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 27 Jul 2017
avatar mbabker mbabker - merge - 27 Jul 2017
avatar infograf768
infograf768 - comment - 28 Jul 2017

@izharaazmi @mbabker

After a good night sleep, I think @izharaazmi was right. 😃
If an admin language filtering may be necessary in the future for other stuff than the modules, we better move the conditional method into JModuleHelper.

This would mean adding a similar method for each element concerned in back-end with the possibility to enable or not such admin lang filtering by checking a specific parameter.

Admins will then be able to choose, element by element, what they want to be filtered or not.

Now that this PR is merged, it is quite easy to make a new PR changing that aspect of it.
It is not as urgent as the new feature.
Would you like me to prepare that now?

avatar izharaazmi
izharaazmi - comment - 28 Jul 2017

I have a patch from last evening ready! :)

avatar infograf768
infograf768 - comment - 28 Jul 2017

LOL
Let's wait for @mbabker feedback

avatar mbabker
mbabker - comment - 28 Jul 2017

You guys will use this more often than me, so whatever makes sense to you roll with it.

avatar infograf768
infograf768 - comment - 28 Jul 2017

@izharaazmi
you do or i do?

avatar izharaazmi
izharaazmi - comment - 28 Jul 2017

@infograf768 I already did the patch which was aimed against your branch
last evening. I can do that against staging now.

avatar izharaazmi
izharaazmi - comment - 28 Jul 2017

See #17314

Add a Comment

Login with GitHub to post a comment