From 9ced47ad539061df83d253d3f02de2a25f01ca45 Mon Sep 17 00:00:00 2001 From: Michael Babker Date: Fri, 11 Mar 2016 11:36:53 -0500 Subject: [PATCH] Misc tweaks and review --- .../Controller/DisplayController.php | 4 +- .../Controller/StartfetchController.php | 1 - .../PatchTester/Model/PullModel.php | 2 +- .../PatchTester/Model/PullsModel.php | 63 ++++++------------- .../PatchTester/Table/PullsTable.php | 2 +- .../PatchTester/Table/TestsTable.php | 2 +- .../PatchTester/View/DefaultHtmlView.php | 4 +- .../PatchTester/View/Pulls/tmpl/default.php | 3 +- .../com_patchtester/patchtester.php | 2 +- .../html/com_patchtester/pulls/default.php | 5 +- 10 files changed, 29 insertions(+), 59 deletions(-) diff --git a/administrator/components/com_patchtester/PatchTester/Controller/DisplayController.php b/administrator/components/com_patchtester/PatchTester/Controller/DisplayController.php index a2ce504..b95a01e 100644 --- a/administrator/components/com_patchtester/PatchTester/Controller/DisplayController.php +++ b/administrator/components/com_patchtester/PatchTester/Controller/DisplayController.php @@ -69,7 +69,7 @@ class DisplayController extends AbstractController if (!class_exists($viewClass)) { throw new \RuntimeException( - sprintf('The view class for the %1$s view in the %2$s format was not found.', $view, $format), 500 + sprintf('A view class for the %1$s view in the %2$s format was not found.', $view, $format), 500 ); } } @@ -113,7 +113,7 @@ class DisplayController extends AbstractController $state->set('filter.applied', $this->getApplication()->getUserStateFromRequest($this->context . '.filter.applied', 'filter_applied', '')); // Pre-fill the limits. - $limit = $this->getApplication()->getUserStateFromRequest('global.list.limit', 'limit', $this->getApplication()->get('list_limit'), 'uint'); + $limit = $this->getApplication()->getUserStateFromRequest('global.list.limit', 'limit', $this->getApplication()->get('list_limit', 20), 'uint'); $state->set('list.limit', $limit); // Check if the ordering field is in the white list, otherwise use the incoming value. diff --git a/administrator/components/com_patchtester/PatchTester/Controller/StartfetchController.php b/administrator/components/com_patchtester/PatchTester/Controller/StartfetchController.php index c698cb7..e85c272 100644 --- a/administrator/components/com_patchtester/PatchTester/Controller/StartfetchController.php +++ b/administrator/components/com_patchtester/PatchTester/Controller/StartfetchController.php @@ -84,7 +84,6 @@ class StartfetchController extends AbstractController $this->getApplication()->close(1); } - // TODO - Decouple the model and context? $model = new PullsModel('com_patchtester.fetch', null, \JFactory::getDbo()); // Initialize the state for the model diff --git a/administrator/components/com_patchtester/PatchTester/Model/PullModel.php b/administrator/components/com_patchtester/PatchTester/Model/PullModel.php index 7fc8eb3..51a8154 100644 --- a/administrator/components/com_patchtester/PatchTester/Model/PullModel.php +++ b/administrator/components/com_patchtester/PatchTester/Model/PullModel.php @@ -304,7 +304,7 @@ class PullModel extends \JModelDatabase /** * Reverts the specified pull request * - * @param integer $id ID of the pull request to Reverts + * @param integer $id ID of the pull request to revert * * @return boolean * diff --git a/administrator/components/com_patchtester/PatchTester/Model/PullsModel.php b/administrator/components/com_patchtester/PatchTester/Model/PullsModel.php index 10f7d6b..20ac258 100644 --- a/administrator/components/com_patchtester/PatchTester/Model/PullsModel.php +++ b/administrator/components/com_patchtester/PatchTester/Model/PullsModel.php @@ -90,13 +90,8 @@ class PullsModel extends \JModelDatabase return $this->cache[$store]; } - // Load the list items. - $query = $this->_getListQuery(); - - $items = $this->_getList($query, $this->getStart(), $this->getState()->get('list.limit')); - - // Add the items to the internal cache. - $this->cache[$store] = $items; + // Load the list items and add the items to the internal cache. + $this->cache[$store] = $this->_getList($this->_getListQuery(), $this->getStart(), $this->getState()->get('list.limit')); return $this->cache[$store]; } @@ -115,12 +110,12 @@ class PullsModel extends \JModelDatabase $query = $db->getQuery(true); // Select the required fields from the table. - $query->select($this->getState()->get('list.select', 'a.*')); - $query->from($db->quoteName('#__patchtester_pulls', 'a')); + $query->select('a.*'); + $query->from('#__patchtester_pulls', 'a'); // Join the tests table to get applied patches - $query->select($db->quoteName('t.id', 'applied')); - $query->join('LEFT', $db->quoteName('#__patchtester_tests', 't') . ' ON t.pull_id = a.pull_id'); + $query->select('t.id AS applied'); + $query->join('LEFT', '#__patchtester_tests AS t ON t.pull_id = a.pull_id'); // Filter by search $search = $this->getState()->get('filter.search'); @@ -129,16 +124,15 @@ class PullsModel extends \JModelDatabase { if (stripos($search, 'id:') === 0) { - $query->where($db->quoteName('a.pull_id') . ' = ' . (int) substr($search, 3)); + $query->where('a.pull_id = ' . (int) substr($search, 3)); } elseif (is_numeric($search)) { - $query->where($db->quoteName('a.pull_id') . ' LIKE ' . $db->quote('%' . (int) $search . '%')); + $query->where('a.pull_id LIKE ' . $db->quote('%' . (int) $search . '%')); } else { - $search = $db->quote('%' . $db->escape($search, true) . '%'); - $query->where('(' . $db->quoteName('a.title') . ' LIKE ' . $search . ')'); + $query->where('(a.title LIKE ' . $db->quote('%' . $db->escape($search, true) . '%') . ')'); } } @@ -189,12 +183,8 @@ class PullsModel extends \JModelDatabase return $this->cache[$store]; } - // Create the pagination object. - $limit = (int) $this->getState()->get('list.limit') - (int) $this->getState()->get('list.links'); - $page = new \JPagination($this->getTotal(), $this->getStart(), $limit); - - // Add the object to the internal cache. - $this->cache[$store] = $page; + // Create the pagination object and add the object to the internal cache. + $this->cache[$store] = new \JPagination($this->getTotal(), $this->getStart(), (int) $this->getState()->get('list.limit', 20)); return $this->cache[$store]; } @@ -252,8 +242,8 @@ class PullsModel extends \JModelDatabase return $this->cache[$store]; } - $start = $this->getState()->get('list.start'); - $limit = $this->getState()->get('list.limit'); + $start = $this->getState()->get('list.start', 0); + $limit = $this->getState()->get('list.limit', 20); $total = $this->getTotal(); if ($start > $total - $limit) @@ -285,13 +275,8 @@ class PullsModel extends \JModelDatabase return $this->cache[$store]; } - // Load the total. - $query = $this->_getListQuery(); - - $total = (int) $this->_getListCount($query); - - // Add the total to the internal cache. - $this->cache[$store] = $total; + // Load the total and add the total to the internal cache. + $this->cache[$store] = (int) $this->_getListCount($this->_getListQuery()); return $this->cache[$store]; } @@ -343,7 +328,7 @@ class PullsModel extends \JModelDatabase { // Build the data object to store in the database $pullData = array( - $pull->number, + (int) $pull->number, $this->getDb()->quote(\JHtml::_('string.truncate', $pull->title, 150)), $this->getDb()->quote(\JHtml::_('string.truncate', $pull->body, 100)), $this->getDb()->quote($pull->html_url), @@ -355,16 +340,8 @@ class PullsModel extends \JModelDatabase $this->getDb()->setQuery( $this->getDb()->getQuery(true) - ->insert($this->getDb()->quoteName('#__patchtester_pulls')) - ->columns( - array( - $this->getDb()->quoteName('pull_id'), - $this->getDb()->quoteName('title'), - $this->getDb()->quoteName('description'), - $this->getDb()->quoteName('pull_url'), - $this->getDb()->quoteName('sha') - ) - ) + ->insert('#__patchtester_pulls') + ->columns(array('pull_id', 'title', 'description', 'pull_url', 'sha')) ->values($data) ); @@ -395,9 +372,7 @@ class PullsModel extends \JModelDatabase */ protected function _getList($query, $limitstart = 0, $limit = 0) { - $this->getDb()->setQuery($query, $limitstart, $limit); - - return $this->getDb()->loadObjectList(); + return $this->getDb()->setQuery($query, $limitstart, $limit)->loadObjectList(); } /** diff --git a/administrator/components/com_patchtester/PatchTester/Table/PullsTable.php b/administrator/components/com_patchtester/PatchTester/Table/PullsTable.php index 0c661dd..c069f3c 100644 --- a/administrator/components/com_patchtester/PatchTester/Table/PullsTable.php +++ b/administrator/components/com_patchtester/PatchTester/Table/PullsTable.php @@ -22,7 +22,7 @@ class PullsTable extends \JTable * * @since 2.0 */ - public function __construct(&$db) + public function __construct(\JDatabaseDriver &$db) { parent::__construct('#__patchtester_pulls', 'id', $db); } diff --git a/administrator/components/com_patchtester/PatchTester/Table/TestsTable.php b/administrator/components/com_patchtester/PatchTester/Table/TestsTable.php index 25fa769..40b0c62 100644 --- a/administrator/components/com_patchtester/PatchTester/Table/TestsTable.php +++ b/administrator/components/com_patchtester/PatchTester/Table/TestsTable.php @@ -22,7 +22,7 @@ class TestsTable extends \JTable * * @since 2.0 */ - public function __construct(&$db) + public function __construct(\JDatabaseDriver &$db) { parent::__construct('#__patchtester_tests', 'id', $db); } diff --git a/administrator/components/com_patchtester/PatchTester/View/DefaultHtmlView.php b/administrator/components/com_patchtester/PatchTester/View/DefaultHtmlView.php index ccb4f23..64836c4 100644 --- a/administrator/components/com_patchtester/PatchTester/View/DefaultHtmlView.php +++ b/administrator/components/com_patchtester/PatchTester/View/DefaultHtmlView.php @@ -59,8 +59,6 @@ class DefaultHtmlView extends \JViewHtml include $path; // Get the layout contents. - $output = ob_get_clean(); - - return $output; + return ob_get_clean(); } } diff --git a/administrator/components/com_patchtester/PatchTester/View/Pulls/tmpl/default.php b/administrator/components/com_patchtester/PatchTester/View/Pulls/tmpl/default.php index f8e92dd..0176c3a 100644 --- a/administrator/components/com_patchtester/PatchTester/View/Pulls/tmpl/default.php +++ b/administrator/components/com_patchtester/PatchTester/View/Pulls/tmpl/default.php @@ -19,7 +19,6 @@ else : $listOrder = $this->escape($this->state->get('list.ordering')); $listDirn = $this->escape($this->state->get('list.direction')); $filterApplied = $this->escape($this->state->get('filter.applied')); -$sortFields = $this->getSortFields(); ?>
@@ -48,7 +47,7 @@ $sortFields = $this->getSortFields();
diff --git a/administrator/components/com_patchtester/patchtester.php b/administrator/components/com_patchtester/patchtester.php index d50fc1a..5c02825 100644 --- a/administrator/components/com_patchtester/patchtester.php +++ b/administrator/components/com_patchtester/patchtester.php @@ -11,7 +11,7 @@ defined('_JEXEC') or die; // Access check. if (!JFactory::getUser()->authorise('core.manage', 'com_patchtester')) { - return JError::raiseWarning(403, JText::_('JERROR_ALERTNOAUTHOR')); + throw new RuntimeException(JText::_('JERROR_ALERTNOAUTHOR'), 403); } // Application reference diff --git a/administrator/templates/hathor/html/com_patchtester/pulls/default.php b/administrator/templates/hathor/html/com_patchtester/pulls/default.php index 3a027d9..07b274a 100644 --- a/administrator/templates/hathor/html/com_patchtester/pulls/default.php +++ b/administrator/templates/hathor/html/com_patchtester/pulls/default.php @@ -16,10 +16,9 @@ if (count($this->envErrors)) : echo $this->loadTemplate('errors'); else : -$listOrder = $this->escape($this->state->get('list.ordering')); -$listDirn = $this->escape($this->state->get('list.direction')); +$listOrder = $this->escape($this->state->get('list.ordering')); +$listDirn = $this->escape($this->state->get('list.direction')); $filterApplied = $this->escape($this->state->get('filter.applied')); -$sortFields = $this->getSortFields(); \JFactory::getDocument()->addStyleDeclaration( '