From 3530752dc37596455b08a9b9c5d518f2b53c8fca Mon Sep 17 00:00:00 2001 From: AnshuMishra17 Date: Wed, 11 Apr 2018 09:30:55 +0530 Subject: [PATCH 1/3] Refactor Code for Mass Order Unhold I have observed that MassAction Unhold is using the collection to release order from hold, whereas MassAction Hold is using Interface to put the order on hold. So, I have refactor code in app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php same as in app/code/Magento/Sales/Controller/Adminhtml/Order/MassHold.php --- .../Controller/Adminhtml/Order/MassUnhold.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php index ebd6ff4a79b06..2d4dfc3b497ca 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php @@ -9,6 +9,7 @@ use Magento\Backend\App\Action\Context; use Magento\Ui\Component\MassAction\Filter; use Magento\Sales\Model\ResourceModel\Order\CollectionFactory; +use Magento\Sales\Api\OrderManagementInterface; class MassUnhold extends AbstractMassAction { @@ -16,16 +17,23 @@ class MassUnhold extends AbstractMassAction * Authorization level of a basic admin session */ const ADMIN_RESOURCE = 'Magento_Sales::unhold'; + + /** + * @var OrderManagementInterface + */ + protected $orderManagement; /** * @param Context $context * @param Filter $filter * @param CollectionFactory $collectionFactory + * @param OrderManagementInterface $orderManagement */ - public function __construct(Context $context, Filter $filter, CollectionFactory $collectionFactory) + public function __construct(Context $context, Filter $filter, CollectionFactory $collectionFactory, OrderManagementInterface $orderManagement) { parent::__construct($context, $filter); $this->collectionFactory = $collectionFactory; + $this->orderManagement = $orderManagement; } /** @@ -40,12 +48,10 @@ protected function massAction(AbstractCollection $collection) /** @var \Magento\Sales\Model\Order $order */ foreach ($collection->getItems() as $order) { - $order->load($order->getId()); if (!$order->canUnhold()) { continue; } - $order->unhold(); - $order->save(); + $this->orderManagement->unHold($order->getEntityId()); $countUnHoldOrder++; } From 74da6881b89b4d1a22f973558439084c951ab9dc Mon Sep 17 00:00:00 2001 From: AnshuMishra17 Date: Wed, 11 Apr 2018 15:03:06 +0530 Subject: [PATCH 2/3] Added backward compatibility. Added backward compatibility for the new orderManagementInterface dependency and made the variable $orderManagement private. --- .../Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php index 2d4dfc3b497ca..82b71041b6cd3 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php @@ -21,7 +21,7 @@ class MassUnhold extends AbstractMassAction /** * @var OrderManagementInterface */ - protected $orderManagement; + private $orderManagement; /** * @param Context $context @@ -33,7 +33,7 @@ public function __construct(Context $context, Filter $filter, CollectionFactory { parent::__construct($context, $filter); $this->collectionFactory = $collectionFactory; - $this->orderManagement = $orderManagement; + $this->orderManagement = $orderManagement ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\Magento\Sales\Api\OrderManagementInterface::class); } /** From d4c762f7b6c6eb5f1a3c60e9d7cb51d12cda8dcb Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Thu, 12 Apr 2018 15:15:15 +0300 Subject: [PATCH 3/3] magento/magento2#14629 Fixed test failures --- .../Controller/Adminhtml/Order/MassUnhold.php | 14 ++++++++++---- .../Adminhtml/Order/MassUnholdTest.php | 16 ++++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php index 82b71041b6cd3..2eb54c9814ef7 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassUnhold.php @@ -27,13 +27,19 @@ class MassUnhold extends AbstractMassAction * @param Context $context * @param Filter $filter * @param CollectionFactory $collectionFactory - * @param OrderManagementInterface $orderManagement + * @param OrderManagementInterface|null $orderManagement */ - public function __construct(Context $context, Filter $filter, CollectionFactory $collectionFactory, OrderManagementInterface $orderManagement) - { + public function __construct( + Context $context, + Filter $filter, + CollectionFactory $collectionFactory, + OrderManagementInterface $orderManagement = null + ) { parent::__construct($context, $filter); $this->collectionFactory = $collectionFactory; - $this->orderManagement = $orderManagement ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\Magento\Sales\Api\OrderManagementInterface::class); + $this->orderManagement = $orderManagement ?: \Magento\Framework\App\ObjectManager::getInstance()->get( + \Magento\Sales\Api\OrderManagementInterface::class + ); } /** diff --git a/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassUnholdTest.php b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassUnholdTest.php index 7003d445e1b50..cddb503925987 100644 --- a/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassUnholdTest.php +++ b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassUnholdTest.php @@ -85,6 +85,11 @@ class MassUnholdTest extends \PHPUnit\Framework\TestCase */ protected $filterMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $orderManagementMock; + protected function setUp() { $objectManagerHelper = new ObjectManagerHelper($this); @@ -146,12 +151,15 @@ protected function setUp() ->method('create') ->willReturn($this->orderCollectionMock); + $this->orderManagementMock = $this->createMock(\Magento\Sales\Api\OrderManagementInterface::class); + $this->massAction = $objectManagerHelper->getObject( \Magento\Sales\Controller\Adminhtml\Order\MassUnhold::class, [ 'context' => $this->contextMock, 'filter' => $this->filterMock, - 'collectionFactory' => $this->orderCollectionFactoryMock + 'collectionFactory' => $this->orderCollectionFactoryMock, + 'orderManagement' => $this->orderManagementMock ] ); } @@ -175,9 +183,7 @@ public function testExecuteOneOrdersReleasedFromHold() ->method('canUnhold') ->willReturn(true); $order1->expects($this->once()) - ->method('unhold'); - $order1->expects($this->once()) - ->method('save'); + ->method('getEntityId'); $this->orderCollectionMock->expects($this->once()) ->method('count') @@ -187,6 +193,8 @@ public function testExecuteOneOrdersReleasedFromHold() ->method('canUnhold') ->willReturn(false); + $this->orderManagementMock->expects($this->atLeastOnce())->method('unHold')->willReturn(true); + $this->messageManagerMock->expects($this->once()) ->method('addError') ->with('1 order(s) were not released from on hold status.');