Merge pull request #1309 from magento-south/PRS

Fixed issues: - MAGETWO-56487 [GitHub] Multi-select option attribute values do not appear in admin #5877 - MAGETWO-54322 ApplyTaxBasedOnVatIdTest test fails on Functional 3rd Party Tests plan
magento · Jul 11, 2017 · 531c257 · 531c257
2 parents 7a6e341 + f8be412
commit 531c257
Show file tree

Hide file tree

Showing 8 changed files with 207 additions and 45 deletions.
diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php
@@ -104,10 +104,13 @@ public function execute()
         $multipleOption = null == $multipleOption ? 'select' : $multipleOption;
 
         if (isset($this->multipleAttributeList[$multipleOption]) && !(null == ($multipleOption))) {
+            $options = $this->getRequest()->getParam($this->multipleAttributeList[$multipleOption]);
             $this->checkUniqueOption(
                 $response,
-                $this->getRequest()->getParam($this->multipleAttributeList[$multipleOption])
+                $options
             );
+            $valueOptions = (isset($options['value']) && is_array($options['value'])) ? $options['value'] : [];
+            $this->checkEmptyOption($response, $valueOptions);
         }
 
         return $this->resultJsonFactory->create()->setJsonData($response->toJson());
@@ -155,13 +158,30 @@ private function setMessageToResponse($response, $messages)
     private function checkUniqueOption(DataObject $response, array $options = null)
     {
         if (is_array($options)
-            && !empty($options['value'])
-            && !empty($options['delete'])
+            && isset($options['value'])
+            && isset($options['delete'])
             && !$this->isUniqueAdminValues($options['value'], $options['delete'])
         ) {
             $this->setMessageToResponse($response, [__("The value of Admin must be unique.")]);
             $response->setError(true);
         }
         return $this;
     }
+
+    /**
+     * Check that admin does not try to create option with empty admin scope option.
+     *
+     * @param DataObject $response
+     * @param array $optionsForCheck
+     * @return void
+     */
+    private function checkEmptyOption(DataObject $response, array $optionsForCheck = null)
+    {
+        foreach ($optionsForCheck as $optionValues) {
+            if (isset($optionValues[0]) && $optionValues[0] == '') {
+                $this->setMessageToResponse($response, [__("The value of Admin scope can't be empty.")]);
+                $response->setError(true);
+            }
+        }
+    }
 }
diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/DataProvider.php b/app/code/Magento/Catalog/Model/Product/Attribute/DataProvider.php
@@ -5,15 +5,14 @@
  */
 namespace Magento\Catalog\Model\Product\Attribute;
 
-use Magento\Catalog\Model\Category;
 use Magento\Framework\Stdlib\ArrayManager;
 use Magento\Store\Api\StoreRepositoryInterface;
 use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
 use Magento\Eav\Model\Entity\Attribute as EavAttribute;
-use Magento\Ui\Component\Form;
 use Magento\Ui\Component\Form\Element\Input;
 use Magento\Ui\Component\Form\Field;
 use Magento\Ui\Component\Form\Element\DataType\Text;
