Skip to content

Commit eb3d172

Browse files
viktorpetrykviktorpetryk
committed
MC-19080: Incorrect behavior after shipping methods disabled
1 parent bc97aed commit eb3d172

File tree

4 files changed

+199
-80
lines changed

4 files changed

+199
-80
lines changed

‎app/code/Magento/Shipping/Model/Shipping.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ public function collectRates(\Magento\Quote\Model\Quote\Address\RateRequest $req
273273
*/
274274
private function prepareCarrier(string $carrierCode, RateRequest $request): AbstractCarrier
275275
{
276-
$carrier = $this->isActive($carrierCode)
276+
$carrier = $this->isShippingCarrierAvailable($carrierCode)
277277
? $this->_carrierFactory->create($carrierCode, $request->getStoreId())
278278
: null;
279279
if (!$carrier) {
@@ -363,6 +363,7 @@ public function composePackagesForCarrier($carrier, $request)
363363
{
364364
$allItems = $request->getAllItems();
365365
$fullItems = [];
366+
$weightItems = [];
366367

367368
$maxWeight = (double)$carrier->getConfigData('max_package_weight');
368369

@@ -428,12 +429,13 @@ public function composePackagesForCarrier($carrier, $request)
428429

429430
if (!empty($decimalItems)) {
430431
foreach ($decimalItems as $decimalItem) {
431-
$fullItems[] = array_fill(0, $decimalItem['qty'] * $qty, $decimalItem['weight']);
432+
$weightItems[] = array_fill(0, $decimalItem['qty'] * $qty, $decimalItem['weight']);
432433
}
433434
} else {
434-
$fullItems[] = array_fill(0, $qty, $itemWeight);
435+
$weightItems[] = array_fill(0, $qty, $itemWeight);
435436
}
436437
}
438+
$fullItems = array_merge($fullItems, ...$weightItems);
437439
sort($fullItems);
438440

439441
return $this->_makePieces($fullItems, $maxWeight);
@@ -537,9 +539,9 @@ public function setCarrierAvailabilityConfigField($code = 'active')
537539
* Checks availability of carrier.
538540
*
539541
* @param string $carrierCode
540-
* @return bool|null
542+
* @return bool
541543
*/
542-
private function isActive(string $carrierCode)
544+
private function isShippingCarrierAvailable(string $carrierCode): bool
543545
{
544546
return $this->_scopeConfig->isSetFlag(
545547
'carriers/' . $carrierCode . '/' . $this->_availabilityConfigField,

‎dev/tests/integration/testsuite/Magento/OfflineShipping/Model/CollectRatesTest.php

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\OfflineShipping\Model;
9+
10+
use Magento\Framework\DataObject;
11+
use Magento\Framework\ObjectManagerInterface;
12+
use Magento\Quote\Model\Quote\Address\RateResult\Error;
13+
use Magento\Quote\Model\Quote\Address\RateResult\Method;
14+
use Magento\Shipping\Model\Rate\Result;
15+
use Magento\Shipping\Model\Shipping;
16+
use Magento\TestFramework\Helper\Bootstrap;
17+
18+
/**
19+
* Integration tests for offline shipping carriers.
20+
* @magentoAppIsolation enabled
21+
*/
22+
class CollectRatesTest extends \PHPUnit\Framework\TestCase
23+
{
24+
/**
25+
* @var ObjectManagerInterface
26+
*/
27+
private $objectManager;
28+
29+
/**
30+
* @var Shipping
31+
*/
32+
protected $shipping;
33+
34+
/**
35+
* @var string
36+
*/
37+
protected $carrier = 'flatrate';
38+
39+
/**
40+
* @var string
41+
*/
42+
protected $errorMessage = 'This shipping method is not available. To use this shipping method, please contact us.';
43+
44+
/**
45+
* @inheritdoc
46+
*/
47+
protected function setUp()
48+
{
49+
$this->objectManager = Bootstrap::getObjectManager();
50+
$this->shipping = $this->objectManager->get(Shipping::class);
51+
}
52+
53+
/**
54+
* @magentoConfigFixture default_store carriers/flatrate/active 1
55+
* @magentoConfigFixture default_store carriers/flatrate/sallowspecific 1
56+
* @magentoConfigFixture default_store carriers/flatrate/specificcountry UK
57+
* @magentoConfigFixture default_store carriers/flatrate/showmethod 1
58+
*/
59+
public function testCollectRatesWhenShippingCarrierIsAvailableAndNotApplicable()
60+
{
61+
$result = $this->shipping->collectRatesByAddress($this->getAddress(), $this->carrier);
62+
$rate = $this->getRate($result->getResult());
63+
64+
static::assertEquals($this->carrier, $rate->getData('carrier'));
65+
static::assertEquals($this->errorMessage, $rate->getData('error_message'));
66+
}
67+
68+
/**
69+
* @magentoConfigFixture default_store carriers/flatrate/active 0
70+
* @magentoConfigFixture default_store carriers/flatrate/sallowspecific 1
71+
* @magentoConfigFixture default_store carriers/flatrate/specificcountry UK
72+
* @magentoConfigFixture default_store carriers/flatrate/showmethod 1
73+
*/
74+
public function testCollectRatesWhenShippingCarrierIsNotAvailableAndNotApplicable()
75+
{
76+
$result = $this->shipping->collectRatesByAddress($this->getAddress(), $this->carrier);
77+
$rate = $this->getRate($result->getResult());
78+
79+
static::assertNull($rate);
80+
}
81+
82+
/**
83+
* @return DataObject
84+
*/
85+
private function getAddress(): DataObject
86+
{
87+
$address = $this->objectManager->create(
88+
DataObject::class,
89+
[
90+
'data' => [
91+
'region_id' => 'CA',
92+
'postcode' => '11111',
93+
'lastname' => 'John',
94+
'firstname' => 'Doe',
95+
'street' => 'Some street',
96+
'city' => 'Los Angeles',
97+
'email' => 'john.doe@example.com',
98+
'telephone' => '11111111',
99+
'country_id' => 'US',
100+
'item_qty' => 1,
101+
],
102+
]
103+
);
104+
105+
return $address;
106+
}
107+
108+
/**
109+
* @param Result $result
110+
* @return Method|Error
111+
*/
112+
private function getRate(Result $result)
113+
{
114+
$rates = $result->getAllRates();
115+
116+
return array_pop($rates);
117+
}
118+
}

‎dev/tests/integration/testsuite/Magento/Shipping/Model/ShippingTest.php

Lines changed: 24 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,16 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6-
76
namespace Magento\Shipping\Model;
87

98
use Magento\Framework\DataObject;
109
use Magento\Framework\ObjectManagerInterface;
11-
use Magento\Quote\Model\Quote\Address\RateResult\Error;
1210
use Magento\Quote\Model\Quote\Address\RateResult\Method;
1311
use Magento\Shipping\Model\Rate\Result;
1412
use Magento\TestFramework\Helper\Bootstrap;
1513

1614
/**
17-
* Contains list of tests for Shipping model.
18-
* @magentoAppIsolation enabled
15+
* Contains list of tests for Shipping model
1916
*/
2017
class ShippingTest extends \PHPUnit\Framework\TestCase
2118
{
@@ -45,8 +42,22 @@ protected function setUp()
4542
*/
4643
public function testCollectRatesByAddress()
4744
{
45+
$address = $this->objectManager->create(DataObject::class, [
46+
'data' => [
47+
'region_id' => 'CA',
48+
'postcode' => '11111',
49+
'lastname' => 'John',
50+
'firstname' => 'Doe',
51+
'street' => 'Some street',
52+
'city' => 'Los Angeles',
53+
'email' => 'john.doe@example.com',
54+
'telephone' => '11111111',
55+
'country_id' => 'US',
56+
'item_qty' => 1
57+
]
58+
]);
4859
/** @var Shipping $result */
49-
$result = $this->model->collectRatesByAddress($this->getAddress(), 'flatrate');
60+
$result = $this->model->collectRatesByAddress($address, 'flatrate');
5061
static::assertInstanceOf(Shipping::class, $result);
5162

5263
return $result->getResult();
@@ -57,81 +68,19 @@ public function testCollectRatesByAddress()
5768
* @covers \Magento\Shipping\Model\Shipping::collectRatesByAddress
5869
* @param Result $result
5970
* @depends testCollectRatesByAddress
71+
* @magentoConfigFixture carriers/flatrate/active 1
72+
* @magentoConfigFixture carriers/flatrate/price 5.00
6073
*/
6174
public function testCollectRates(Result $result)
6275
{
63-
$rate = $this->getRate($result);
64-
static::assertInstanceOf(Method::class, $rate);
65-
static::assertEquals('flatrate', $rate->getData('carrier'));
66-
static::assertEquals(5, $rate->getData('price'));
67-
}
76+
$rates = $result->getAllRates();
77+
static::assertNotEmpty($rates);
6878

69-
/**
70-
* @magentoConfigFixture default_store carriers/flatrate/active 1
71-
* @magentoConfigFixture default_store carriers/flatrate/sallowspecific 1
72-
* @magentoConfigFixture default_store carriers/flatrate/specificcountry UK
73-
* @magentoConfigFixture default_store carriers/flatrate/showmethod 1
74-
*/
75-
public function testShippingMethodIsActiveAndNotApplicable()
76-
{
77-
$result = $this->model->collectRatesByAddress($this->getAddress(), 'flatrate');
78-
$rate = $this->getRate($result->getResult());
79+
/** @var Method $rate */
80+
$rate = array_pop($rates);
7981

82+
static::assertInstanceOf(Method::class, $rate);
8083
static::assertEquals('flatrate', $rate->getData('carrier'));
81-
static::assertEquals(
82-
'This shipping method is not available. To use this shipping method, please contact us.',
83-
$rate->getData('error_message')
84-
);
85-
}
86-
87-
/**
88-
* @magentoConfigFixture default_store carriers/flatrate/active 0
89-
* @magentoConfigFixture default_store carriers/flatrate/sallowspecific 1
90-
* @magentoConfigFixture default_store carriers/flatrate/specificcountry UK
91-
* @magentoConfigFixture default_store carriers/flatrate/showmethod 1
92-
*/
93-
public function testShippingMethodIsNotActiveAndNotApplicable()
94-
{
95-
$result = $this->model->collectRatesByAddress($this->getAddress(), 'flatrate');
96-
$rate = $this->getRate($result->getResult());
97-
98-
static::assertNull($rate);
99-
}
100-
101-
/**
102-
* @return DataObject
103-
*/
104-
private function getAddress(): DataObject
105-
{
106-
$address = $this->objectManager->create(
107-
DataObject::class,
108-
[
109-
'data' => [
110-
'region_id' => 'CA',
111-
'postcode' => '11111',
112-
'lastname' => 'John',
113-
'firstname' => 'Doe',
114-
'street' => 'Some street',
115-
'city' => 'Los Angeles',
116-
'email' => 'john.doe@example.com',
117-
'telephone' => '11111111',
118-
'country_id' => 'US',
119-
'item_qty' => 1,
120-
],
121-
]
122-
);
123-
124-
return $address;
125-
}
126-
127-
/**
128-
* @param Result $result
129-
* @return Method|Error
130-
*/
131-
private function getRate(Result $result)
132-
{
133-
$rates = $result->getAllRates();
134-
135-
return array_pop($rates);
84+
static::assertEquals(5, $rate->getData('price'));
13685
}
13786
}

‎dev/tests/integration/testsuite/Magento/Ups/Model/CollectRatesTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Ups\Model;
9+
10+
/**
11+
* Integration tests for online shipping carriers.
12+
* @magentoAppIsolation enabled
13+
*/
14+
class CollectRatesTest extends \Magento\OfflineShipping\Model\CollectRatesTest
15+
{
16+
/**
17+
* @var string
18+
*/
19+
protected $carrier = 'ups';
20+
21+
/**
22+
* @var string
23+
*/
24+
protected $errorMessage = 'This shipping method is currently unavailable. ' .
25+
'If you would like to ship using this shipping method, please contact us.';
26+
27+
/**
28+
* @magentoConfigFixture default_store carriers/ups/active 1
29+
* @magentoConfigFixture default_store carriers/ups/type UPS
30+
* @magentoConfigFixture default_store carriers/ups/sallowspecific 1
31+
* @magentoConfigFixture default_store carriers/ups/specificcountry UK
32+
* @magentoConfigFixture default_store carriers/ups/showmethod 1
33+
*/
34+
public function testCollectRatesWhenShippingCarrierIsAvailableAndNotApplicable()
35+
{
36+
parent::testCollectRatesWhenShippingCarrierIsAvailableAndNotApplicable();
37+
}
38+
39+
/**
40+
* @magentoConfigFixture default_store carriers/ups/active 0
41+
* @magentoConfigFixture default_store carriers/ups/type UPS
42+
* @magentoConfigFixture default_store carriers/ups/sallowspecific 1
43+
* @magentoConfigFixture default_store carriers/ups/specificcountry UK
44+
* @magentoConfigFixture default_store carriers/ups/showmethod 1
45+
*/
46+
public function testCollectRatesWhenShippingCarrierIsNotAvailableAndNotApplicable()
47+
{
48+
parent::testCollectRatesWhenShippingCarrierIsNotAvailableAndNotApplicable();
49+
}
50+
}

0 commit comments

Comments
 (0)