Misc tweaks and review

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.

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

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
 	 *

administrator/components/com_patchtester/PatchTester/Model/PullsModel.php

@@ -90,13 +90,8 @@ class PullsModel extends \JModelDatabase
 			return $this->cache[$store];
 		}
 
-		// Load the list items.
+		// Load the list items and add the items to the internal cache.
-		$query = $this->_getListQuery();
+		$this->cache[$store] = $this->_getList($this->_getListQuery(), $this->getStart(), $this->getState()->get('list.limit'));
-
-		$items = $this->_getList($query, $this->getStart(), $this->getState()->get('list.limit'));
-
-		// Add the items to the internal cache.
-		$this->cache[$store] = $items;
 
 		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->select('a.*');
-		$query->from($db->quoteName('#__patchtester_pulls', 'a'));
+		$query->from('#__patchtester_pulls', 'a');
 
 		// Join the tests table to get applied patches
-		$query->select($db->quoteName('t.id', 'applied'));
+		$query->select('t.id AS applied');
-		$query->join('LEFT', $db->quoteName('#__patchtester_tests', 't') . ' ON t.pull_id = a.pull_id');
+		$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('(a.title LIKE ' . $db->quote('%' . $db->escape($search, true) . '%') . ')');
-				$query->where('(' . $db->quoteName('a.title') . ' LIKE ' . $search . ')');
 			}
 		}
 
@@ -189,12 +183,8 @@ class PullsModel extends \JModelDatabase
 			return $this->cache[$store];
 		}
 
-		// Create the pagination object.
+		// Create the pagination object and add the object to the internal cache.
-		$limit = (int) $this->getState()->get('list.limit') - (int) $this->getState()->get('list.links');
+		$this->cache[$store] = new \JPagination($this->getTotal(), $this->getStart(), (int) $this->getState()->get('list.limit', 20));
-		$page = new \JPagination($this->getTotal(), $this->getStart(), $limit);
-
-		// Add the object to the internal cache.
-		$this->cache[$store] = $page;
 
 		return $this->cache[$store];
 	}
@@ -252,8 +242,8 @@ class PullsModel extends \JModelDatabase
 			return $this->cache[$store];
 		}
 
-		$start = $this->getState()->get('list.start');
+		$start = $this->getState()->get('list.start', 0);
-		$limit = $this->getState()->get('list.limit');
+		$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.
+		// Load the total and add the total to the internal cache.
-		$query = $this->_getListQuery();
+		$this->cache[$store] = (int) $this->_getListCount($this->_getListQuery());
-
-		$total = (int) $this->_getListCount($query);
-
-		// Add the total to the internal cache.
-		$this->cache[$store] = $total;
 
 		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'))
+				->insert('#__patchtester_pulls')
-				->columns(
+				->columns(array('pull_id', 'title', 'description', 'pull_url', 'sha'))
-					array(
-						$this->getDb()->quoteName('pull_id'),
-						$this->getDb()->quoteName('title'),
-						$this->getDb()->quoteName('description'),
-						$this->getDb()->quoteName('pull_url'),
-						$this->getDb()->quoteName('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()->setQuery($query, $limitstart, $limit)->loadObjectList();
-
-		return $this->getDb()->loadObjectList();
 	}
 
 	/**

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);
 	}

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);
 	}

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 ob_get_clean();
-
-		return $output;
 	}
 }

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();
 ?>
 <form action="<?php echo \JRoute::_('index.php?option=com_patchtester&view=pulls'); ?>" method="post" name="adminForm" id="adminForm" data-order="<?php echo $listOrder; ?>">
 	<div id="j-main-container">
@@ -48,7 +47,7 @@ $sortFields    = $this->getSortFields();
 				<label for="sortTable" class="element-invisible"><?php echo \JText::_('JGLOBAL_SORT_BY'); ?></label>
 				<select name="sortTable" id="sortTable" class="input-medium" onchange="PatchTester.orderTable()">
 					<option value=""><?php echo \JText::_('JGLOBAL_SORT_BY'); ?></option>
-					<?php echo \JHtml::_('select.options', $sortFields, 'value', 'text', $listOrder);?>
+					<?php echo \JHtml::_('select.options', $this->getSortFields(), 'value', 'text', $listOrder);?>
 				</select>
 			</div>
 			<div class="btn-group pull-right">

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

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'));
+$listOrder     = $this->escape($this->state->get('list.ordering'));
-$listDirn  = $this->escape($this->state->get('list.direction'));
+$listDirn      = $this->escape($this->state->get('list.direction'));
 $filterApplied = $this->escape($this->state->get('filter.applied'));
-$sortFields    = $this->getSortFields();
 
 \JFactory::getDocument()->addStyleDeclaration(
 	'