From 8e0b8039d79a9b4423a36c3c22ef697c343732a0 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:16:16 -0400
Subject: [PATCH 01/15] Check that xml extension is loaded on install

---
 OneShipStationPlugin.php | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/OneShipStationPlugin.php b/OneShipStationPlugin.php
index 957f34a..3226d17 100644
--- a/OneShipStationPlugin.php
+++ b/OneShipStationPlugin.php
@@ -36,14 +36,16 @@ public function onBeforeInstall() {
 
         // require PHP 5.4+
         if (!defined('PHP_VERSION_ID') || PHP_VERSION_ID < 50400) {
-            Craft::log('One ShipStation requires PHP 5.4+ in order to run.', LogLevel::Error);
-            return false;
+            throw new Exception('One ShipStation requires PHP 5.4+ in order to run.');
         }
 
         // require Craft Commerce 1.0+
         if (!($commerce = craft()->plugins->getPlugin('commerce')) || version_compare($commerce->getVersion(), '1.0', '<')) {
-            Craft::log('One ShipStation requires Craft Commerce 1.0+.', LogLevel::Error);
-            return false;
+            throw new Exception('One ShipStation requires Craft Commerce 1.0+.');
+        }
+
+        if (!extension_loaded('xml')) {
+            throw new Exception('One ShipStation requires the xml extension to be installed.');
         }
 
         return true;

From 2da69277d5df8d6482fe0df6bb7acaa3cb74024c Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:16:55 -0400
Subject: [PATCH 02/15] Catch all controller exceptions, log them, and return a
 json error to be able to view it in ShipStation

---
 .../OneShipStation_OrdersController.php       | 30 +++++++++++--------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index 5529a7c..766af42 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -17,13 +17,18 @@ public function actionProcess(array $variables=[]) {
         if (!$this->authenticate()) {
             throw new HttpException(401);
         }
-        switch (craft()->request->getParam('action')) {
-            case 'export':
-                return $this->getOrders();
-            case 'shipnotify':
-                return $this->postShipment();
-            default:
-                throw new HttpException(400);
+        try {
+            switch (craft()->request->getParam('action')) {
+                case 'export':
+                    return $this->getOrders();
+                case 'shipnotify':
+                    return $this->postShipment();
+                default:
+                    throw new HttpException(400);
+            }
+        } catch (Exception $e) {
+            Craft::log($e->getMessage(), LogLevel::Error, true);
+            return $this->returnErrorJson($e->getMessage());
         }
     }
 
@@ -105,7 +110,7 @@ protected function parseDate($field_name) {
                     return date('Y-m-d H:i:s', $date);
                 else
                     return date('Y-m-d H:i:59', $date);
-            }  
+            }
         }
         return null;
     }
@@ -122,21 +127,22 @@ protected function postShipment() {
         $order = $this->orderFromParams();
 
         $status = craft()->commerce_orderStatuses->getOrderStatusByHandle('shipped');
-        if (!$status) { throw new ErrorException("Failed to find Commerce OrderStatus 'Shipped'"); }
+        if (!$status) {
+            throw new ErrorException("Failed to find Commerce OrderStatus 'Shipped'");
+        }
 
         $order->orderStatusId = $status->id;
         $order->message = $this->orderStatusMessageFromShipstationParams();
 
         if (craft()->commerce_orders->saveOrder($order)) {
-
             $shippingInformation = $this->getShippingInformationFromParams();
             if (!craft()->oneShipStation_shippingLog->logShippingInformation($order, $shippingInformation)) {
-                Craft::log('Logging shipping information failed');
+                throw new ErrorException('Logging shipping information failed for order ' . $order->id);
             }
 
             $this->returnJson(['success' => true]); //TODO return 200 success
         } else {
-            throw new ErrorException('Failed to save order');
+            throw new ErrorException('Failed to save order with id ' . $order->id);
         }
     }
 

From fdc5c527c8b11d6cd5e9c750732e517d4fcad264 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:24:28 -0400
Subject: [PATCH 03/15] Spacing and brackets please

---
 services/OneShipStation_XmlService.php | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/services/OneShipStation_XmlService.php b/services/OneShipStation_XmlService.php
