From 4ba2c46dd52149339c0baf4edbf1069cadc5cd0d Mon Sep 17 00:00:00 2001 From: David Manners Date: Sat, 16 Jun 2018 12:56:42 +0000 Subject: [PATCH 01/23] magento-engcom/import-export-improvements#44: update Customer sample CSV - update the sample csv for customer to remove columns reward_update_notification and reward_warning_notification - after this PR is merged you will be able to download the sample customer csv and import it directly without getting an error message --- app/code/Magento/ImportExport/Files/Sample/customer.csv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/ImportExport/Files/Sample/customer.csv b/app/code/Magento/ImportExport/Files/Sample/customer.csv index 64c09574aea..522e59f5667 100644 --- a/app/code/Magento/ImportExport/Files/Sample/customer.csv +++ b/app/code/Magento/ImportExport/Files/Sample/customer.csv @@ -1,2 +1,2 @@ -email,_website,_store,confirmation,created_at,created_in,disable_auto_group_change,dob,firstname,gender,group_id,lastname,middlename,password_hash,prefix,reward_update_notification,reward_warning_notification,rp_token,rp_token_created_at,store_id,suffix,taxvat,updated_at,website_id,password -jondoe@example.com,base,default,,"2015-10-30 12:49:47","Default Store View",0,,Jon,,1,Doe,,d708be3fe0fe0120840e8b13c8faae97424252c6374227ff59c05814f1aecd79:mgLqkqgTwLPLlCljzvF8hp67fNOOvOZb:1,,,,07e71459c137f4da15292134ff459cba,"2015-10-30 12:49:48",1,,,"2015-10-30 12:49:48",1, +email,_website,_store,confirmation,created_at,created_in,disable_auto_group_change,dob,firstname,gender,group_id,lastname,middlename,password_hash,prefix,rp_token,rp_token_created_at,store_id,suffix,taxvat,updated_at,website_id,password +jondoe@example.com,base,default,,"2015-10-30 12:49:47","Default Store View",0,,Jon,,1,Doe,,d708be3fe0fe0120840e8b13c8faae97424252c6374227ff59c05814f1aecd79:mgLqkqgTwLPLlCljzvF8hp67fNOOvOZb:1,,07e71459c137f4da15292134ff459cba,"2015-10-30 12:49:48",1,,,"2015-10-30 12:49:48",1, From b49d2ff51cfad58d6b78cacaedf418aeadbd88ab Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sat, 16 Jun 2018 16:49:22 +0200 Subject: [PATCH 02/23] magento-engcom/import-export-improvements#88: Set store id for product category initialization to 0. Would otherwise be null and set to 1 in \Magento\Catalog\Model\ResourceModel\Collection\AbstractCollection::getStoreId, leading to unwanted store values for category names. --- .../Model/Import/Product/CategoryProcessor.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php index 5de9d3880b5..db5b07279ed 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php @@ -75,6 +75,7 @@ protected function initCategories() $collection->addAttributeToSelect('name') ->addAttributeToSelect('url_key') ->addAttributeToSelect('url_path'); + $collection->setStoreId(\Magento\Store\Model\Store::DEFAULT_STORE_ID); /* @var $collection \Magento\Catalog\Model\ResourceModel\Category\Collection */ foreach ($collection as $category) { $structure = explode(self::DELIMITER_CATEGORY, $category->getPath()); From 94dcc4ea2df2b59adc90c1deb4d78863bd83ba2c Mon Sep 17 00:00:00 2001 From: Tiago Sampaio Date: Tue, 26 Jun 2018 18:12:28 -0300 Subject: [PATCH 03/23] Changes: - Replacing the method proccessAdditionalValidation by processAdditionalValidation. It's was mistype error. --- app/code/Magento/Dhl/Model/Carrier.php | 12 ++++++++++++ .../Shipping/Model/Carrier/AbstractCarrier.php | 15 ++++++++++++++- .../Model/Carrier/AbstractCarrierInterface.php | 2 +- .../Model/Carrier/AbstractCarrierOnline.php | 14 ++++++++++++++ app/code/Magento/Shipping/Model/Shipping.php | 2 +- .../Model/Carrier/AbstractCarrierOnlineTest.php | 2 +- 6 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Dhl/Model/Carrier.php b/app/code/Magento/Dhl/Model/Carrier.php index 258dcbe9595..6262d71c4ed 100644 --- a/app/code/Magento/Dhl/Model/Carrier.php +++ b/app/code/Magento/Dhl/Model/Carrier.php @@ -1263,8 +1263,20 @@ protected function _doShipmentRequest(\Magento\Framework\DataObject $request) * * @param \Magento\Framework\DataObject $request * @return $this|\Magento\Framework\DataObject|boolean + * @deprecated */ public function proccessAdditionalValidation(\Magento\Framework\DataObject $request) + { + return $this->processAdditionalValidation($request); + } + + /** + * Processing additional validation to check is carrier applicable. + * + * @param \Magento\Framework\DataObject $request + * @return $this|\Magento\Framework\DataObject|boolean + */ + public function processAdditionalValidation(\Magento\Framework\DataObject $request) { //Skip by item validation if there is no items in request if (!count($this->getAllItems($request))) { diff --git a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrier.php b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrier.php index 5575792c346..6763046373c 100644 --- a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrier.php +++ b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrier.php @@ -329,11 +329,24 @@ public function checkAvailableShipCountries(\Magento\Framework\DataObject $reque * @return $this|bool|\Magento\Framework\DataObject * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function proccessAdditionalValidation(\Magento\Framework\DataObject $request) + public function processAdditionalValidation(\Magento\Framework\DataObject $request) { return $this; } + /** + * Processing additional validation to check is carrier applicable. + * + * @param \Magento\Framework\DataObject $request + * @return $this|bool|\Magento\Framework\DataObject + * @deprecated + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function proccessAdditionalValidation(\Magento\Framework\DataObject $request) + { + return $this->processAdditionalValidation($request); + } + /** * Determine whether current carrier enabled for activity * diff --git a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php index ee59fae657e..adf7ad1ed2e 100644 --- a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php +++ b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php @@ -90,7 +90,7 @@ public function checkAvailableShipCountries(\Magento\Framework\DataObject $reque * @return $this|\Magento\Framework\DataObject|boolean * @api */ - public function proccessAdditionalValidation(\Magento\Framework\DataObject $request); + public function processAdditionalValidation(\Magento\Framework\DataObject $request); /** * Determine whether current carrier enabled for activity diff --git a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierOnline.php b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierOnline.php index 8244fcc4bad..be2588dc487 100644 --- a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierOnline.php +++ b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierOnline.php @@ -302,10 +302,24 @@ public function getAllItems(RateRequest $request) * * @param \Magento\Framework\DataObject $request * @return $this|bool|\Magento\Framework\DataObject + * @deprecated * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ public function proccessAdditionalValidation(\Magento\Framework\DataObject $request) + { + return $this->processAdditionalValidation($request); + } + + /** + * Processing additional validation to check if carrier applicable. + * + * @param \Magento\Framework\DataObject $request + * @return $this|bool|\Magento\Framework\DataObject + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.NPathComplexity) + */ + public function processAdditionalValidation(\Magento\Framework\DataObject $request) { //Skip by item validation if there is no items in request if (!count($this->getAllItems($request))) { diff --git a/app/code/Magento/Shipping/Model/Shipping.php b/app/code/Magento/Shipping/Model/Shipping.php index 2223cb8ae3b..57e055e83a5 100644 --- a/app/code/Magento/Shipping/Model/Shipping.php +++ b/app/code/Magento/Shipping/Model/Shipping.php @@ -259,7 +259,7 @@ public function collectCarrierRates($carrierCode, $request) $carrier->setActiveFlag($this->_availabilityConfigField); $result = $carrier->checkAvailableShipCountries($request); if (false !== $result && !$result instanceof \Magento\Quote\Model\Quote\Address\RateResult\Error) { - $result = $carrier->proccessAdditionalValidation($request); + $result = $carrier->processAdditionalValidation($request); } /* * Result will be false if the admin set not to show the shipping module diff --git a/app/code/Magento/Shipping/Test/Unit/Model/Carrier/AbstractCarrierOnlineTest.php b/app/code/Magento/Shipping/Test/Unit/Model/Carrier/AbstractCarrierOnlineTest.php index 6f87eb171a3..b40f5b26b89 100644 --- a/app/code/Magento/Shipping/Test/Unit/Model/Carrier/AbstractCarrierOnlineTest.php +++ b/app/code/Magento/Shipping/Test/Unit/Model/Carrier/AbstractCarrierOnlineTest.php @@ -100,7 +100,7 @@ public function testComposePackages() $this->stockItemData->expects($this->atLeastOnce())->method('getIsDecimalDivided') ->will($this->returnValue(true)); - $this->carrier->proccessAdditionalValidation($request); + $this->carrier->processAdditionalValidation($request); } public function testParseXml() From a8ee55551ecf13eaeefc665de14145f94bc757aa Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sun, 17 Jun 2018 13:08:10 +0200 Subject: [PATCH 04/23] magento-engcom/import-export-improvements#30: Do required attribute check before attribute validation. Add check if the required attribute is not empty after trimming. --- .../CustomerImportExport/Model/Import/Customer.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php index e5cc543db6a..3d4c40fbbed 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php @@ -593,6 +593,12 @@ protected function _validateRowForUpdate(array $rowData, $rowNumber) if (in_array($attributeCode, $this->_ignoredAttributes)) { continue; } + if ($attributeParams['is_required'] + && ((!isset($rowData[$attributeCode]) && !$this->_getCustomerId($email, $website)) + || (isset($rowData[$attributeCode]) && '' === trim($rowData[$attributeCode])))) { + $this->addRowError(self::ERROR_VALUE_IS_REQUIRED, $rowNumber, $attributeCode); + continue; + } if (isset($rowData[$attributeCode]) && strlen($rowData[$attributeCode])) { $this->isAttributeValid( $attributeCode, @@ -603,8 +609,6 @@ protected function _validateRowForUpdate(array $rowData, $rowNumber) ? $this->_parameters[Import::FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR] : Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR ); - } elseif ($attributeParams['is_required'] && !$this->_getCustomerId($email, $website)) { - $this->addRowError(self::ERROR_VALUE_IS_REQUIRED, $rowNumber, $attributeCode); } } } From 0832e295c57f4140faf6101db73589f620d33896 Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sat, 30 Jun 2018 20:42:15 +0200 Subject: [PATCH 05/23] magento-engcom/import-export-improvements#30: Extend isErrorAlreadyAdded check to also check column name so several columns in the same row may throw the same error and they can be distinguished. --- .../Import/ErrorProcessing/ProcessingErrorAggregator.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/ImportExport/Model/Import/ErrorProcessing/ProcessingErrorAggregator.php b/app/code/Magento/ImportExport/Model/Import/ErrorProcessing/ProcessingErrorAggregator.php index 079b2e60c47..7f49e2022c4 100644 --- a/app/code/Magento/ImportExport/Model/Import/ErrorProcessing/ProcessingErrorAggregator.php +++ b/app/code/Magento/ImportExport/Model/Import/ErrorProcessing/ProcessingErrorAggregator.php @@ -77,7 +77,7 @@ public function addError( $errorMessage = null, $errorDescription = null ) { - if ($this->isErrorAlreadyAdded($rowNumber, $errorCode)) { + if ($this->isErrorAlreadyAdded($rowNumber, $errorCode, $columnName)) { return $this; } $this->processErrorStatistics($errorLevel); @@ -333,13 +333,14 @@ public function clear() /** * @param int $rowNum * @param string $errorCode + * @param string $columnName * @return bool */ - protected function isErrorAlreadyAdded($rowNum, $errorCode) + protected function isErrorAlreadyAdded($rowNum, $errorCode, $columnName = null) { $errors = $this->getErrorsByCode([$errorCode]); foreach ($errors as $error) { - if ($rowNum == $error->getRowNumber()) { + if ($rowNum == $error->getRowNumber() && $columnName == $error->getColumnName()) { return true; } } From 2e04d5092554a7f0e6961408d4bd5ded05cdbaf7 Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sat, 30 Jun 2018 23:34:44 +0200 Subject: [PATCH 06/23] magento-engcom/import-export-improvements#88: Need to adjust test CSV according to code change. For category identification, admin name must be used. --- .../Model/Import/_files/products_to_import_with_two_stores.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_with_two_stores.csv b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_with_two_stores.csv index f343cd20ecc..eda9f3a01bf 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_with_two_stores.csv +++ b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_with_two_stores.csv @@ -1,4 +1,4 @@ product_websites,store_view_code,attribute_set_code,product_type,categories,sku,price,name,url_key -base,,Default,simple,Default Category/category-defaultstore,product,123,product,product +base,,Default,simple,Default Category/category-admin,product,123,product,product ,default,Default,simple,,product,,product-default,product-default ,fixturestore,Default,simple,,product,,product-fixture,product-fixture From b4ce314d6228206551dd8d5bd128b2ebbf9ec0a5 Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Thu, 5 Jul 2018 21:46:04 +0200 Subject: [PATCH 07/23] magento-engcom/import-export-improvements#30: Refactoring for readability. --- .../CustomerImportExport/Model/Import/Customer.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php index 3d4c40fbbed..ab940c9e845 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php @@ -593,12 +593,18 @@ protected function _validateRowForUpdate(array $rowData, $rowNumber) if (in_array($attributeCode, $this->_ignoredAttributes)) { continue; } - if ($attributeParams['is_required'] - && ((!isset($rowData[$attributeCode]) && !$this->_getCustomerId($email, $website)) - || (isset($rowData[$attributeCode]) && '' === trim($rowData[$attributeCode])))) { + + $isFieldRequired = $attributeParams['is_required']; + $isFieldNotSetAndCustomerDoesNotExist = + !isset($rowData[$attributeCode]) && !$this->_getCustomerId($email, $website); + $isFieldSetAndTrimmedValueIsEmpty + = isset($rowData[$attributeCode]) && '' === trim($rowData[$attributeCode]); + + if ($isFieldRequired && ($isFieldNotSetAndCustomerDoesNotExist || $isFieldSetAndTrimmedValueIsEmpty)) { $this->addRowError(self::ERROR_VALUE_IS_REQUIRED, $rowNumber, $attributeCode); continue; } + if (isset($rowData[$attributeCode]) && strlen($rowData[$attributeCode])) { $this->isAttributeValid( $attributeCode, From 9f0584adabd853a0640decf71be20d606ff01fdf Mon Sep 17 00:00:00 2001 From: Freek Vandeursen Date: Thu, 19 Oct 2017 11:16:39 +0200 Subject: [PATCH 08/23] Improve attribute checking On a store with a large number of attribute sets, a lot of repeated checking is done for the same attributes. So instead we can keep track of the attributes we already checked, and skip them the next time. --- .../Import/Product/Type/AbstractType.php | 53 ++++++++++++------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php index dd33b944236..8744a208bd4 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php @@ -28,6 +28,13 @@ abstract class AbstractType */ public static $commonAttributesCache = []; + /** + * Maintain a list of invisible attributes + * + * @var array + */ + public static $invisibleAttributesCache = []; + /** * Attribute Code to Id cache * @@ -278,7 +285,10 @@ protected function _initAttributes() } } foreach ($absentKeys as $attributeSetName => $attributeIds) { - $this->attachAttributesById($attributeSetName, $attributeIds); + $unknownAttributeIds = array_diff($attributeIds, array_keys(self::$commonAttributesCache), self::$invisibleAttributesCache); + if ($unknownAttributeIds) { + $this->attachAttributesById($attributeSetName, $attributeIds); + } } foreach ($entityAttributes as $attributeRow) { if (isset(self::$commonAttributesCache[$attributeRow['attribute_id']])) { @@ -310,30 +320,35 @@ protected function attachAttributesById($attributeSetName, $attributeIds) $attributeId = $attribute->getId(); if ($attribute->getIsVisible() || in_array($attributeCode, $this->_forcedAttributesCodes)) { - self::$commonAttributesCache[$attributeId] = [ - 'id' => $attributeId, - 'code' => $attributeCode, - 'is_global' => $attribute->getIsGlobal(), - 'is_required' => $attribute->getIsRequired(), - 'is_unique' => $attribute->getIsUnique(), - 'frontend_label' => $attribute->getFrontendLabel(), - 'is_static' => $attribute->isStatic(), - 'apply_to' => $attribute->getApplyTo(), - 'type' => \Magento\ImportExport\Model\Import::getAttributeType($attribute), - 'default_value' => strlen( - $attribute->getDefaultValue() - ) ? $attribute->getDefaultValue() : null, - 'options' => $this->_entityModel->getAttributeOptions( - $attribute, - $this->_indexValueAttributes - ), - ]; + if (!isset(self::$commonAttributesCache[$attributeId])) { + self::$commonAttributesCache[$attributeId] = [ + 'id' => $attributeId, + 'code' => $attributeCode, + 'is_global' => $attribute->getIsGlobal(), + 'is_required' => $attribute->getIsRequired(), + 'is_unique' => $attribute->getIsUnique(), + 'frontend_label' => $attribute->getFrontendLabel(), + 'is_static' => $attribute->isStatic(), + 'apply_to' => $attribute->getApplyTo(), + 'type' => \Magento\ImportExport\Model\Import::getAttributeType($attribute), + 'default_value' => strlen( + $attribute->getDefaultValue() + ) ? $attribute->getDefaultValue() : null, + 'options' => $this->_entityModel->getAttributeOptions( + $attribute, + $this->_indexValueAttributes + ), + ]; + } + self::$attributeCodeToId[$attributeCode] = $attributeId; $this->_addAttributeParams( $attributeSetName, self::$commonAttributesCache[$attributeId], $attribute ); + } else { + self::$invisibleAttributesCache[] = $attributeId; } } } From ed01a4a29e8d87650c758e7bd8afd1a6ec0a4de0 Mon Sep 17 00:00:00 2001 From: Freek Vandeursen Date: Thu, 19 Oct 2017 13:53:00 +0200 Subject: [PATCH 09/23] Fix code style, too long line --- .../Model/Import/Product/Type/AbstractType.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php index 8744a208bd4..ed0e8591f44 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php @@ -285,7 +285,11 @@ protected function _initAttributes() } } foreach ($absentKeys as $attributeSetName => $attributeIds) { - $unknownAttributeIds = array_diff($attributeIds, array_keys(self::$commonAttributesCache), self::$invisibleAttributesCache); + $unknownAttributeIds = array_diff( + $attributeIds, + array_keys(self::$commonAttributesCache), + self::$invisibleAttributesCache + ); if ($unknownAttributeIds) { $this->attachAttributesById($attributeSetName, $attributeIds); } From c0d696ef095561cef154ac7e4d81154a6a1b951d Mon Sep 17 00:00:00 2001 From: Freek Vandeursen Date: Thu, 26 Oct 2017 11:00:55 +0200 Subject: [PATCH 10/23] Fix too long variable name --- .../Model/Import/Product/Type/AbstractType.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php index ed0e8591f44..efcda3caf98 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php @@ -33,7 +33,7 @@ abstract class AbstractType * * @var array */ - public static $invisibleAttributesCache = []; + public static $invAttributesCache = []; /** * Attribute Code to Id cache @@ -288,7 +288,7 @@ protected function _initAttributes() $unknownAttributeIds = array_diff( $attributeIds, array_keys(self::$commonAttributesCache), - self::$invisibleAttributesCache + self::$invAttributesCache ); if ($unknownAttributeIds) { $this->attachAttributesById($attributeSetName, $attributeIds); @@ -352,7 +352,7 @@ protected function attachAttributesById($attributeSetName, $attributeIds) $attribute ); } else { - self::$invisibleAttributesCache[] = $attributeId; + self::$invAttributesCache[] = $attributeId; } } } From b5d41306eec61f76ec20f6eac8f50dfba4112405 Mon Sep 17 00:00:00 2001 From: Freek Vandeursen Date: Wed, 16 May 2018 00:09:15 +0200 Subject: [PATCH 11/23] Handle forced attribute codes --- .../Model/Import/Product/Type/AbstractType.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php index efcda3caf98..afd018f077d 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php @@ -290,7 +290,7 @@ protected function _initAttributes() array_keys(self::$commonAttributesCache), self::$invAttributesCache ); - if ($unknownAttributeIds) { + if ($unknownAttributeIds || $this->_forcedAttributesCodes) { $this->attachAttributesById($attributeSetName, $attributeIds); } } @@ -317,8 +317,11 @@ protected function _initAttributes() protected function attachAttributesById($attributeSetName, $attributeIds) { foreach ($this->_prodAttrColFac->create()->addFieldToFilter( - 'main_table.attribute_id', - ['in' => $attributeIds] + ['main_table.attribute_id', 'main_table.attribute_code'], + [ + ['in' => $attributeIds], + ['in' => $this->_forcedAttributesCodes] + ] ) as $attribute) { $attributeCode = $attribute->getAttributeCode(); $attributeId = $attribute->getId(); From fb4a81b71afc99532ba046ccdc95174ed0ce4a8e Mon Sep 17 00:00:00 2001 From: Freek Vandeursen Date: Fri, 13 Jul 2018 15:55:28 +0200 Subject: [PATCH 12/23] Update test --- .../Import/Product/Type/AbstractTypeTest.php | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Type/AbstractTypeTest.php b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Type/AbstractTypeTest.php index 70ac3a4fa2e..bd2fe896b8c 100644 --- a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Type/AbstractTypeTest.php +++ b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Type/AbstractTypeTest.php @@ -134,8 +134,28 @@ protected function setUp() ->expects($this->any()) ->method('addFieldToFilter') ->with( - 'main_table.attribute_id', - ['in' => ['attribute_id', 'boolean_attribute']] + ['main_table.attribute_id', 'main_table.attribute_code'], + [ + [ + 'in' => + [ + 'attribute_id', + 'boolean_attribute', + ], + ], + [ + 'in' => + [ + 'related_tgtr_position_behavior', + 'related_tgtr_position_limit', + 'upsell_tgtr_position_behavior', + 'upsell_tgtr_position_limit', + 'thumbnail_label', + 'small_image_label', + 'image_label', + ], + ], + ] ) ->willReturn([$attribute1, $attribute2]); From 0430946c8073a05b7c9562cb07294f709ac326a4 Mon Sep 17 00:00:00 2001 From: Justin Rhyne Date: Tue, 10 Jul 2018 17:37:09 -0400 Subject: [PATCH 13/23] Add sort order to user agent rules table headers --- .../Backend/view/adminhtml/ui_component/design_config_form.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Backend/view/adminhtml/ui_component/design_config_form.xml b/app/code/Magento/Backend/view/adminhtml/ui_component/design_config_form.xml index 46902363f1f..79c98738329 100644 --- a/app/code/Magento/Backend/view/adminhtml/ui_component/design_config_form.xml +++ b/app/code/Magento/Backend/view/adminhtml/ui_component/design_config_form.xml @@ -92,7 +92,7 @@ - + false From 48696723d2a312a162fc364a46294c76e73acc02 Mon Sep 17 00:00:00 2001 From: Tommy Quissens Date: Tue, 10 Jul 2018 12:47:04 +0200 Subject: [PATCH 14/23] Updated security issues details The e-mail address is responding with an autoreply which tells to use bugcrowd. So just mention bugcrowd instead so people don't loose time typing & encrypting a mail which has no use. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a2cf536bb65..6b2ac458eb4 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ To learn more about issue gate labels click [here](https://github.com/magento/ma

