Merge pull request #35 from onedesign/bugfix/cleanup

Bugfix/cleanup
onedesign · Sep 7, 2017 · bf4bfd4 · bf4bfd4
2 parents dec3f13 + 55445ab
commit bf4bfd4
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 24 deletions.
diff --git a/OneShipStationPlugin.php b/OneShipStationPlugin.php
@@ -28,24 +28,29 @@ 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.');
         }
 
         // 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.');
         }
+    }
 
+    public function onBeforeInstall() {
+        $this->doDepChecks();
         return true;
     }
 
@@ -57,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

diff --git 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(implode(array($message, basename($filename), $lineno), ': '), $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,16 +24,36 @@ class Oneshipstation_OrdersController extends BaseController
      * @throws HttpException for malformed requests
      */
     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);
+        set_error_handler(array($this, 'handleError'));
+
+        try {
+            if (!$this->authenticate()) {
+                throw new HttpException(401, 'Invalid OneShipStation username or password.');
+            }
+
+            switch (craft()->request->getParam('action')) {
+                case 'export':
+                    return $this->getOrders();
+                case 'shipnotify':
+                    return $this->postShipment();
+                default:
+                    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);
+            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,
+                'message' => $e->getMessage(),
+            ));
+        } catch (Exception $e) {
+            OneShipStationPlugin::log($e->getMessage(), LogLevel::Error, true);
+            HeaderHelper::setHeader("HTTP/1.0 500");
+            return $this->returnErrorJson($e->getMessage());
         }
     }
 
@@ -52,7 +82,10 @@ 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;
@@ -105,7 +138,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 +155,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);
         }
     }
 

diff --git 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'])) {