From 9380c95ba8a12eac0f57b92a682b932b1ceb6a1f Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 19 Oct 2017 13:14:58 +0600 Subject: [PATCH 1/3] Even existing credit memos should be refundable if their state is open Credit memos can have the state open: `\Magento\Sales\Model\Order\Creditmemo::STATE_OPEN`. This means that it is possible to have a creditmemo with an ID which still has to be refunded. I'm creating a module that introduces a validation step for refund payments before the actual refund can take place. It uses the open state of credit memos to wait for approval and then refunds the creditmemo. Right now the credit memo is not refundable once it has an ID. I think this is incorrect in case of open credit memos. --- app/code/Magento/Sales/Model/Service/CreditmemoService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Sales/Model/Service/CreditmemoService.php b/app/code/Magento/Sales/Model/Service/CreditmemoService.php index 2f08c26de9058..c7541e6cb7e48 100644 --- a/app/code/Magento/Sales/Model/Service/CreditmemoService.php +++ b/app/code/Magento/Sales/Model/Service/CreditmemoService.php @@ -195,7 +195,7 @@ public function refund( */ protected function validateForRefund(\Magento\Sales\Api\Data\CreditmemoInterface $creditmemo) { - if ($creditmemo->getId()) { + if ($creditmemo->getId() && $creditmemo->getState() !== \Magento\Sales\Model\Order\Creditmemo::STATE_OPEN) { throw new \Magento\Framework\Exception\LocalizedException( __('We cannot register an existing credit memo.') ); From 8a6c3f2d833b9ec4a9cea76c3926b1a193a038f7 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Sun, 22 Oct 2017 13:54:46 +0600 Subject: [PATCH 2/3] Loose integer check for creditmemo state `$creditmemo->getState() = "1"` `Creditmemo::STATE_OPEN = 1` --- app/code/Magento/Sales/Model/Service/CreditmemoService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Sales/Model/Service/CreditmemoService.php b/app/code/Magento/Sales/Model/Service/CreditmemoService.php index c7541e6cb7e48..24f56c0dbd595 100644 --- a/app/code/Magento/Sales/Model/Service/CreditmemoService.php +++ b/app/code/Magento/Sales/Model/Service/CreditmemoService.php @@ -195,7 +195,7 @@ public function refund( */ protected function validateForRefund(\Magento\Sales\Api\Data\CreditmemoInterface $creditmemo) { - if ($creditmemo->getId() && $creditmemo->getState() !== \Magento\Sales\Model\Order\Creditmemo::STATE_OPEN) { + if ($creditmemo->getId() && $creditmemo->getState() != \Magento\Sales\Model\Order\Creditmemo::STATE_OPEN) { throw new \Magento\Framework\Exception\LocalizedException( __('We cannot register an existing credit memo.') ); From 480f8ae7f4948918cffcf9d63838a584dce9b5b4 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Tue, 24 Oct 2017 17:06:17 +0600 Subject: [PATCH 3/3] add tests for refunding pending credit memos --- .../Model/Service/CreditmemoServiceTest.php | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/app/code/Magento/Sales/Test/Unit/Model/Service/CreditmemoServiceTest.php b/app/code/Magento/Sales/Test/Unit/Model/Service/CreditmemoServiceTest.php index 9ecab6cf9ab52..2e668f0b0d6f1 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Service/CreditmemoServiceTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Service/CreditmemoServiceTest.php @@ -243,6 +243,78 @@ public function testRefund() $this->assertSame($creditMemoMock, $this->creditmemoService->refund($creditMemoMock, true)); } + public function testRefundPendingCreditMemo() + { + $creditMemoMock = $this->getMockBuilder(\Magento\Sales\Api\Data\CreditmemoInterface::class) + ->setMethods(['getId', 'getOrder', 'getState', 'getInvoice']) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $creditMemoMock->expects($this->once())->method('getId')->willReturn(444); + $creditMemoMock->expects($this->once())->method('getState') + ->willReturn(\Magento\Sales\Model\Order\Creditmemo::STATE_OPEN); + $orderMock = $this->getMockBuilder(Order::class)->disableOriginalConstructor()->getMock(); + + $creditMemoMock->expects($this->atLeastOnce())->method('getOrder')->willReturn($orderMock); + $orderMock->expects($this->once())->method('getBaseTotalRefunded')->willReturn(0); + $orderMock->expects($this->once())->method('getBaseTotalPaid')->willReturn(10); + $creditMemoMock->expects($this->once())->method('getBaseGrandTotal')->willReturn(10); + + $this->priceCurrencyMock->expects($this->any()) + ->method('round') + ->willReturnArgument(0); + + // Set payment adapter dependency + $refundAdapterMock = $this->getMockBuilder(\Magento\Sales\Model\Order\RefundAdapterInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->objectManagerHelper->setBackwardCompatibleProperty( + $this->creditmemoService, + 'refundAdapter', + $refundAdapterMock + ); + + // Set resource dependency + $resourceMock = $this->getMockBuilder(\Magento\Framework\App\ResourceConnection::class) + ->disableOriginalConstructor() + ->getMock(); + $this->objectManagerHelper->setBackwardCompatibleProperty( + $this->creditmemoService, + 'resource', + $resourceMock + ); + + // Set order repository dependency + $orderRepositoryMock = $this->getMockBuilder(\Magento\Sales\Api\OrderRepositoryInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->objectManagerHelper->setBackwardCompatibleProperty( + $this->creditmemoService, + 'orderRepository', + $orderRepositoryMock + ); + + $adapterMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\AdapterInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $resourceMock->expects($this->once())->method('getConnection')->with('sales')->willReturn($adapterMock); + $adapterMock->expects($this->once())->method('beginTransaction'); + $refundAdapterMock->expects($this->once()) + ->method('refund') + ->with($creditMemoMock, $orderMock, false) + ->willReturn($orderMock); + $orderRepositoryMock->expects($this->once()) + ->method('save') + ->with($orderMock); + $creditMemoMock->expects($this->once()) + ->method('getInvoice') + ->willReturn(null); + $adapterMock->expects($this->once())->method('commit'); + $this->creditmemoRepositoryMock->expects($this->once()) + ->method('save'); + + $this->assertSame($creditMemoMock, $this->creditmemoService->refund($creditMemoMock, true)); + } + /** * @expectedExceptionMessage The most money available to refund is 1. * @expectedException \Magento\Framework\Exception\LocalizedException