Skip to content

Commit b3d9a05

Browse files
joanhejoanhe
authored
Merge pull request #3581 from magento-borg/MC-5570
[borg] MC-5570: Different Order Status of two created credit memo orders
2 parents 9291b8a + 5d742e8 commit b3d9a05

File tree

11 files changed

+146
-148
lines changed

11 files changed

+146
-148
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\Order\ProductOption;
@@ -764,13 +765,25 @@ public function canShip()
764765
}
765766

766767
foreach ($this->getAllItems() as $item) {
767-
if ($item->getQtyToShip() > 0 && !$item->getIsVirtual() && !$item->getLockedDoShip()) {
768+
if ($item->getQtyToShip() > 0 && !$item->getIsVirtual() &&
769+
!$item->getLockedDoShip() && !$this->isRefunded($item)) {
768770
return true;
769771
}
770772
}
771773
return false;
772774
}
773775

776+
/**
777+
* Check if item is refunded.
778+
*
779+
* @param OrderItemInterface $item
780+
* @return bool
781+
*/
782+
private function isRefunded(OrderItemInterface $item)
783+
{
784+
return $item->getQtyRefunded() == $item->getQtyOrdered();
785+
}
786+
774787
/**
775788
* Retrieve order edit availability
776789
*

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

Lines changed: 14 additions & 17 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,24 +23,21 @@ 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-
}
39-
}
40-
}
41-
if ($order->getState() == Order::STATE_NEW && $order->getIsInProcess()) {
26+
$currentState = $order->getState();
27+
if ($currentState == Order::STATE_NEW && $order->getIsInProcess()) {
4228
$order->setState(Order::STATE_PROCESSING)
4329
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_PROCESSING));
30+
$currentState = Order::STATE_PROCESSING;
31+
}
32+
33+
if (!$order->isCanceled() && !$order->canUnhold() && !$order->canInvoice()) {
34+
if (in_array($currentState, [Order::STATE_PROCESSING, Order::STATE_COMPLETE]) && !$order->canCreditmemo()) {
35+
$order->setState(Order::STATE_CLOSED)
36+
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_CLOSED));
37+
} elseif ($currentState === Order::STATE_PROCESSING && !$order->canShip()) {
38+
$order->setState(Order::STATE_COMPLETE)
39+
->setStatus($order->getConfig()->getStateDefaultStatus(Order::STATE_COMPLETE));
40+
}
4441
}
4542
return $this;
4643
}

‎app/code/Magento/Sales/Test/Mftf/Test/CreditMemoTotalAfterShippingDiscountTest.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@
124124

125125
<click selector="{{AdminInvoiceMainActionsSection.submitInvoice}}" stepKey="clickSubmitInvoice"/>
126126
<see selector="{{AdminOrderDetailsMessagesSection.successMessage}}" userInput="The invoice has been created." stepKey="seeSuccessMessage1"/>
127+
<see selector="{{AdminOrderDetailsInformationSection.orderStatus}}" userInput="Processing" stepKey="seeOrderProcessing"/>
127128

128129
<!--Create Credit Memo-->
129130
<comment userInput="Admin creates credit memo" stepKey="createCreditMemoComment"/>

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

Lines changed: 77 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ class StateTest extends \PHPUnit\Framework\TestCase
2424

2525
protected function setUp()
2626
{
27-
$this->orderMock = $this->createPartialMock(\Magento\Sales\Model\Order::class, [
27+
$this->orderMock = $this->createPartialMock(
28+
\Magento\Sales\Model\Order::class,
29+
[
2830
'__wakeup',
2931
'getId',
3032
'hasCustomerNoteNotify',
@@ -35,13 +37,12 @@ protected function setUp()
3537
'canShip',
3638
'getBaseGrandTotal',
3739
'canCreditmemo',
38-
'getState',
39-
'setState',
4040
'getTotalRefunded',
4141
'hasForcedCanCreditmemo',
4242
'getIsInProcess',
4343
'getConfig',
44-
]);
44+
]
45+
);
4546
$this->orderMock->expects($this->any())
4647
->method('getConfig')
4748
->willReturnSelf();
@@ -53,127 +54,88 @@ protected function setUp()
5354
}
5455

5556
/**
56-
* test check order - order without id
57-
*/
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
57+
* @param bool $isCanceled
58+
* @param bool $canUnhold
59+
* @param bool $canInvoice
60+
* @param bool $canShip
61+
* @param int $callCanSkipNum
62+
* @param bool $canCreditmemo
63+
* @param int $callCanCreditmemoNum
64+
* @param string $currentState
65+
* @param string $expectedState
66+
* @param int $callSetStateNum
67+
* @dataProvider stateCheckDataProvider
68+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
7169
*/
72-
public function testCheckSetStateComplete()
73-
{
70+
public function testCheck(
71+
bool $canCreditmemo,
72+
int $callCanCreditmemoNum,
73+
bool $canShip,
74+
int $callCanSkipNum,
75+
string $currentState,
76+
string $expectedState = '',
77+
bool $isInProcess = false,
78+
int $callGetIsInProcessNum = 0,
79+
bool $isCanceled = false,
80+
bool $canUnhold = false,
81+
bool $canInvoice = false
82+
) {
83+
$this->orderMock->setState($currentState);
7484
$this->orderMock->expects($this->any())
75-
->method('getId')
76-
->will($this->returnValue(1));
77-
$this->orderMock->expects($this->once())
7885
->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-
{
86+
->willReturn($isCanceled);
11087
$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())
11788
->method('canUnhold')
118-
->will($this->returnValue(false));
119-
$this->orderMock->expects($this->once())
89+
->willReturn($canUnhold);
90+
$this->orderMock->expects($this->any())
12091
->method('canInvoice')
121-
->will($this->returnValue(false));
122-
$this->orderMock->expects($this->once())
92+
->willReturn($canInvoice);
93+
$this->orderMock->expects($this->exactly($callCanSkipNum))
12394
->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())
95+
->willReturn($canShip);
96+
$this->orderMock->expects($this->exactly($callCanCreditmemoNum))
12997
->method('canCreditmemo')
130-
->will($this->returnValue(false));
131-
$this->orderMock->expects($this->exactly(2))
132-
->method('getTotalRefunded')
133-
->will($this->returnValue(null));
134-
$this->orderMock->expects($this->once())
135-
->method('hasForcedCanCreditmemo')
136-
->will($this->returnValue(true));
137-
$this->orderMock->expects($this->exactly(2))
138-
->method('getState')
139-
->will($this->returnValue(Order::STATE_PROCESSING));
140-
$this->orderMock->expects($this->once())
141-
->method('setState')
142-
->with(Order::STATE_CLOSED)
143-
->will($this->returnSelf());
144-
$this->assertEquals($this->state, $this->state->check($this->orderMock));
98+
->willReturn($canCreditmemo);
99+
$this->orderMock->expects($this->exactly($callGetIsInProcessNum))
100+
->method('getIsInProcess')
101+
->willReturn($isInProcess);
102+
$this->state->check($this->orderMock);
103+
$this->assertEquals($expectedState, $this->orderMock->getState());
145104
}
146105

147-
/**
148-
* test check order - set state processing
149-
*/
150-
public function testCheckSetStateProcessing()
106+
public function stateCheckDataProvider()
151107
{
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));
108+
return [
109+
'processing - !canCreditmemo!canShip -> closed' =>
110+
[false, 1, false, 0, Order::STATE_PROCESSING, Order::STATE_CLOSED],
111+
'complete - !canCreditmemo,!canShip -> closed' =>
112+
[false, 1, false, 0, Order::STATE_COMPLETE, Order::STATE_CLOSED],
113+
'processing - !canCreditmemo,canShip -> closed' =>
114+
[false, 1, true, 0, Order::STATE_PROCESSING, Order::STATE_CLOSED],
115+
'complete - !canCreditmemo,canShip -> closed' =>
116+
[false, 1, true, 0, Order::STATE_COMPLETE, Order::STATE_CLOSED],
117+
'processing - canCreditmemo,!canShip -> complete' =>
118+
[true, 1, false, 1, Order::STATE_PROCESSING, Order::STATE_COMPLETE],
119+
'complete - canCreditmemo,!canShip -> complete' =>
120+
[true, 1, false, 0, Order::STATE_COMPLETE, Order::STATE_COMPLETE],
121+
'processing - canCreditmemo, canShip -> processing' =>
122+
[true, 1, true, 1, Order::STATE_PROCESSING, Order::STATE_PROCESSING],
123+
'complete - canCreditmemo, canShip -> complete' =>
124+
[true, 1, true, 0, Order::STATE_COMPLETE, Order::STATE_COMPLETE],
125+
'new - canCreditmemo, canShip, IsInProcess -> processing' =>
126+
[true, 1, true, 1, Order::STATE_NEW, Order::STATE_PROCESSING, true, 1],
127+
'new - canCreditmemo, !canShip, IsInProcess -> processing' =>
128+
[true, 1, false, 1, Order::STATE_NEW, Order::STATE_COMPLETE, true, 1],
129+
'new - canCreditmemo, canShip, !IsInProcess -> new' =>
130+
[true, 0, true, 0, Order::STATE_NEW, Order::STATE_NEW, false, 1],
131+
'hold - canUnhold -> hold' =>
132+
[true, 0, true, 0, Order::STATE_HOLDED, Order::STATE_HOLDED, false, 0, false, true],
133+
'payment_review - canUnhold -> payment_review' =>
134+
[true, 0, true, 0, Order::STATE_PAYMENT_REVIEW, Order::STATE_PAYMENT_REVIEW, false, 0, false, true],
135+
'pending_payment - canUnhold -> pending_payment' =>
136+
[true, 0, true, 0, Order::STATE_PENDING_PAYMENT, Order::STATE_PENDING_PAYMENT, false, 0, false, true],
137+
'cancelled - isCanceled -> cancelled' =>
138+
[true, 0, true, 0, Order::STATE_HOLDED, Order::STATE_HOLDED, false, 0, true],
139+
];
178140
}
179141
}

