Skip to content

Work on Site components #12261

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.

Sign up for GitHub

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

Closed
wants to merge 19 commits into from
Closed

Work on Site components #12261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2aa44e9
Elvis
frankmayer Oct 2, 2016
cafbc0b
Merge nested positive ifs
frankmayer Oct 2, 2016
330d82c
Replaced some " with ' and proposed HEREDOC for bigger SQL Statements…
frankmayer Oct 2, 2016
22859e7
Removed unnecessary parentheses
frankmayer Oct 2, 2016
66f954e
Removed senseless return;
frankmayer Oct 2, 2016
585bbc8
Inline one-time use variables
frankmayer Oct 2, 2016
be1c3f8
Optimized non-optimal if conditions
frankmayer Oct 2, 2016
7e6df29
Fixed callable call in loop termination condition
frankmayer Oct 2, 2016
2ad8098
Replaced is_null() with ===
frankmayer Oct 2, 2016
f0c7392
Fixed non-optimal regular expressions
frankmayer Oct 2, 2016
d6c7e42
Short syntax for applied operation
frankmayer Oct 2, 2016
b89b4e1
Do not use strlen to check for empty string
frankmayer Oct 2, 2016
656a9c2
Replace strstr() with strpos() for some cases
frankmayer Oct 2, 2016
79c41e3
Replace some substr() with strpos()
frankmayer Oct 2, 2016
7a93567
Merge unset statements
frankmayer Oct 2, 2016
94a33cf
Remove silly assignment :smile:
frankmayer Oct 2, 2016
f575776
Made string comparisons type safe
frankmayer Oct 2, 2016
e11305b
More type safe comparisons
frankmayer Oct 2, 2016
0d5b62c
Codesniffer...
frankmayer Oct 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/com_ajax/ajax.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
$class = 'Mod' . ucfirst($module) . 'Helper';
}

$method = $input->get('method') ? $input->get('method') : 'get';
$method = $input->get('method') ?: 'get';

