From e803e6a7f23d9dd5a5aa1c33b9235097c5058052 Mon Sep 17 00:00:00 2001 From: AnshuMishra17 Date: Wed, 11 Apr 2018 11:29:04 +0530 Subject: [PATCH 1/4] Refactor Mass Order Cancel code to use Interface I have observed that MassAction Cancel is using the collection for orders cancel, whereas Order Cancel (order cancel from order edit section) is using Interface to put the order on hold. So, I have refactor the Mass Order Cancel code to use Sales Order Interface. # Conflicts: # app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php --- .../Sales/Controller/Adminhtml/Order/MassCancel.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php index 5d7a55bc4a56c..4413ee268ce0b 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php @@ -11,6 +11,7 @@ use Magento\Ui\Component\MassAction\Filter; use Magento\Sales\Model\ResourceModel\Order\CollectionFactory; use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Sales\Api\OrderManagementInterface; class MassCancel extends \Magento\Sales\Controller\Adminhtml\Order\AbstractMassAction { @@ -18,16 +19,23 @@ class MassCancel extends \Magento\Sales\Controller\Adminhtml\Order\AbstractMassA * Authorization level of a basic admin session */ const ADMIN_RESOURCE = 'Magento_Sales::cancel'; + + /** + * @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; } /** @@ -57,8 +65,7 @@ protected function massAction(AbstractCollection $collection) if (!$order->canCancel()) { continue; } - $order->cancel(); - $order->save(); + $this->orderManagement->cancel($order->getEntityId()); $countCancelOrder++; } $countNonCancelOrder = $collection->count() - $countCancelOrder; From 21dfeaeec760044547c1b7e10f3b0b713ecc6d2c Mon Sep 17 00:00:00 2001 From: Magento Community Engineering <31669971+magento-engcom-team@users.noreply.github.com> Date: Thu, 12 Apr 2018 06:53:16 -0500 Subject: [PATCH 2/4] ENGCOM-1266: Refactor Mass Order Cancel code to use Interface #14630 - Merge Pull Request magento/magento2#14630 from AnshuMishra17/magento2:patch-2 - Merged commits: 1. dab8c3897444df70498b2388735b2d06e218d5f3 2. 97f5978c0e8c7b3d042ce74ca44eca691729b860 3. 2b4ca2e0205a6b4edf15f36c6e5e983f0fdeb981 # Conflicts: # app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php # app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassCancelTest.php --- .../Controller/Adminhtml/Order/MassCancel.php | 18 +- .../Adminhtml/Order/MassCancelTest.php | 291 ++++++++++++++++++ 2 files changed, 304 insertions(+), 5 deletions(-) create mode 100644 app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassCancelTest.php diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php index 4413ee268ce0b..5ae55226c5b50 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php @@ -23,18 +23,25 @@ class MassCancel extends \Magento\Sales\Controller\Adminhtml\Order\AbstractMassA /** * @var OrderManagementInterface */ - protected $orderManagement; + private $orderManagement; /** * @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; } @@ -62,7 +69,8 @@ protected function massAction(AbstractCollection $collection) { $countCancelOrder = 0; foreach ($collection->getItems() as $order) { - if (!$order->canCancel()) { + $isCanceled = $this->orderManagement->cancel($order->getEntityId()); + if ($isCanceled === false) { continue; } $this->orderManagement->cancel($order->getEntityId()); diff --git a/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassCancelTest.php b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassCancelTest.php new file mode 100644 index 0000000000000..756bade3c83c9 --- /dev/null +++ b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/MassCancelTest.php @@ -0,0 +1,291 @@ +contextMock = $this->createMock(\Magento\Backend\App\Action\Context::class); + $this->messageManagerMock = $this->createMock(\Magento\Framework\Message\Manager::class); + $this->responseMock = $this->createMock(\Magento\Framework\App\ResponseInterface::class); + $this->requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class); + $this->objectManagerMock = $this->createMock(\Magento\Framework\ObjectManager\ObjectManager::class); + + $resultRedirectFactory = $this->createMock(\Magento\Backend\Model\View\Result\RedirectFactory::class); + + $this->orderCollectionMock = $this->getMockBuilder(\Magento\Sales\Model\ResourceModel\Order\Collection::class) + ->disableOriginalConstructor() + ->getMock(); + + $resourceCollection = \Magento\Sales\Model\ResourceModel\Order\CollectionFactory::class; + $this->orderCollectionFactoryMock = $this->getMockBuilder($resourceCollection) + ->disableOriginalConstructor() + ->setMethods(['create']) + ->getMock(); + + $this->sessionMock = $this->createPartialMock(\Magento\Backend\Model\Session::class, ['setIsUrlNotice']); + $this->actionFlagMock = $this->createPartialMock(\Magento\Framework\App\ActionFlag::class, ['get', 'set']); + $this->helperMock = $this->createPartialMock(\Magento\Backend\Helper\Data::class, ['getUrl']); + $this->resultRedirectMock = $this->createMock(\Magento\Backend\Model\View\Result\Redirect::class); + $resultRedirectFactory->expects($this->any())->method('create')->willReturn($this->resultRedirectMock); + + $redirectMock = $this->getMockBuilder(\Magento\Backend\Model\View\Result\Redirect::class) + ->disableOriginalConstructor() + ->getMock(); + + $resultFactoryMock = $this->getMockBuilder(\Magento\Framework\Controller\ResultFactory::class) + ->disableOriginalConstructor() + ->getMock(); + $resultFactoryMock->expects($this->any()) + ->method('create') + ->with(\Magento\Framework\Controller\ResultFactory::TYPE_REDIRECT) + ->willReturn($redirectMock); + + $this->contextMock->expects($this->once())->method('getMessageManager')->willReturn($this->messageManagerMock); + $this->contextMock->expects($this->once())->method('getRequest')->willReturn($this->requestMock); + $this->contextMock->expects($this->once())->method('getResponse')->willReturn($this->responseMock); + $this->contextMock->expects($this->once())->method('getObjectManager')->willReturn($this->objectManagerMock); + $this->contextMock->expects($this->once())->method('getSession')->willReturn($this->sessionMock); + $this->contextMock->expects($this->once())->method('getActionFlag')->willReturn($this->actionFlagMock); + $this->contextMock->expects($this->once())->method('getHelper')->willReturn($this->helperMock); + $this->contextMock->expects($this->once()) + ->method('getResultRedirectFactory') + ->willReturn($resultRedirectFactory); + $this->contextMock->expects($this->any()) + ->method('getResultFactory') + ->willReturn($resultFactoryMock); + $this->filterMock = $this->createMock(\Magento\Ui\Component\MassAction\Filter::class); + $this->filterMock->expects($this->once()) + ->method('getCollection') + ->with($this->orderCollectionMock) + ->willReturn($this->orderCollectionMock); + $this->orderCollectionFactoryMock->expects($this->once()) + ->method('create') + ->willReturn($this->orderCollectionMock); + $this->orderManagementMock = $this->createMock(\Magento\Sales\Api\OrderManagementInterface::class); + + $this->massAction = $objectManagerHelper->getObject( + \Magento\Sales\Controller\Adminhtml\Order\MassCancel::class, + [ + 'context' => $this->contextMock, + 'filter' => $this->filterMock, + 'collectionFactory' => $this->orderCollectionFactoryMock, + 'orderManagement' => $this->orderManagementMock + ] + ); + } + + /** + * Test for selected orders + * Two orders, only $order1 can be canceled + */ + public function testExecuteCanCancelOneOrder() + { + $order1id = 100; + $order2id = 200; + + $order1 = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + ->disableOriginalConstructor() + ->getMock(); + $order2 = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + ->disableOriginalConstructor() + ->getMock(); + $orders = [$order1, $order2]; + $countOrders = count($orders); + + $this->orderCollectionMock->expects($this->any()) + ->method('getItems') + ->willReturn($orders); + + $order1->expects($this->once()) + ->method('getEntityId') + ->willReturn($order1id); + + $order2->expects($this->once()) + ->method('getEntityId') + ->willReturn($order2id); + + $this->orderCollectionMock->expects($this->once()) + ->method('count') + ->willReturn($countOrders); + + $this->orderManagementMock->expects($this->at(0))->method('cancel')->with($order1id)->willReturn(true); + $this->orderManagementMock->expects($this->at(1))->method('cancel')->with($order2id)->willReturn(false); + + $this->messageManagerMock->expects($this->once()) + ->method('addError') + ->with('1 order(s) cannot be canceled.'); + + $this->messageManagerMock->expects($this->once()) + ->method('addSuccess') + ->with('We canceled 1 order(s).'); + + $this->resultRedirectMock->expects($this->once()) + ->method('setPath') + ->with('sales/*/') + ->willReturnSelf(); + + $this->massAction->execute(); + } + + /** + * Test for excluded orders + * Two orders could't be canceled + */ + public function testExcludedCannotCancelOrders() + { + $order1 = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + ->disableOriginalConstructor() + ->getMock(); + $order2 = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + ->disableOriginalConstructor() + ->getMock(); + + $orders = [$order1, $order2]; + $countOrders = count($orders); + + $order1->expects($this->once()) + ->method('getEntityId') + ->willReturn(100); + + $order2->expects($this->once()) + ->method('getEntityId') + ->willReturn(200); + + $this->orderCollectionMock->expects($this->any()) + ->method('getItems') + ->willReturn([$order1, $order2]); + + $this->orderCollectionMock->expects($this->once()) + ->method('count') + ->willReturn($countOrders); + + $this->orderManagementMock->expects($this->atLeastOnce())->method('cancel')->willReturn(false); + + $this->messageManagerMock->expects($this->once()) + ->method('addError') + ->with('You cannot cancel the order(s).'); + + $this->resultRedirectMock->expects($this->once()) + ->method('setPath') + ->with('sales/*/') + ->willReturnSelf(); + + $this->massAction->execute(); + } + + /** + * Order throws exception while canceling + */ + public function testException() + { + $exception = new \Exception('Can not cancel'); + + $order1 = $this->getMockBuilder(\Magento\Sales\Model\Order::class) + ->disableOriginalConstructor() + ->getMock(); + $this->orderCollectionMock->expects($this->any()) + ->method('getItems') + ->willReturn([$order1]); + + $order1->expects($this->once()) + ->method('getEntityId') + ->willReturn(100); + + $this->orderManagementMock->expects($this->atLeastOnce())->method('cancel')->willThrowException($exception); + + $this->messageManagerMock->expects($this->once()) + ->method('addError') + ->with('Can not cancel'); + + $this->massAction->execute(); + } +} From 0ecf1fae15f0e481d21bc0114ae8d35195d56038 Mon Sep 17 00:00:00 2001 From: Jeroen Date: Thu, 4 Oct 2018 10:55:08 +0200 Subject: [PATCH 3/4] Remove duplicate --- app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php index 5ae55226c5b50..8a012397492f5 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php @@ -42,7 +42,6 @@ public function __construct( $this->orderManagement = $orderManagement ?: \Magento\Framework\App\ObjectManager::getInstance()->get( \Magento\Sales\Api\OrderManagementInterface::class ); - $this->orderManagement = $orderManagement; } /** From 2569a0bf546008c9734155a928a968ee1e954b63 Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Thu, 4 Oct 2018 12:15:32 +0300 Subject: [PATCH 4/4] Removed double call of orderManagement service --- app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php index 8a012397492f5..05a22245dc004 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/MassCancel.php @@ -72,7 +72,6 @@ protected function massAction(AbstractCollection $collection) if ($isCanceled === false) { continue; } - $this->orderManagement->cancel($order->getEntityId()); $countCancelOrder++; } $countNonCancelOrder = $collection->count() - $countCancelOrder;