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

[New Feature] Implementing filtering admin modules per admin languages (including custom menu modules) #17215

Merged
merged 12 commits into from Jul 27, 2017

Conversation

@infograf768
Member

infograf768 commented Jul 23, 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.

infograf768 added some commits Jul 24, 2017

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 24, 2017

Member

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?

Member

infograf768 commented Jul 24, 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?

<?php elseif ($item->language == '*'):?>
<?php echo JText::alt('JALL', 'language'); ?>
<?php else:?>
<?php echo $this->escape($item->language); ?>

This comment has been minimized.

@izharaazmi

izharaazmi Jul 24, 2017

Contributor

IIIRC, we are displaying language flags somewhere.

@izharaazmi

izharaazmi Jul 24, 2017

Contributor

IIIRC, we are displaying language flags somewhere.

This comment has been minimized.

@infograf768

infograf768 Jul 24, 2017

Member

We should NOT display language flags because this is independant of the Content Languages and can be used on monolanguage sites where Content Languages are not necessary.
The language field is fetching administrator languages NOT content languages.

@infograf768

infograf768 Jul 24, 2017

Member

We should NOT display language flags because this is independant of the Content Languages and can be used on monolanguage sites where Content Languages are not necessary.
The language field is fetching administrator languages NOT content languages.

@@ -76,6 +76,47 @@ public function display($cachable = false, $urlparams = false)
// Load the submenu.
ModulesHelper::addSubmenu($this->input->get('view', 'modules'));
// Check custom administrator menu modules
if (JLanguageMultilang::isAdminEnabled())

This comment has been minimized.

@izharaazmi

izharaazmi Jul 24, 2017

Contributor

I think its not good, not enough to test this condition in module manager and menu manager alone. This check should be something that occurs always. Would it be any good if we do this in postinstall messages?

@izharaazmi

izharaazmi Jul 24, 2017

Contributor

I think its not good, not enough to test this condition in module manager and menu manager alone. This check should be something that occurs always. Would it be any good if we do this in postinstall messages?

This comment has been minimized.

@infograf768

infograf768 Jul 24, 2017

Member

Postinstall messages may show only once and then dismissed.
The ideal solution would be in the index.php of the admin template or, better, when we load the template if we want to the proper check all the time.

@infograf768

infograf768 Jul 24, 2017

Member

Postinstall messages may show only once and then dismissed.
The ideal solution would be in the index.php of the admin template or, better, when we load the template if we want to the proper check all the time.

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 24, 2017

Contributor

@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."
Contributor

izharaazmi commented Jul 24, 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."
@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 24, 2017

Member

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

Member

infograf768 commented Jul 24, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 24, 2017

Member

@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?

Member

infograf768 commented Jul 24, 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?

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jul 24, 2017

Contributor

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

Contributor

brianteeman commented Jul 24, 2017

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

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jul 24, 2017

Contributor

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

Contributor

brianteeman commented Jul 24, 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

@RCheesley

This comment has been minimized.

Show comment
Hide comment
@RCheesley

RCheesley Jul 24, 2017

RCheesley commented Jul 24, 2017

@mbabker mbabker added this to the Joomla 3.8.0 milestone Jul 24, 2017

@mbabker mbabker added the New Feature label Jul 24, 2017

@mbabker mbabker added this to Testing/Review in [3.8] General Jul 24, 2017

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 24, 2017

Member

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

Member

infograf768 commented Jul 24, 2017

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

@infograf768 infograf768 changed the title from Implementing Multilang custom admin menus to [New Feature] Implementing filtering admin modules per admin languages (including custom menu modules) Jul 25, 2017

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 25, 2017

Member

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.

Member

infograf768 commented Jul 25, 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.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jul 25, 2017

Contributor

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?

Contributor

brianteeman commented Jul 25, 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?

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jul 25, 2017

Contributor

thanks

Contributor

brianteeman commented on b2b0888 Jul 25, 2017

thanks

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 25, 2017

Member

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.

Member

infograf768 commented Jul 25, 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.

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 26, 2017

Contributor

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

Contributor

izharaazmi commented Jul 26, 2017

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

Merge remote-tracking branch 'upstream/staging' into
multilang_custom_adminmenus

Conflicts:
	libraries/cms/language/multilang.php
	libraries/cms/module/helper.php
@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 26, 2017

Member

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

Member

infograf768 commented Jul 26, 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

@wojsmol

This comment has been minimized.

Show comment
Hide comment
@wojsmol

wojsmol Jul 27, 2017

Contributor

Currently I am working on a system check feature view

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

Contributor

wojsmol commented Jul 27, 2017

Currently I am working on a system check feature view

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 27, 2017

Member

@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

Member

infograf768 commented Jul 27, 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

@wojsmol

This comment has been minimized.

Show comment
Hide comment
@wojsmol

wojsmol Jul 27, 2017

Contributor

@infograf768 I will test tonight (Polish time).

Contributor

wojsmol commented Jul 27, 2017

@infograf768 I will test tonight (Polish time).

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor
Contributor

izharaazmi commented Jul 27, 2017

@AlexRed

This comment has been minimized.

Show comment
Hide comment
@AlexRed

AlexRed Jul 27, 2017

Contributor

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.

Contributor

AlexRed commented Jul 27, 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.

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

@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.
Contributor

izharaazmi commented Jul 27, 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.
@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

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

Contributor

izharaazmi commented Jul 27, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 27, 2017

Member

@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.

Member

infograf768 commented Jul 27, 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.

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

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?

Contributor

izharaazmi commented Jul 27, 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?

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

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

Contributor

izharaazmi commented Jul 27, 2017

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

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

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

Contributor

izharaazmi commented Jul 27, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 27, 2017

Member

@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

Member

infograf768 commented Jul 27, 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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 27, 2017

Member

did not get these, weird.
Looking

Member

infograf768 commented Jul 27, 2017

did not get these, weird.
Looking

infograf768 added some commits Jul 27, 2017

Update adminlanguage.php
since version
@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 27, 2017

Member

Done, if I understood correctly

Member

infograf768 commented Jul 27, 2017

Done, if I understood correctly

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

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.
Contributor

izharaazmi commented Jul 27, 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.
@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

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

Contributor

izharaazmi commented Jul 27, 2017

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

*
* @since 3.8.0
*/
public static function isAdminEnabled()

This comment has been minimized.

@izharaazmi

izharaazmi Jul 27, 2017

Contributor

I think this method does not fit here. This check is specific to modules so this should be not in global CMS\Language\Multilanguage context.

@izharaazmi

izharaazmi Jul 27, 2017

Contributor

I think this method does not fit here. This check is specific to modules so this should be not in global CMS\Language\Multilanguage context.

This comment has been minimized.

@infograf768

infograf768 Jul 27, 2017

Member

already replied

@infograf768

infograf768 Jul 27, 2017

Member

already replied

Show outdated Hide outdated libraries/cms/html/adminlanguage.php
Show outdated Hide outdated libraries/cms/html/adminlanguage.php
@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 27, 2017

Contributor

Is this RTC?

Contributor

izharaazmi commented Jul 27, 2017

Is this RTC?

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.8.0 milestone Jul 27, 2017

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jul 27, 2017

RTC after two successful tests.

franz-wohlkoenig commented Jul 27, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label Jul 27, 2017

@wojsmol

This comment has been minimized.

Show comment
Hide comment
@wojsmol

wojsmol Jul 27, 2017

Contributor

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

Contributor

wojsmol commented Jul 27, 2017

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

@mbabker mbabker added this to the Joomla 3.8.0 milestone Jul 27, 2017

@mbabker mbabker merged commit 1bcfb4a into joomla:staging Jul 27, 2017

5 of 6 checks passed

continuous-integration/jenkins/pr-merge This commit is being built
Details
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@joomla-cms-bot joomla-cms-bot removed the RTC label Jul 27, 2017

@mbabker mbabker moved this from Testing/Review to Completed in [3.8] General Jul 27, 2017

@infograf768 infograf768 deleted the infograf768:multilang_custom_adminmenus branch Jul 28, 2017

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 28, 2017

Member

@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?

Member

infograf768 commented Jul 28, 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?

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 28, 2017

Contributor
Contributor

izharaazmi commented Jul 28, 2017

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 28, 2017

Member

LOL
Let's wait for @mbabker feedback

Member

infograf768 commented Jul 28, 2017

LOL
Let's wait for @mbabker feedback

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Jul 28, 2017

Member

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

Member

mbabker commented Jul 28, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 28, 2017

Member

@izharaazmi
you do or i do?

Member

infograf768 commented Jul 28, 2017

@izharaazmi
you do or i do?

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 28, 2017

Contributor
Contributor

izharaazmi commented Jul 28, 2017

@izharaazmi

This comment has been minimized.

Show comment
Hide comment
@izharaazmi

izharaazmi Jul 28, 2017

Contributor

See #17314

Contributor

izharaazmi commented Jul 28, 2017

See #17314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment