From 34409a8931811a83f9062c672c58e970376291be Mon Sep 17 00:00:00 2001 From: Oscar Recio Date: Tue, 26 Sep 2017 16:44:13 +0200 Subject: [PATCH 1/4] Add Guard Clause to check if invoice has been canceled previously --- app/code/Magento/Sales/Model/Order/Invoice.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/code/Magento/Sales/Model/Order/Invoice.php b/app/code/Magento/Sales/Model/Order/Invoice.php index 014ad8fd5fe3a..84778d71ec2cf 100644 --- a/app/code/Magento/Sales/Model/Order/Invoice.php +++ b/app/code/Magento/Sales/Model/Order/Invoice.php @@ -407,6 +407,10 @@ public function void() */ public function cancel() { + if (!$this->canCancel()) { + return $this; + } + $order = $this->getOrder(); $order->getPayment()->cancelInvoice($this); foreach ($this->getAllItems() as $item) { From 82eb45dc3e4b2103268762b81fa74b5a0843881a Mon Sep 17 00:00:00 2001 From: Oscar Recio Date: Wed, 27 Sep 2017 09:27:44 +0200 Subject: [PATCH 2/4] Fix Codacy Issue: 'Whitespace found at end of line' Code Style --- app/code/Magento/Sales/Model/Order/Invoice.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/Sales/Model/Order/Invoice.php b/app/code/Magento/Sales/Model/Order/Invoice.php index 84778d71ec2cf..3f2fa1f72f6e5 100644 --- a/app/code/Magento/Sales/Model/Order/Invoice.php +++ b/app/code/Magento/Sales/Model/Order/Invoice.php @@ -410,7 +410,6 @@ public function cancel() if (!$this->canCancel()) { return $this; } - $order = $this->getOrder(); $order->getPayment()->cancelInvoice($this); foreach ($this->getAllItems() as $item) { From c4e1b29b86d223b381fc2da10d960d4b26b3b436 Mon Sep 17 00:00:00 2001 From: Oscar Recio Date: Thu, 5 Oct 2017 20:56:04 +0200 Subject: [PATCH 3/4] Add Test to cover cancel Invoice --- .../Test/Unit/Model/Order/InvoiceTest.php | 68 +++++++++++++++++-- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php index 0962e32dfb6ed..f046a95b6cc61 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php @@ -8,12 +8,12 @@ namespace Magento\Sales\Test\Unit\Model\Order; +use Magento\Sales\Api\Data\InvoiceInterface; +use Magento\Sales\Model\Order; use Magento\Sales\Model\Order\Invoice; +use Magento\Sales\Model\ResourceModel\Order\Invoice\Collection as InvoiceCollection; use Magento\Sales\Model\ResourceModel\OrderFactory; -use Magento\Sales\Model\Order; -use Magento\TestFramework\Helper\Bootstrap; use PHPUnit_Framework_MockObject_MockObject as MockObject; -use Magento\Sales\Model\ResourceModel\Order\Invoice\Collection as InvoiceCollection; /** * Class InvoiceTest @@ -72,7 +72,7 @@ protected function setUp() ->setMethods( [ 'getPayment', '__wakeup', 'load', 'setHistoryEntityName', 'getStore', 'getBillingAddress', - 'getShippingAddress' + 'getShippingAddress', 'getConfig', ] ) ->getMock(); @@ -83,7 +83,7 @@ protected function setUp() $this->paymentMock = $this->getMockBuilder( \Magento\Sales\Model\Order\Payment::class )->disableOriginalConstructor()->setMethods( - ['canVoid', '__wakeup', 'canCapture', 'capture', 'pay'] + ['canVoid', '__wakeup', 'canCapture', 'capture', 'pay', 'cancelInvoice'] )->getMock(); $this->orderFactory = $this->createPartialMock(\Magento\Sales\Model\OrderFactory::class, ['create']); @@ -407,4 +407,62 @@ private function getOrderInvoiceCollection() return $collection; } + + /** + * Assert open invoice can be canceled, and its status changes + */ + public function testCancelOpenInvoice() + { + $orderConfigMock = $this->getMockBuilder(\Magento\Sales\Model\Order\Config::class) + ->disableOriginalConstructor()->setMethods( + ['getStateDefaultStatus'] + )->getMock(); + + $orderConfigMock->expects($this->once())->method('getStateDefaultStatus') + ->with(Order::STATE_PROCESSING) + ->willReturn(Order::STATE_PROCESSING); + + $this->order->expects($this->once())->method('getPayment')->willReturn($this->paymentMock); + $this->order->expects($this->once())->method('getConfig')->willReturn($orderConfigMock); + + $this->paymentMock->expects($this->once())->method('cancelInvoice')->willReturn($this->paymentMock); + + $this->eventManagerMock->expects($this->once())->method('dispatch')->with('sales_order_invoice_cancel'); + + $this->model->setData(InvoiceInterface::ITEMS, []); + $this->model->setState(Invoice::STATE_OPEN); + $this->model->cancel(); + + self::assertEquals(Invoice::STATE_CANCELED, $this->model->getState()); + } + + /** + * Assert open invoice can be canceled, and its status changes + * + * @param $initialInvoiceStatus + * @param $expectedInvoiceStatus + * @dataProvider getNotOpenedInvoiceStatuses + */ + public function testCannotCancelNotOpenedInvoice($initialInvoiceStatus, $expectedInvoiceStatus) + { + $this->order->expects($this->never())->method('getPayment'); + $this->paymentMock->expects($this->never())->method('cancelInvoice'); + $this->eventManagerMock->expects($this->never())->method('dispatch')->with('sales_order_invoice_cancel'); + + $this->model->setState($initialInvoiceStatus); + $this->model->cancel(); + + self::assertEquals($expectedInvoiceStatus, $this->model->getState()); + } + + /** + * @return array + */ + public function getNotOpenedInvoiceStatuses() + { + return [ + [Invoice::STATE_PAID, Invoice::STATE_PAID], + [Invoice::STATE_CANCELED, Invoice::STATE_CANCELED], + ]; + } } From 8965b5e318eb7273e9e049eb4bc7b0771d95a656 Mon Sep 17 00:00:00 2001 From: Oscar Recio Date: Thu, 5 Oct 2017 22:27:43 +0200 Subject: [PATCH 4/4] Fix Code Style and Codacy Variable too long --- .../Test/Unit/Model/Order/InvoiceTest.php | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php index f046a95b6cc61..a115f22dd548e 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/InvoiceTest.php @@ -425,9 +425,13 @@ public function testCancelOpenInvoice() $this->order->expects($this->once())->method('getPayment')->willReturn($this->paymentMock); $this->order->expects($this->once())->method('getConfig')->willReturn($orderConfigMock); - $this->paymentMock->expects($this->once())->method('cancelInvoice')->willReturn($this->paymentMock); + $this->paymentMock->expects($this->once()) + ->method('cancelInvoice') + ->willReturn($this->paymentMock); - $this->eventManagerMock->expects($this->once())->method('dispatch')->with('sales_order_invoice_cancel'); + $this->eventManagerMock->expects($this->once()) + ->method('dispatch') + ->with('sales_order_invoice_cancel'); $this->model->setData(InvoiceInterface::ITEMS, []); $this->model->setState(Invoice::STATE_OPEN); @@ -440,19 +444,21 @@ public function testCancelOpenInvoice() * Assert open invoice can be canceled, and its status changes * * @param $initialInvoiceStatus - * @param $expectedInvoiceStatus + * @param $finalInvoiceStatus * @dataProvider getNotOpenedInvoiceStatuses */ - public function testCannotCancelNotOpenedInvoice($initialInvoiceStatus, $expectedInvoiceStatus) + public function testCannotCancelNotOpenedInvoice($initialInvoiceStatus, $finalInvoiceStatus) { $this->order->expects($this->never())->method('getPayment'); $this->paymentMock->expects($this->never())->method('cancelInvoice'); - $this->eventManagerMock->expects($this->never())->method('dispatch')->with('sales_order_invoice_cancel'); + $this->eventManagerMock->expects($this->never()) + ->method('dispatch') + ->with('sales_order_invoice_cancel'); $this->model->setState($initialInvoiceStatus); $this->model->cancel(); - self::assertEquals($expectedInvoiceStatus, $this->model->getState()); + self::assertEquals($finalInvoiceStatus, $this->model->getState()); } /**