ENGCOM-3679: Use repository to load order when manually creating an invoice #19727

Author: sivaschenko

app/code/Magento/Sales/Controller/Adminhtml/Order/Invoice/NewAction.php

@@ -1,18 +1,21 @@
 <?php
 /**
- *
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
 
 namespace Magento\Sales\Controller\Adminhtml\Order\Invoice;
 
-use Magento\Framework\App\Action\HttpGetActionInterface as HttpGetActionInterface;
+use Magento\Framework\App\Action\HttpGetActionInterface;
 use Magento\Backend\App\Action;
 use Magento\Framework\Registry;
 use Magento\Framework\View\Result\PageFactory;
+use Magento\Sales\Api\OrderRepositoryInterface;
 use Magento\Sales\Model\Service\InvoiceService;
 
+/**
+ * Create new invoice action.
+ */
 class NewAction extends \Magento\Backend\App\Action implements HttpGetActionInterface
 {
     /**
@@ -37,22 +40,32 @@ class NewAction extends \Magento\Backend\App\Action implements HttpGetActionInte
      */
     private $invoiceService;
 
+    /**
+     * @var OrderRepositoryInterface
+     */
+    private $orderRepository;
+
     /**
      * @param Action\Context $context
      * @param Registry $registry
      * @param PageFactory $resultPageFactory
      * @param InvoiceService $invoiceService
+     * @param OrderRepositoryInterface|null $orderRepository
      */
     public function __construct(
         Action\Context $context,
         Registry $registry,
         PageFactory $resultPageFactory,
-        InvoiceService $invoiceService
+        InvoiceService $invoiceService,
+        OrderRepositoryInterface $orderRepository = null
     ) {
+        parent::__construct($context);
+
         $this->registry = $registry;
         $this->resultPageFactory = $resultPageFactory;
-        parent::__construct($context);
         $this->invoiceService = $invoiceService;
+        $this->orderRepository = $orderRepository ?: \Magento\Framework\App\ObjectManager::getInstance()
+            ->get(OrderRepositoryInterface::class);
     }
 
     /**
@@ -78,14 +91,11 @@ public function execute()
     {
         $orderId = $this->getRequest()->getParam('order_id');
         $invoiceData = $this->getRequest()->getParam('invoice', []);
-        $invoiceItems = isset($invoiceData['items']) ? $invoiceData['items'] : [];
+        $invoiceItems = $invoiceData['items'] ?? [];
 
         try {
             /** @var \Magento\Sales\Model\Order $order */
-            $order = $this->_objectManager->create(\Magento\Sales\Model\Order::class)->load($orderId);
-            if (!$order->getId()) {
-                throw new \Magento\Framework\Exception\LocalizedException(__('The order no longer exists.'));
-            }
+            $order = $this->orderRepository->get($orderId);
 
             if (!$order->canInvoice()) {
                 throw new \Magento\Framework\Exception\LocalizedException(

app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Invoice/NewActionTest.php

@@ -6,12 +6,14 @@
 namespace Magento\Sales\Test\Unit\Controller\Adminhtml\Order\Invoice;
 
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use Magento\Sales\Api\OrderRepositoryInterface;
 
 /**
  * Class NewActionTest
  * @package Magento\Sales\Controller\Adminhtml\Order\Invoice
  * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ * @SuppressWarnings(PHPMD.TooManyFields)
  */
 class NewActionTest extends \PHPUnit\Framework\TestCase
 {
@@ -90,6 +92,11 @@ class NewActionTest extends \PHPUnit\Framework\TestCase
      */
     protected $invoiceServiceMock;
 
+    /**
+     * @var OrderRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $orderRepositoryMock;
+
     protected function setUp()
     {
         $objectManager = new ObjectManager($this);
@@ -215,12 +222,15 @@ protected function setUp()
             ->disableOriginalConstructor()
             ->getMock();
 
+        $this->orderRepositoryMock = $this->createMock(OrderRepositoryInterface::class);
+
         $this->controller = $objectManager->getObject(
             \Magento\Sales\Controller\Adminhtml\Order\Invoice\NewAction::class,
             [
                 'context' => $contextMock,
                 'resultPageFactory' => $this->resultPageFactoryMock,
-                'invoiceService' => $this->invoiceServiceMock
+                'invoiceService' => $this->invoiceServiceMock,
+                'orderRepository' => $this->orderRepositoryMock
             ]
         );
     }
@@ -250,19 +260,17 @@ public function testExecute()
 
         $orderMock = $this->getMockBuilder(\Magento\Sales\Model\Order::class)
             ->disableOriginalConstructor()
-            ->setMethods(['load', 'getId', 'canInvoice'])
+            ->setMethods(['load', 'canInvoice'])
             ->getMock();
-        $orderMock->expects($this->once())
-            ->method('load')
-            ->with($orderId)
-            ->willReturnSelf();
-        $orderMock->expects($this->once())
-            ->method('getId')
-            ->willReturn($orderId);
         $orderMock->expects($this->once())
             ->method('canInvoice')
             ->willReturn(true);
 
+        $this->orderRepositoryMock->expects($this->once())
+            ->method('get')
+            ->with($orderId)
+            ->willReturn($orderMock);
+
         $this->invoiceServiceMock->expects($this->once())
             ->method('prepareInvoice')
             ->with($orderMock, [])
@@ -285,11 +293,7 @@ public function testExecute()
             ->with(true)
             ->will($this->returnValue($commentText));
 
-        $this->objectManagerMock->expects($this->at(0))
-            ->method('create')
-            ->with(\Magento\Sales\Model\Order::class)
-            ->willReturn($orderMock);
-        $this->objectManagerMock->expects($this->at(1))
+        $this->objectManagerMock->expects($this->once())
             ->method('get')
             ->with(\Magento\Backend\Model\Session::class)
             ->will($this->returnValue($this->sessionMock));
@@ -318,19 +322,12 @@ public function testExecuteNoOrder()
 
         $orderMock = $this->getMockBuilder(\Magento\Sales\Model\Order::class)
             ->disableOriginalConstructor()
-            ->setMethods(['load', 'getId', 'canInvoice'])
+            ->setMethods(['canInvoice'])
             ->getMock();
-        $orderMock->expects($this->once())
-            ->method('load')
-            ->with($orderId)
-            ->willReturnSelf();
-        $orderMock->expects($this->once())
-            ->method('getId')
-            ->willReturn(null);
 
-        $this->objectManagerMock->expects($this->at(0))
-            ->method('create')
-            ->with(\Magento\Sales\Model\Order::class)
+        $this->orderRepositoryMock->expects($this->once())
+            ->method('get')
+            ->with($orderId)
             ->willReturn($orderMock);
 
         $resultRedirect = $this->getMockBuilder(\Magento\Backend\Model\View\Result\Redirect::class)