Minor updates as highlighted by Scrutinizer (#787)

- OrderTotalCalculator.php: added missing public function declarations - Used setField to apply values to fields in OrderProcessor.php, OrderActionsForm.php, Order.php & OrderStatusLog.php - OrderProcessor.php: `Company` variable exists on the Address class (not Order) Docs - ShoppingCartController.php: added `string` to return type - Product.php: added `false` as a possible return type to documentation of addLink, removeLink and removeallLink functions - Variation.php: added `false` as a possible return type to the Link function - CheckoutComponentConfig.php: corrected docs for the getComponentByType function Removed unused variables - $page & $termsPage in PopulateShopTask.php - $success in VariationForm.php - $response in OrderStatusLogTest.php - $filterperiod in ProductReport.php - $record variable in imageByFilename & setParent functions in ProductBulkLoader.php
silvershop · Jul 13, 2023 · d8d585a · d8d585a
1 parent d9d5d4b
commit d8d585a
Show file tree

Hide file tree

Showing 14 changed files with 36 additions and 38 deletions.
diff --git a/src/Admin/ProductBulkLoader.php b/src/Admin/ProductBulkLoader.php
@@ -145,7 +145,7 @@ public function processRecord($record, $columnMap, &$results, $preview = false)
     }
 
     // set image, based on filename
-    public function imageByFilename(&$obj, $val, $record)
+    public function imageByFilename(&$obj, $val)
     {
         $filename = trim(strtolower(Convert::raw2sql($val)));
         $filenamedashes = str_replace(' ', '-', $filename);
@@ -165,7 +165,7 @@ public function imageByFilename(&$obj, $val, $record)
     }
 
     // find product group parent (ie Cateogry)
