Merge remote-tracking branch 'origin/MC-29444' into 2.4-develop-pr1

zakdma · zakdma · commit 6213ff9dc479 · 2020-01-10T23:09:06.000+02:00
diff --git a/app/code/Magento/Ups/Model/Carrier.php b/app/code/Magento/Ups/Model/Carrier.php
@@ -47,7 +47,8 @@
 use Zend_Http_Client;
 
 /**
- * UPS shipping implementation
+ * UPS shipping implementation.
+ *
  * @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
@@ -155,7 +156,9 @@ class Carrier extends AbstractCarrierOnline implements CarrierInterface
      * @inheritdoc
      */
     protected $_debugReplacePrivateDataKeys = [
-        'UserId', 'Password', 'AccessLicenseNumber'
+        'UserId',
+        'Password',
+        'AccessLicenseNumber',
     ];
 
     /**
@@ -372,9 +375,10 @@ public function setRequest(RateRequest $request)
             $destCountry = self::USA_COUNTRY_ID;
         }
 
-        //for UPS, puero rico state for US will assume as puerto rico country
-        if ($destCountry == self::USA_COUNTRY_ID && ($request->getDestPostcode() == '00912' ||
-            $request->getDestRegionCode() == self::PUERTORICO_COUNTRY_ID)
+        //for UPS, puerto rico state for US will assume as puerto rico country
+        if ($destCountry == self::USA_COUNTRY_ID
+            && ($request->getDestPostcode() == '00912'
+                || $request->getDestRegionCode() == self::PUERTORICO_COUNTRY_ID)
         ) {
             $destCountry = self::PUERTORICO_COUNTRY_ID;
         }
@@ -1322,20 +1326,28 @@ public function getResponse()
     }
 
     /**
-     * Get allowed shipping methods
+     * Get allowed shipping methods.
      *
      * @return array
      */
     public function getAllowedMethods()
     {
-        $allowed = explode(',', $this->getConfigData('allowed_methods'));
-        $arr = [];
-        $isByCode = $this->getConfigData('type') == 'UPS_XML';
-        foreach ($allowed as $code) {
-            $arr[$code] = $isByCode ? $this->getShipmentByCode($code) : $this->configHelper->getCode('method', $code);
+        $allowedMethods = explode(',', (string)$this->getConfigData('allowed_methods'));
+        $isUpsXml = $this->getConfigData('type') === 'UPS_XML';
+        $origin = $this->getConfigData('origin_shipment');
+
+        $availableByTypeMethods = $isUpsXml
+            ? $this->configHelper->getCode('originShipment', $origin)
+            : $this->configHelper->getCode('method');
+
+        $methods = [];
+        foreach ($availableByTypeMethods as $methodCode => $methodData) {
+            if (in_array($methodCode, $allowedMethods)) {
+                $methods[$methodCode] = $methodData->getText();
+            }
         }
 
-        return $arr;
+        return $methods;
     }
 
     /**
@@ -1896,20 +1908,18 @@ public function getContainerTypes(DataObject $params = null)
                     ];
                 }
                 $containerTypes = $containerTypes + [
-                    '03' => __('UPS Tube'),
-                    '04' => __('PAK'),
-                    '2a' => __('Small Express Box'),
-                    '2b' => __('Medium Express Box'),
-                    '2c' => __('Large Express Box'),
-                ];
+                        '03' => __('UPS Tube'),
+                        '04' => __('PAK'),
+                        '2a' => __('Small Express Box'),
+                        '2b' => __('Medium Express Box'),
+                        '2c' => __('Large Express Box'),
+                    ];
             }
 
             return ['00' => __('Customer Packaging')] + $containerTypes;
-        } elseif ($countryShipper == self::USA_COUNTRY_ID &&
-            $countryRecipient == self::PUERTORICO_COUNTRY_ID &&
-            ($method == '03' ||
-            $method == '02' ||
-            $method == '01')
+        } elseif ($countryShipper == self::USA_COUNTRY_ID
+            && $countryRecipient == self::PUERTORICO_COUNTRY_ID
+            && in_array($method, ['01', '02', '03'])
         ) {
             // Container types should be the same as for domestic
             $params->setCountryRecipient(self::USA_COUNTRY_ID);
diff --git a/app/code/Magento/Ups/Test/Unit/Model/CarrierTest.php b/app/code/Magento/Ups/Test/Unit/Model/CarrierTest.php
@@ -3,15 +3,16 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Ups\Test\Unit\Model;
 
 use Magento\Directory\Model\Country;
 use Magento\Directory\Model\CountryFactory;
 use Magento\Framework\App\Config\ScopeConfigInterface;
-use Magento\Framework\DataObject;
 use Magento\Framework\HTTP\ClientFactory;
 use Magento\Framework\HTTP\ClientInterface;
 use Magento\Framework\Model\AbstractModel;
+use Magento\Framework\Phrase;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\Quote\Model\Quote\Address\RateRequest;
 use Magento\Quote\Model\Quote\Address\RateResult\Error;
@@ -20,11 +21,15 @@
 use Magento\Shipping\Model\Rate\ResultFactory;
 use Magento\Shipping\Model\Simplexml\Element;
 use Magento\Shipping\Model\Simplexml\ElementFactory;
+use Magento\Store\Model\ScopeInterface;
+use Magento\Ups\Helper\Config;
 use Magento\Ups\Model\Carrier;
 use PHPUnit_Framework_MockObject_MockObject as MockObject;
 use Psr\Log\LoggerInterface;
 
 /**
+ * Unit tests for \Magento\Ups\Model\Carrier class.
+ *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class CarrierTest extends \PHPUnit\Framework\TestCase
@@ -92,17 +97,23 @@ class CarrierTest extends \PHPUnit\Framework\TestCase
      */
     private $logger;
 
+    /**
+     * @var Config|MockObject
+     */
+    private $configHelper;
+
+    /**
+     * @inheritdoc
+     */
     protected function setUp()
     {
         $this->helper = new ObjectManager($this);
 
         $this->scope = $this->getMockBuilder(ScopeConfigInterface::class)
             ->disableOriginalConstructor()
+            ->setMethods(['getValue', 'isSetFlag'])
             ->getMock();
 
-        $this->scope->method('getValue')
-            ->willReturnCallback([$this, 'scopeConfigGetValue']);
-
         $this->error = $this->getMockBuilder(Error::class)
             ->setMethods(['setCarrier', 'setCarrierTitle', 'setErrorMessage'])
             ->getMock();
@@ -143,6 +154,11 @@ protected function setUp()
 
         $this->logger = $this->getMockForAbstractClass(LoggerInterface::class);
 
+        $this->configHelper = $this->getMockBuilder(Config::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['getCode'])
+            ->getMock();
+
         $this->model = $this->helper->getObject(
             Carrier::class,
             [
@@ -153,6 +169,7 @@ protected function setUp()
                 'xmlElFactory' => $xmlFactory,
                 'logger' => $this->logger,
                 'httpClientFactory' => $httpClientFactory,
+                'configHelper' => $this->configHelper,
             ]
         );
     }
@@ -189,14 +206,17 @@ public function scopeConfigGetValue(string $path)
      * @param bool $freeShippingEnabled
      * @param int $requestSubtotal
      * @param int $expectedPrice
+     * @return void
      */
     public function testGetMethodPrice(
-        $cost,
-        $shippingMethod,
-        $freeShippingEnabled,
-        $requestSubtotal,
-        $expectedPrice
-    ) {
+        int $cost,
+        string $shippingMethod,
+        bool $freeShippingEnabled,
+        int $requestSubtotal,
+        int $expectedPrice
+    ): void {
+        $this->scope->method('getValue')
+            ->willReturnCallback([$this, 'scopeConfigGetValue']);
         $path = 'carriers/' . $this->model->getCarrierCode() . '/';
         $this->scope->method('isSetFlag')
             ->with($path . 'free_shipping_enable')
@@ -244,8 +264,13 @@ public function getMethodPriceProvider()
         ];
     }
 
