🔃 [EngCom] Public Pull Requests - 2.3-develop

Accepted Public Pull Requests:
 - #16961: [Forwardport] Improve "Invalid country code" error message on tax import (by @ihor-sviziev)
 - #16962: [Forwardport] Add Confirm Modal Width (by @ihor-sviziev)
 - #16769: [Forwardport] Added 'title' attribute to 'img' tag in knockout template files. (by @sanganinamrata)
 - #16947: [Forwardport] Fix newsletter subscription behaviour for registered customer.  (by @eduard13)
 - #16876: FIXED: FTP user and password strings urldecoded (by @javierperezm)
 - #16875: Remove unused comments from _initDiscount() function (by @mageprince)
 - #16888: Code improvement (by @mage2pratik)
 - #16892: [Forwardport] Fix for #12081: missing translations in the js-translations.json (by @mage2pratik)
 - magento-engcom/import-export-improvements#115: import-export-improvements #82 : super attribute error message improvements (by @tadhgbowe)


Fixed GitHub Issues:
 - #12081: Magento 2.2.0: Translations for 'Item in Cart' missing in mini cart. (reported by @jhruehl) has been fixed in #16892 by @mage2pratik in 2.3-develop branch
   Related commits:
     1. 18d11f5

app/code/Magento/Braintree/Setup/Patch/Data/ConvertSerializedDataToJson.php

@@ -12,7 +12,7 @@
 use Magento\Framework\Setup\Patch\PatchVersionInterface;
 
 /**
- * Convert data fro php native serialized data to JSON.
+ * Convert data from php native serialized data to JSON.
  */
 class ConvertSerializedDataToJson implements DataPatchInterface, PatchVersionInterface
 {

app/code/Magento/Braintree/view/frontend/web/template/payment/paypal.html

@@ -12,7 +12,7 @@
                data-bind="attr: {'id': getCode()}, value: getCode(), checked: isChecked, click: selectPaymentMethod, visible: isRadioButtonVisible()" />
         <label class="label" data-bind="attr: {'for': getCode()}">
             <!-- PayPal Logo -->
-            <img data-bind="attr: {src: getPaymentAcceptanceMarkSrc(), alt: $t('Acceptance Mark')}"
+            <img data-bind="attr: {src: getPaymentAcceptanceMarkSrc(), alt: $t('Acceptance Mark')}, title: $t('Acceptance Mark')}"
                  class="payment-icon"/>
             <!-- PayPal Logo -->
             <span text="getTitle()"></span>

app/code/Magento/Bundle/Test/Unit/Pricing/Price/BundleSelectionPriceTest.php

@@ -121,7 +121,7 @@ protected function setupSelectionPrice($useRegularPrice = false)
     }
 
     /**
-     *  test fro method getValue with dynamic productType
+     *  Test for method getValue with dynamic productType
      *
      * @param bool $useRegularPrice
      * @dataProvider useRegularPriceDataProvider

app/code/Magento/Captcha/view/frontend/web/template/checkout/captcha.html

@@ -15,6 +15,7 @@
                 <div class="control captcha-image">
                     <img data-bind="attr: {
                                         alt: $t('Please type the letters and numbers below'),
+                                        title: $t('Please type the letters and numbers below'),
                                         height: imageHeight(),
                                         src: getImageSource(),
                                         }"

app/code/Magento/Catalog/etc/adminhtml/system.xml

@@ -98,7 +98,6 @@
                 <clone_fields>1</clone_fields>
                 <clone_model>Magento\Catalog\Model\Config\CatalogClone\Media\Image</clone_model>
                 <field id="placeholder" type="image" sortOrder="1" showInDefault="1" showInWebsite="1" showInStore="1">
-                    <label></label>
                     <backend_model>Magento\Config\Model\Config\Backend\Image</backend_model>
                     <upload_dir config="system/filesystem/media" scope_info="1">catalog/product/placeholder</upload_dir>
                     <base_url type="media" scope_info="1">catalog/product/placeholder</base_url>

app/code/Magento/Catalog/view/adminhtml/web/template/image-preview.html

@@ -14,7 +14,8 @@
                 event="load: $parent.onPreviewLoad.bind($parent)"
                 attr="
                     src: $parent.getFilePreview($file),
-                    alt: $file.name">
+                    alt: $file.name,
+                    title: $file.name">
         </a>
 
         <div class="actions">

app/code/Magento/Catalog/view/base/web/template/product/list/columns/image.html

@@ -11,6 +11,7 @@
              class="product-image-photo"
              attr="src: getImageUrl($row()),
                alt: getLabel($row()),
+               title: getLabel($row()),
                width: getResizedImageWidth($row()),
                height: getResizedImageHeight($row())"/>
     </a>

app/code/Magento/Catalog/view/base/web/template/product/list/columns/image_with_borders.html

@@ -14,7 +14,7 @@
                   data-bind="style: {'padding-bottom': getHeight($row())/getWidth($row()) * 100 + '%'}">
                 <img class="product-image-photo"
                      data-bind="attr: {src: getImageUrl($row()),
-                                       alt: getLabel($row())}" />
+                                       alt: getLabel($row()), title: getLabel($row())}" />
             </span>
         </span>
     </a>

app/code/Magento/Checkout/view/frontend/web/template/summary/item/details/thumbnail.html

@@ -8,6 +8,6 @@
       data-bind="attr: {'style': 'height: ' + getHeight($parents[1]) + 'px; width: ' + getWidth($parents[1]) + 'px;' }">
     <span class="product-image-wrapper">
         <img
-            data-bind="attr: {'src': getSrc($parents[1]), 'width': getWidth($parents[1]), 'height': getHeight($parents[1]), 'alt': getAlt($parents[1]) }"/>
+            data-bind="attr: {'src': getSrc($parents[1]), 'width': getWidth($parents[1]), 'height': getHeight($parents[1]), 'alt': getAlt($parents[1]), 'title': getAlt($parents[1]) }"/>
     </span>
 </span>

app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php

@@ -25,6 +25,12 @@ class Configurable extends \Magento\CatalogImportExport\Model\Import\Product\Typ
     /**
      * Error codes.
      */
