From 16079d95e30cc6be2aa6a35b79f80db9e90ac155 Mon Sep 17 00:00:00 2001 From: Mudit Shukla Date: Sat, 11 Jan 2020 03:27:50 +0530 Subject: [PATCH 1/5] Update Reorder.php Update Reorder.php so that it can handle the Localized Exception and added try catch block --- .../Adminhtml/Order/Create/Reorder.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php index 995a6c216d3c2..04013df790a1d 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php @@ -90,10 +90,18 @@ public function execute() } $resultRedirect->setPath('sales/order/view', ['order_id' => $orderId]); } else { - $order->setReordered(true); - $this->_getSession()->setUseOldShippingMethod(true); - $this->_getOrderCreateModel()->initFromOrder($order); - $resultRedirect->setPath('sales/*'); + try { + $order->setReordered(true); + $this->_getSession()->setUseOldShippingMethod(true); + $this->_getOrderCreateModel()->initFromOrder($order); + $resultRedirect->setPath('sales/*'); + } catch (\Magento\Framework\Exception\LocalizedException $e) { + $this->messageManager->addErrorMessage($e->getMessage()); + return $resultRedirect->setPath('sales/*'); + } catch (\Exception $e) { + $this->messageManager->addException($e, __('Error while processing order.')); + return $resultRedirect->setPath('sales/*'); + } } return $resultRedirect; From a21b2466f6d63e5b9d120ff0295f80f51c2ec7cc Mon Sep 17 00:00:00 2001 From: engcom-Echo Date: Thu, 6 Feb 2020 13:57:24 +0200 Subject: [PATCH 2/5] Cover unit test --- .../Adminhtml/Order/Create/ReorderTest.php | 193 +++++++++++++----- 1 file changed, 144 insertions(+), 49 deletions(-) diff --git a/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Create/ReorderTest.php b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Create/ReorderTest.php index b11d73de736d4..ce777046b584d 100644 --- a/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Create/ReorderTest.php +++ b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Create/ReorderTest.php @@ -3,6 +3,8 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); + namespace Magento\Sales\Test\Unit\Controller\Adminhtml\Order\Create; use Magento\Backend\App\Action\Context; @@ -14,19 +16,24 @@ use Magento\Framework\App\RequestInterface; use Magento\Framework\Message\ManagerInterface; use Magento\Framework\ObjectManagerInterface; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Sales\Api\OrderRepositoryInterface; use Magento\Sales\Controller\Adminhtml\Order\Create\Reorder; use Magento\Sales\Model\AdminOrder\Create; use Magento\Sales\Model\Order; use Magento\Sales\Model\Order\Reorder\UnavailableProductsProvider; use Magento\Sales\Helper\Reorder as ReorderHelper; +use Magento\Framework\Exception\NoSuchEntityException; +use PHPUnit\Framework\TestCase; +use PHPUnit_Framework_MockObject_MockObject as MockObject; /** - * Class ReorderTest + * Verify reorder class. + * * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.TooManyFields) */ -class ReorderTest extends \PHPUnit\Framework\TestCase +class ReorderTest extends TestCase { /** * @var Reorder @@ -39,67 +46,67 @@ class ReorderTest extends \PHPUnit\Framework\TestCase private $context; /** - * @var RequestInterface|\PHPUnit_Framework_MockObject_MockObject + * @var RequestInterface|MockObject */ private $requestMock; /** - * @var ObjectManagerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ObjectManagerInterface|MockObject */ private $objectManagerMock; /** - * @var Order|\PHPUnit_Framework_MockObject_MockObject + * @var Order|MockObject */ private $orderMock; /** - * @var ManagerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var ManagerInterface|MockObject */ private $messageManagerMock; /** - * @var ForwardFactory|\PHPUnit_Framework_MockObject_MockObject + * @var ForwardFactory|MockObject */ private $resultForwardFactoryMock; /** - * @var RedirectFactory|\PHPUnit_Framework_MockObject_MockObject + * @var RedirectFactory|MockObject */ private $resultRedirectFactoryMock; /** - * @var Redirect|\PHPUnit_Framework_MockObject_MockObject + * @var Redirect|MockObject */ private $resultRedirectMock; /** - * @var Forward|\PHPUnit_Framework_MockObject_MockObject + * @var Forward|MockObject */ private $resultForwardMock; /** - * @var Quote|\PHPUnit_Framework_MockObject_MockObject + * @var Quote|MockObject */ private $quoteSessionMock; /** - * @var OrderRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject + * @var OrderRepositoryInterface|MockObject */ private $orderRepositoryMock; /** - * @var ReorderHelper|\PHPUnit_Framework_MockObject_MockObject + * @var ReorderHelper|MockObject */ private $reorderHelperMock; /** - * @var UnavailableProductsProvider|\PHPUnit_Framework_MockObject_MockObject + * @var UnavailableProductsProvider|MockObject */ private $unavailableProductsProviderMock; /** - * @var Create|\PHPUnit_Framework_MockObject_MockObject + * @var Create|MockObject */ private $orderCreateMock; @@ -109,39 +116,33 @@ class ReorderTest extends \PHPUnit\Framework\TestCase private $orderId; /** - * @return void + * @inheritDoc */ protected function setUp() { $this->orderId = 111; - $this->orderRepositoryMock = $this->getMockBuilder(OrderRepositoryInterface::class)->getMockForAbstractClass(); + $this->orderRepositoryMock = $this->createMock(OrderRepositoryInterface::class); + $this->requestMock = $this->createMock(RequestInterface::class); + $this->objectManagerMock = $this->createMock(ObjectManagerInterface::class); + $this->resultForwardFactoryMock = $this->createMock(ForwardFactory::class); + $this->resultRedirectFactoryMock = $this->createMock(RedirectFactory::class); + $this->resultRedirectMock = $this->createMock(Redirect::class); + $this->resultForwardMock = $this->createMock(Forward::class); + $this->reorderHelperMock = $this->createMock(ReorderHelper::class); + $this->unavailableProductsProviderMock = $this->createMock(UnavailableProductsProvider::class); + $this->orderCreateMock = $this->createMock(Create::class); $this->orderMock = $this->getMockBuilder(Order::class) ->disableOriginalConstructor() ->setMethods(['getEntityId', 'getId', 'setReordered']) ->getMock(); - $this->requestMock = $this->getMockBuilder(RequestInterface::class)->getMockForAbstractClass(); - $this->objectManagerMock = $this->getMockBuilder(ObjectManagerInterface::class)->getMockForAbstractClass(); - $this->resultForwardFactoryMock = $this->getMockBuilder(ForwardFactory::class) - ->disableOriginalConstructor() - ->getMock(); - $this->resultRedirectFactoryMock = $this->getMockBuilder(RedirectFactory::class) - ->disableOriginalConstructor() - ->getMock(); - $this->resultRedirectMock = $this->getMockBuilder(Redirect::class)->disableOriginalConstructor()->getMock(); - $this->resultForwardMock = $this->getMockBuilder(Forward::class)->disableOriginalConstructor()->getMock(); $this->quoteSessionMock = $this->getMockBuilder(Quote::class) ->disableOriginalConstructor() ->setMethods(['clearStorage', 'setUseOldShippingMethod']) ->getMock(); - $this->reorderHelperMock = $this->getMockBuilder(ReorderHelper::class) - ->disableOriginalConstructor() - ->getMock(); - $this->messageManagerMock = $this->getMockBuilder(ManagerInterface::class)->getMockForAbstractClass(); - $this->unavailableProductsProviderMock = $this->getMockBuilder(UnavailableProductsProvider::class) - ->disableOriginalConstructor() - ->getMock(); - $this->orderCreateMock = $this->getMockBuilder(Create::class)->disableOriginalConstructor()->getMock(); - $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->messageManagerMock = $this->getMockBuilder(ManagerInterface::class) + ->getMockForAbstractClass(); + + $objectManager = new ObjectManager($this); $this->context = $objectManager->getObject( Context::class, [ @@ -165,9 +166,11 @@ protected function setUp() } /** + * Verify execute with no route. + * * @return void */ - public function testExecuteForward() + public function testExecuteForward(): void { $this->clearStorage(); $this->getOrder(); @@ -178,9 +181,11 @@ public function testExecuteForward() } /** + * Verify execute redirect order grid + * * @return void */ - public function testExecuteRedirectOrderGrid() + public function testExecuteRedirectOrderGrid(): void { $this->clearStorage(); $this->getOrder(); @@ -193,9 +198,11 @@ public function testExecuteRedirectOrderGrid() } /** + * Verify execute redirect back. + * * @return void */ - public function testExecuteRedirectBack() + public function testExecuteRedirectBack(): void { $this->clearStorage(); $this->getOrder(); @@ -210,9 +217,11 @@ public function testExecuteRedirectBack() } /** + * Verify execute redirect new order. + * * @return void */ - public function testExecuteRedirectNewOrder() + public function testExecuteRedirectNewOrder(): void { $this->clearStorage(); $this->getOrder(); @@ -227,9 +236,75 @@ public function testExecuteRedirectNewOrder() } /** + * Verify redirect new order with throws exception. + * + * @return void + */ + public function testExecuteRedirectNewOrderWithThrowsException(): void + { + $exception = new NoSuchEntityException(); + + $this->clearStorage(); + $this->getOrder(); + $this->canReorder(true); + $this->createRedirect(); + $this->getOrderId($this->orderId); + $this->getUnavailableProducts([]); + + $this->orderMock->expects($this->once()) + ->method('setReordered') + ->with(true) + ->willThrowException($exception); + $this->messageManagerMock + ->expects($this->once()) + ->method('addErrorMessage') + ->willReturnSelf(); + $this->resultRedirectMock + ->expects($this->once()) + ->method('setPath') + ->with('sales/*') + ->willReturnSelf(); + $this->assertInstanceOf(Redirect::class, $this->reorder->execute()); + } + + /** + * Verify redirect new order with exception. + * * @return void */ - private function clearStorage() + public function testExecuteRedirectNewOrderWithException(): void + { + $exception = new \Exception(); + + $this->clearStorage(); + $this->getOrder(); + $this->canReorder(true); + $this->createRedirect(); + $this->getOrderId($this->orderId); + $this->getUnavailableProducts([]); + $this->orderMock->expects($this->once()) + ->method('setReordered') + ->with(true) + ->willThrowException(new $exception); + $this->messageManagerMock + ->expects($this->once()) + ->method('addException') + ->with($exception, __('Error while processing order.')) + ->willReturnSelf(); + $this->resultRedirectMock + ->expects($this->once()) + ->method('setPath') + ->with('sales/*') + ->willReturnSelf(); + $this->assertInstanceOf(Redirect::class, $this->reorder->execute()); + } + + /** + * Mock clear storage. + * + * @return void + */ + private function clearStorage(): void { $this->objectManagerMock->expects($this->at(0)) ->method('get') @@ -239,9 +314,11 @@ private function clearStorage() } /** + * Mock get order. + * * @return void */ - private function getOrder() + private function getOrder(): void { $this->requestMock->expects($this->once()) ->method('getParam') @@ -254,9 +331,12 @@ private function getOrder() } /** + * Mock and return 'canReorder' method. + * * @param bool $result + * @return void */ - private function canReorder($result) + private function canReorder(bool $result): void { $entityId = 1; $this->orderMock->expects($this->once())->method('getEntityId')->willReturn($entityId); @@ -267,18 +347,22 @@ private function canReorder($result) } /** + * Mock result forward. + * * @return void */ - private function prepareForward() + private function prepareForward(): void { $this->resultForwardFactoryMock->expects($this->once())->method('create')->willReturn($this->resultForwardMock); $this->resultForwardMock->expects($this->once())->method('forward')->with('noroute')->willReturnSelf(); } /** + * Mock create. + * * @return void */ - private function createRedirect() + private function createRedirect(): void { $this->resultRedirectFactoryMock->expects($this->once()) ->method('create') @@ -286,26 +370,35 @@ private function createRedirect() } /** + * Mock order 'getId' method. + * * @param null|int $orderId + * @return void */ - private function getOrderId($orderId) + private function getOrderId($orderId): void { $this->orderMock->expects($this->once())->method('getId')->willReturn($orderId); } /** + * Mock result redirect 'setPath' method. + * * @param string $path * @param null|array $params + * @return void */ - private function setPath($path, $params = []) + private function setPath(string $path, $params = []): void { $this->resultRedirectMock->expects($this->once())->method('setPath')->with($path, $params); } /** + * Mock unavailable products provider. + * * @param array $unavailableProducts + * @return void */ - private function getUnavailableProducts(array $unavailableProducts) + private function getUnavailableProducts(array $unavailableProducts): void { $this->unavailableProductsProviderMock->expects($this->any()) ->method('getForOrder') @@ -314,9 +407,11 @@ private function getUnavailableProducts(array $unavailableProducts) } /** + * Mock init form order. + * * @return void */ - private function initFromOrder() + private function initFromOrder(): void { $this->orderMock->expects($this->once())->method('setReordered')->with(true)->willReturnSelf(); $this->objectManagerMock->expects($this->at(1)) From 43f9e2b61dca391ca4c887f0c0c939dcbace34b7 Mon Sep 17 00:00:00 2001 From: engcom-Echo Date: Thu, 6 Feb 2020 14:50:28 +0200 Subject: [PATCH 3/5] Add implement HttpPostActionInterface for renderer controller --- .../Adminhtml/Order/Create/Reorder.php | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php index 04013df790a1d..481fa669b72d3 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php @@ -3,16 +3,28 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); + namespace Magento\Sales\Controller\Adminhtml\Order\Create; use Magento\Backend\App\Action; +use Magento\Backend\Model\View\Result\Forward; use Magento\Backend\Model\View\Result\ForwardFactory; +use Magento\Backend\Model\View\Result\Redirect; +use Magento\Catalog\Helper\Product; +use Magento\Framework\Escaper; use Magento\Framework\View\Result\PageFactory; -use Magento\Sales\Model\Order\Reorder\UnavailableProductsProvider; use Magento\Sales\Api\OrderRepositoryInterface; +use Magento\Sales\Controller\Adminhtml\Order\Create; use Magento\Sales\Helper\Reorder as ReorderHelper; +use Magento\Sales\Model\Order; +use Magento\Sales\Model\Order\Reorder\UnavailableProductsProvider; +use Magento\Framework\App\Action\HttpPostActionInterface; -class Reorder extends \Magento\Sales\Controller\Adminhtml\Order\Create +/** + * Controller create order. + */ +class Reorder extends Create implements HttpPostActionInterface { /** * @var UnavailableProductsProvider @@ -31,8 +43,8 @@ class Reorder extends \Magento\Sales\Controller\Adminhtml\Order\Create /** * @param Action\Context $context - * @param \Magento\Catalog\Helper\Product $productHelper - * @param \Magento\Framework\Escaper $escaper + * @param Product $productHelper + * @param Escaper $escaper * @param PageFactory $resultPageFactory * @param ForwardFactory $resultForwardFactory * @param UnavailableProductsProvider $unavailableProductsProvider @@ -41,8 +53,8 @@ class Reorder extends \Magento\Sales\Controller\Adminhtml\Order\Create */ public function __construct( Action\Context $context, - \Magento\Catalog\Helper\Product $productHelper, - \Magento\Framework\Escaper $escaper, + Product $productHelper, + Escaper $escaper, PageFactory $resultPageFactory, ForwardFactory $resultForwardFactory, UnavailableProductsProvider $unavailableProductsProvider, @@ -62,19 +74,21 @@ public function __construct( } /** - * @return \Magento\Backend\Model\View\Result\Forward|\Magento\Backend\Model\View\Result\Redirect + * Adminhtml controller create order. + * + * @return Forward|Redirect */ public function execute() { $this->_getSession()->clearStorage(); $orderId = $this->getRequest()->getParam('order_id'); - /** @var \Magento\Sales\Model\Order $order */ + /** @var Order $order */ $order = $this->orderRepository->get($orderId); if (!$this->reorderHelper->canReorder($order->getEntityId())) { return $this->resultForwardFactory->create()->forward('noroute'); } - /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */ + /** @var Redirect $resultRedirect */ $resultRedirect = $this->resultRedirectFactory->create(); if (!$order->getId()) { $resultRedirect->setPath('sales/order/'); @@ -90,7 +104,7 @@ public function execute() } $resultRedirect->setPath('sales/order/view', ['order_id' => $orderId]); } else { - try { + try { $order->setReordered(true); $this->_getSession()->setUseOldShippingMethod(true); $this->_getOrderCreateModel()->initFromOrder($order); From 6c5fbcb16196a5e7757fe82c77f358da3aae1678 Mon Sep 17 00:00:00 2001 From: engcom-Echo Date: Thu, 20 Feb 2020 12:01:14 +0200 Subject: [PATCH 4/5] Changed implements class --- .../Sales/Controller/Adminhtml/Order/Create/Reorder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php index 481fa669b72d3..925e8bfdd05aa 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php @@ -12,6 +12,7 @@ use Magento\Backend\Model\View\Result\ForwardFactory; use Magento\Backend\Model\View\Result\Redirect; use Magento\Catalog\Helper\Product; +use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\Escaper; use Magento\Framework\View\Result\PageFactory; use Magento\Sales\Api\OrderRepositoryInterface; @@ -19,12 +20,11 @@ use Magento\Sales\Helper\Reorder as ReorderHelper; use Magento\Sales\Model\Order; use Magento\Sales\Model\Order\Reorder\UnavailableProductsProvider; -use Magento\Framework\App\Action\HttpPostActionInterface; /** * Controller create order. */ -class Reorder extends Create implements HttpPostActionInterface +class Reorder extends Create implements HttpGetActionInterface { /** * @var UnavailableProductsProvider From f7819a64af4ce9789fa86a71948870eaccbf8c95 Mon Sep 17 00:00:00 2001 From: engcom-Echo Date: Thu, 12 Mar 2020 12:18:45 +0200 Subject: [PATCH 5/5] Added an error logging --- .../Controller/Adminhtml/Order/Create/Reorder.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php index 925e8bfdd05aa..eeaf4bee1b1c2 100644 --- a/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php +++ b/app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php @@ -20,6 +20,7 @@ use Magento\Sales\Helper\Reorder as ReorderHelper; use Magento\Sales\Model\Order; use Magento\Sales\Model\Order\Reorder\UnavailableProductsProvider; +use Psr\Log\LoggerInterface; /** * Controller create order. @@ -41,6 +42,11 @@ class Reorder extends Create implements HttpGetActionInterface */ private $reorderHelper; + /** + * @var LoggerInterface + */ + private $logger; + /** * @param Action\Context $context * @param Product $productHelper @@ -50,6 +56,7 @@ class Reorder extends Create implements HttpGetActionInterface * @param UnavailableProductsProvider $unavailableProductsProvider * @param OrderRepositoryInterface $orderRepository * @param ReorderHelper $reorderHelper + * @param LoggerInterface $logger */ public function __construct( Action\Context $context, @@ -59,11 +66,13 @@ public function __construct( ForwardFactory $resultForwardFactory, UnavailableProductsProvider $unavailableProductsProvider, OrderRepositoryInterface $orderRepository, - ReorderHelper $reorderHelper + ReorderHelper $reorderHelper, + LoggerInterface $logger ) { $this->unavailableProductsProvider = $unavailableProductsProvider; $this->orderRepository = $orderRepository; $this->reorderHelper = $reorderHelper; + $this->logger = $logger; parent::__construct( $context, $productHelper, @@ -110,9 +119,11 @@ public function execute() $this->_getOrderCreateModel()->initFromOrder($order); $resultRedirect->setPath('sales/*'); } catch (\Magento\Framework\Exception\LocalizedException $e) { + $this->logger->critical($e); $this->messageManager->addErrorMessage($e->getMessage()); return $resultRedirect->setPath('sales/*'); } catch (\Exception $e) { + $this->logger->critical($e); $this->messageManager->addException($e, __('Error while processing order.')); return $resultRedirect->setPath('sales/*'); }