From d8d585a843d2a2c437c9101f9ec1cc64a5b9eb1d Mon Sep 17 00:00:00 2001 From: Antony Thorpe <1023740+AntonyThorpe@users.noreply.github.com> Date: Thu, 13 Jul 2023 19:11:27 +1200 Subject: [PATCH] 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 --- src/Admin/ProductBulkLoader.php | 4 ++-- src/Cart/OrderTotalCalculator.php | 8 ++++---- src/Cart/ShoppingCartController.php | 10 +++++----- src/Checkout/CheckoutComponentConfig.php | 2 +- src/Checkout/OrderProcessor.php | 10 +++++----- src/Forms/OrderActionsForm.php | 2 +- src/Forms/VariationForm.php | 1 - src/Model/Order.php | 4 ++-- src/Model/OrderStatusLog.php | 14 +++++++------- src/Model/Variation/Variation.php | 2 +- src/Page/Product.php | 6 +++--- src/Reports/ProductReport.php | 1 - src/Tasks/PopulateShopTask.php | 6 +++--- tests/php/Model/OrderStatusLogTest.php | 4 ++-- 14 files changed, 36 insertions(+), 38 deletions(-) diff --git a/src/Admin/ProductBulkLoader.php b/src/Admin/ProductBulkLoader.php index cece1319c..024913de3 100644 --- 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 index 60ae1f198..633c1b093 100644 --- 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 index 240591ce4..cd5f2f143 100644 --- 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 index e3f277407..bd8e56ccf 100644 --- 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 index 725aca247..2b177493f 100644 --- 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 index 8e0b0b404..fce67860d 100644 --- 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 index b599b59df..c3079aae7 100644 --- 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 index 81f670c36..e157da4b6 100644 --- 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 index f4517897d..f96f5cef7 100644 --- 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 index 602c45ada..6ac081b38 100755 --- 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 index ef66e52b5..42b624708 100644 --- 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 index 83af54d61..70584364d 100644 --- 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 index 4b600897d..bc7682ff8 100644 --- 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 index d5e2d93b8..44c25f18a 100644 --- 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();