Skip to content

Commit 387b0e7

Browse files
author
Joan He
committed
MC-5570: Different Order Status of two created credit memo orders
1 parent c49da73 commit 387b0e7

File tree

3 files changed

+96
-123
lines changed

3 files changed

+96
-123
lines changed

‎app/code/Magento/Sales/Model/Order.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Framework\Locale\ResolverInterface;
1212
use Magento\Framework\Pricing\PriceCurrencyInterface;
1313
use Magento\Sales\Api\Data\OrderInterface;
14+
use Magento\Sales\Api\Data\OrderItemInterface;
1415
use Magento\Sales\Api\Data\OrderStatusHistoryInterface;
1516
use Magento\Sales\Model\Order\Payment;
1617
use Magento\Sales\Model\ResourceModel\Order\Address\Collection;
@@ -761,13 +762,25 @@ public function canShip()
761762
}
762763

763764
foreach ($this->getAllItems() as $item) {
764-
if ($item->getQtyToShip() > 0 && !$item->getIsVirtual() && !$item->getLockedDoShip()) {
765+
if ($item->getQtyToShip() > 0 && !$item->getIsVirtual() &&
766+
!$item->getLockedDoShip() && !$this->isRefunded($item)) {
765767
return true;
766768
}
767769
}
768770
return false;
769771
}
770772

773+
/**
774+
* Check if item is refunded.
775+
*
776+
* @param OrderItemInterface $item
777+
* @return bool
778+
*/
779+
private function isRefunded(OrderItemInterface $item)
780+
{
781+
return $item->getQtyRefunded() == $item->getQtyOrdered();
782+
}
783+
771784
/**
772785
* Retrieve order edit availability
773786
*

‎app/code/Magento/Sales/Model/ResourceModel/Order/Handler/State.php

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
class State
1515
{
1616
/**
17-
* Check order status before save
17+
* Check order status and adjust the status before save
1818
*
1919
* @param Order $order
2020
* @return $this
@@ -23,22 +23,18 @@ class State
2323
*/
2424
public function check(Order $order)
2525
{
26-
if (!$order->isCanceled() && !$order->canUnhold() && !$order->canInvoice() && !$order->canShip()) {
27-
if (0 == $order->getBaseGrandTotal() || $order->canCreditmemo()) {
28-
if ($order->getState() !== Order::STATE_COMPLETE) {
29-
$order->setState(Order::STATE_COMPLETE)
30-
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_COMPLETE));
31-
}
32-
} elseif ((float)$order->getTotalRefunded()
33-
|| !$order->getTotalRefunded() && $order->hasForcedCanCreditmemo()
34-
) {
35-
if ($order->getState() !== Order::STATE_CLOSED) {
36-
$order->setState(Order::STATE_CLOSED)
37-
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_CLOSED));
38-
}
26+
$currentState = $order->getState();
27+
if (!$order->isCanceled() && !$order->canUnhold() && !$order->canInvoice()) {
28+
if (in_array($currentState, [Order::STATE_PROCESSING, Order::STATE_COMPLETE]) && !$order->canCreditmemo()) {
29+
$order->setState(Order::STATE_CLOSED)
30+
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_CLOSED));
31+
} elseif ($currentState === Order::STATE_PROCESSING && !$order->canShip()) {
32+
$order->setState(Order::STATE_COMPLETE)
33+
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_COMPLETE));
3934
}
4035
}
41-
if ($order->getState() == Order::STATE_NEW && $order->getIsInProcess()) {
36+
37+
if ($currentState == Order::STATE_NEW && $order->getIsInProcess()) {
4238
$order->setState(Order::STATE_PROCESSING)
4339
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_PROCESSING));
4440
}

‎app/code/Magento/Sales/Test/Unit/Model/ResourceModel/Order/Handler/StateTest.php

Lines changed: 71 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -53,127 +53,91 @@ protected function setUp()
5353
}
5454

