Skip to content

Commit 4b81e7b

Browse files
viktorpetrykviktorpetryk
committed
MC-29444: Extra Values of UPS and USPS are in dropdown list Cart Price Rule - Condition - Shipping Method
1 parent 1ad65a3 commit 4b81e7b

File tree

3 files changed

+130
-37
lines changed

3 files changed

+130
-37
lines changed

‎app/code/Magento/Ups/Model/Carrier.php

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
use Magento\Shipping\Model\Shipment\Request as Shipment;
2828

2929
/**
30-
* UPS shipping implementation
30+
* UPS shipping implementation.
31+
*
3132
* @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
3233
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
3334
*/
@@ -135,7 +136,9 @@ class Carrier extends AbstractCarrierOnline implements CarrierInterface
135136
* @inheritdoc
136137
*/
137138
protected $_debugReplacePrivateDataKeys = [
138-
'UserId', 'Password', 'AccessLicenseNumber'
139+
'UserId',
140+
'Password',
141+
'AccessLicenseNumber',
139142
];
140143

141144
/**
@@ -352,9 +355,10 @@ public function setRequest(RateRequest $request)
352355
$destCountry = self::USA_COUNTRY_ID;
353356
}
354357

355-
//for UPS, puero rico state for US will assume as puerto rico country
356-
if ($destCountry == self::USA_COUNTRY_ID && ($request->getDestPostcode() == '00912' ||
357-
$request->getDestRegionCode() == self::PUERTORICO_COUNTRY_ID)
358+
//for UPS, puerto rico state for US will assume as puerto rico country
359+
if ($destCountry == self::USA_COUNTRY_ID
360+
&& ($request->getDestPostcode() == '00912'
361+
|| $request->getDestRegionCode() == self::PUERTORICO_COUNTRY_ID)
358362
) {
359363
$destCountry = self::PUERTORICO_COUNTRY_ID;
360364
}
@@ -727,7 +731,7 @@ protected function _getXmlQuotes()
727731
<StateProvinceCode>{$shipperStateProvince}</StateProvinceCode>
728732
</Address>
729733
</Shipper>
730-
734+
731735
<ShipTo>
732736
<Address>
733737
<PostalCode>{$params['19_destPostal']}</PostalCode>
@@ -743,7 +747,7 @@ protected function _getXmlQuotes()
743747
$xmlParams .= <<<XMLRequest
744748
</Address>
745749
</ShipTo>
746-
750+
747751
<ShipFrom>
748752
<Address>
749753
<PostalCode>{$params['15_origPostal']}</PostalCode>
@@ -1302,20 +1306,23 @@ public function getResponse()
13021306
}
13031307

13041308
/**
1305-
* Get allowed shipping methods
1309+
* Get allowed shipping methods.
13061310
*
13071311
* @return array
13081312
*/
13091313
public function getAllowedMethods()
13101314
{
1311-
$allowed = explode(',', $this->getConfigData('allowed_methods'));
1312-
$arr = [];
1313-
$isByCode = $this->getConfigData('type') == 'UPS_XML';
1314-
foreach ($allowed as $code) {
1315-
$arr[$code] = $isByCode ? $this->getShipmentByCode($code) : $this->configHelper->getCode('method', $code);
1315+
$isUpsXml = $this->getConfigData('type') === 'UPS_XML';
1316+
$origin = $this->getConfigData('origin_shipment');
1317+
$allowedMethods = $isUpsXml
1318+
? $this->configHelper->getCode('originShipment', $origin)
1319+
: $this->configHelper->getCode('method');
1320+
$methods = [];
1321+
foreach ($allowedMethods as $methodCode => $methodData) {
1322+
$methods[$methodCode] = $methodData->getText();
13161323
}
13171324

1318-
return $arr;
1325+
return $methods;
13191326
}
13201327

13211328
/**
@@ -1876,20 +1883,18 @@ public function getContainerTypes(\Magento\Framework\DataObject $params = null)
18761883
];
18771884
}
18781885
$containerTypes = $containerTypes + [
1879-
'03' => __('UPS Tube'),
1880-
'04' => __('PAK'),
1881-
'2a' => __('Small Express Box'),
1882-
'2b' => __('Medium Express Box'),
1883-
'2c' => __('Large Express Box'),
1884-
];
1886+
'03' => __('UPS Tube'),
1887+
'04' => __('PAK'),
1888+
'2a' => __('Small Express Box'),
1889+
'2b' => __('Medium Express Box'),
1890+
'2c' => __('Large Express Box'),
1891+
];
18851892
}
18861893

18871894
return ['00' => __('Customer Packaging')] + $containerTypes;
1888-
} elseif ($countryShipper == self::USA_COUNTRY_ID &&
1889-
$countryRecipient == self::PUERTORICO_COUNTRY_ID &&
1890-
($method == '03' ||
1891-
$method == '02' ||
1892-
$method == '01')
1895+
} elseif ($countryShipper == self::USA_COUNTRY_ID
1896+
&& $countryRecipient == self::PUERTORICO_COUNTRY_ID
1897+
&& ($method == '03' || $method == '02' || $method == '01')
18931898
) {
18941899
// Container types should be the same as for domestic
18951900
$params->setCountryRecipient(self::USA_COUNTRY_ID);

‎app/code/Magento/Ups/Test/Unit/Model/CarrierTest.php

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

89
use Magento\Directory\Model\Country;
910
use Magento\Directory\Model\CountryFactory;
1011
use Magento\Framework\App\Config\ScopeConfigInterface;
11-
use Magento\Framework\DataObject;
1212
use Magento\Framework\HTTP\ClientFactory;
1313
use Magento\Framework\HTTP\ClientInterface;
1414
use Magento\Framework\Model\AbstractModel;
15+
use Magento\Framework\Phrase;
1516
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
1617
use Magento\Quote\Model\Quote\Address\RateRequest;
1718
use Magento\Quote\Model\Quote\Address\RateResult\Error;
@@ -20,11 +21,15 @@
2021
use Magento\Shipping\Model\Rate\ResultFactory;
2122
use Magento\Shipping\Model\Simplexml\Element;
2223
use Magento\Shipping\Model\Simplexml\ElementFactory;
24+
use Magento\Store\Model\ScopeInterface;
25+
use Magento\Ups\Helper\Config;
2326
use Magento\Ups\Model\Carrier;
2427
use PHPUnit_Framework_MockObject_MockObject as MockObject;
2528
use Psr\Log\LoggerInterface;
2629

2730
/**
31+
* Unit tests for \Magento\Ups\Model\Carrier class.
32+
*
2833
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2934
*/
3035
class CarrierTest extends \PHPUnit\Framework\TestCase
@@ -92,17 +97,23 @@ class CarrierTest extends \PHPUnit\Framework\TestCase
9297
*/
9398
private $logger;
9499

100+
/**
101+
* @var Config|MockObject
102+
*/
103+
private $configHelper;
104+
105+
/**
106+
* @inheritdoc
107+
*/
95108
protected function setUp()
96109
{
97110
$this->helper = new ObjectManager($this);
98111

99112
$this->scope = $this->getMockBuilder(ScopeConfigInterface::class)
100113
->disableOriginalConstructor()
114+
->setMethods(['getValue', 'isSetFlag'])
101115
->getMock();
102116

103-
$this->scope->method('getValue')
104-
->willReturnCallback([$this, 'scopeConfigGetValue']);
105-
106117
$this->error = $this->getMockBuilder(Error::class)
107118
->setMethods(['setCarrier', 'setCarrierTitle', 'setErrorMessage'])
108119
->getMock();
@@ -143,6 +154,11 @@ protected function setUp()
143154

144155
$this->logger = $this->getMockForAbstractClass(LoggerInterface::class);
145156

157+
$this->configHelper = $this->getMockBuilder(Config::class)
158+
->disableOriginalConstructor()
159+
->setMethods(['getCode'])
160+
->getMock();
161+
146162
$this->model = $this->helper->getObject(
147163
Carrier::class,
148164
[
@@ -153,6 +169,7 @@ protected function setUp()
153169
'xmlElFactory' => $xmlFactory,
154170
'logger' => $this->logger,
155171
'httpClientFactory' => $httpClientFactory,
172+
'configHelper' => $this->configHelper,
156173
]
157174
);
158175
}
@@ -189,14 +206,17 @@ public function scopeConfigGetValue(string $path)
189206
* @param bool $freeShippingEnabled
190207
* @param int $requestSubtotal
191208
* @param int $expectedPrice
209+
* @return void
192210
*/
193211
public function testGetMethodPrice(
194-
$cost,
195-
$shippingMethod,
196-
$freeShippingEnabled,
197-
$requestSubtotal,
198-
$expectedPrice
199-
) {
212+
int $cost,
213+
string $shippingMethod,
214+
bool $freeShippingEnabled,
215+
int $requestSubtotal,
216+
int $expectedPrice
217+
): void {
218+
$this->scope->method('getValue')
219+
->willReturnCallback([$this, 'scopeConfigGetValue']);
200220
$path = 'carriers/' . $this->model->getCarrierCode() . '/';
201221
$this->scope->method('isSetFlag')
202222
->with($path . 'free_shipping_enable')
@@ -244,8 +264,13 @@ public function getMethodPriceProvider()
244264
];
245265
}
246266