index eac07d7..be6814d 100644
--- a/services/OneShipStation_XmlService.php
+++ b/services/OneShipStation_XmlService.php
@@ -59,12 +59,13 @@ public function order(\SimpleXMLElement $xml, Commerce_OrderModel $order, $name=
 
         $this->shippingMethod($order_xml, $order);
 
-        if ($paymentObj = $order->paymentMethod)
+        if ($paymentObj = $order->paymentMethod) {
             $this->addChildWithCDATA($order_xml, 'PaymentMethod', $paymentObj->name);
+        }
 
         $items_xml = $this->items($order_xml, $order->getLineItems());
         $this->discount($items_xml, $order);
-        
+
         $customer = $order->getCustomer();
         $customer_xml = $this->customer($order_xml, $customer);
 
@@ -135,7 +136,7 @@ public function item(\SimpleXMLElement $xml, Commerce_LineItemModel $item, $name
                                                 'cdata' => false]
         ];
         $this->mapCraftModel($item_xml, $item_mapping, $item);
- 
+
         $item_xml->addChild('WeightUnits', 'Grams');
 
         if (isset($item->snapshot['options'])) {

From 837a25a1b79ec10ad9a8c119eaff4da7feff2242 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:26:20 -0400
Subject: [PATCH 04/15] Catch ErrorExceptions, too

---
 controllers/OneShipStation_OrdersController.php | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index 766af42..3d9a361 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -26,6 +26,9 @@ public function actionProcess(array $variables=[]) {
                 default:
                     throw new HttpException(400);
             }
+        } catch (ErrorException $e) {
+            Craft::log($e->getMessage(), LogLevel::Error, true);
+            return $this->returnErrorJson($e->getMessage());
         } catch (Exception $e) {
             Craft::log($e->getMessage(), LogLevel::Error, true);
             return $this->returnErrorJson($e->getMessage());

From 95ec8f59d2fd8183d25b71c8433727db07669463 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:29:18 -0400
Subject: [PATCH 05/15] Use the plugin's log handler

---
 controllers/OneShipStation_OrdersController.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index 3d9a361..41de02b 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -27,10 +27,10 @@ public function actionProcess(array $variables=[]) {
                     throw new HttpException(400);
             }
         } catch (ErrorException $e) {
-            Craft::log($e->getMessage(), LogLevel::Error, true);
+            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
             return $this->returnErrorJson($e->getMessage());
         } catch (Exception $e) {
-            Craft::log($e->getMessage(), LogLevel::Error, true);
+            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
             return $this->returnErrorJson($e->getMessage());
         }
     }

From b785973d2b55ba93753d36ed421f7082d97da5eb Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:40:25 -0400
Subject: [PATCH 06/15] Show an actual error when username/password are
 incorrect