5555
/**
56-
* test check order - order without id
56+
* @param bool $isCanceled
57+
* @param bool $canUnhold
58+
* @param bool $canInvoice
59+
* @param bool $canShip
60+
* @param int $callCanSkipNum
61+
* @param bool $canCreditmemo
62+
* @param int $callCanCreditmemoNum
63+
* @param string $currentState
64+
* @param string $expectedState
65+
* @param int $callSetStateNum
66+
* @dataProvider stateCheckDataProvider
5767
*/
58-
public function testCheckOrderEmpty()
59-
{
60-
$this->orderMock->expects($this->once())
61-
->method('getBaseGrandTotal')
62-
->willReturn(100);
63-
$this->orderMock->expects($this->never())
64-
->method('setState');
65-
66-
$this->state->check($this->orderMock);
67-
}
68-
69-
/**
70-
* test check order - set state complete
71-
*/
72-
public function testCheckSetStateComplete()
73-
{
68+
public function testCheck(
69+
bool $canCreditmemo,
70+
int $callCanCreditmemoNum,
71+
bool $canShip,
72+
int $callCanSkipNum,
73+
string $currentState,
74+
string $expectedState = '',
75+
int $callSetStateNum = 0,
76+
bool $isInProcess = false,
77+
int $callGetIsInProcessNum = 0,
78+
bool $isCanceled = false,
79+
bool $canUnhold = false,
80+
bool $canInvoice = false
81+
) {
7482
$this->orderMock->expects($this->any())
75-
->method('getId')
76-
->will($this->returnValue(1));
77-
$this->orderMock->expects($this->once())
7883
->method('isCanceled')
79-
->will($this->returnValue(false));
80-
$this->orderMock->expects($this->once())
81-
->method('canUnhold')
82-
->will($this->returnValue(false));
83-
$this->orderMock->expects($this->once())
84-
->method('canInvoice')
85-
->will($this->returnValue(false));
86-
$this->orderMock->expects($this->once())
87-
->method('canShip')
88-
->will($this->returnValue(false));
89-
$this->orderMock->expects($this->once())
90-
->method('getBaseGrandTotal')
91-
->will($this->returnValue(100));
92-
$this->orderMock->expects($this->once())
93-
->method('canCreditmemo')
94-
->will($this->returnValue(true));
95-
$this->orderMock->expects($this->exactly(2))
96-
->method('getState')
97-
->will($this->returnValue(Order::STATE_PROCESSING));
98-
$this->orderMock->expects($this->once())
99-
->method('setState')
100-
->with(Order::STATE_COMPLETE)
101-
->will($this->returnSelf());
102-
$this->assertEquals($this->state, $this->state->check($this->orderMock));
103-
}
104-
105-
/**
106-
* test check order - set state closed
107-
*/
108-
public function testCheckSetStateClosed()
109-
{
84+
->willReturn($isCanceled);
11085
$this->orderMock->expects($this->any())
111-
->method('getId')
112-
->will($this->returnValue(1));
113-
$this->orderMock->expects($this->once())
114-
->method('isCanceled')
115-
->will($this->returnValue(false));
116-
$this->orderMock->expects($this->once())
11786
->method('canUnhold')
118-
->will($this->returnValue(false));
119-
$this->orderMock->expects($this->once())
87+
->willReturn($canUnhold);
88+
$this->orderMock->expects($this->any())
12089
->method('canInvoice')
121-
->will($this->returnValue(false));
122-
$this->orderMock->expects($this->once())
90+
->willReturn($canInvoice);
91+
$this->orderMock->expects($this->exactly($callCanSkipNum))
12392
->method('canShip')
124-
->will($this->returnValue(false));
125-
$this->orderMock->expects($this->once())
126-
->method('getBaseGrandTotal')
127-
->will($this->returnValue(100));
128-
$this->orderMock->expects($this->once())
93+
->willReturn($canShip);
94+
$this->orderMock->expects($this->exactly($callCanCreditmemoNum))
12995
->method('canCreditmemo')
130-
->will($this->returnValue(false));
131-
$this->orderMock->expects($this->exactly(2))
132-
->method('getTotalRefunded')
133-
->will($this->returnValue(null));
96+
->willReturn($canCreditmemo);
13497
$this->orderMock->expects($this->once())
135-
->method('hasForcedCanCreditmemo')
136-
->will($this->returnValue(true));
137-
$this->orderMock->expects($this->exactly(2))
13898
->method('getState')
139-
->will($this->returnValue(Order::STATE_PROCESSING));
140-
$this->orderMock->expects($this->once())
99+
->willReturn($currentState);
100+
$this->orderMock->expects($this->exactly($callGetIsInProcessNum))
101+
->method('getIsInProcess')
102+
->willReturn($isInProcess);
103+
$this->orderMock->expects($this->exactly($callSetStateNum))
141104
->method('setState')
142-
->with(Order::STATE_CLOSED)
105+
->with($expectedState)
143106
->will($this->returnSelf());
144-
$this->assertEquals($this->state, $this->state->check($this->orderMock));
107+
$this->state->check($this->orderMock);
145108
}
146109

147-
/**
148-
* test check order - set state processing
149-
*/
150-
public function testCheckSetStateProcessing()
110+
public function stateCheckDataProvider()
151111
{
152-
$this->orderMock->expects($this->any())
153-
->method('getId')
154-
->will($this->returnValue(1));
155-
$this->orderMock->expects($this->once())
156-
->method('isCanceled')
157-
->will($this->returnValue(false));
158-
$this->orderMock->expects($this->once())
159-
->method('canUnhold')
160-
->will($this->returnValue(false));
161-
$this->orderMock->expects($this->once())
162-
->method('canInvoice')
163-
->will($this->returnValue(false));
164-
$this->orderMock->expects($this->once())
165-
->method('canShip')
166-
->will($this->returnValue(true));
167-
$this->orderMock->expects($this->once())
168-
->method('getState')
169-
->will($this->returnValue(Order::STATE_NEW));
170-
$this->orderMock->expects($this->once())
171-
->method('getIsInProcess')
172-
->will($this->returnValue(true));
173-
$this->orderMock->expects($this->once())
174-
->method('setState')
175-
->with(Order::STATE_PROCESSING)
176-
->will($this->returnSelf());
177-
$this->assertEquals($this->state, $this->state->check($this->orderMock));
112+
return [
113+
'processing - !canCreditmemo!canShip -> closed' =>
114+
[false, 1, false, 0, Order::STATE_PROCESSING, Order::STATE_CLOSED, 1],
115+
'complete - !canCreditmemo,!canShip -> closed' =>
116+
[false, 1, false, 0, Order::STATE_COMPLETE, Order::STATE_CLOSED, 1],
117+
'processing - !canCreditmemo,canShip -> closed' =>
118+
[false, 1, true, 0, Order::STATE_PROCESSING, Order::STATE_CLOSED, 1],
119+
'complete - !canCreditmemo,canShip -> closed' =>
120+
[false, 1, true, 0, Order::STATE_COMPLETE, Order::STATE_CLOSED, 1],
121+
'processing - canCreditmemo,!canShip -> complete' =>
122+
[true, 1, false, 1, Order::STATE_PROCESSING, Order::STATE_COMPLETE, 1],
123+
'complete - canCreditmemo,!canShip -> complete' =>
124+
[true, 1, false, 0, Order::STATE_COMPLETE],
125+
'processing - canCreditmemo, canShip -> processing' =>
126+
[true, 1, true, 1, Order::STATE_PROCESSING],
127+
'complete - canCreditmemo, canShip -> complete' =>
128+
[true, 1, true, 0, Order::STATE_COMPLETE],
129+
'new - canCreditmemo, canShip, IsInProcess -> processing' =>
130+
[true, 0, true, 0, Order::STATE_NEW, Order::STATE_PROCESSING, 1, true, 1],
131+
'new - canCreditmemo, canShip, !IsInProcess -> new' =>
132+
[true, 0, true, 0, Order::STATE_NEW, '', 0, false, 1],
133+
'hold - canUnhold -> hold' =>
134+
[true, 0, true, 0, Order::STATE_HOLDED, '', 0, false, 0, false, true, false],
135+
'payment_review - canUnhold -> payment_review' =>
136+
[true, 0, true, 0, Order::STATE_PAYMENT_REVIEW, '', 0, false, 0, false, false, false],
137+
'pending_payment - canUnhold -> pending_payment' =>
138+
[true, 0, true, 0, Order::STATE_PENDING_PAYMENT, '', 0, false, 0, false, false, false],
139+
'cancelled - isCanceled -> cancelled' =>
140+
[true, 0, true, 0, Order::STATE_HOLDED, '', 0, false, 0, true, false, false],
141+
];
178142
}
179143
}

0 commit comments

Comments
 (0)