247-
public function testCollectRatesErrorMessage()
267+
/**
268+
* @return void
269+
*/
270+
public function testCollectRatesErrorMessage(): void
248271
{
272+
$this->scope->method('getValue')
273+
->willReturnCallback([$this, 'scopeConfigGetValue']);
249274
$this->scope->method('isSetFlag')
250275
->willReturn(false);
251276

@@ -373,6 +398,69 @@ public function countryDataProvider()
373398
];
374399
}
375400

401+
/**
402+
* @dataProvider allowedMethodsDataProvider
403+
* @param string $carrierType
404+
* @param string $methodType
405+
* @param string $methodCode
406+
* @param string $methodTitle
407+
* @param array $expectedMethods
408+
* @return void
409+
*/
410+
public function testGetAllowedMethods(
411+
string $carrierType,
412+
string $methodType,
413+
string $methodCode,
414+
string $methodTitle,
415+
array $expectedMethods
416+
): void {
417+
$this->scope->method('getValue')
418+
->willReturnMap(
419+
[
420+
[
421+
'carriers/ups/type',
422+
ScopeInterface::SCOPE_STORE,
423+
null,
424+
$carrierType
425+
],
426+
[
427+
'carriers/ups/origin_shipment',
428+
ScopeInterface::SCOPE_STORE,
429+
null,
430+
'Shipments Originating in United States',
431+
],
432+
]
433+
);
434+
$this->configHelper->method('getCode')
435+
->with($methodType)
436+
->willReturn([$methodCode => new Phrase($methodTitle)]);
437+
$actualMethods = $this->model->getAllowedMethods();
438+
$this->assertEquals($expectedMethods, $actualMethods);
439+
}
440+
441+
/**
442+
* @return array
443+
*/
444+
public function allowedMethodsDataProvider(): array
445+
{
446+
return [
447+
[
448+
'UPS',
449+
'method',
450+
'1DM',
451+
'Next Day Air Early AM',
452+
['1DM' => 'Next Day Air Early AM'],
453+
],
454+
[
455+
'UPS_XML',
456+
'originShipment',
457+
'01',
458+
'UPS Next Day Air',
459+
['01' => 'UPS Next Day Air'],
460+
],
461+
];
462+
}
463+
376464
/**
377465
* Creates mock for XML factory.
378466
*

‎app/code/Magento/Usps/etc/config.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<usps>
1212
<active>0</active>
1313
<sallowspecific>0</sallowspecific>
14-
<allowed_methods>0_FCLE,0_FCL,0_FCP,1,2,3,4,6,7,13,16,17,22,23,25,27,28,33,34,35,36,37,42,43,53,55,56,57,61,INT_1,INT_2,INT_4,INT_6,INT_7,INT_8,INT_9,INT_10,INT_11,INT_12,INT_13,INT_14,INT_15,INT_16,INT_20,INT_26</allowed_methods>
14+
<allowed_methods>0_FCLE,0_FCL,0_FCP,1,2,3,4,6,7,13,16,17,22,23,25,27,28,33,34,35,36,37,42,43,53,57,61,INT_1,INT_2,INT_4,INT_6,INT_7,INT_8,INT_9,INT_10,INT_11,INT_12,INT_13,INT_14,INT_15,INT_16,INT_20</allowed_methods>
1515
<container>VARIABLE</container>
1616
<cutoff_cost />
1717
<free_method />

0 commit comments

Comments
 (0)