Reporting security issues

-To report security vulnerabilities in Magento software or web sites, please e-mail security@magento.com. Please do not report security issues using GitHub. Be sure to encrypt your e-mail with our encryption key if it includes sensitive information. Learn more about reporting security issues here. +To report security vulnerabilities in Magento software or web sites, please create a Bugcrowd researcher account there to submit and follow-up your issue. Learn more about reporting security issues here. Stay up-to-date on the latest security news and patches for Magento by signing up for Security Alert Notifications. From 1dd2017ffae83cbebadf78f02f7bc1eb9c860302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karla=20Saarem=C3=A4e?= Date: Sat, 14 Jul 2018 18:11:54 +0300 Subject: [PATCH 15/23] Update _rating.less --- lib/web/css/source/lib/_rating.less | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web/css/source/lib/_rating.less b/lib/web/css/source/lib/_rating.less index e585e4489d6..535fa446160 100644 --- a/lib/web/css/source/lib/_rating.less +++ b/lib/web/css/source/lib/_rating.less @@ -38,7 +38,7 @@ input[type="radio"] { .lib-visually-hidden(); - &:focus, + &:hover, &:checked { + label { &:before { From e92064ada057a657140d2ecb7380f25f301d75c5 Mon Sep 17 00:00:00 2001 From: Torrey Tsui Date: Wed, 4 Jul 2018 16:37:21 +0100 Subject: [PATCH 16/23] Fix zero price simple failed to resolve as default --- .../Pricing/Price/ConfigurablePriceResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/ConfigurableProduct/Pricing/Price/ConfigurablePriceResolver.php b/app/code/Magento/ConfigurableProduct/Pricing/Price/ConfigurablePriceResolver.php index 3d42217de5f..5581fcc07b8 100644 --- a/app/code/Magento/ConfigurableProduct/Pricing/Price/ConfigurablePriceResolver.php +++ b/app/code/Magento/ConfigurableProduct/Pricing/Price/ConfigurablePriceResolver.php @@ -64,7 +64,7 @@ public function resolvePrice(\Magento\Framework\Pricing\SaleableInterface $produ foreach ($this->lowestPriceOptionsProvider->getProducts($product) as $subProduct) { $productPrice = $this->priceResolver->resolvePrice($subProduct); - $price = $price ? min($price, $productPrice) : $productPrice; + $price = isset($price) ? min($price, $productPrice) : $productPrice; } return (float)$price; From f500af37927dbc67dcf382b181e27a9e4b699c23 Mon Sep 17 00:00:00 2001 From: torreytsui Date: Fri, 6 Jul 2018 13:44:50 +0100 Subject: [PATCH 17/23] Extract resolvePrice() test cases --- .../Price/ConfigurablePriceResolverTest.php | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php b/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php index 99c31420473..59094492023 100644 --- a/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php +++ b/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php @@ -55,24 +55,31 @@ protected function setUp() * situation: one product is supplying the price, which could be a price of zero (0) * * @dataProvider resolvePriceDataProvider + * + * @param $variantPrices + * @param $expectedPrice */ - public function testResolvePrice($expectedValue) + public function testResolvePrice($variantPrices, $expectedPrice) { - $price = $expectedValue; - $product = $this->getMockBuilder( \Magento\Catalog\Model\Product::class )->disableOriginalConstructor()->getMock(); $product->expects($this->never())->method('getSku'); - $this->lowestPriceOptionsProvider->expects($this->once())->method('getProducts')->willReturn([$product]); - $this->priceResolver->expects($this->once()) + $products = array_map(function () { + return $this->getMockBuilder(\Magento\Catalog\Model\Product::class) + ->disableOriginalConstructor() + ->getMock(); + }, $variantPrices); + + $this->lowestPriceOptionsProvider->expects($this->once())->method('getProducts')->willReturn($products); + $this->priceResolver ->method('resolvePrice') - ->with($product) - ->willReturn($price); + ->willReturnOnConsecutiveCalls(...$variantPrices); - $this->assertEquals($expectedValue, $this->resolver->resolvePrice($product)); + $actualPrice = $this->resolver->resolvePrice($product); + self::assertSame($expectedPrice, $actualPrice); } /** @@ -81,8 +88,18 @@ public function testResolvePrice($expectedValue) public function resolvePriceDataProvider() { return [ - 'price of zero' => [0.00], - 'price of five' => [5], + 'Single variant at price 0.00 (float), should return 0.00 (float)' => [ + $variantPrices = [ + 0.00, + ], + $expectedPrice = 0.00, + ], + 'Single variant at price 5 (integer), should return 5.00 (float)' => [ + $variantPrices = [ + 5, + ], + $expectedPrice = 5.00, + ], ]; } } From 8110f5fd293843746bba90d614b822d5a71df8de Mon Sep 17 00:00:00 2001 From: torreytsui Date: Fri, 6 Jul 2018 13:48:36 +0100 Subject: [PATCH 18/23] Test price at 0 against resolvePrice() --- .../Price/ConfigurablePriceResolverTest.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php b/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php index 59094492023..189730e1808 100644 --- a/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php +++ b/app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php @@ -100,6 +100,28 @@ public function resolvePriceDataProvider() ], $expectedPrice = 5.00, ], + 'Single variants at price null (null), should return 0.00 (float)' => [ + $variantPrices = [ + null, + ], + $expectedPrice = 0.00, + ], + 'Multiple variants at price 0, 10, 20, should return 0.00 (float)' => [ + $variantPrices = [ + 0, + 10, + 20, + ], + $expectedPrice = 0.00, + ], + 'Multiple variants at price 10, 0, 20, should return 0.00 (float)' => [ + $variantPrices = [ + 10, + 0, + 20, + ], + $expectedPrice = 0.00, + ], ]; } } From c0869b92844fad1d8121d8ab37817fcee8ff1e03 Mon Sep 17 00:00:00 2001 From: Sean Templeton Date: Tue, 3 Jul 2018 16:09:28 -0500 Subject: [PATCH 19/23] Fix responsive tables showing broken heading --- lib/web/css/source/lib/_tables.less | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web/css/source/lib/_tables.less b/lib/web/css/source/lib/_tables.less index 9c37e17b4fe..43b63152946 100644 --- a/lib/web/css/source/lib/_tables.less +++ b/lib/web/css/source/lib/_tables.less @@ -530,7 +530,7 @@ display: block; .lib-css(padding, @_table-responsive-cell-padding); - &:before { + &[data-th]:before { .lib-css(padding-right, @table-cell__padding-horizontal); content: attr(data-th)': '; display: inline-block; From 5905236f3b55987fc5ce24f3c72c7e8da77f69cb Mon Sep 17 00:00:00 2001 From: Valerij Ivashchenko Date: Thu, 5 Jul 2018 18:26:42 +0300 Subject: [PATCH 20/23] smallest fix in Option/Type/Text.php --- app/code/Magento/Catalog/Model/Product/Option/Type/Text.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Model/Product/Option/Type/Text.php b/app/code/Magento/Catalog/Model/Product/Option/Type/Text.php index fd0eae188fe..9ffe75e513b 100644 --- a/app/code/Magento/Catalog/Model/Product/Option/Type/Text.php +++ b/app/code/Magento/Catalog/Model/Product/Option/Type/Text.php @@ -84,7 +84,7 @@ public function validateUserValue($values) */ public function prepareForCart() { - if ($this->getIsValid() && strlen($this->getUserValue()) > 0) { + if ($this->getIsValid() && ($this->getUserValue() !== '')) { return $this->getUserValue(); } else { return null; From 955fbf2b92cee5336489c0fdede4d862dee275fd Mon Sep 17 00:00:00 2001 From: Jeroen van Leusden Date: Thu, 8 Feb 2018 17:07:49 +0100 Subject: [PATCH 21/23] Correctly save Product Custom Option values --- .../Catalog/Model/Product/Option/Value.php | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Option/Value.php b/app/code/Magento/Catalog/Model/Product/Option/Value.php index fb7759b210b..55058afd2bc 100644 --- a/app/code/Magento/Catalog/Model/Product/Option/Value.php +++ b/app/code/Magento/Catalog/Model/Product/Option/Value.php @@ -202,27 +202,29 @@ public function getProduct() public function saveValues() { foreach ($this->getValues() as $value) { - $this->isDeleted(false); - $this->setData( + $optionValue = clone $this; + $optionValue->isDeleted(false); + + $optionValue->setData( $value )->setData( 'option_id', - $this->getOption()->getId() + $optionValue->getOption()->getId() )->setData( 'store_id', - $this->getOption()->getStoreId() + $optionValue->getOption()->getStoreId() ); - if ($this->getData('is_delete') == '1') { - if ($this->getId()) { - $this->deleteValues($this->getId()); - $this->delete(); + if ($optionValue->getData('is_delete') == '1') { + if ($optionValue->getId()) { + $optionValue->deleteValues($optionValue->getId()); + $optionValue->delete(); } } else { - $this->save(); + $optionValue->save(); } } - //eof foreach() + return $this; } From db95d8ab1dee5030a3d592f9ecdea117b6b34a1f Mon Sep 17 00:00:00 2001 From: Jeroen van Leusden Date: Thu, 17 May 2018 16:47:00 +0200 Subject: [PATCH 22/23] Correctly save Product Custom Option values --- .../Magento/Catalog/Model/Product/Option.php | 28 +++++++++++----- .../Catalog/Model/Product/Option/Value.php | 32 ++++++++----------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Option.php b/app/code/Magento/Catalog/Model/Product/Option.php index 39595cdaa60..acfc454883e 100644 --- a/app/code/Magento/Catalog/Model/Product/Option.php +++ b/app/code/Magento/Catalog/Model/Product/Option.php @@ -8,6 +8,7 @@ use Magento\Catalog\Api\Data\ProductCustomOptionInterface; use Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface; +use Magento\Catalog\Api\Data\ProductCustomOptionValuesInterfaceFactory; use Magento\Catalog\Api\Data\ProductInterface; use Magento\Catalog\Model\Product; use Magento\Catalog\Model\ResourceModel\Product\Option\Value\Collection; @@ -102,6 +103,11 @@ class Option extends AbstractExtensibleModel implements ProductCustomOptionInter */ private $metadataPool; + /** + * @var ProductCustomOptionValuesInterfaceFactory + */ + private $customOptionValuesFactory; + /** * @param \Magento\Framework\Model\Context $context * @param \Magento\Framework\Registry $registry @@ -114,6 +120,7 @@ class Option extends AbstractExtensibleModel implements ProductCustomOptionInter * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource * @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection * @param array $data + * @param ProductCustomOptionValuesInterfaceFactory|null $customOptionValuesFactory * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -127,12 +134,16 @@ public function __construct( Option\Validator\Pool $validatorPool, \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null, \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null, - array $data = [] + array $data = [], + ProductCustomOptionValuesInterfaceFactory $customOptionValuesFactory = null ) { $this->productOptionValue = $productOptionValue; $this->optionTypeFactory = $optionFactory; $this->validatorPool = $validatorPool; $this->string = $string; + $this->customOptionValuesFactory = $customOptionValuesFactory ?: + \Magento\Framework\App\ObjectManager::getInstance()->get(ProductCustomOptionValuesInterfaceFactory::class); + parent::__construct( $context, $registry, @@ -390,20 +401,21 @@ public function beforeSave() */ public function afterSave() { - $this->getValueInstance()->unsetValues(); $values = $this->getValues() ?: $this->getData('values'); if (is_array($values)) { foreach ($values as $value) { - if ($value instanceof \Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface) { + if ($value instanceof ProductCustomOptionValuesInterface) { $data = $value->getData(); } else { $data = $value; } - $this->getValueInstance()->addValue($data); - } - $this->getValueInstance()->setOption($this)->saveValues(); - } elseif ($this->getGroupByType($this->getType()) == self::OPTION_GROUP_SELECT) { + $this->customOptionValuesFactory->create() + ->addValue($data) + ->setOption($this) + ->saveValues(); + } + } elseif ($this->getGroupByType($this->getType()) === self::OPTION_GROUP_SELECT) { throw new LocalizedException(__('Select type options required values rows.')); } @@ -804,7 +816,7 @@ public function setImageSizeY($imageSizeY) } /** - * @param \Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface[] $values + * @param ProductCustomOptionValuesInterface[] $values * @return $this */ public function setValues(array $values = null) diff --git a/app/code/Magento/Catalog/Model/Product/Option/Value.php b/app/code/Magento/Catalog/Model/Product/Option/Value.php index 55058afd2bc..ebbc060c99e 100644 --- a/app/code/Magento/Catalog/Model/Product/Option/Value.php +++ b/app/code/Magento/Catalog/Model/Product/Option/Value.php @@ -76,6 +76,7 @@ class Value extends AbstractModel implements \Magento\Catalog\Api\Data\ProductCu * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource * @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection * @param array $data + * @param CustomOptionPriceCalculator|null $customOptionPriceCalculator */ public function __construct( \Magento\Framework\Model\Context $context, @@ -89,6 +90,7 @@ public function __construct( $this->_valueCollectionFactory = $valueCollectionFactory; $this->customOptionPriceCalculator = $customOptionPriceCalculator ?? \Magento\Framework\App\ObjectManager::getInstance()->get(CustomOptionPriceCalculator::class); + parent::__construct( $context, $registry, @@ -201,27 +203,21 @@ public function getProduct() */ public function saveValues() { + $option = $this->getOption(); + foreach ($this->getValues() as $value) { - $optionValue = clone $this; - $optionValue->isDeleted(false); - - $optionValue->setData( - $value - )->setData( - 'option_id', - $optionValue->getOption()->getId() - )->setData( - 'store_id', - $optionValue->getOption()->getStoreId() - ); - - if ($optionValue->getData('is_delete') == '1') { - if ($optionValue->getId()) { - $optionValue->deleteValues($optionValue->getId()); - $optionValue->delete(); + $this->isDeleted(false); + $this->setData($value) + ->setData('option_id', $option->getId()) + ->setData('store_id', $option->getStoreId()); + + if ((bool) $this->getData('is_delete') === true) { + if ($this->getId()) { + $this->deleteValues($this->getId()); + $this->delete(); } } else { - $optionValue->save(); + $this->save(); } } From f73b1193bbdf4ecea6a92b53a2eab2d1bed6f6eb Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Wed, 18 Jul 2018 15:10:12 +0300 Subject: [PATCH 23/23] Revert backward incompatible changes --- .../Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php index adf7ad1ed2e..ee59fae657e 100644 --- a/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php +++ b/app/code/Magento/Shipping/Model/Carrier/AbstractCarrierInterface.php @@ -90,7 +90,7 @@ public function checkAvailableShipCountries(\Magento\Framework\DataObject $reque * @return $this|\Magento\Framework\DataObject|boolean * @api */ - public function processAdditionalValidation(\Magento\Framework\DataObject $request); + public function proccessAdditionalValidation(\Magento\Framework\DataObject $request); /** * Determine whether current carrier enabled for activity