Refactor \Order\Shipment\Save Controller to use ResultInterface

Author: Jeroen

app/code/Magento/Shipping/Controller/Adminhtml/Order/Shipment/Save.php

@@ -1,13 +1,13 @@
 <?php
 /**
- *
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Shipping\Controller\Adminhtml\Order\Shipment;
 
-use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface;
-use Magento\Backend\App\Action;
+use Magento\Framework\App\Action\HttpPostActionInterface;
+use Magento\Framework\Controller\ResultFactory;
 use Magento\Sales\Model\Order\Shipment\Validation\QuantityValidator;
 
 /**
@@ -48,17 +48,22 @@ class Save extends \Magento\Backend\App\Action implements HttpPostActionInterfac
      * @param \Magento\Shipping\Controller\Adminhtml\Order\ShipmentLoader $shipmentLoader
      * @param \Magento\Shipping\Model\Shipping\LabelGenerator $labelGenerator
      * @param \Magento\Sales\Model\Order\Email\Sender\ShipmentSender $shipmentSender
+     * @param \Magento\Sales\Model\Order\Shipment\ShipmentValidatorInterface|null $shipmentValidator
      */
     public function __construct(
         \Magento\Backend\App\Action\Context $context,
         \Magento\Shipping\Controller\Adminhtml\Order\ShipmentLoader $shipmentLoader,
         \Magento\Shipping\Model\Shipping\LabelGenerator $labelGenerator,
-        \Magento\Sales\Model\Order\Email\Sender\ShipmentSender $shipmentSender
+        \Magento\Sales\Model\Order\Email\Sender\ShipmentSender $shipmentSender,
+        \Magento\Sales\Model\Order\Shipment\ShipmentValidatorInterface $shipmentValidator = null
     ) {
+        parent::__construct($context);
+
         $this->shipmentLoader = $shipmentLoader;
         $this->labelGenerator = $labelGenerator;
         $this->shipmentSender = $shipmentSender;
-        parent::__construct($context);
+        $this->shipmentValidator = $shipmentValidator ?: \Magento\Framework\App\ObjectManager::getInstance()
+            ->get(\Magento\Sales\Model\Order\Shipment\ShipmentValidatorInterface::class);
     }
 
     /**
@@ -86,7 +91,7 @@ protected function _saveShipment($shipment)
      * Save shipment
      * We can save only new shipment. Existing shipments are not editable
      *
-     * @return void
+     * @return \Magento\Backend\Model\View\Result\Redirect
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @SuppressWarnings(PHPMD.NPathComplexity)
      */
@@ -98,7 +103,7 @@ public function execute()
         $formKeyIsValid = $this->_formKeyValidator->validate($this->getRequest());
         $isPost = $this->getRequest()->isPost();
         if (!$formKeyIsValid || !$isPost) {
-            $this->messageManager->addError(__('We can\'t save the shipment right now.'));
+            $this->messageManager->addErrorMessage(__('We can\'t save the shipment right now.'));
             return $resultRedirect->setPath('sales/order/index');
         }
 
@@ -118,8 +123,7 @@ public function execute()
             $this->shipmentLoader->setTracking($this->getRequest()->getParam('tracking'));
             $shipment = $this->shipmentLoader->load();
             if (!$shipment) {
-                $this->_forward('noroute');
-                return;
+                return $this->resultFactory->create(ResultFactory::TYPE_FORWARD)->forward('noroute');
             }
 
             if (!empty($data['comment_text'])) {
@@ -132,15 +136,13 @@ public function execute()
                 $shipment->setCustomerNote($data['comment_text']);
                 $shipment->setCustomerNoteNotify(isset($data['comment_customer_notify']));
             }