if (is_file($helperFile))
{
Expand Down
47 changes: 30 additions & 17 deletions components/com_banners/models/banners.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ protected function getListQuery()
$cid = $this->getState('filter.client_id');
$categoryId = $this->getState('filter.category_id');
$keywords = $this->getState('filter.keywords');
$randomise = ($ordering == 'random');
$randomise = ($ordering === 'random');
$nullDate = $db->quote($db->getNullDate());
$nowDate = $db->quote(JFactory::getDate()->toSql());

Expand Down Expand Up @@ -120,7 +120,7 @@ protected function getListQuery()
$query->where($categoryEquals);
}
}
elseif ((is_array($categoryId)) && (count($categoryId) > 0))
elseif (is_array($categoryId) && (count($categoryId) > 0))
{
$categoryId = ArrayHelper::toInteger($categoryId);
$categoryId = implode(',', $categoryId);
Expand All @@ -134,7 +134,7 @@ protected function getListQuery()

if ($tagSearch)
{
if (count($keywords) == 0)
if (count($keywords) === 0)
{
$query->where('0');
}
Expand All @@ -152,31 +152,44 @@ protected function getListQuery()
foreach ($keywords as $keyword)
{
$keyword = trim($keyword);
$condition1 = "a.own_prefix=1 "
. " AND a.metakey_prefix=SUBSTRING(" . $db->quote($keyword) . ",1,LENGTH( a.metakey_prefix)) "
. " OR a.own_prefix=0 "
. " AND cl.own_prefix=1 "
. " AND cl.metakey_prefix=SUBSTRING(" . $db->quote($keyword) . ",1,LENGTH(cl.metakey_prefix)) "
. " OR a.own_prefix=0 "
. " AND cl.own_prefix=0 "
. " AND " . ($prefix == substr($keyword, 0, strlen($prefix)) ? '1' : '0');

$condition2 = "a.metakey REGEXP '[[:<:]]" . $db->escape($keyword) . "[[:>:]]'";
$quotedKeyword = $db->quote($keyword);
$prefixCondition = ($prefix === substr($keyword, 0, strlen($prefix)) ? '1' : '0');

$condition1 = /** @lang SQL */
Copy link
Contributor

@andrepereiradasilva andrepereiradasilva Oct 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this part the fields names should use the API quoteName

Also, i think the API has the ´orWhere´ and ´andWhere´
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query.php#L1429
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query.php#L1448

Copy link
Contributor Author

@frankmayer frankmayer Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's why I wrote the comment below those lines.
What I did in this part, was to just refactor concatenated strings (without the query builder) into HEREDOC strings.
As I noted in those comments, it will not make any sense, if the query builder is used instead and all queries will be migrated to that methodology.

Copy link
Contributor

@andrepereiradasilva andrepereiradasilva Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one relevant information

decide whether or not we are going to enforce the use of quoteName() and quote() in database queries. (Roland)

The PLT has decided that it will enforce the use of quoteName() and quote() methods on any new pull requests involving database queries. This to have consistent database queries that will work best across SQL platforms and with maximum security over time.

See https://volunteers.joomla.org/leadership/production-leadership-team/reports/296-plt-meeting-july-26-2016

<<<SQL
a.own_prefix=1
AND a.metakey_prefix=SUBSTRING($quotedKeyword,1,LENGTH( a.metakey_prefix))
OR a.own_prefix=0
AND cl.own_prefix=1
AND cl.metakey_prefix=SUBSTRING($quotedKeyword,1,LENGTH(cl.metakey_prefix))
OR a.own_prefix=0
AND cl.own_prefix=0
AND $prefixCondition
SQL;

/* Todo: Check with other Joomla members if the above change makes enough sense if the above is not a complete
Statement and there are other conditions following.
The other question is of course, if such queries will in future all be done with the query builder
*/

$escapedKeyword = $db->escape($keyword);
$condition2 = "a.metakey REGEXP '[[:<:]]" . $escapedKeyword . "[[:>:]]'";

if ($cid)
{
$condition2 .= " OR cl.metakey REGEXP '[[:<:]]" . $db->escape($keyword) . "[[:>:]]'";
$condition2 .= " OR cl.metakey REGEXP '[[:<:]]" . $escapedKeyword . "[[:>:]]'";
}

if ($categoryId)
{
$condition2 .= " OR cat.metakey REGEXP '[[:<:]]" . $db->escape($keyword) . "[[:>:]]'";
$condition2 .= " OR cat.metakey REGEXP '[[:<:]]" . $escapedKeyword . "[[:>:]]'";
}

$temp[] = "($condition1) AND ($condition2)";
$temp = "($condition1) AND ($condition2)";
}

$query->where('(' . implode(' OR ', $temp) . ')');
$query->where($temp);
}
}

Expand Down Expand Up @@ -288,7 +301,7 @@ public function impress()
JError::raiseError(500, $e->getMessage());
}