-    public function setParent(&$obj, $val, $record)
+    public function setParent(&$obj, $val)
     {
         $title = strtolower(Convert::raw2sql($val));
         if ($title) {

diff --git a/src/Cart/OrderTotalCalculator.php b/src/Cart/OrderTotalCalculator.php
@@ -35,7 +35,7 @@ class OrderTotalCalculator
      */
     protected $order;
 
-    function __construct(Order $order)
+    public function __construct(Order $order)
     {
         $this->order = $order;
     }
@@ -45,19 +45,19 @@ function __construct(Order $order)
      * @throws Exception
      * @throws \Psr\Container\NotFoundExceptionInterface
      */
-    function calculate()
+    public function calculate()
     {
         $runningtotal = $this->order->SubTotal();
         $sort = 1;
         $existingmodifiers = $this->order->Modifiers();
-         
+
         $modifierclasses = Order::config()->modifiers;
 
         //check if modifiers are even in use
         if (!is_array($modifierclasses) || empty($modifierclasses)) {
             return $runningtotal;
         }
-        
+
         $modifierclasses = array_unique($modifierclasses);
 
         if (DB::get_conn()->supportsTransactions()) {

diff --git a/src/Cart/ShoppingCartController.php b/src/Cart/ShoppingCartController.php
@@ -191,7 +191,7 @@ protected function buyableFromRequest()
      *
      * @param HTTPRequest $request
      *
-     * @return HTTPResponse
+     * @return string|HTTPResponse
      * @throws \SilverStripe\Control\HTTPResponse_Exception
      */
     public function add($request)
@@ -216,7 +216,7 @@ public function add($request)
      *
      * @param HTTPRequest $request
      *
-     * @return HTTPResponse
+     * @return string|HTTPResponse
      * @throws \SilverStripe\Control\HTTPResponse_Exception
      */
     public function remove($request)
@@ -235,7 +235,7 @@ public function remove($request)
      *
      * @param HTTPRequest $request
      *
-     * @return HTTPResponse
+     * @return string|HTTPResponse
      * @throws \SilverStripe\Control\HTTPResponse_Exception
      */
     public function removeall($request)
@@ -254,7 +254,7 @@ public function removeall($request)
      *
      * @param HTTPRequest $request
      *
-     * @return HTTPResponse
+     * @return string|HTTPResponse
      * @throws \SilverStripe\Control\HTTPResponse_Exception
      */
     public function setquantity($request)
@@ -275,7 +275,7 @@ public function setquantity($request)
      *
      * @param HTTPRequest $request
      *
-     * @return HTTPResponse
+     * @return string|HTTPResponse
      */
     public function clear($request)
     {

diff --git a/src/Checkout/CheckoutComponentConfig.php b/src/Checkout/CheckoutComponentConfig.php
@@ -80,7 +80,7 @@ public function getComponents()
     /**
      * Returns the first available component with the given class or interface.
      *
-     * @param String ClassName
+     * @param string $type ClassName
      *
      * @return CheckoutComponent
      */

diff --git a/src/Checkout/OrderProcessor.php b/src/Checkout/OrderProcessor.php
@@ -159,7 +159,7 @@ protected function getGatewayData($customData)
                 'firstName'        => $this->order->FirstName,
                 'lastName'         => $this->order->Surname,
                 'email'            => $this->order->Email,
-                'company'          => $this->order->Company,
+                'company'          => $billing->Company,
                 'billingAddress1'  => $billing->Address,
                 'billingAddress2'  => $billing->AddressLine2,
                 'billingCity'      => $billing->City,
@@ -231,7 +231,7 @@ public function completePayment()
                 || ($this->order->GrandTotal() == 0 && Order::config()->allow_zero_order_total)
             ) {
                 //set order as paid
-                $this->order->Status = 'Paid';
+                $this->order->setField('Status', 'Paid');
                 $this->order->write();
             }
         }
@@ -287,13 +287,13 @@ public function placeOrder()
 
         //update status
         if ($this->order->TotalOutstanding(false)) {
-            $this->order->Status = 'Unpaid';
+            $this->order->setField('Status', 'Unpaid');
         } else {
-            $this->order->Status = 'Paid';
+            $this->order->setField('Status', 'Paid');
         }
 
         if (!$this->order->Placed) {
-            $this->order->Placed = DBDatetime::now()->Rfc2822(); //record placed order datetime
+            $this->order->setField('Placed', DBDatetime::now()->Rfc2822()); //record placed order datetime
             if ($request = Controller::curr()->getRequest()) {
                 $this->order->IPAddress = $request->getIP(); //record client IP
             }

diff --git a/src/Forms/OrderActionsForm.php b/src/Forms/OrderActionsForm.php
@@ -198,7 +198,7 @@ public function docancel($data, $form)
         if (self::config()->allow_cancelling
             && $this->order->canCancel()
         ) {
-            $this->order->Status = 'MemberCancelled';
+            $this->order->setField('Status', 'MemberCancelled');
             $this->order->write();
 
             if (self::config()->email_notification) {

diff --git a/src/Forms/VariationForm.php b/src/Forms/VariationForm.php
@@ -55,7 +55,6 @@ public function addtocart($data, $form)
             // if we are in just doing a validation step then check
             if ($this->getRequest()->requestVar('ValidateVariant')) {
                 $message = '';
-                $success = false;
 
                 try {
                     $success = $variation->canPurchase(null, $data['Quantity']);

diff --git a/src/Model/Order.php b/src/Model/Order.php
@@ -838,7 +838,7 @@ protected function statusTransition($fromStatus, $toStatus)
         $this->extend('onStatusChange', $fromStatus, $toStatus);
 
         if ($toStatus == 'Paid' && !$this->Paid) {
-            $this->Paid = DBDatetime::now()->Rfc2822();
+            $this->setField('Paid', DBDatetime::now()->Rfc2822());
             foreach ($this->Items() as $item) {
                 $item->onPayment();
             }
@@ -847,7 +847,7 @@ protected function statusTransition($fromStatus, $toStatus)
 
             if (!$this->ReceiptSent) {
                 OrderEmailNotifier::create($this)->sendReceipt();
-                $this->ReceiptSent = DBDatetime::now()->Rfc2822();
+                $this->setField('ReceiptSent', DBDatetime::now()->Rfc2822());
             }
         }
 

diff --git a/src/Model/OrderStatusLog.php b/src/Model/OrderStatusLog.php
@@ -168,13 +168,13 @@ protected function updateWithLastInfo()
                 ->first();
 
             if ($latestLog) {
-                $this->DispatchedBy = $latestLog->DispatchedBy;
-                $this->DispatchedOn = $latestLog->DispatchedOn;
-                $this->DispatchTicket = $latestLog->DispatchTicket;
-                $this->PaymentCode = $latestLog->PaymentCode;
-                $this->PaymentOK = $latestLog->PaymentOK;
-                $this->SentToCustomer = $latestLog->SentToCustomer;
-                $this->VisibleToCustomer = $latestLog->VisibleToCustomer;
+                $this->setField('DispatchedBy', $latestLog->DispatchedBy);
+                $this->setField('DispatchedOn', $latestLog->DispatchedOn);
+                $this->setField('DispatchTicket', $latestLog->DispatchTicket);
+                $this->setField('PaymentCode', $latestLog->PaymentCode);
+                $this->setField('PaymentOK', $latestLog->PaymentOK);
+                $this->setField('SentToCustomer', $latestLog->SentToCustomer);
+                $this->setField('VisibleToCustomer', $latestLog->VisibleToCustomer);
             }
         }
     }

diff --git a/src/Model/Variation/Variation.php b/src/Model/Variation/Variation.php
@@ -323,7 +323,7 @@ public function addLink()
      *
      * @param $action string
      *
-     * @return string
+     * @return string|false
      */
     public function Link($action = null)
     {

diff --git a/src/Page/Product.php b/src/Page/Product.php
@@ -467,7 +467,7 @@ public function Image()
     /**
      * Link to add this product to cart.
      *
-     * @return string link
+     * @return string|false link
      */
     public function addLink()
     {
@@ -477,7 +477,7 @@ public function addLink()
     /**
      * Link to remove one of this product from cart.
      *
-     * @return string link
+     * @return string|false link
      */
     public function removeLink()
     {
@@ -487,7 +487,7 @@ public function removeLink()
     /**
      * Link to remove all of this product from cart.
      *
-     * @return string link
+     * @return string|false link
      */
     public function removeallLink()
     {

diff --git a/src/Reports/ProductReport.php b/src/Reports/ProductReport.php
@@ -65,7 +65,6 @@ public function query($params)
         $query->setFrom('"' . $table . '"');
 
         $whereClue = '1';
-        $filterperiod = $this->periodfield;
         if ($start && $end) {
             $whereClue = sprintf(
                 'DATE("o"."Placed") BETWEEN DATE(\'%s\') AND DATE(\'%s\')',

diff --git a/src/Tasks/PopulateShopTask.php b/src/Tasks/PopulateShopTask.php
@@ -89,7 +89,7 @@ public function run($request)
         }
 
         //cart page
-        if (!$page = CartPage::get()->first()) {
+        if (!CartPage::get()->first()) {
             $fixture = YamlFixture::create($fixtureDir . '/pages/Cart.yml');
             $fixture->writeInto($factory);
             $page = $factory->get(CartPage::class, 'cart');
@@ -100,7 +100,7 @@ public function run($request)
         }
 
         //checkout page
-        if (!$page = CheckoutPage::get()->first()) {
+        if (!CheckoutPage::get()->first()) {
             $fixture = YamlFixture::create($fixtureDir . '/pages/Checkout.yml');
             $fixture->writeInto($factory);
             $page = $factory->get(CheckoutPage::class, 'checkout');
@@ -122,7 +122,7 @@ public function run($request)
         }
 
         //terms page
-        if (!$termsPage = Page::get()->filter('URLSegment', 'terms-and-conditions')->first()) {
+        if (!Page::get()->filter('URLSegment', 'terms-and-conditions')->first()) {
             $fixture = YamlFixture::create($fixtureDir . '/pages/TermsConditions.yml');
             $fixture->writeInto($factory);
             $page = $factory->get('Page', 'termsconditions');

diff --git a/tests/php/Model/OrderStatusLogTest.php b/tests/php/Model/OrderStatusLogTest.php
@@ -56,7 +56,7 @@ public function testOrderStatusLogItemsWithMember()
         );
 
         $processor = OrderProcessor::create($order);
-        $response = $processor->makePayment("Manual", []);
+        $processor->makePayment("Manual", []);
         $order->Status = "Paid";
         $order->write();
 
@@ -280,7 +280,7 @@ public function testOrderPlacedByGuest()
         );
 
         $processor_guest = OrderProcessor::create($order);
-        $response = $processor_guest->makePayment("Manual", []);
+        $processor_guest->makePayment("Manual", []);
         $order->Status = "Paid";
         $order->write();