+use Magento\Store\Model\Store;
 
 /**
  * Data provider for the form of adding new product attribute.
@@ -149,40 +148,36 @@ private function customizeOptions($meta)
         $sortOrder = 1;
         foreach ($this->storeRepository->getList() as $store) {
             $storeId = $store->getId();
-
+            $storeLabelConfiguration = [
+                'dataType' => 'text',
+                'formElement' => 'input',
+                'component' => 'Magento_Catalog/js/form/element/input',
+                'template' => 'Magento_Catalog/form/element/input',
+                'prefixName' => 'option.value',
+                'prefixElementName' => 'option_',
+                'suffixName' => (string)$storeId,
+                'label' => $store->getName(),
+                'sortOrder' => $sortOrder,
+                'componentType' => Field::NAME,
+            ];
+            // JS code can't understand 'required-entry' => false|null, we have to avoid even empty property.
+            if ($store->getCode() == Store::ADMIN_CODE) {
+                $storeLabelConfiguration['validation'] = [
+                    'required-entry' => true,
+                ];
+            }
             $meta['attribute_options_select_container']['children']['attribute_options_select']['children']
             ['record']['children']['value_option_' . $storeId] = $this->arrayManager->set(
                 'arguments/data/config',
                 [],
-                [
-                    'dataType' => 'text',
-                    'formElement' => 'input',
-                    'component' => 'Magento_Catalog/js/form/element/input',
-                    'template' => 'Magento_Catalog/form/element/input',
-                    'prefixName' => 'option.value',
-                    'prefixElementName' => 'option_',
-                    'suffixName' => (string)$storeId,
-                    'label' => $store->getName(),
-                    'sortOrder' => $sortOrder,
-                    'componentType' => Field::NAME,
-                ]
+                $storeLabelConfiguration
             );
+
             $meta['attribute_options_multiselect_container']['children']['attribute_options_multiselect']['children']
             ['record']['children']['value_option_' . $storeId] = $this->arrayManager->set(
                 'arguments/data/config',
                 [],
-                [
-                    'dataType' => 'text',
-                    'formElement' => 'input',
-                    'component' => 'Magento_Catalog/js/form/element/input',
-                    'template' => 'Magento_Catalog/form/element/input',
-                    'prefixName' => 'option.value',
-                    'prefixElementName' => 'option_',
-                    'suffixName' => (string)$storeId,
-                    'label' => $store->getName(),
-                    'sortOrder' => $sortOrder,
-                    'componentType' => Field::NAME,
-                ]
+                $storeLabelConfiguration
             );
             ++$sortOrder;
         }

diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php
@@ -251,4 +251,77 @@ public function provideUniqueData()
             ],
         ];
     }
+
+    /**
+     * Check that empty admin scope labels will trigger error.
+     *
+     * @dataProvider provideEmptyOption
+     * @param array $options
+     * @throws \Magento\Framework\Exception\NotFoundException
+     */
+    public function testEmptyOption(array $options, $result)
+    {
+        $this->requestMock->expects($this->any())
+            ->method('getParam')
+            ->willReturnMap([
+                ['frontend_label', null, null],
+                ['frontend_input', 'select', 'multipleselect'],
+                ['attribute_code', null, "test_attribute_code"],
+                ['new_attribute_set_name', null, 'test_attribute_set_name'],
+                ['option', null, $options],
+                ['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
+            ]);
+
+        $this->objectManagerMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->attributeMock);
+
+        $this->attributeMock->expects($this->once())
+            ->method('loadByCode')
+            ->willReturnSelf();
+
+        $this->resultJsonFactoryMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->resultJson);
+
+        $this->resultJson->expects($this->once())
+            ->method('setJsonData')
+            ->willReturnArgument(0);
+
+        $response = $this->getModel()->execute();
+        $responseObject = json_decode($response);
+        $this->assertEquals($responseObject, $result);
+    }
+
+    /**
+     * Dataprovider for testEmptyOption.
+     *
+     * @return array
+     */
+    public function provideEmptyOption()
+    {
+        return [
+            'empty admin scope options' => [
+                [
+                    'value' => [
+                        "option_0" => [''],
+                    ],
+                ],
+                (object) [
+                    'error' => true,
+                    'message' => 'The value of Admin scope can\'t be empty.',
+                ]
+            ],
+            'not empty admin scope options' => [
+                [
+                    'value' => [
+                        "option_0" => ['asdads'],
+                    ],
+                ],
+                (object) [
+                    'error' => false,
+                ]
+            ]
+        ];
+    }
 }
diff --git a/app/code/Magento/Catalog/etc/adminhtml/di.xml b/app/code/Magento/Catalog/etc/adminhtml/di.xml
@@ -51,6 +51,7 @@
         <arguments>
             <argument name="multipleAttributeList" xsi:type="array">
                 <item name="select" xsi:type="string">option</item>
+                <item name="multiselect" xsi:type="string">option</item>
             </argument>
         </arguments>
     </type>

diff --git a/app/code/Magento/Catalog/view/adminhtml/web/template/form/element/input.html b/app/code/Magento/Catalog/view/adminhtml/web/template/form/element/input.html
@@ -17,3 +17,4 @@
             id: uid,
             disabled: disabled
     }"/>
+<label class="admin__field-error" if="error" attr="for: uid" text="error"/>
diff --git a/app/code/Magento/Ui/view/base/web/templates/dynamic-rows/templates/default.html b/app/code/Magento/Ui/view/base/web/templates/dynamic-rows/templates/default.html
@@ -22,9 +22,8 @@
             <table class="admin__dynamic-rows admin__control-table" data-role="grid" attr="{'data-index': index}">
                 <thead if="element.columnsHeader">
                     <tr>
-                        <th if="dndConfig.enabled"/>
+                        <th if="dndConfig.enabled" />
                         <th repeat="foreach: labels, item: '$label'"
-                            text="$label().label"
                             css="setClasses($label())"
                             visible="$label().visible"
                             disable="$label().disabled">

