From 970b2ef4f011d86e4ee4ed7364001b5b64e394ce Mon Sep 17 00:00:00 2001
From: Deon George <deon@leenooks.net>
Date: Fri, 10 May 2013 00:42:54 +1000
Subject: [PATCH] Improved exception handling and other minor fixes

---
 application/classes/Config.php                |  5 +---
 .../classes/Controller/User/Search.php        |  8 ++---
 application/classes/HTTP/Exception/403.php    | 15 ++++++++++
 application/classes/HTTP/Exception/404.php    | 14 ++-------
 application/classes/HTTP/Exception/501.php    | 23 ++------------
 application/classes/Model/RTM.php             |  2 +-
 application/classes/URL.php                   |  4 +++
 application/views/errors/403.php              |  5 ++++
 application/views/errors/404.php              | 30 +++----------------
 application/views/errors/501.php              |  8 +++--
 .../classes/Controller/Reseller/Invoice.php   |  6 ++--
 11 files changed, 45 insertions(+), 75 deletions(-)
 create mode 100644 application/classes/HTTP/Exception/403.php
 create mode 100644 application/views/errors/403.php

diff --git a/application/classes/Config.php b/application/classes/Config.php
index 29b6aa82..9a26fea5 100644
--- a/application/classes/Config.php
+++ b/application/classes/Config.php
@@ -172,10 +172,7 @@ class Config extends Kohana_Config {
 
 	public static function theme() {
 		// If we are using user admin pages (and login), we'll choose the admin theme.
-		if (Request::current() AND (! empty(URL::$method_directory[strtolower(Request::current()->directory())]) OR in_array(strtolower(Request::current()->controller()),array('login'))))
-			return 'theme/'.Kohana::$config->load('config')->theme_admin;
-		else
-			return 'theme/'.Kohana::$config->load('config')->theme;
+		return URL::admin_url() ? 'theme/'.Kohana::$config->load('config')->theme_admin : 'theme/'.Kohana::$config->load('config')->theme;
 	}
 
 	public static function time($date) {
diff --git a/application/classes/Controller/User/Search.php b/application/classes/Controller/User/Search.php
index db7a74b8..923d3901 100644
--- a/application/classes/Controller/User/Search.php
+++ b/application/classes/Controller/User/Search.php
@@ -21,12 +21,12 @@ class Controller_User_Search extends Controller_Search {
 		$result = array();
 
 		if (isset($_REQUEST['term']) AND trim($_REQUEST['term'])) {
-			$result = array_merge($result,ORM::factory('Account')->list_autocomplete($_REQUEST['term'],'url','id',array('ACC %s: %s'=>array('id','name(TRUE)')),array(),array('urlprefix'=>'a/service/list/')));
-			$result = array_merge($result,ORM::factory('Invoice')->list_autocomplete($_REQUEST['term'],'url','id',array('INV %s: %s'=>array('id','account->name(TRUE)')),array(),array('urlprefix'=>'u/invoice/view/')));
-			$result = array_merge($result,ORM::factory('Service')->list_autocomplete($_REQUEST['term'],'url','id',array('SVC %s: %s'=>array('id','service_name()')),array(),array('urlprefix'=>'u/service/view/')));
+			$result = Arr::merge($result,ORM::factory('Account')->list_autocomplete($_REQUEST['term'],'url','id',array('ACC %s: %s'=>array('id','name(TRUE)')),array(),array('urlprefix'=>'a/service/list/')));
+			$result = Arr::merge($result,ORM::factory('Invoice')->list_autocomplete($_REQUEST['term'],'url','id',array('INV %s: %s'=>array('id','account->name(TRUE)')),array(),array('urlprefix'=>'u/invoice/view/')));
+			$result = Arr::merge($result,ORM::factory('Service')->list_autocomplete($_REQUEST['term'],'url','id',array('SVC %s: %s'=>array('id','service_name()')),array(),array('urlprefix'=>'u/service/view/')));
 
 			foreach (array('Service_Plugin_Adsl','Service_Plugin_Domain','Service_Plugin_Host') as $o)
-				$result = array_merge($result,ORM::factory($o)->list_autocomplete($_REQUEST['term'],'url','service_id',array('SVC %s: %s'=>array('service_id','service_name()')),array(),array('urlprefix'=>'u/service/view/')));
+				$result = Arr::merge($result,ORM::factory($o)->list_autocomplete($_REQUEST['term'],'url','service_id',array('SVC %s: %s'=>array('service_id','service_name()')),array(),array('urlprefix'=>'u/service/view/')));
 		}
 
 		$this->auto_render = FALSE;
diff --git a/application/classes/HTTP/Exception/403.php b/application/classes/HTTP/Exception/403.php
new file mode 100644
index 00000000..9d78aa48
--- /dev/null
+++ b/application/classes/HTTP/Exception/403.php
@@ -0,0 +1,15 @@
+<?php defined('SYSPATH') or die('No direct access allowed.');
+
+/**
+ * This class overrides Kohana's 403 Exception
+ *
+ * @package    OSB
+ * @category   Exceptions
+ * @author     Deon George
+ * @copyright  (c) 2009-2013 Open Source Billing
+ * @license    http://dev.osbill.net/license.html
+ */
+class HTTP_Exception_403 extends HTTP_Exception {
+	protected $_code = 403;
+}
+?>
diff --git a/application/classes/HTTP/Exception/404.php b/application/classes/HTTP/Exception/404.php
index 14bf5100..d1277d63 100644
--- a/application/classes/HTTP/Exception/404.php
+++ b/application/classes/HTTP/Exception/404.php
@@ -9,17 +9,7 @@
  * @copyright  (c) 2009-2013 Open Source Billing
  * @license    http://dev.osbill.net/license.html
  */
-class HTTP_Exception_404 extends Kohana_HTTP_Exception_404 {
-	public function get_response() {
-		$response = Response::factory();
-		$response->status($this->_code);
-
-		$view = View::factory('errors/404');
-		$view->message = $this->getMessage();
- 
-		$response->body($view->render());
- 
-		return $response;
-	}
+class HTTP_Exception_404 extends HTTP_Exception {
+	protected $_code = 404;
 }
 ?>
diff --git a/application/classes/HTTP/Exception/501.php b/application/classes/HTTP/Exception/501.php
index e79b7f1a..e1afe147 100644
--- a/application/classes/HTTP/Exception/501.php
+++ b/application/classes/HTTP/Exception/501.php
@@ -1,7 +1,7 @@
 <?php defined('SYSPATH') or die('No direct access allowed.');
 
 /**
- * This class provides a custom 501 Exception to catch OSB specific errors.
+ * This class overrides Kohana's 501 Exception to catch OSB specific errors
  *
  * @package    OSB
  * @category   Exceptions
@@ -9,26 +9,7 @@
  * @copyright  (c) 2009-2013 Open Source Billing
  * @license    http://dev.osbill.net/license.html
  */
-class HTTP_Exception_501 extends Kohana_HTTP_Exception {
+class HTTP_Exception_501 extends HTTP_Exception {
 	protected $_code = 501;
-
-	public function __construct($message='',array $variables=NULL,$code=0) {
-		parent::__construct($message,$variables,$code);
-
-		$response = Response::factory();
-		$response->status($this->_code);
-
-		// @todo This is not working as cleanly as I would like - ie: we shouldnt need to publish the headers ourselves?
-		header(':', true, 501);
-
-		if (Kohana::$config->load('debug')->show_errors)
-			$response->body(View::factory('errors/501')->set('message',$this->getMessage())->render());
-		else
-			$response->body('Dang, something went wrong, tell us how we got here...');
-
-		echo $response->render();
-
-		exit (501);
-	}
 }
 ?>
diff --git a/application/classes/Model/RTM.php b/application/classes/Model/RTM.php
index a5e11e22..497c933a 100644
--- a/application/classes/Model/RTM.php
+++ b/application/classes/Model/RTM.php
@@ -27,7 +27,7 @@ class Model_RTM extends ORM_OSB {
 		$result = array();
 
 		foreach ($rtmo->agents_direct() as $artmo)
-			$result = $result+$rtmo->customers($artmo);
+			$result = Arr::merge($result,$rtmo->customers($artmo));
 
 		foreach ($rtmo->customers_direct() as $ao)
 			array_push($result,$ao);
diff --git a/application/classes/URL.php b/application/classes/URL.php
index a7c02294..03a5fa87 100644
--- a/application/classes/URL.php
+++ b/application/classes/URL.php
@@ -18,6 +18,10 @@ class URL extends Kohana_URL {
 		'task'=>'task',
 	);
 
+	public static function admin_url() {
+		return (Request::current() AND ((Auth::instance()->logged_in() AND ! empty(URL::$method_directory[strtolower(Request::current()->directory())])) OR in_array(strtolower(Request::current()->controller()),array('login'))));
+	}
+
 	/**
 	 * Wrapper to provide a URL::site() link based on function
 	 */
diff --git a/application/views/errors/403.php b/application/views/errors/403.php
new file mode 100644
index 00000000..e0e0516a
--- /dev/null
+++ b/application/views/errors/403.php
@@ -0,0 +1,5 @@
+<h1>Oops!</h1>
+<h2>403 Not Found or Not Authorised?</h2>
+<div class="error-details">
+	Sorry, either the item doesnt exist, or you are not authorised to see it.
+</div>
diff --git a/application/views/errors/404.php b/application/views/errors/404.php
index d840cc74..539ad469 100644
--- a/application/views/errors/404.php
+++ b/application/views/errors/404.php
@@ -1,27 +1,5 @@
-<?php
-	Style::factory()
-		->type('file')
-		->data('media/css/pages/error.css');
-
-	echo View::factory('theme/focusbusiness/page')
-		->set('meta',new Meta)
-		->set('navbar','')
-		->set('content','
-<div class="container">
-	<div class="row">
-		<div class="span8">
-			<div class="error-container">
-				<h1>Oops!</h1>
-				<h2>404 Not Found?</h2>
-				<div class="error-details">
-					Sorry, an error has occured, requested page not found?
-				</div>
-				<div class="error-actions">
-					'.HTML::anchor('/','<i class="icon-chevron-left"></i> Back to Home',array('class'=>'btn btn-large btn-primary')).'
-				</div>
-			</div>
-		</div>
-	</div>
+<h1>Oops!</h1>
+<h2>404 Not Found?</h2>
+<div class="error-details">
+	Sorry, an error has occured, requested page not found?
 </div>
-');
-?>
diff --git a/application/views/errors/501.php b/application/views/errors/501.php
index 866088a5..29350929 100644
--- a/application/views/errors/501.php
+++ b/application/views/errors/501.php
@@ -1,4 +1,6 @@
-Bother, something went wrong - 501.
+<h1>Oops!</h1>
+<h2>501 Bother, something went wrong.</h2>
 <?php echo $message; ?>
-<hr/>
-<?php echo Debug::vars(array('cookie'=>$_COOKIE,'request'=>$_REQUEST,'session'=>$_SESSION,'server'=>$_SERVER)); ?>
+<div class="error-details">
+	If this keeps happening, please let us know.
+</div>
diff --git a/modules/invoice/classes/Controller/Reseller/Invoice.php b/modules/invoice/classes/Controller/Reseller/Invoice.php
index 6b4d3d3a..7985c7dc 100644
--- a/modules/invoice/classes/Controller/Reseller/Invoice.php
+++ b/modules/invoice/classes/Controller/Reseller/Invoice.php
@@ -15,10 +15,8 @@ class Controller_Reseller_Invoice extends Controller_Invoice {
 
 		$ao = ORM::factory('Account',$id);
 
-		if (! $ao->loaded() OR ! Auth::instance()->authorised($ao)) {
-			$this->template->content = 'Unauthorised or doesnt exist?';
-			return FALSE;
-		}
+		if (! $ao->loaded() OR ! Auth::instance()->authorised($ao))
+			throw HTTP_Exception::factory(403,'Unauthorised or doesnt exist?');
 
 		Block::add(array(
 			'body'=>$output,