if ($db->getAffectedRows() == 0)
if ($db->getAffectedRows() === 0)
{
// Insert new count
$query->clear();
Expand Down
2 changes: 1 addition & 1 deletion components/com_config/controller/config/display.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function execute()

if (class_exists($viewClass))
{
if ($viewName != 'close')
if ($viewName !== 'close')
{
$model = new $modelClass;

Expand Down
2 changes: 1 addition & 1 deletion components/com_config/controller/display.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function execute()
$view->document = $document;

// Reply for service requests
if ($viewFormat == 'json')
if ($viewFormat === 'json')
{
return $view->render();
}
Expand Down
6 changes: 3 additions & 3 deletions components/com_config/controller/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function parseController($app)
}
}

if (empty($tasks[0]) || $tasks[0] == 'Config')
if (empty($tasks[0]) || $tasks[0] === 'Config')
{
$location = 'Config';
}
Expand Down Expand Up @@ -90,15 +90,15 @@ public function parseController($app)
// Some special handling for com_config administrator
$option = $app->input->get('option');

if ($app->isAdmin() && $option == 'com_config')
if ($option === 'com_config' && $app->isAdmin())
{
$component = $app->input->get('component');

if (!empty($component))
{
$view = 'Component';
}
elseif ($option == 'com_config')
elseif ($option === 'com_config')
{
$view = 'Application';
}
Expand Down
2 changes: 1 addition & 1 deletion components/com_config/controller/templates/display.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function execute()

if (class_exists($viewClass))
{
if ($viewName != 'close')
if ($viewName !== 'close')
{
$model = new $modelClass;

Expand Down
4 changes: 2 additions & 2 deletions components/com_config/model/cms.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ protected function cleanCache($group = null, $client_id = 0)
$dispatcher = JEventDispatcher::getInstance();

$options = array(
'defaultgroup' => ($group) ? $group : (isset($this->option) ? $this->option : JFactory::getApplication()->input->get('option')),
'cachebase' => ($client_id) ? JPATH_ADMINISTRATOR . '/cache' : $conf->get('cache_path', JPATH_SITE . '/cache'));
'defaultgroup' => $group ?: (isset($this->option) ? $this->option : JFactory::getApplication()->input->get('option')),
'cachebase' => $client_id ? JPATH_ADMINISTRATOR . '/cache' : $conf->get('cache_path', JPATH_SITE . '/cache'));

$cache = JCache::getInstance('callback', $options);
$cache->clean();
Expand Down
7 changes: 2 additions & 5 deletions components/com_config/model/templates.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,10 @@ protected function preprocessForm(JForm $form, $data, $group = 'content')
$formFile = JPath::clean(JPATH_BASE . '/templates/' . $template . '/templateDetails.xml');
}

if (file_exists($formFile))
// Get the template form.
if (file_exists($formFile) && !$form->loadFile($formFile, false, '//config'))
{
// Get the template form.
if (!$form->loadFile($formFile, false, '//config'))
{
throw new Exception(JText::_('JERROR_LOADFILE_FAILED'));
}
}

// Attempt to load the xml file.
Expand Down
3 changes: 1 addition & 2 deletions components/com_config/view/cms/html.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ public function loadTemplate($tpl = null)
if ($this->_template != false)
{
// Unset so as not to introduce into template scope
unset($tpl);
unset($file);
unset($tpl, $file);

// Never allow a 'this' property
if (isset($this->this))
Expand Down
2 changes: 1 addition & 1 deletion components/com_config/view/modules/tmpl/default.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
JHtml::_('behavior.combobox');
JHtml::_('formbehavior.chosen', 'select');

$hasContent = empty($this->item['module']) || $this->item['module'] == 'custom' || $this->item['module'] == 'mod_custom';
$hasContent = empty($this->item['module']) || $this->item['module'] === 'custom' || $this->item['module'] === 'mod_custom';

// If multi-language site, make language read-only
if (JLanguageMultilang::isEnabled())
Expand Down
2 changes: 1 addition & 1 deletion components/com_config/view/modules/tmpl/default_options.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<div class="controls">
<?php
// If multi-language site, make menu-type selection read-only
if (JLanguageMultilang::isEnabled() && $this->item['module'] == 'mod_menu' && $field->getAttribute('name') == 'menutype')
if (JLanguageMultilang::isEnabled() && $this->item['module'] === 'mod_menu' && $field->getAttribute('name') === 'menutype')
{
$field->__set('readonly', true);
}
Expand Down
2 changes: 1 addition & 1 deletion components/com_contact/controllers/contact.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function submit()
// Check for a valid session cookie
if ($params->get('validate_session', 0))
{
if (JFactory::getSession()->getState() != 'active')
if (JFactory::getSession()->getState() !== 'active')
{
JError::raiseWarning(403, JText::_('JLIB_ENVIRONMENT_SESSION_INVALID'));

Expand Down
6 changes: 3 additions & 3 deletions components/com_contact/helpers/association.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ abstract class ContactHelperAssociation extends CategoryHelperAssociation
public static function getAssociations($id = 0, $view = null)
{
$jinput = JFactory::getApplication()->input;
$view = is_null($view) ? $jinput->get('view') : $view;
$view = $view === null ? $jinput->get('view') : $view;
$id = empty($id) ? $jinput->getInt('id') : $id;

if ($view == 'contact')
if ($view === 'contact')
{
if ($id)
{
Expand All @@ -53,7 +53,7 @@ public static function getAssociations($id = 0, $view = null)
}
}

if ($view == 'category' || $view == 'categories')
if ($view === 'category' || $view === 'categories')
{
return self::getCategoryAssociations($id, 'com_contact');
}
Expand Down
12 changes: 6 additions & 6 deletions components/com_contact/helpers/route.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static function getContactRoute($id, $catid, $language = 0)
}
}

if ($language && $language != "*" && JLanguageMultilang::isEnabled())
if ($language && $language !== '*' && JLanguageMultilang::isEnabled())
{
$link .= '&lang=' . $language;
$needles['language'] = $language;
Expand Down Expand Up @@ -106,7 +106,7 @@ public static function getCategoryRoute($catid, $language = 0)
$needles['category'] = $catids;
$needles['categories'] = $catids;

if ($language && $language != "*" && JLanguageMultilang::isEnabled())
if ($language && $language !== '*' && JLanguageMultilang::isEnabled())
{
$link .= '&lang=' . $language;
$needles['language'] = $language;
Expand Down Expand Up @@ -145,7 +145,7 @@ protected static function _findItem($needles = null)
$attributes = array('component_id');
$values = array($component->id);

if ($language != '*')
if ($language !== '*')
{
$attributes[] = 'language';
$values[] = array($needles['language'], '*');
Expand All @@ -155,7 +155,7 @@ protected static function _findItem($needles = null)

foreach ($items as $item)
{
if (isset($item->query) && isset($item->query['view']))
if (isset($item->query, $item->query['view']))
{
$view = $item->query['view'];

Expand All @@ -171,7 +171,7 @@ protected static function _findItem($needles = null)
* language != * can override existing entries
* language == * cannot override existing entries
*/
if (!isset(self::$lookup[$language][$view][$item->query['id']]) || $item->language != '*')
if ($item->language !== '*' || !isset(self::$lookup[$language][$view][$item->query['id']]))
{
self::$lookup[$language][$view][$item->query['id']] = $item->id;
}
Expand Down Expand Up @@ -199,7 +199,7 @@ protected static function _findItem($needles = null)

// Check if the active menuitem matches the requested language
$active = $menus->getActive();
if ($active && ($language == '*' || in_array($active->language, array('*', $language)) || !JLanguageMultilang::isEnabled()))
if ($active && ($language === '*' || in_array($active->language, array('*', $language)) || !JLanguageMultilang::isEnabled()))
{
return $active->id;
}
Expand Down
6 changes: 3 additions & 3 deletions components/com_contact/models/category.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ protected function getListQuery()

// Join over the users for the author and modified_by names.
$query->select("CASE WHEN a.created_by_alias > ' ' THEN a.created_by_alias ELSE ua.name END AS author")
->select("ua.email AS author_email")
->select('ua.email AS author_email')

->join('LEFT', '#__users AS ua ON ua.id = a.created_by')
->join('LEFT', '#__users AS uam ON uam.id = a.modified_by');
Expand Down Expand Up @@ -201,7 +201,7 @@ protected function getListQuery()
}

// Set sortname ordering if selected
if ($this->getState('list.ordering') == 'sortname')
if ($this->getState('list.ordering') === 'sortname')
{
$query->order($db->escape('a.sortname1') . ' ' . $db->escape($this->getState('list.direction', 'ASC')))
->order($db->escape('a.sortname2') . ' ' . $db->escape($this->getState('list.direction', 'ASC')))
Expand Down Expand Up @@ -235,7 +235,7 @@ protected function populateState($ordering = null, $direction = null)
// List state information
$format = $app->input->getWord('format');

if ($format == 'feed')
if ($format === 'feed')
{
$limit = $app->get('feed_limit');
}
Expand Down
2 changes: 1 addition & 1 deletion components/com_contact/models/contact.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public function &getItem($pk = null)
}

// Check for published state if filter set.
if (((is_numeric($published)) || (is_numeric($archived))) && (($data->published != $published) && ($data->published != $archived)))
if ((is_numeric($published) || is_numeric($archived)) && (($data->published != $published) && ($data->published != $archived)))
{
JError::raiseError(404, JText::_('COM_CONTACT_ERROR_CONTACT_NOT_FOUND'));
}
Expand Down
Loading