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
Changes from 2 commits
7ce7d64
9141551
85205a8
91e2e74
1af85b3
58aa2fe
b2b0888
ad47caa
37f75f3
c9536f3
664f5b8
31abf2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Postinstall messages may show only once and then dismissed. |
||
{ | ||
// Check if we have any mod_menu module set to All languages | ||
$db = JFactory::getDbo(); | ||
$query = $db->getQuery(true) | ||
->select('COUNT(*)') | ||
->from($db->qn('#__modules')) | ||
->where($db->qn('module') . ' = ' . $db->quote('mod_menu')) | ||
->where($db->qn('published') . ' = 1') | ||
->where($db->qn('client_id') . ' = 1') | ||
->where($db->qn('language') . ' = ' . $db->quote('*')); | ||
$db->setQuery($query); | ||
|
||
$modulesAll = (int) $db->loadResult(); | ||
|
||
// If none, check that we have a mod_menu module for each admin language | ||
if ($modulesAll == 0) | ||
{ | ||
$adminLanguages = count(JLanguageHelper::getInstalledLanguages(1)); | ||
|
||
$db = JFactory::getDbo(); | ||
$query = $db->getQuery(true) | ||
->select('COUNT(*)') | ||
->from($db->qn('#__modules')) | ||
->where($db->qn('module') . ' = ' . $db->quote('mod_menu')) | ||
->where($db->qn('published') . ' = 1') | ||
->where($db->qn('client_id') . ' = 1') | ||
->where($db->qn('language') . ' != ' . $db->quote('*')); | ||
$db->setQuery($query); | ||
|
||
$totalModules = (int) $db->loadResult(); | ||
|
||
if ($totalModules != $adminLanguages) | ||
{ | ||
$msg = JText::_('JMENU_MULTILANG_WARNING'); | ||
JFactory::getApplication()->enqueueMessage($msg, 'warning'); | ||
} | ||
} | ||
} | ||
|
||
return parent::display(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<form> | ||
<fieldset addfieldpath="/administrator/components/com_modules/models/fields" /> | ||
|
||
<field | ||
name="client_id" | ||
type="list" | ||
label="" | ||
filtermode="selector" | ||
layout="default" | ||
onchange="jQuery('#filter_position, #filter_module, #filter_language').val('');this.form.submit();" | ||
> | ||
<option value="0">JSITE</option> | ||
<option value="1">JADMINISTRATOR</option> | ||
</field> | ||
<fields name="filter"> | ||
<field | ||
name="search" | ||
type="text" | ||
label="COM_MODULES_MODULES_FILTER_SEARCH_LABEL" | ||
description="COM_MODULES_MODULES_FILTER_SEARCH_DESC" | ||
hint="JSEARCH_FILTER" | ||
noresults="COM_MODULES_MSG_MANAGE_NO_MODULES" | ||
/> | ||
<field | ||
name="state" | ||
type="status" | ||
label="JSTATUS" | ||
filter="*,-2,0,1" | ||
onchange="this.form.submit();" | ||
> | ||
<option value="">JOPTION_SELECT_PUBLISHED</option> | ||
</field> | ||
<field | ||
name="position" | ||
type="modulesposition" | ||
label="COM_MODULES_FIELD_POSITION_LABEL" | ||
onchange="this.form.submit();" | ||
> | ||
<option value="">COM_MODULES_OPTION_SELECT_POSITION</option> | ||
</field> | ||
<field | ||
name="module" | ||
type="ModulesModule" | ||
label="COM_MODULES_OPTION_SELECT_MODULE" | ||
onchange="this.form.submit();" | ||
> | ||
<option value="">COM_MODULES_OPTION_SELECT_MODULE</option> | ||
</field> | ||
<field | ||
name="access" | ||
type="accesslevel" | ||
label="JOPTION_FILTER_ACCESS" | ||
description="JOPTION_FILTER_ACCESS_DESC" | ||
onchange="this.form.submit();" | ||
> | ||
<option value="">JOPTION_SELECT_ACCESS</option> | ||
</field> | ||
<field | ||
name="language" | ||
type="language" | ||
label="JOPTION_FILTER_LANGUAGE" | ||
description="JOPTION_FILTER_LANGUAGE_DESC" | ||
client="administrator" | ||
onchange="this.form.submit();" | ||
> | ||
<option value="">JOPTION_SELECT_LANGUAGE</option> | ||
<option value="*">JALL</option> | ||
</field> | ||
</fields> | ||
<fields name="list"> | ||
<field | ||
name="fullordering" | ||
type="list" | ||
label="JGLOBAL_SORT_BY" | ||
description="JGLOBAL_SORT_BY" | ||
statuses="*,0,1,-2" | ||
onchange="this.form.submit();" | ||
default="a.position ASC" | ||
> | ||
<option value="">JGLOBAL_SORT_BY</option> | ||
<option value="a.ordering ASC">JGRID_HEADING_ORDERING_ASC</option> | ||
<option value="a.ordering DESC">JGRID_HEADING_ORDERING_DESC</option> | ||
<option value="a.published ASC">JSTATUS_ASC</option> | ||
<option value="a.published DESC">JSTATUS_DESC</option> | ||
<option value="a.title ASC">JGLOBAL_TITLE_ASC</option> | ||
<option value="a.title DESC">JGLOBAL_TITLE_DESC</option> | ||
<option value="a.position ASC">COM_MODULES_HEADING_POSITION_ASC</option> | ||
<option value="a.position DESC">COM_MODULES_HEADING_POSITION_DESC</option> | ||
<option value="name ASC">COM_MODULES_HEADING_MODULE_ASC</option> | ||
<option value="name DESC">COM_MODULES_HEADING_MODULE_DESC</option> | ||
<option value="ag.title ASC">JGRID_HEADING_ACCESS_ASC</option> | ||
<option value="ag.title DESC">JGRID_HEADING_ACCESS_DESC</option> | ||
<option value="a.language ASC">JGRID_HEADING_LANGUAGE_ASC</option> | ||
<option value="a.language DESC">JGRID_HEADING_LANGUAGE_DESC</option> | ||
<option value="a.id ASC">JGRID_HEADING_ID_ASC</option> | ||
<option value="a.id DESC">JGRID_HEADING_ID_DESC</option> | ||
</field> | ||
<field | ||
name="limit" | ||
type="limitbox" | ||
label="COM_MODULES_LIST_LIMIT" | ||
description="JFIELD_PLG_SEARCH_SEARCHLIMIT_DESC" | ||
class="input-mini" | ||
default="25" | ||
onchange="this.form.submit();" | ||
/> | ||
</fields> | ||
</form> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,6 +103,14 @@ | |
label="JFIELD_ORDERING_LABEL" | ||
/> | ||
|
||
<field | ||
name="language" | ||
type="hidden" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better to set this as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do |
||
default="*" | ||
> | ||
<option value="*">JALL</option> | ||
</field> | ||
|
||
<field name="content" type="editor" | ||
buttons="true" | ||
description="COM_MODULES_FIELD_CONTENT_DESC" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -541,6 +541,15 @@ public function getForm($data = array(), $loadData = true) | |
if ($clientId == 1) | ||
{ | ||
$form = $this->loadForm('com_modules.module.admin', 'moduleadmin', array('control' => 'jform', 'load_data' => $loadData), true); | ||
|
||
// Display language field to filter admin custom menus per language | ||
if ($module == 'mod_menu' && JLanguageMultilang::isAdminEnabled()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we allow language filtering for all modules both client? I thing you have done all the work already and we can quickly expand the scope here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was my question above: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh right. That would be a great thing. |
||
{ | ||
$form->setFieldAttribute('language', 'type', 'language'); | ||
$form->setFieldAttribute('language', 'label', 'JFIELD_LANGUAGE_LABEL'); | ||
$form->setFieldAttribute('language', 'description', 'JFIELD_MODULE_LANGUAGE_DESC'); | ||
$form->setFieldAttribute('language', 'client', 'administrator'); | ||
} | ||
} | ||
else | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,10 @@ | |
<th width="10%" class="nowrap hidden-phone"> | ||
<?php echo JHtml::_('searchtools.sort', 'JGRID_HEADING_LANGUAGE', 'l.title', $listDirn, $listOrder); ?> | ||
</th> | ||
<?php elseif ($clientId === 1 && JLanguageMultilang::isAdminEnabled()) : ?> | ||
<th width="10%" class="nowrap hidden-phone"> | ||
<?php echo JHtml::_('searchtools.sort', 'JGRID_HEADING_LANGUAGE', 'a.language', $listDirn, $listOrder); ?> | ||
</th> | ||
<?php endif; ?> | ||
<th width="1%" class="nowrap center hidden-phone"> | ||
<?php echo JHtml::_('searchtools.sort', 'JGRID_HEADING_ID', 'a.id', $listDirn, $listOrder); ?> | ||
|
@@ -187,6 +191,16 @@ | |
<td class="small hidden-phone"> | ||
<?php echo JLayoutHelper::render('joomla.content.language', $item); ?> | ||
</td> | ||
<?php elseif ($clientId === 1 && JLanguageMultilang::isAdminEnabled()) : ?> | ||
<td class="small hidden-phone"> | ||
<?php if ($item->language == ''):?> | ||
<?php echo JText::_('JUNDEFINED'); ?> | ||
<?php elseif ($item->language == '*'):?> | ||
<?php echo JText::alt('JALL', 'language'); ?> | ||
<?php else:?> | ||
<?php echo $this->escape($item->language); ?> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIIRC, we are displaying language flags somewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
<?php endif; ?> | ||
</td> | ||
<?php endif; ?> | ||
<td class="hidden-phone"> | ||
<?php echo (int) $item->id; ?> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,19 +39,17 @@ public function display($tpl = null) | |
$this->total = $this->get('Total'); | ||
$this->filterForm = $this->get('FilterForm'); | ||
$this->activeFilters = $this->get('ActiveFilters'); | ||
$this->clientId = $this->state->get('client_id'); | ||
|
||
// Check for errors. | ||
if (count($errors = $this->get('Errors'))) | ||
{ | ||
throw new Exception(implode("\n", $errors), 500); | ||
} | ||
|
||
// We do not need the Page and the Language filters when filtering by administrator | ||
if ($this->state->get('client_id') == 1) | ||
// We do not need the Language filter when custom admin menus are not translated | ||
if ($this->clientId == 1 && !JLanguageMultilang::isAdminEnabled()) | ||
{ | ||
unset($this->activeFilters['menuitem']); | ||
$this->filterForm->removeField('menuitem', 'filter'); | ||
|
||
unset($this->activeFilters['language']); | ||
$this->filterForm->removeField('language', 'filter'); | ||
} | ||
|
@@ -176,11 +174,26 @@ protected function addToolbar() | |
*/ | ||
protected function getSortFields() | ||
{ | ||
if ($this->getLayout() == 'default') | ||
$this->state = $this->get('State'); | ||
|
||
if ($this->state->get('client_id') == 0) | ||
{ | ||
if ($this->getLayout() == 'default') | ||
{ | ||
return array( | ||
'ordering' => JText::_('JGRID_HEADING_ORDERING'), | ||
'a.published' => JText::_('JSTATUS'), | ||
'a.title' => JText::_('JGLOBAL_TITLE'), | ||
'position' => JText::_('COM_MODULES_HEADING_POSITION'), | ||
'name' => JText::_('COM_MODULES_HEADING_MODULE'), | ||
'pages' => JText::_('COM_MODULES_HEADING_PAGES'), | ||
'a.access' => JText::_('JGRID_HEADING_ACCESS'), | ||
'language_title' => JText::_('JGRID_HEADING_LANGUAGE'), | ||
'a.id' => JText::_('JGRID_HEADING_ID') | ||
); | ||
} | ||
|
||
return array( | ||
'ordering' => JText::_('JGRID_HEADING_ORDERING'), | ||
'a.published' => JText::_('JSTATUS'), | ||
'a.title' => JText::_('JGLOBAL_TITLE'), | ||
'position' => JText::_('COM_MODULES_HEADING_POSITION'), | ||
'name' => JText::_('COM_MODULES_HEADING_MODULE'), | ||
|
@@ -190,15 +203,30 @@ protected function getSortFields() | |
'a.id' => JText::_('JGRID_HEADING_ID') | ||
); | ||
} | ||
else | ||
{ | ||
if ($this->getLayout() == 'default') | ||
{ | ||
return array( | ||
'ordering' => JText::_('JGRID_HEADING_ORDERING'), | ||
'a.published' => JText::_('JSTATUS'), | ||
'a.title' => JText::_('JGLOBAL_TITLE'), | ||
'position' => JText::_('COM_MODULES_HEADING_POSITION'), | ||
'name' => JText::_('COM_MODULES_HEADING_MODULE'), | ||
'a.access' => JText::_('JGRID_HEADING_ACCESS'), | ||
'a.language' => JText::_('JGRID_HEADING_LANGUAGE'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alignment |
||
'a.id' => JText::_('JGRID_HEADING_ID') | ||
); | ||
} | ||
|
||
return array( | ||
'a.title' => JText::_('JGLOBAL_TITLE'), | ||
'position' => JText::_('COM_MODULES_HEADING_POSITION'), | ||
'name' => JText::_('COM_MODULES_HEADING_MODULE'), | ||
'pages' => JText::_('COM_MODULES_HEADING_PAGES'), | ||
'a.access' => JText::_('JGRID_HEADING_ACCESS'), | ||
'language_title' => JText::_('JGRID_HEADING_LANGUAGE'), | ||
'a.id' => JText::_('JGRID_HEADING_ID') | ||
); | ||
return array( | ||
'a.title' => JText::_('JGLOBAL_TITLE'), | ||
'position' => JText::_('COM_MODULES_HEADING_POSITION'), | ||
'name' => JText::_('COM_MODULES_HEADING_MODULE'), | ||
'a.access' => JText::_('JGRID_HEADING_ACCESS'), | ||
'a.language' => JText::_('JGRID_HEADING_LANGUAGE'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alignment |
||
'a.id' => JText::_('JGRID_HEADING_ID') | ||
); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@infograf768 Installed 3 languages - en, fr, de and having module counts - en = 1, fr = 0, de = 2 will pass this test but its not what we want. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do not have any custom admin menu with module set to ALL, then we should change this conditional to
if ($totalModules < $adminLanguages)
it would solve the issue and display the warning I guess.
Can you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@izharaazmi
Can you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, no. This is indeed not enough
We need to improve this by fetching each admin language and make sure we have at least one module assigned to each.
Let's work on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
group by
andIN (<language list>)
should solve the issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@izharaazmi
Without taking these out of the controllers for now, can you propose here a full snippet to replace the existing one? I am not good a groupby stuff ;)
we have now: