Skip to content

Commit bc97aed

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

File tree

3 files changed

+125
-33
lines changed

3 files changed

+125
-33
lines changed

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

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

89
use Magento\Framework\App\ObjectManager;
@@ -272,7 +273,9 @@ public function collectRates(\Magento\Quote\Model\Quote\Address\RateRequest $req
272273
*/
273274
private function prepareCarrier(string $carrierCode, RateRequest $request): AbstractCarrier
274275
{
275-
$carrier = $this->_carrierFactory->create($carrierCode, $request->getStoreId());
276+
$carrier = $this->isActive($carrierCode)
277+
? $this->_carrierFactory->create($carrierCode, $request->getStoreId())
278+
: null;
276279
if (!$carrier) {
277280
throw new \RuntimeException('Failed to initialize carrier');
278281
}
@@ -425,13 +428,10 @@ public function composePackagesForCarrier($carrier, $request)
425428

426429
if (!empty($decimalItems)) {
427430
foreach ($decimalItems as $decimalItem) {
428-
$fullItems = array_merge(
429-
$fullItems,
430-
array_fill(0, $decimalItem['qty'] * $qty, $decimalItem['weight'])
431-
);
431+
$fullItems[] = array_fill(0, $decimalItem['qty'] * $qty, $decimalItem['weight']);
432432
}
433433
} else {
434-
$fullItems = array_merge($fullItems, array_fill(0, $qty, $itemWeight));
434+
$fullItems[] = array_fill(0, $qty, $itemWeight);
435435
}
436436
}
437437
sort($fullItems);
@@ -532,4 +532,18 @@ public function setCarrierAvailabilityConfigField($code = 'active')
532532
$this->_availabilityConfigField = $code;
533533
return $this;
534534
}
535+
536+
/**
537+
* Checks availability of carrier.
538+
*
539+
* @param string $carrierCode
540+
* @return bool|null
541+
*/
542+
private function isActive(string $carrierCode)
543+
{
544+
return $this->_scopeConfig->isSetFlag(
545+
'carriers/' . $carrierCode . '/' . $this->_availabilityConfigField,
546+
\Magento\Store\Model\ScopeInterface::SCOPE_STORE
547+
);
548+
}
535549
}

‎app/code/Magento/Shipping/Test/Unit/Model/ShippingTest.php

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Shipping\Test\Unit\Model;
78

89
use Magento\Catalog\Model\Product;
910
use Magento\Catalog\Model\Product\Type as ProductType;
1011
use Magento\CatalogInventory\Model\Stock\Item as StockItem;
1112
use Magento\CatalogInventory\Model\StockRegistry;
13+
use Magento\Framework\App\Config\ScopeConfigInterface;
1214
use Magento\Quote\Model\Quote\Item as QuoteItem;
1315
use Magento\Shipping\Model\Carrier\AbstractCarrierInterface;
1416
use Magento\Shipping\Model\CarrierFactory;
@@ -19,12 +21,14 @@
1921
use PHPUnit_Framework_MockObject_MockObject as MockObject;
2022

2123
/**
22-
* @see Shipping
24+
* Unit tests for \Magento\Shipping\Model\Shipping class.
25+
*
26+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2327
*/
2428
class ShippingTest extends \PHPUnit\Framework\TestCase
2529
{
2630
/**
27-
* Test identification number of product
31+
* Test identification number of product.
2832
*
2933
* @var int
3034
*/
@@ -50,22 +54,34 @@ class ShippingTest extends \PHPUnit\Framework\TestCase
5054
*/
5155
private $carrier;
5256

57+
/**
58+
* @var ScopeConfigInterface|MockObject
59+
*/
60+
private $scopeConfig;
61+
62+
/**
63+
* @inheritdoc
64+
*/
5365
protected function setUp()
5466
{
5567
$this->stockRegistry = $this->createMock(StockRegistry::class);
5668
$this->stockItemData = $this->createMock(StockItem::class);
69+
$this->scopeConfig = $this->createMock(ScopeConfigInterface::class);
5770

5871
$this->shipping = (new ObjectManagerHelper($this))->getObject(
5972
Shipping::class,
6073
[
6174
'stockRegistry' => $this->stockRegistry,
6275
'carrierFactory' => $this->getCarrierFactory(),
76+
'scopeConfig' => $this->scopeConfig,
6377
]
6478
);
6579
}
6680

6781
/**
82+
* Compose Packages For Carrier.
6883
*
84+
* @return void
6985
*/
7086
public function testComposePackages()
7187
{
@@ -125,14 +141,25 @@ function ($key) {
125141

126142
/**
127143
* Active flag should be set before collecting carrier rates.
144+
*
145+
* @return void
128146
*/
129147
public function testCollectCarrierRatesSetActiveFlag()
130148
{
149+
$carrierCode = 'carrier';
150+
$scopeStore = 'store';
151+
$this->scopeConfig->expects($this->once())
152+
->method('isSetFlag')
153+
->with(
154+
'carriers/' . $carrierCode . '/active',
155+
$scopeStore
156+
)
157+
->willReturn(true);
131158
$this->carrier->expects($this->atLeastOnce())
132159
->method('setActiveFlag')
133160
->with('active');
134161

135-
$this->shipping->collectCarrierRates('carrier', new RateRequest());
162+
$this->shipping->collectCarrierRates($carrierCode, new RateRequest());
136163
}
137164

138165
/**

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

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

89
use Magento\Framework\DataObject;
910
use Magento\Framework\ObjectManagerInterface;
11+
use Magento\Quote\Model\Quote\Address\RateResult\Error;
1012
use Magento\Quote\Model\Quote\Address\RateResult\Method;
1113
use Magento\Shipping\Model\Rate\Result;
1214
use Magento\TestFramework\Helper\Bootstrap;
1315

1416
/**
15-
* Contains list of tests for Shipping model
17+
* Contains list of tests for Shipping model.
18+
* @magentoAppIsolation enabled
1619
*/
1720
class ShippingTest extends \PHPUnit\Framework\TestCase
1821
{
@@ -42,22 +45,8 @@ protected function setUp()
4245
*/
4346
public function testCollectRatesByAddress()
4447
{
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-
]);
5948
/** @var Shipping $result */
60-
$result = $this->model->collectRatesByAddress($address, 'flatrate');
49+
$result = $this->model->collectRatesByAddress($this->getAddress(), 'flatrate');
6150
static::assertInstanceOf(Shipping::class, $result);
6251

6352
return $result->getResult();
@@ -68,19 +57,81 @@ public function testCollectRatesByAddress()
6857
* @covers \Magento\Shipping\Model\Shipping::collectRatesByAddress
6958
* @param Result $result
7059
* @depends testCollectRatesByAddress
71-
* @magentoConfigFixture carriers/flatrate/active 1
72-
* @magentoConfigFixture carriers/flatrate/price 5.00
7360
*/
7461
public function testCollectRates(Result $result)
7562
{
76-
$rates = $result->getAllRates();
77-
static::assertNotEmpty($rates);
78-
79-
/** @var Method $rate */
80-
$rate = array_pop($rates);
81-
63+
$rate = $this->getRate($result);
8264
static::assertInstanceOf(Method::class, $rate);
8365
static::assertEquals('flatrate', $rate->getData('carrier'));
8466
static::assertEquals(5, $rate->getData('price'));
8567
}
68+
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+
80+
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);
136+
}
86137
}

0 commit comments

Comments
 (0)