‎dev/tests/api-functional/testsuite/Magento/GraphQl/Sales/OrdersTest.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,21 @@ public function testOrdersQuery()
8383
$actualData = $response['customerOrders']['items'];
8484

8585
foreach ($expectedData as $key => $data) {
86-
$this->assertEquals($data['increment_id'], $actualData[$key]['increment_id']);
87-
$this->assertEquals($data['grand_total'], $actualData[$key]['grand_total']);
88-
$this->assertEquals($data['status'], $actualData[$key]['status']);
86+
$this->assertEquals(
87+
$data['increment_id'],
88+
$actualData[$key]['increment_id'],
89+
"increment_id is different than the expected for order - " . $data['increment_id']
90+
);
91+
$this->assertEquals(
92+
$data['grand_total'],
93+
$actualData[$key]['grand_total'],
94+
"grand_total is different than the expected for order - " . $data['increment_id']
95+
);
96+
$this->assertEquals(
97+
$data['status'],
98+
$actualData[$key]['status'],
99+
"status is different than the expected for order - " . $data['increment_id']
100+
);
89101
}
90102
}
91103

‎dev/tests/api-functional/testsuite/Magento/Sales/Service/V1/RefundOrderTest.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
namespace Magento\Sales\Service\V1;
77

8+
use Magento\Sales\Model\Order;
9+
810
/**
911
* API test for creation of Creditmemo for certain Order.
1012
*/
@@ -86,10 +88,10 @@ public function testShortRequest()
8688
'Failed asserting that proper shipping amount of the Order was refunded'
8789
);
8890

89-
$this->assertNotEquals(
90-
$existingOrder->getStatus(),
91+
$this->assertEquals(
92+
Order::STATE_COMPLETE,
9193
$updatedOrder->getStatus(),
92-
'Failed asserting that order status was changed'
94+
'Failed asserting that order status has not changed'
9395
);
9496
} catch (\Magento\Framework\Exception\NoSuchEntityException $e) {
9597
$this->fail('Failed asserting that Creditmemo was created');

dev/tests/functional/tests/app/Magento/Sales/Test/Constraint/AssertOrderCancelMassActionPartialFailMessage.php renamed to dev/tests/functional/tests/app/Magento/Sales/Test/Constraint/AssertOrderCancelMassActionFailMessage.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@
1111
use Magento\Mtf\Constraint\AbstractConstraint;
1212

1313
/**
14-
* Class AssertOrderCancelAndSuccessMassActionFailMessage
14+
* Class AssertOrderCancelMassActionFailMessage
1515
* Assert cancel fail message is displayed on order index page
1616
*/
17-
class AssertOrderCancelMassActionPartialFailMessage extends AbstractConstraint
17+
class AssertOrderCancelMassActionFailMessage extends AbstractConstraint
1818
{
1919
/**
2020
* Text value to be checked

‎dev/tests/functional/tests/app/Magento/Sales/Test/Constraint/AssertOrderStatusIsCorrect.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public function processAssert(
3939
/** @var \Magento\Sales\Test\Block\Adminhtml\Order\View\Tab\Info $infoTab */
4040
$infoTab = $salesOrderView->getOrderForm()->openTab('info')->getTab('info');
4141
\PHPUnit\Framework\Assert::assertEquals(
42-
$infoTab->getOrderStatus(),
43-
$orderStatus
42+
$orderStatus,
43+
$infoTab->getOrderStatus()
4444
);
4545
}
4646

0 commit comments

Comments
 (0)