+    const ERROR_ATTRIBUTE_CODE_DOES_NOT_EXIST = 'attrCodeDoesNotExist';
+
+    const ERROR_ATTRIBUTE_CODE_NOT_GLOBAL_SCOPE = 'attrCodeNotGlobalScope';
+
+    const ERROR_ATTRIBUTE_CODE_NOT_TYPE_SELECT = 'attrCodeNotTypeSelect';
+
     const ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER = 'attrCodeIsNotSuper';
 
     const ERROR_INVALID_OPTION_VALUE = 'invalidOptionValue';
@@ -39,10 +45,19 @@ class Configurable extends \Magento\CatalogImportExport\Model\Import\Product\Typ
      * Validation failure message template definitions
      *
      * @var array
+     *
+     * Note: Some of these messages exceed maximum limit of 120 characters per line. Split up accordingly.
      */
     protected $_messageTemplates = [
-        self::ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER => 'Attribute with code "%s" is not super',
-        self::ERROR_INVALID_OPTION_VALUE => 'Invalid option value for attribute "%s"',
+        self::ERROR_ATTRIBUTE_CODE_DOES_NOT_EXIST => 'Column configurable_variations: Attribute with code ' .
+            '"%s" does not exist or is missing from product attribute set',
+        self::ERROR_ATTRIBUTE_CODE_NOT_GLOBAL_SCOPE => 'Column configurable_variations: Attribute with code ' .
+            '"%s" is not super - it needs to have Global Scope',
+        self::ERROR_ATTRIBUTE_CODE_NOT_TYPE_SELECT => 'Column configurable_variations: Attribute with code ' .
+            '"%s" is not super - it needs to be Input Type of Dropdown, Visual Swatch or Text Swatch',
+        self::ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER => 'Column configurable_variations: Attribute with code ' .
+            '"%s" is not super',
+        self::ERROR_INVALID_OPTION_VALUE => 'Column configurable_variations: Invalid option value for attribute "%s"',
         self::ERROR_INVALID_WEBSITE => 'Invalid website code for super attribute',
         self::ERROR_DUPLICATED_VARIATIONS => 'SKU %s contains duplicated variations',
         self::ERROR_UNIDENTIFIABLE_VARIATION => 'Configurable variation "%s" is unidentifiable',
@@ -289,10 +304,11 @@ protected function _isParticularAttributesValid(array $rowData, $rowNum)
     {
         if (!empty($rowData['_super_attribute_code'])) {
             $superAttrCode = $rowData['_super_attribute_code'];
-
             if (!$this->_isAttributeSuper($superAttrCode)) {
-                // check attribute superity
-                $this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER, $rowNum, $superAttrCode);
+                // Identify reason why attribute is not super:
+                if (!$this->identifySuperAttributeError($superAttrCode, $rowNum)) {
+                    $this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER, $rowNum, $superAttrCode);
+                }
                 return false;
             } elseif (isset($rowData['_super_attribute_option']) && strlen($rowData['_super_attribute_option'])) {
                 $optionKey = strtolower($rowData['_super_attribute_option']);
@@ -305,6 +321,66 @@ protected function _isParticularAttributesValid(array $rowData, $rowNum)
         return true;
     }
 
+    /**
+     * Identify exactly why a super attribute code is not super.
+     *
+     * @param string $superAttrCode
+     * @param int $rowNum
+     * @return bool
+     */
+    private function identifySuperAttributeError($superAttrCode, $rowNum)
+    {
+        // This attribute code is not a super attribute. Need to give a clearer message why?
+        $reasonFound = false;
+        $codeExists = false;
+
+        // Does this attribute code exist?
+        $sourceAttribute = $this->doesSuperAttributeExist($superAttrCode);
+        if (is_array($sourceAttribute)) {
+            $codeExists = true;
+            // Does attribute have the correct settings?
+            if (isset($sourceAttribute['is_global']) && $sourceAttribute['is_global'] !== '1') {
+                $this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_NOT_GLOBAL_SCOPE, $rowNum, $superAttrCode);
+                $reasonFound = true;
+            } elseif (isset($sourceAttribute['type']) && $sourceAttribute['type'] !== 'select') {
+                $this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_NOT_TYPE_SELECT, $rowNum, $superAttrCode);
+                $reasonFound = true;
+            }
+        }
+
+        if ($codeExists === false) {
+            $this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_DOES_NOT_EXIST, $rowNum, $superAttrCode);
+            $reasonFound = true;
+        }
+
+        return $reasonFound;
+    }
+
+    /**
+     * Does the super attribute exist in the current attribute set?
+     *
+     * @param string $superAttrCode
+     * @return array
+     */
+    private function doesSuperAttributeExist($superAttrCode)
+    {
+        $returnAttributeArray = null;
+        if (is_array(self::$commonAttributesCache)) {
+            $filteredAttribute = array_filter(
+                self::$commonAttributesCache,
+                function ($element) use ($superAttrCode) {
+                    return $element['code'] == $superAttrCode;
+                }
+            );
+
+            // Return the first element of the filtered array (if found).
+            if (count($filteredAttribute)) {
+                $returnAttributeArray = array_shift($filteredAttribute);
+            }
+        }
+        return $returnAttributeArray;
+    }
+
     /**
      * Array of SKU to array of super attribute values for all products.
      *

app/code/Magento/Newsletter/Model/Subscriber.php

@@ -419,7 +419,6 @@ public function subscribe($email)
             self::XML_PATH_CONFIRMATION_FLAG,
             \Magento\Store\Model\ScopeInterface::SCOPE_STORE
         ) == 1 ? true : false;
-        $isOwnSubscribes = false;
 
         $isSubscribeOwnEmail = $this->_customerSession->isLoggedIn()
             && $this->_customerSession->getCustomerDataObject()->getEmail() == $email;
@@ -428,13 +427,7 @@ public function subscribe($email)
             || $this->getStatus() == self::STATUS_NOT_ACTIVE
         ) {
             if ($isConfirmNeed === true) {
-                // if user subscribes own login email - confirmation is not needed
-                $isOwnSubscribes = $isSubscribeOwnEmail;
-                if ($isOwnSubscribes == true) {
-                    $this->setStatus(self::STATUS_SUBSCRIBED);
-                } else {
-                    $this->setStatus(self::STATUS_NOT_ACTIVE);
-                }
+                $this->setStatus(self::STATUS_NOT_ACTIVE);
             } else {
                 $this->setStatus(self::STATUS_SUBSCRIBED);
             }
@@ -460,9 +453,7 @@ public function subscribe($email)
         try {
             /* Save model before sending out email */
             $this->save();
