MSI: magento#131

- Move Сarrier Сodes validation to service layer - Add New intigration Test
VitaliyBoyko · Oct 24, 2017 · 5f26681 · 5f26681
1 parent 3090cb4
commit 5f26681
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 23 deletions.
diff --git a/app/code/Magento/Inventory/Controller/Adminhtml/Source/SourceCarrierDataProcessor.php b/app/code/Magento/Inventory/Controller/Adminhtml/Source/SourceCarrierDataProcessor.php
@@ -46,7 +46,6 @@ public function process(array $data)
             && isset($data['carrier_codes'])
             && is_array($data['carrier_codes'])
         ) {
-            $this->checkCarrierCodes($data['carrier_codes']);
             $data[SourceInterface::CARRIER_LINKS] = $this->getCarrierLinksData($data['carrier_codes']);
         } else {
             $data[SourceInterface::CARRIER_LINKS] = [];
@@ -55,21 +54,6 @@ public function process(array $data)
         return $data;
     }
 
-    /**
-     * @param array $carrierCodes
-     * @return void
-     * @throws InputException
-     */
-    private function checkCarrierCodes(array $carrierCodes)
-    {
-        $availableCarriers = $this->shippingConfig->getAllCarriers();
-
-        // TODO: move validation to service (management + tests)
-        if (count(array_intersect_key(array_flip($carrierCodes), $availableCarriers)) !== count($carrierCodes)) {
-            throw new InputException(__('Wrong carrier codes data'));
-        }
-    }
-
     /**
      * @param array $carrierCodes
      * @return array

diff --git a/app/code/Magento/Inventory/Model/SourceCarrierLinkManagement.php b/app/code/Magento/Inventory/Model/SourceCarrierLinkManagement.php
@@ -3,17 +3,20 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Inventory\Model;
 
 use Magento\Framework\Api\SearchCriteria\CollectionProcessorInterface;
 use Magento\Framework\Api\SearchCriteriaBuilder;
 use Magento\Framework\App\ResourceConnection;
-use Magento\Inventory\Model\ResourceModel\SourceCarrierLink as SourceCarrierLinkResourceModel;
+use Magento\Framework\Validation\ValidationException;
 use Magento\Inventory\Model\ResourceModel\SourceCarrierLink;
+use Magento\Inventory\Model\ResourceModel\SourceCarrierLink as SourceCarrierLinkResourceModel;
 use Magento\Inventory\Model\ResourceModel\SourceCarrierLink\Collection;
 use Magento\Inventory\Model\ResourceModel\SourceCarrierLink\CollectionFactory;
 use Magento\InventoryApi\Api\Data\SourceCarrierLinkInterface;
 use Magento\InventoryApi\Api\Data\SourceInterface;
+use Magento\Shipping\Model\Config;
 
 /**
  * @inheritdoc
@@ -45,25 +48,35 @@ class SourceCarrierLinkManagement implements SourceCarrierLinkManagementInterfac
      */
     private $searchCriteriaBuilder;
 