-    public function testCollectRatesErrorMessage()
+    /**
+     * @return void
+     */
+    public function testCollectRatesErrorMessage(): void
     {
+        $this->scope->method('getValue')
+            ->willReturnCallback([$this, 'scopeConfigGetValue']);
         $this->scope->method('isSetFlag')
             ->willReturn(false);
 
@@ -373,6 +398,87 @@ public function countryDataProvider()
         ];
     }
 
+    /**
+     * @dataProvider allowedMethodsDataProvider
+     * @param string $carrierType
+     * @param string $methodType
+     * @param string $methodCode
+     * @param string $methodTitle
+     * @param string $allowedMethods
+     * @param array $expectedMethods
+     * @return void
+     */
+    public function testGetAllowedMethods(
+        string $carrierType,
+        string $methodType,
+        string $methodCode,
+        string $methodTitle,
+        string $allowedMethods,
+        array $expectedMethods
+    ): void {
+        $this->scope->method('getValue')
+            ->willReturnMap(
+                [
+                    [
+                        'carriers/ups/allowed_methods',
+                        ScopeInterface::SCOPE_STORE,
+                        null,
+                        $allowedMethods,
+                    ],
+                    [
+                        'carriers/ups/type',
+                        ScopeInterface::SCOPE_STORE,
+                        null,
+                        $carrierType,
+                    ],
+                    [
+                        'carriers/ups/origin_shipment',
+                        ScopeInterface::SCOPE_STORE,
+                        null,
+                        'Shipments Originating in United States',
+                    ],
+                ]
+            );
+        $this->configHelper->method('getCode')
+            ->with($methodType)
+            ->willReturn([$methodCode => new Phrase($methodTitle)]);
+        $actualMethods = $this->model->getAllowedMethods();
+        $this->assertEquals($expectedMethods, $actualMethods);
+    }
+
+    /**
+     * @return array
+     */
+    public function allowedMethodsDataProvider(): array
+    {
+        return [
+            [
+                'UPS',
+                'method',
+                '1DM',
+                'Next Day Air Early AM',
+                '',
+                [],
+            ],
+            [
+                'UPS',
+                'method',
+                '1DM',
+                'Next Day Air Early AM',
+                '1DM,1DML,1DA',
+                ['1DM' => 'Next Day Air Early AM'],
+            ],
+            [
+                'UPS_XML',
+                'originShipment',
+                '01',
+                'UPS Next Day Air',
+                '01,02,03',
+                ['01' => 'UPS Next Day Air'],
+            ],
+        ];
+    }
+
     /**
      * Creates mock for XML factory.
      *
diff --git a/app/code/Magento/Usps/etc/config.xml b/app/code/Magento/Usps/etc/config.xml
@@ -11,7 +11,7 @@
             <usps>
                 <active>0</active>
                 <sallowspecific>0</sallowspecific>
-                <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>
+                <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>
                 <container>VARIABLE</container>
                 <cutoff_cost />
                 <free_method />
