Skip to content

Commit 8fcac9d

Browse files
engcom-Charlieengcom-Charlie
committed
fix iterate on null for getActiveCarriers, add unit test
1 parent 73edcb1 commit 8fcac9d

File tree

2 files changed

+183
-13
lines changed

2 files changed

+183
-13
lines changed

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

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,21 @@
44
* See COPYING.txt for license details.
55
*/
66

7+
declare(strict_types=1);
8+
79
namespace Magento\Shipping\Model;
810

11+
use Magento\Framework\App\Config\ScopeConfigInterface;
12+
use Magento\Framework\DataObject;
913
use Magento\Shipping\Model\Carrier\AbstractCarrierInterface;
14+
use Magento\Store\Model\ScopeInterface;
1015

1116
/**
12-
* Class Config
17+
* Config model for shipping
1318
* @api
1419
* @since 100.0.2
1520
*/
16-
class Config extends \Magento\Framework\DataObject
21+
class Config extends DataObject
1722
{
1823
/**
1924
* Shipping origin settings
@@ -29,25 +34,25 @@ class Config extends \Magento\Framework\DataObject
2934
/**
3035
* Core store config
3136
*
32-
* @var \Magento\Framework\App\Config\ScopeConfigInterface
37+
* @var ScopeConfigInterface
3338
*/
3439
protected $_scopeConfig;
3540

3641
/**
37-
* @var \Magento\Shipping\Model\CarrierFactory
42+
* @var CarrierFactory
3843
*/
3944
protected $_carrierFactory;
4045

4146
/**
4247
* Constructor
4348
*
44-
* @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
45-
* @param \Magento\Shipping\Model\CarrierFactory $carrierFactory
49+
* @param ScopeConfigInterface $scopeConfig
50+
* @param CarrierFactory $carrierFactory
4651
* @param array $data
4752
*/
4853
public function __construct(
49-
\Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig,
50-
\Magento\Shipping\Model\CarrierFactory $carrierFactory,
54+
ScopeConfigInterface $scopeConfig,
55+
CarrierFactory $carrierFactory,
5156
array $data = []
5257
) {
5358
$this->_scopeConfig = $scopeConfig;
@@ -61,14 +66,14 @@ public function __construct(
6166
* @param mixed $store
6267
* @return AbstractCarrierInterface[]
6368
*/
64-
public function getActiveCarriers($store = null)
69+
public function getActiveCarriers($store = null): array
6570
{
6671
$carriers = [];
67-
$config = $this->_scopeConfig->getValue('carriers', \Magento\Store\Model\ScopeInterface::SCOPE_STORE, $store);
72+
$config = $this->getCarriersConfig($store);
6873
foreach (array_keys($config) as $carrierCode) {
6974
if ($this->_scopeConfig->isSetFlag(
7075
'carriers/' . $carrierCode . '/active',
71-
\Magento\Store\Model\ScopeInterface::SCOPE_STORE,
76+
ScopeInterface::SCOPE_STORE,
7277
$store
7378
)) {
7479
$carrierModel = $this->_carrierFactory->create($carrierCode, $store);
@@ -77,6 +82,7 @@ public function getActiveCarriers($store = null)
7782
}
7883
}
7984
}
85+
8086
return $carriers;
8187
}
8288

@@ -86,16 +92,28 @@ public function getActiveCarriers($store = null)
8692
* @param mixed $store
8793
* @return AbstractCarrierInterface[]
8894
*/
89-
public function getAllCarriers($store = null)
95+
public function getAllCarriers($store = null): array
9096
{
9197
$carriers = [];
92-
$config = $this->_scopeConfig->getValue('carriers', \Magento\Store\Model\ScopeInterface::SCOPE_STORE, $store) ?: [];
98+
$config = $this->getCarriersConfig($store);
9399
foreach (array_keys($config) as $carrierCode) {
94100
$model = $this->_carrierFactory->create($carrierCode, $store);
95101
if ($model) {
96102
$carriers[$carrierCode] = $model;
97103
}
98104
}
105+
99106
return $carriers;
100107
}
108+
109+
/**
110+
* Returns carriers config by store
111+
*
112+
* @param mixed $store
113+
* @return array
114+
*/
115+
private function getCarriersConfig($store = null): array
116+
{
117+
return $this->_scopeConfig->getValue('carriers', ScopeInterface::SCOPE_STORE, $store) ?: [];
118+
}
101119
}

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
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\Shipping\Test\Unit\Model;
9+
10+
use Magento\Framework\App\Config\ScopeConfigInterface;
11+
use Magento\Shipping\Model\CarrierFactory;
12+
use Magento\Store\Model\ScopeInterface;
13+
use PHPUnit\Framework\MockObject\MockObject;
14+
use PHPUnit\Framework\TestCase;
15+
use Magento\Shipping\Model\Config;
16+
17+
/**
18+
* Test for \Magento\Shipping\Model\Config.
19+
*/
20+
class ConfigTest extends TestCase
21+
{
22+
private const STUB_STORE_CODE = 'default';
23+
24+
/**
25+
* @var array
26+
*/
27+
private $shippingCarriersData = [
28+
'flatrate' => [
29+
'active' => '1',
30+
'name' => 'Fixed',
31+
'title' => 'Flat Rate',
32+
],
33+
'tablerate' => [
34+
'active' => '0',
35+
'name' => 'Table Rate',
36+
'title' => 'Best Way',
37+
]
38+
];
39+
40+
/**
41+
* @var Config
42+
*/
43+
private $model;
44+
45+
/**
46+
* @var ScopeConfigInterface|MockObject
47+
*/
48+
private $scopeConfigMock;
49+
50+
/**
51+
* @var CarrierFactory|MockObject
52+
*/
53+
private $carrierFactoryMock;
54+
55+
/**
56+
* @inheritdoc
57+
*/
58+
protected function setUp(): void
59+
{
60+
$this->scopeConfigMock = $this->createMock(ScopeConfigInterface::class);
61+
$this->carrierFactoryMock = $this->createMock(CarrierFactory::class);
62+
63+
$this->model = new Config($this->scopeConfigMock, $this->carrierFactoryMock, []);
64+
}
65+
66+
/**
67+
* Get active carriers when there is no active on the store
68+
*
69+
* @return void
70+
*/
71+
public function testGetActiveCarriersWhenThereIsNoAvailable(): void
72+
{
73+
$this->scopeConfigMock->expects($this->once())
74+
->method('getValue')
75+
->with('carriers', ScopeInterface::SCOPE_STORE, null)
76+
->willReturn(null);
77+
78+
$this->assertEquals([], $this->model->getActiveCarriers());
79+
}
80+
81+
/**
82+
* Test for getActiveCarriers
83+
*
84+
* @return void
85+
*/
86+
public function testGetActiveCarriers(): void
87+
{
88+
$this->scopeConfigMock->expects($this->once())
89+
->method('getValue')
90+
->with('carriers', ScopeInterface::SCOPE_STORE, self::STUB_STORE_CODE)
91+
->willReturn($this->shippingCarriersData);
92+
93+
$this->scopeConfigMock->expects($this->exactly(2))
94+
->method('isSetFlag')
95+
->withConsecutive(
96+
['carriers/flatrate/active', ScopeInterface::SCOPE_STORE, self::STUB_STORE_CODE],
97+
['carriers/tablerate/active', ScopeInterface::SCOPE_STORE, self::STUB_STORE_CODE],
98+
)
99+
->willReturnOnConsecutiveCalls(
100+
true,
101+
false,
102+
);
103+
104+
$this->carrierFactoryMock->expects($this->once())
105+
->method('create')
106+
->with('flatrate', self::STUB_STORE_CODE)
107+
->willReturn(true);
108+
109+
$this->assertEquals(['flatrate' => true], $this->model->getActiveCarriers(self::STUB_STORE_CODE));
110+
}
111+
112+
/**
113+
* Get all carriers when there is no carriers available on the store
114+
*
115+
* @return void
116+
*/
117+
public function testGetAllCarriersWhenThereIsNoAvailable(): void
118+
{
119+
$this->scopeConfigMock->expects($this->once())
120+
->method('getValue')
121+
->with('carriers', ScopeInterface::SCOPE_STORE, null)
122+
->willReturn(null);
123+
124+
$this->assertEquals([], $this->model->getAllCarriers());
125+
}
126+
127+
/**
128+
* Test for getAllCarriers
129+
*
130+
* @return void
131+
*/
132+
public function testGetAllCarriers(): void
133+
{
134+
$this->scopeConfigMock->expects($this->once())
135+
->method('getValue')
136+
->with('carriers', ScopeInterface::SCOPE_STORE, self::STUB_STORE_CODE)
137+
->willReturn($this->shippingCarriersData);
138+
139+
$this->carrierFactoryMock->expects($this->exactly(2))
140+
->method('create')
141+
->withConsecutive(
142+
['flatrate', self::STUB_STORE_CODE],
143+
['tablerate', self::STUB_STORE_CODE],
144+
)
145+
->willReturnOnConsecutiveCalls(
146+
true,
147+
false,
148+
);
149+
150+
$this->assertEquals(['flatrate' => true], $this->model->getAllCarriers(self::STUB_STORE_CODE));
151+
}
152+
}

0 commit comments

Comments
 (0)