+    /**
+     * Shipping config
+     *
+     * @var Config
+     */
+    private $shippingConfig;
+
     /**
      * @param ResourceConnection $resourceConnection
      * @param SourceCarrierLinkResourceModel $sourceCarrierLinkResource
      * @param CollectionProcessorInterface $collectionProcessor
      * @param CollectionFactory $carrierLinkCollectionFactory
      * @param SearchCriteriaBuilder $searchCriteriaBuilder
+     * @param Config $shippingConfig
      */
     public function __construct(
         ResourceConnection $resourceConnection,
         SourceCarrierLinkResourceModel $sourceCarrierLinkResource,
         CollectionProcessorInterface $collectionProcessor,
         CollectionFactory $carrierLinkCollectionFactory,
-        SearchCriteriaBuilder $searchCriteriaBuilder
+        SearchCriteriaBuilder $searchCriteriaBuilder,
+        Config $shippingConfig
     ) {
         $this->resourceConnection = $resourceConnection;
         $this->sourceCarrierLinkResource = $sourceCarrierLinkResource;
         $this->collectionProcessor = $collectionProcessor;
         $this->carrierLinkCollectionFactory = $carrierLinkCollectionFactory;
         $this->searchCriteriaBuilder = $searchCriteriaBuilder;
+        $this->shippingConfig = $shippingConfig;
     }
 
     /**
@@ -95,14 +108,24 @@ private function deleteCurrentCarrierLinks(SourceInterface $source)
     /**
      * @param SourceInterface $source
      * @return void
+     * @throws ValidationException
      */
     private function saveNewCarrierLinks(SourceInterface $source)
     {
         $carrierLinkData = [];
+
+        $availableCarriers = $this->shippingConfig->getAllCarriers();
+
         foreach ($source->getCarrierLinks() as $carrierLink) {
+            $carrierCode = $carrierLink->getCarrierCode();
+
+            if (array_key_exists($carrierCode, $availableCarriers) === false) {
+                throw new ValidationException(__('CarrierCode %1 dos not exists', $carrierCode));
+            }
+
             $carrierLinkData[] = [
                 'source_id' => $source->getSourceId(),
-                SourceCarrierLinkInterface::CARRIER_CODE => $carrierLink->getCarrierCode(),
+                SourceCarrierLinkInterface::CARRIER_CODE => $carrierCode,
                 SourceCarrierLinkInterface::POSITION => $carrierLink->getPosition(),
             ];
         }

diff --git a/app/code/Magento/InventoryApi/Test/Api/SourceRepository/CarrierLinkManagementTest.php b/app/code/Magento/InventoryApi/Test/Api/SourceRepository/CarrierLinkManagementTest.php
@@ -66,23 +66,23 @@ public function dataProviderCarrierLinks(): array
                         SourceCarrierLinkInterface::POSITION => 200,
                     ],
                     [
-                        SourceCarrierLinkInterface::CARRIER_CODE => 'new-link-1',
+                        SourceCarrierLinkInterface::CARRIER_CODE => 'dhl',
                         SourceCarrierLinkInterface::POSITION => 300,
                     ],
                     [
-                        SourceCarrierLinkInterface::CARRIER_CODE => 'new-link-2',
+                        SourceCarrierLinkInterface::CARRIER_CODE => 'fedex',
                         SourceCarrierLinkInterface::POSITION => 400,
                     ],
                 ],
             ],
             'replace_carrier_links' => [
                 [
                     [
-                        SourceCarrierLinkInterface::CARRIER_CODE => 'new-link-1',
+                        SourceCarrierLinkInterface::CARRIER_CODE => 'dhl',
                         SourceCarrierLinkInterface::POSITION => 100,
                     ],
                     [
-                        SourceCarrierLinkInterface::CARRIER_CODE => 'new-link-2',
+                        SourceCarrierLinkInterface::CARRIER_CODE => 'fedex',
                         SourceCarrierLinkInterface::POSITION => 200,
                     ],
                 ],

diff --git a/app/code/Magento/InventoryApi/Test/Api/SourceRepository/CreateTest.php b/app/code/Magento/InventoryApi/Test/Api/SourceRepository/CreateTest.php
@@ -68,6 +68,54 @@ public function testCreate()
         AssertArrayContains::assert($expectedData, $this->getSourceDataById($sourceId));
     }
 
+    public function testCreateWithWrongCarrierLinks()
+    {
+        $expectedData = [
+            SourceInterface::NAME => 'source-name-1',
+            SourceInterface::CONTACT_NAME => 'source-contact-name',
+            SourceInterface::EMAIL => 'source-email',
+            SourceInterface::ENABLED => true,
+            SourceInterface::DESCRIPTION => 'source-description',
+            SourceInterface::LATITUDE => 11.123456,
+            SourceInterface::LONGITUDE => 12.123456,
+            SourceInterface::COUNTRY_ID => 'US',
+            SourceInterface::REGION_ID => 10,
+            SourceInterface::CITY => 'source-city',
+            SourceInterface::STREET => 'source-street',
+            SourceInterface::POSTCODE => 'source-postcode',
+            SourceInterface::PHONE => 'source-phone',
+            SourceInterface::FAX => 'source-fax',
+            SourceInterface::PRIORITY => 200,
+            SourceInterface::USE_DEFAULT_CARRIER_CONFIG => 0,
+            SourceInterface::CARRIER_LINKS => [
+                [
+                    SourceCarrierLinkInterface::CARRIER_CODE => 'no_exists_1',
+                    SourceCarrierLinkInterface::POSITION => 100,
+                ],
+                [
+                    SourceCarrierLinkInterface::CARRIER_CODE => 'no_exists_2',
+                    SourceCarrierLinkInterface::POSITION => 200,
+                ],
+            ],
+        ];
+        $serviceInfo = [
+            'rest' => [
+                'resourcePath' => self::RESOURCE_PATH,
+                'httpMethod' => Request::HTTP_METHOD_POST,
+            ],
+            'soap' => [
+                'service' => self::SERVICE_NAME,
+                'operation' => self::SERVICE_NAME . 'Save',
+            ],
+        ];
+
+        try {
+            $this->_webApiCall($serviceInfo, ['source' => $expectedData]);
+        } catch (\Exception $e) {
+            self::assertEquals(\Magento\Framework\Webapi\Exception::HTTP_BAD_REQUEST, $e->getCode());
+        }
+    }
+
     protected function tearDown()
     {
         /** @var ResourceConnection $connection */