diff --git a/dev/tests/functional/tests/app/Magento/Customer/Test/Handler/Customer/Webapi.php b/dev/tests/functional/tests/app/Magento/Customer/Test/Handler/Customer/Webapi.php
@@ -150,9 +150,14 @@ protected function prepareCustomAttributes(array $data)
      */
     protected function getCustomerGroup(Customer $customer)
     {
-        return $customer->hasData('group_id')
-            ? $customer->getDataFieldConfig('group_id')['source']->getCustomerGroup()->getCustomerGroupId()
-            : self::GENERAL_GROUP;
+        if ($customer->hasData('group_id')) {
+            if ($customer->getDataFieldConfig('group_id')['source']->getCustomerGroup()) {
+                return $customer->getDataFieldConfig('group_id')['source']->getCustomerGroup()->getCustomerGroupId();
+            }
+            return $customer->getData('group_id');
+        } else {
+            return self::GENERAL_GROUP;
+        }
     }
 
     /**

diff --git a/dev/tests/functional/tests/app/Magento/Tax/Test/TestCase/ApplyTaxBasedOnVatIdTest.php b/dev/tests/functional/tests/app/Magento/Tax/Test/TestCase/ApplyTaxBasedOnVatIdTest.php
@@ -8,11 +8,9 @@
 
 use Magento\Tax\Test\Fixture\TaxRule;
 use Magento\Checkout\Test\Fixture\Cart;
-use Magento\Customer\Test\Fixture\Customer;
 use Magento\Config\Test\Fixture\ConfigData;
 use Magento\Checkout\Test\Page\CheckoutCart;
 use Magento\Sales\Test\Fixture\OrderInjectable;
-use Magento\Catalog\Test\Fixture\CatalogProductSimple;
 use Magento\Customer\Test\TestCase\AbstractApplyVatIdTest;
 
 /**
@@ -80,23 +78,28 @@ public function test(
     ) {
         // Preconditions
         $this->configData = $configData;
+        $this->customer = $order->getDataFieldConfig('customer_id')['source']->getCustomer();
         $this->objectManager->create(
             \Magento\Config\Test\TestStep\SetupConfigurationStep::class,
             ['configData' => $this->configData]
         )->run();
-        $taxRule->persist();
+
         // Prepare data
-        $this->customer = $order->getDataFieldConfig('customer_id')['source']->getCustomer();
-        $address = $this->customer->getDataFieldConfig('address')['source']->getAddresses()[0];
-        $this->prepareVatConfig($vatConfig, $customerGroup);
-        $poducts = $order->getEntityId()['products'];
+        $taxRule->persist();
+        $this->prepareVatConfiguration($vatConfig);
+        $this->prepareCustomer($customerGroup);
+
+        $products = $order->getEntityId()['products'];
         $cart = $this->fixtureFactory->createByCode(
             'cart',
-            ['data' => array_merge($cart->getData(), ['items' => ['products' => $poducts]])]
+            ['data' => array_merge($cart->getData(), ['items' => ['products' => $products]])]
         );
+        $address = $this->customer->getDataFieldConfig('address')['source']->getAddresses()[0];
 
-        // Steps
         $order->persist();
+        $this->updateCustomer($customerGroup);
+
+        // Steps
         $this->objectManager->create(
             \Magento\Customer\Test\TestStep\LoginCustomerOnFrontendStep::class,
             ['customer' => $this->customer]
@@ -115,8 +118,73 @@ public function test(
             'address' => $address,
             'orderId' => $order->getId(),
             'cart' => $cart,
-            'products' => $poducts
+            'products' => $products
+        ];
+    }
+
+    /**
+     * Persist vat configuration
+     *
+     * @param string $vatConfig
+     * @return void
+     */
+    private function prepareVatConfiguration($vatConfig)
+    {
+        $groupConfig = [
+            'customer/create_account/viv_domestic_group' => [
+                'value' => $this->vatGroups['valid_domestic_group']->getCustomerGroupId()
+            ],
+            'customer/create_account/viv_intra_union_group' => [
+                'value' => $this->vatGroups['valid_intra_union_group']->getCustomerGroupId()
+            ],
+            'customer/create_account/viv_invalid_group' => [
+                'value' => $this->vatGroups['invalid_group']->getCustomerGroupId()
+            ],
+            'customer/create_account/viv_error_group' => [
+                'value' => $this->vatGroups['error_group']->getCustomerGroupId()
+            ]
         ];
+        $vatConfig = $this->fixtureFactory->createByCode(
+            'configData',
+            ['data' => array_replace_recursive($vatConfig->getSection(), $groupConfig)]
+        );
+        $vatConfig->persist();
+    }
+
+    /**
+     * Persist customer with valid customer group
+     *
+     * @param string $customerGroup
+     * @return void
+     */
+    private function prepareCustomer($customerGroup)
+    {
+        $customerData = array_merge(
+            $this->customer->getData(),
+            ['group_id' => ['value' => $this->vatGroups[$customerGroup]->getCustomerGroupId()]],
+            ['address' => ['addresses' => $this->customer->getDataFieldConfig('address')['source']->getAddresses()]],
+            ['email' => 'JohnDoe%isolation%@example.com']
+        );
+
+        unset($customerData['id']);
+        $this->customer = $this->fixtureFactory->createByCode('customer', ['data' => $customerData]);
+        $this->customer->persist();
+    }
+
+    /**
+     * Update customer with customer group code for assert
+     *
+     * @param string $customerGroup
+     * @return void
+     */
+    private function updateCustomer($customerGroup)
+    {
+        $customerData = array_merge(
+            $this->customer->getData(),
+            ['group_id' => ['value' => $this->vatGroups[$customerGroup]->getCustomerGroupCode()]]
+        );
+
+        $this->customer = $this->fixtureFactory->createByCode('customer', ['data' => $customerData]);
     }
 
     /**