---
 controllers/OneShipStation_OrdersController.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index 41de02b..bbc45ad 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -15,6 +15,7 @@ class Oneshipstation_OrdersController extends BaseController
      */
     public function actionProcess(array $variables=[]) {
         if (!$this->authenticate()) {
+            return $this->returnErrorJson('Invalid username/password.');
             throw new HttpException(401);
         }
         try {

From 0d5cd8ab3251329b7548fa9b0d3f4e33a72b51e1 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:44:17 -0400
Subject: [PATCH 07/15] Perform dependecy checks on ever run of the plugin

---
 OneShipStationPlugin.php | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/OneShipStationPlugin.php b/OneShipStationPlugin.php
index 3226d17..3437369 100644
--- a/OneShipStationPlugin.php
+++ b/OneShipStationPlugin.php
@@ -28,7 +28,7 @@ public function getDeveloperUrl() {
         return 'https://onedesigncompany.com';
     }
 
-    public function onBeforeInstall() {
+    protected function doDepChecks() {
         // require Craft 2.5+
         if (version_compare(craft()->getVersion(), '2.5', '<')) {
             throw new Exception('One ShipStation requires Craft CMS 2.5+ in order to run.');
@@ -44,10 +44,13 @@ public function onBeforeInstall() {
             throw new Exception('One ShipStation requires Craft Commerce 1.0+.');
         }
 
-        if (!extension_loaded('xml')) {
+        if (extension_loaded('xml')) {
             throw new Exception('One ShipStation requires the xml extension to be installed.');
         }
+    }
 
+    public function onBeforeInstall() {
+        $this->doDepChecks();
         return true;
     }
 
@@ -59,6 +62,10 @@ public function createTables() {}
 
     public function dropTables() {}
 
+    public function init() {
+        $this->doDepChecks();
+    }
+
     /*
      * WARNING: Do not register any routes that ShipStation will use here.
      *          ShipStation sends a parameter `action` with every request, which collides with

From 8da0ad9b66f222dbed377686a61ff64e37852aff Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 21:44:55 -0400
Subject: [PATCH 08/15] Oops, forgot to fix a ! after testing

---
 OneShipStationPlugin.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OneShipStationPlugin.php b/OneShipStationPlugin.php
index 3437369..dc99f95 100644
--- a/OneShipStationPlugin.php
+++ b/OneShipStationPlugin.php
@@ -44,7 +44,7 @@ protected function doDepChecks() {
             throw new Exception('One ShipStation requires Craft Commerce 1.0+.');
         }
 
-        if (extension_loaded('xml')) {
+        if (!extension_loaded('xml')) {
             throw new Exception('One ShipStation requires the xml extension to be installed.');
         }
     }

From 1efccf1f31236333050055f3958a6c5dc4e6ec7a Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Mon, 14 Aug 2017 22:01:52 -0400
Subject: [PATCH 09/15] Adds better error messages for http exceptions

---
 controllers/OneShipStation_OrdersController.php | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index bbc45ad..af272c2 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -15,8 +15,7 @@ class Oneshipstation_OrdersController extends BaseController
      */
     public function actionProcess(array $variables=[]) {
         if (!$this->authenticate()) {
-            return $this->returnErrorJson('Invalid username/password.');
-            throw new HttpException(401);
+            return $this->returnErrorJson('Invalid OneShipStation username or password.');
         }
         try {
             switch (craft()->request->getParam('action')) {
@@ -25,7 +24,7 @@ public function actionProcess(array $variables=[]) {
                 case 'shipnotify':
                     return $this->postShipment();
                 default:
-                    throw new HttpException(400);
+                    throw new HttpException(400, 'No action set. Set the ?action= parameter as `export` or `shipnotify`.');
             }
         } catch (ErrorException $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
@@ -33,6 +32,12 @@ public function actionProcess(array $variables=[]) {
         } catch (Exception $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
             return $this->returnErrorJson($e->getMessage());
+        } catch (HttpException $e) {
+            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
+            return $this->returnErrorJson(array(
+                'code' => $e->statusCode,
+                'error' => $e->getMessage(),
+            ));
         }
     }
 

From 49d3fe44d27c5f7983dd92efa85c2c001cc44277 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Wed, 6 Sep 2017 10:24:25 -0400
Subject: [PATCH 10/15] Moves generic Exception catch to the bottom

---
 controllers/OneShipStation_OrdersController.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index af272c2..a45ea2e 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -29,15 +29,15 @@ public function actionProcess(array $variables=[]) {
         } catch (ErrorException $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
             return $this->returnErrorJson($e->getMessage());
-        } catch (Exception $e) {
-            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
-            return $this->returnErrorJson($e->getMessage());
         } catch (HttpException $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
             return $this->returnErrorJson(array(
                 'code' => $e->statusCode,
                 'error' => $e->getMessage(),
             ));
+        } catch (Exception $e) {
+            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
+            return $this->returnErrorJson($e->getMessage());
         }
     }
 

From 684f5274f0449a2a42795b452898abff73758ce5 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Thu, 7 Sep 2017 11:15:37 -0400
Subject: [PATCH 11/15] Fixes logic to extract dates from params. start_date
 was always resulting to `true`

---
 controllers/OneShipStation_OrdersController.php | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index a45ea2e..0cf04a3 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -66,9 +66,14 @@ protected function authenticate() {
      */
     protected function getOrders() {
         $criteria = craft()->elements->getCriteria('Commerce_Order');
-        if ($start_date = $this->parseDate('start_date') && $end_date = $this->parseDate('end_date')) {
+
+        $start_date = $this->parseDate('start_date');
+        $end_date = $this->parseDate('end_date');
+
+        if ($start_date && $end_date) {
             $criteria->dateOrdered = array('and', '> '.$start_date, '< '.$end_date);
         }
+
         $criteria->orderStatusId = true;
 
         $num_pages = $this->paginateOrders($criteria);

From 68f06ee6623df8aa05ff5a0a207a683f19d58068 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Thu, 7 Sep 2017 11:33:09 -0400
Subject: [PATCH 12/15] Adds an error handler to avoid HTML error responses

---
 .../OneShipStation_OrdersController.php       | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index 0cf04a3..824690d 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -5,6 +5,16 @@ class Oneshipstation_OrdersController extends BaseController
 {
     protected $allowAnonymous = true;
 
+    /**
+     * Override Craft's error handler because it returns HTML that ShipStation cuts off
+     * So, this will throw an exception with the error message in order to output JSON
+     */
+    function handleError($severity, $message, $filename, $lineno) {
+      if (error_reporting() & $severity) {
+        throw new ErrorException($message, $severity);
+      }
+    }
+
     /**
      * ShipStation will hit this action for processing orders, both POSTing and GETting.
      *   ShipStation will send a GET param 'action' of either shipnotify or export.
@@ -14,6 +24,8 @@ class Oneshipstation_OrdersController extends BaseController
      * @throws HttpException for malformed requests
      */
     public function actionProcess(array $variables=[]) {
+        set_error_handler(array($this, 'handleError'));
+
         if (!$this->authenticate()) {
             return $this->returnErrorJson('Invalid OneShipStation username or password.');
         }
@@ -38,6 +50,9 @@ public function actionProcess(array $variables=[]) {
         } catch (Exception $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
             return $this->returnErrorJson($e->getMessage());
+        } catch (ErrorException $e) {
+            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
+            return $this->returnErrorJson($e->getMessage());
         }
     }
 
@@ -66,14 +81,15 @@ protected function authenticate() {
      */
     protected function getOrders() {
         $criteria = craft()->elements->getCriteria('Commerce_Order');
-
         $start_date = $this->parseDate('start_date');
         $end_date = $this->parseDate('end_date');
 
+        $myArray = [];
+        $myArray->error;
+
         if ($start_date && $end_date) {
             $criteria->dateOrdered = array('and', '> '.$start_date, '< '.$end_date);
         }
-
         $criteria->orderStatusId = true;
 
         $num_pages = $this->paginateOrders($criteria);

From f06217ba96e007fd10a04d86388e6f0841ca10b8 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Thu, 7 Sep 2017 11:35:36 -0400
Subject: [PATCH 13/15] Adds more info to error messages, but doesn't expose
 filesystem structure

---
 controllers/OneShipStation_OrdersController.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index 824690d..0f4a094 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -11,7 +11,7 @@ class Oneshipstation_OrdersController extends BaseController
      */
     function handleError($severity, $message, $filename, $lineno) {
       if (error_reporting() & $severity) {
-        throw new ErrorException($message, $severity);
+        throw new ErrorException(implode(array($message, basename($filename), $lineno), ': '), $severity);
       }
     }
 

From 0933e0d87985a33536bf42368ce30dba0b5d11ab Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Thu, 7 Sep 2017 11:36:34 -0400
Subject: [PATCH 14/15] Oops, left debug lines in the code.

---
 controllers/OneShipStation_OrdersController.php | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index 0f4a094..f4d4a1d 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -84,9 +84,6 @@ protected function getOrders() {
         $start_date = $this->parseDate('start_date');
         $end_date = $this->parseDate('end_date');
 
-        $myArray = [];
-        $myArray->error;
-
         if ($start_date && $end_date) {
             $criteria->dateOrdered = array('and', '> '.$start_date, '< '.$end_date);
         }

From 55445ab8d1c144d2ab58422bd8124b785a890eb4 Mon Sep 17 00:00:00 2001
From: mkornatz <mkornatz@gmail.com>
Date: Thu, 7 Sep 2017 12:59:14 -0400
Subject: [PATCH 15/15] Sets HTTP error codes when serving errors

---
 controllers/OneShipStation_OrdersController.php | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/controllers/OneShipStation_OrdersController.php b/controllers/OneShipStation_OrdersController.php
index f4d4a1d..8348e77 100644
--- a/controllers/OneShipStation_OrdersController.php
+++ b/controllers/OneShipStation_OrdersController.php
@@ -26,10 +26,11 @@ function handleError($severity, $message, $filename, $lineno) {
     public function actionProcess(array $variables=[]) {
         set_error_handler(array($this, 'handleError'));
 
-        if (!$this->authenticate()) {
-            return $this->returnErrorJson('Invalid OneShipStation username or password.');
-        }
         try {
+            if (!$this->authenticate()) {
+                throw new HttpException(401, 'Invalid OneShipStation username or password.');
+            }
+
             switch (craft()->request->getParam('action')) {
                 case 'export':
                     return $this->getOrders();
@@ -40,18 +41,18 @@ public function actionProcess(array $variables=[]) {
             }
         } catch (ErrorException $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
+            HeaderHelper::setHeader("HTTP/1.0 500");
             return $this->returnErrorJson($e->getMessage());
         } catch (HttpException $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
+            HeaderHelper::setHeader("HTTP/1.0 {$e->statusCode}");
             return $this->returnErrorJson(array(
                 'code' => $e->statusCode,
-                'error' => $e->getMessage(),
+                'message' => $e->getMessage(),
             ));
         } catch (Exception $e) {
             OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
-            return $this->returnErrorJson($e->getMessage());
-        } catch (ErrorException $e) {
-            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
+            HeaderHelper::setHeader("HTTP/1.0 500");
             return $this->returnErrorJson($e->getMessage());
         }
     }