-            if ($isConfirmNeed === true
-                && $isOwnSubscribes === false
-            ) {
+            if ($isConfirmNeed === true) {
                 $this->sendConfirmationRequestEmail();
             } else {
                 $this->sendConfirmationSuccessEmail();

app/code/Magento/Newsletter/Test/Unit/Model/SubscriberTest.php

@@ -5,6 +5,8 @@
  */
 namespace Magento\Newsletter\Test\Unit\Model;
 
+use Magento\Newsletter\Model\Subscriber;
+
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
@@ -116,7 +118,7 @@ public function testSubscribe()
         $email = 'subscriber_email@magento.com';
         $this->resource->expects($this->any())->method('loadByEmail')->willReturn(
             [
-                'subscriber_status' => 3,
+                'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,
                 'subscriber_email' => $email,
                 'name' => 'subscriber_name'
             ]
@@ -133,15 +135,15 @@ public function testSubscribe()
         $this->sendEmailCheck();
         $this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();
 
-        $this->assertEquals(1, $this->subscriber->subscribe($email));
+        $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));
     }
 
     public function testSubscribeNotLoggedIn()
     {
         $email = 'subscriber_email@magento.com';
         $this->resource->expects($this->any())->method('loadByEmail')->willReturn(
             [
-                'subscriber_status' => 3,
+                'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,
                 'subscriber_email' => $email,
                 'name' => 'subscriber_name'
             ]
@@ -158,7 +160,7 @@ public function testSubscribeNotLoggedIn()
         $this->sendEmailCheck();
         $this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();
 
-        $this->assertEquals(2, $this->subscriber->subscribe($email));
+        $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));
     }
 
     public function testUpdateSubscription()