-            $validationResult = $this->getShipmentValidator()
-                ->validate($shipment, [QuantityValidator::class]);
+            $validationResult = $this->shipmentValidator->validate($shipment, [QuantityValidator::class]);
 
             if ($validationResult->hasMessages()) {
-                $this->messageManager->addError(
+                $this->messageManager->addErrorMessage(
                     __("Shipment Document Validation Error(s):\n" . implode("\n", $validationResult->getMessages()))
                 );
-                $this->_redirect('*/*/new', ['order_id' => $this->getRequest()->getParam('order_id')]);
-                return;
+                return $resultRedirect->setPath('*/*/new', ['order_id' => $this->getRequest()->getParam('order_id')]);
             }
             $shipment->register();
 
@@ -160,7 +162,7 @@ public function execute()
             $shipmentCreatedMessage = __('The shipment has been created.');
             $labelCreatedMessage = __('You created the shipping label.');
 
-            $this->messageManager->addSuccess(
+            $this->messageManager->addSuccessMessage(
                 $isNeedCreateLabel ? $shipmentCreatedMessage . ' ' . $labelCreatedMessage : $shipmentCreatedMessage
             );
             $this->_objectManager->get(\Magento\Backend\Model\Session::class)->getCommentText(true);
@@ -169,38 +171,23 @@ public function execute()
                 $responseAjax->setError(true);
                 $responseAjax->setMessage($e->getMessage());
             } else {
-                $this->messageManager->addError($e->getMessage());
-                $this->_redirect('*/*/new', ['order_id' => $this->getRequest()->getParam('order_id')]);
+                $this->messageManager->addErrorMessage($e->getMessage());
+                return $resultRedirect->setPath('*/*/new', ['order_id' => $this->getRequest()->getParam('order_id')]);
             }
         } catch (\Exception $e) {
             $this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e);
             if ($isNeedCreateLabel) {
                 $responseAjax->setError(true);
                 $responseAjax->setMessage(__('An error occurred while creating shipping label.'));
             } else {
-                $this->messageManager->addError(__('Cannot save shipment.'));
-                $this->_redirect('*/*/new', ['order_id' => $this->getRequest()->getParam('order_id')]);
+                $this->messageManager->addErrorMessage(__('Cannot save shipment.'));
+                return $resultRedirect->setPath('*/*/new', ['order_id' => $this->getRequest()->getParam('order_id')]);
             }
         }
         if ($isNeedCreateLabel) {
-            $this->getResponse()->representJson($responseAjax->toJson());
-        } else {
-            $this->_redirect('sales/order/view', ['order_id' => $shipment->getOrderId()]);
-        }
-    }
-
-    /**
-     * @return \Magento\Sales\Model\Order\Shipment\ShipmentValidatorInterface
-     * @deprecated 100.1.1
-     */
-    private function getShipmentValidator()
-    {
-        if ($this->shipmentValidator === null) {
-            $this->shipmentValidator = $this->_objectManager->get(
-                \Magento\Sales\Model\Order\Shipment\ShipmentValidatorInterface::class
-            );
+            return $this->resultFactory->create(ResultFactory::TYPE_JSON)->setJsonData($responseAjax->toJson());
         }
 
-        return $this->shipmentValidator;
+        return $resultRedirect->setPath('sales/order/view', ['order_id' => $shipment->getOrderId()]);
     }
 }

app/code/Magento/Shipping/Test/Unit/Controller/Adminhtml/Order/Shipment/SaveTest.php

@@ -142,7 +142,7 @@ protected function setUp()
         );
         $this->messageManager = $this->createPartialMock(
             \Magento\Framework\Message\Manager::class,
-            ['addSuccess', 'addError']
+            ['addSuccessMessage', 'addErrorMessage']
         );
         $this->session = $this->createPartialMock(
             \Magento\Backend\Model\Session::class,
@@ -236,7 +236,7 @@ public function testExecute($formKeyIsValid, $isPost)
 
         if (!$formKeyIsValid || !$isPost) {
             $this->messageManager->expects($this->once())
-                ->method('addError');
+                ->method('addErrorMessage');
 
             $this->resultRedirect->expects($this->once())
                 ->method('setPath')
@@ -325,12 +325,11 @@ public function testExecute($formKeyIsValid, $isPost)
                 ->method('get')
                 ->with(\Magento\Backend\Model\Session::class)
                 ->will($this->returnValue($this->session));
-            $path = 'sales/order/view';
             $arguments = ['order_id' => $orderId];
             $shipment->expects($this->once())
                 ->method('getOrderId')
                 ->will($this->returnValue($orderId));
-            $this->prepareRedirect($path, $arguments);
+            $this->prepareRedirect($arguments);
 
             $this->shipmentValidatorMock->expects($this->once())
                 ->method('validate')
@@ -360,10 +359,9 @@ public function executeDataProvider()
     }
 
     /**
-     * @param string $path
      * @param array $arguments
      */
-    protected function prepareRedirect($path, array $arguments = [])
+    protected function prepareRedirect(array $arguments = [])
     {
         $this->actionFlag->expects($this->any())
             ->method('get')
@@ -372,14 +370,8 @@ protected function prepareRedirect($path, array $arguments = [])
         $this->session->expects($this->any())
             ->method('setIsUrlNotice')
             ->with(true);
-
-        $url = $path . '/' . (!empty($arguments) ? $arguments['order_id'] : '');
-        $this->helper->expects($this->atLeastOnce())
-            ->method('getUrl')
-            ->with($path, $arguments)
-            ->will($this->returnValue($url));
-        $this->response->expects($this->atLeastOnce())
-            ->method('setRedirect')
-            ->with($url);
+        $this->resultRedirect->expects($this->once())
+            ->method('setPath')
+            ->with('sales/order/view', $arguments);
     }
 }