@@ -175,7 +177,7 @@ public function testUpdateSubscription()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 1
+                    'subscriber_status' => Subscriber::STATUS_SUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -210,7 +212,7 @@ public function testUnsubscribeCustomerById()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 1
+                    'subscriber_status' => Subscriber::STATUS_SUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -236,7 +238,7 @@ public function testSubscribeCustomerById()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 3
+                    'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -262,7 +264,7 @@ public function testSubscribeCustomerById1()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 3
+                    'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -276,7 +278,7 @@ public function testSubscribeCustomerById1()
         $this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);
 
         $this->subscriber->subscribeCustomerById($customerId);
-        $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());
+        $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());
     }
 
     public function testSubscribeCustomerByIdAfterConfirmation()
@@ -293,7 +295,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 4
+                    'subscriber_status' => Subscriber::STATUS_UNCONFIRMED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -305,7 +307,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()
         $this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);
 
         $this->subscriber->updateSubscription($customerId);
-        $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());
+        $this->assertEquals(Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());
     }
 
     public function testUnsubscribe()

app/code/Magento/Paypal/etc/adminhtml/system/express_checkout.xml

@@ -198,7 +198,6 @@
                     <attribute type="shared">1</attribute>
                 </field>
                 <field id="bml_wizard" translate="button_label" sortOrder="15" showInDefault="1" showInWebsite="1">
-                    <label></label>
                     <button_label>Get Publisher ID from PayPal</button_label>
                     <button_url><![CDATA[https:/financing.paypal.com/ppfinportal/cart/index?dcp=4eff8563b9cc505e0b9afaff3256705081553c79]]></button_url>
                     <frontend_model>Magento\Paypal\Block\Adminhtml\System\Config\BmlApiWizard</frontend_model>

app/code/Magento/Sales/view/adminhtml/web/order/create/scripts.js

@@ -47,7 +47,7 @@ define([
                             var buttons = controlButtonArea.childElements();
                             for (var i = 0; i < buttons.length; i++) {
                                 if (buttons[i].innerHTML.include(button.label)) {
-                                    return ;
+                                    return;
                                 }
                             }
                             button.insertIn(controlButtonArea, 'top');

app/code/Magento/Tax/Block/Sales/Order/Tax.php

@@ -264,12 +264,6 @@ protected function _initShipping()
      */
     protected function _initDiscount()
     {
-        //        $store  = $this->getStore();
-        //        $parent = $this->getParentBlock();
-        //        if ($this->_config->displaySales) {
-        //
-        //        } elseif ($this->_config->displaySales) {
-        //        }
     }
 
     /**

app/code/Magento/TaxImportExport/Model/Rate/CsvImportHandler.php

@@ -237,7 +237,7 @@ protected function _importRate(array $rateData, array $regionsCache, array $stor
         $countryCode = $rateData[1];
         $country = $this->_countryFactory->create()->loadByCode($countryCode, 'iso2_code');
         if (!$country->getId()) {
-            throw new \Magento\Framework\Exception\LocalizedException(__('One of the countries has invalid code.'));
+            throw new \Magento\Framework\Exception\LocalizedException(__('Country code is invalid: %1', $countryCode));
         }
         $regionsCache = $this->_addCountryRegionsToCache($countryCode, $regionsCache);
 

app/code/Magento/TaxImportExport/i18n/en_US.csv

@@ -12,7 +12,7 @@ Rate,Rate
 "Invalid file upload attempt","Invalid file upload attempt"
 "Invalid file upload attempt.","Invalid file upload attempt."
 "Invalid file format.","Invalid file format."
-"One of the countries has invalid code.","One of the countries has invalid code."
+"Country code is invalid: %1","Country code is invalid: %1"
 "Import Tax Rates","Import Tax Rates"
 "Export Tax Rates","Export Tax Rates"
 CSV,CSV

app/code/Magento/Translation/etc/di.xml

@@ -67,6 +67,7 @@
                 <item name="translate_wrapping" xsi:type="string"><![CDATA[~translate\=("')([^\'].*?)\'\"~]]></item>
                 <item name="mage_translation_widget" xsi:type="string"><![CDATA[~(?:\$|jQuery)\.mage\.__\((?s)[^'"]*?(['"])(.+?)(?<!\\)\1(?s).*?\)~]]></item>
                 <item name="mage_translation_static" xsi:type="string"><![CDATA[~\$t\((?s)[^'"]*?(["'])(.+?)\1(?s).*?\)~]]></item>
+                <item name="translate_args" xsi:type="string"><![CDATA[~translate args\=("|'|"')([^\'].*?)('"|'|")~]]></item>
             </argument>
         </arguments>
     </type>