From a3a3fbb37cec074dc257d0c8537d3083be74d9cf Mon Sep 17 00:00:00 2001 From: wokisajn Date: Fri, 18 May 2018 15:05:23 +0200 Subject: [PATCH 1/4] ISSUE:7121 - warning when try to create multiple products with the same SKU --- .../Model/Product/Attribute/Backend/Sku.php | 29 ++++++++++--------- .../Magento/Catalog/Setup/UpgradeSchema.php | 17 +++++++++++ app/code/Magento/Catalog/etc/module.xml | 2 +- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php index a9da2a3400de9..2383b2f68bfb0 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -14,6 +14,7 @@ namespace Magento\Catalog\Model\Product\Attribute\Backend; use Magento\Catalog\Model\Product; +use Magento\Framework\Exception\AlreadyExistsException; class Sku extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend { @@ -64,27 +65,27 @@ public function validate($object) } /** - * Generate and set unique SKU to product + * Check unique SKU for product * * @param Product $object * @return void */ - protected function _generateUniqueSku($object) + protected function _checkUniqueSku($object) { $attribute = $this->getAttribute(); $entity = $attribute->getEntity(); - $attributeValue = $object->getData($attribute->getAttributeCode()); - $increment = null; + $attributeCode = $attribute->getAttributeCode(); + $attributeValue = $object->getData($attributeCode); while (!$entity->checkAttributeUniqueValue($attribute, $object)) { - if ($increment === null) { - $increment = $this->_getLastSimilarAttributeValueIncrement($attribute, $object); - } - $sku = trim($attributeValue); - if (strlen($sku . '-' . ++$increment) > self::SKU_MAX_LENGTH) { - $sku = substr($sku, 0, -strlen($increment) - 1); - } - $sku = $sku . '-' . $increment; - $object->setData($attribute->getAttributeCode(), $sku); + throw new AlreadyExistsException( + __( + 'Duplicated %attribute found: %value', + [ + 'attribute' => $attributeCode, + 'value' => $attributeValue + ] + ) + ); } } @@ -96,7 +97,7 @@ protected function _generateUniqueSku($object) */ public function beforeSave($object) { - $this->_generateUniqueSku($object); + $this->_checkUniqueSku($object); return parent::beforeSave($object); } diff --git a/app/code/Magento/Catalog/Setup/UpgradeSchema.php b/app/code/Magento/Catalog/Setup/UpgradeSchema.php index 1ccfd7c243320..0f25600d5ce7c 100755 --- a/app/code/Magento/Catalog/Setup/UpgradeSchema.php +++ b/app/code/Magento/Catalog/Setup/UpgradeSchema.php @@ -142,9 +142,26 @@ public function upgrade(SchemaSetupInterface $setup, ModuleContextInterface $con $this->enableSegmentation($setup); } + if (version_compare($context->getVersion(), '2.2.6', '<')) { + $this->modifySkuColumn($setup); + } + $setup->endSetup(); } + /** + * @param SchemaSetupInterface $setup + */ + private function modifySkuColumn(SchemaSetupInterface $setup) + { + $setup->getConnection()->addIndex( + $setup->getTable('catalog_product_entity'), + $setup->getIdxName('catalog_product_entity', ['sku']), + ['sku'], + \Magento\Framework\DB\Adapter\AdapterInterface::INDEX_TYPE_UNIQUE + ); + } + /** * Change definition of customer group id column * diff --git a/app/code/Magento/Catalog/etc/module.xml b/app/code/Magento/Catalog/etc/module.xml index cb27a54c2b58b..23e130aa8a991 100644 --- a/app/code/Magento/Catalog/etc/module.xml +++ b/app/code/Magento/Catalog/etc/module.xml @@ -6,7 +6,7 @@ */ --> - + From f1c4662c70f6ea3916e5facdaf659569233b4ce6 Mon Sep 17 00:00:00 2001 From: wokisajn Date: Fri, 18 May 2018 16:09:05 +0200 Subject: [PATCH 2/4] ISSUE:7121 - original method is reverted and deprecated --- .../Model/Product/Attribute/Backend/Sku.php | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php index 2383b2f68bfb0..b284a00c8931e 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -70,7 +70,7 @@ public function validate($object) * @param Product $object * @return void */ - protected function _checkUniqueSku($object) + private function checkUniqueSku($object) { $attribute = $this->getAttribute(); $entity = $attribute->getEntity(); @@ -89,6 +89,32 @@ protected function _checkUniqueSku($object) } } + /** + * Generate and set unique SKU to product + * + * @param Product $object + * @return void + * @deprecated + */ + protected function _generateUniqueSku($object) + { + $attribute = $this->getAttribute(); + $entity = $attribute->getEntity(); + $attributeValue = $object->getData($attribute->getAttributeCode()); + $increment = null; + while (!$entity->checkAttributeUniqueValue($attribute, $object)) { + if ($increment === null) { + $increment = $this->_getLastSimilarAttributeValueIncrement($attribute, $object); + } + $sku = trim($attributeValue); + if (strlen($sku . '-' . ++$increment) > self::SKU_MAX_LENGTH) { + $sku = substr($sku, 0, -strlen($increment) - 1); + } + $sku = $sku . '-' . $increment; + $object->setData($attribute->getAttributeCode(), $sku); + } + } + /** * Make SKU unique before save * @@ -97,7 +123,7 @@ protected function _checkUniqueSku($object) */ public function beforeSave($object) { - $this->_checkUniqueSku($object); + $this->checkUniqueSku($object); return parent::beforeSave($object); } @@ -107,6 +133,7 @@ public function beforeSave($object) * @param \Magento\Eav\Model\Entity\Attribute\AbstractAttribute $attribute * @param Product $object * @return int + * @deprecated */ protected function _getLastSimilarAttributeValueIncrement($attribute, $object) { From 545575d60484c8805972acd7dc57a60ee39e8102 Mon Sep 17 00:00:00 2001 From: ecomchallenger Date: Fri, 25 May 2018 07:51:32 +0200 Subject: [PATCH 3/4] Customer Address Custom Attribute gets deleted upon update #6411 (Integration tests, improvement for product duplication) --- .../Model/Product/Attribute/Backend/Sku.php | 26 ++++-- .../Product/Attribute/Backend/SkuTest.php | 86 +++++++++++++++++-- 2 files changed, 100 insertions(+), 12 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php index b284a00c8931e..d1739561ba502 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -11,6 +11,7 @@ * * @author Magento Core Team */ + namespace Magento\Catalog\Model\Product\Attribute\Backend; use Magento\Catalog\Model\Product; @@ -44,6 +45,7 @@ public function __construct(\Magento\Framework\Stdlib\StringUtils $string) * Validate SKU * * @param Product $object + * * @return bool * @throws \Magento\Framework\Exception\LocalizedException * @throws \Magento\Framework\Exception\LocalizedException @@ -53,7 +55,9 @@ public function validate($object) $attrCode = $this->getAttribute()->getAttributeCode(); $value = $object->getData($attrCode); if ($this->getAttribute()->getIsRequired() && strlen($value) === 0) { - throw new \Magento\Framework\Exception\LocalizedException(__('The value of attribute "%1" must be set', $attrCode)); + throw new \Magento\Framework\Exception\LocalizedException( + __('The value of attribute "%1" must be set', $attrCode) + ); } if ($this->string->strlen($object->getSku()) > self::SKU_MAX_LENGTH) { @@ -61,6 +65,7 @@ public function validate($object) __('SKU length should be %1 characters maximum.', self::SKU_MAX_LENGTH) ); } + return true; } @@ -68,7 +73,10 @@ public function validate($object) * Check unique SKU for product * * @param Product $object + * * @return void + * + * @throws AlreadyExistsException */ private function checkUniqueSku($object) { @@ -82,7 +90,7 @@ private function checkUniqueSku($object) 'Duplicated %attribute found: %value', [ 'attribute' => $attributeCode, - 'value' => $attributeValue + 'value' => $attributeValue, ] ) ); @@ -93,8 +101,8 @@ private function checkUniqueSku($object) * Generate and set unique SKU to product * * @param Product $object + * * @return void - * @deprecated */ protected function _generateUniqueSku($object) { @@ -119,11 +127,17 @@ protected function _generateUniqueSku($object) * Make SKU unique before save * * @param Product $object + * * @return $this */ public function beforeSave($object) { - $this->checkUniqueSku($object); + if (true === $object->getIsDuplicate()) { + $this->_generateUniqueSku($object); + } else { + $this->checkUniqueSku($object); + } + return parent::beforeSave($object); } @@ -132,6 +146,7 @@ public function beforeSave($object) * * @param \Magento\Eav\Model\Entity\Attribute\AbstractAttribute $attribute * @param Product $object + * * @return int * @deprecated */ @@ -153,6 +168,7 @@ protected function _getLastSimilarAttributeValueIncrement($attribute, $object) 1 ); $data = $connection->fetchOne($select, $bind); - return abs((int)str_replace($value, '', $data)); + + return abs((int) str_replace($value, '', $data)); } } diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php index f557919897869..9e022b71baf0d 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php @@ -3,10 +3,12 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Catalog\Model\Product\Attribute\Backend; /** * Test class for \Magento\Catalog\Model\Product\Attribute\Backend\Sku. + * * @magentoAppArea adminhtml */ class SkuTest extends \PHPUnit\Framework\TestCase @@ -14,7 +16,28 @@ class SkuTest extends \PHPUnit\Framework\TestCase /** * @magentoDataFixture Magento/Catalog/_files/product_simple.php */ - public function testGenerateUniqueSkuExistingProduct() + public function testGenerateUniqueSkuDuplicatedProduct() + { + $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\Catalog\Model\ProductRepository::class + ); + $product = $repository->get('simple'); + /** @var \Magento\Catalog\Model\Product\Copier $copier */ + $copier = $this->getCopier(); + /** @var \Magento\Catalog\Model\Product; $product2 */ + $product2 = $copier->copy($product); + + $this->assertEquals('simple', $product->getSku()); + $product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2); + $this->assertEquals('simple-1', $product2->getSku()); + } + + /** + * Checks if generation of unique sku is not allowed + * + * @magentoDataFixture Magento/Catalog/_files/product_simple.php + */ + public function testPreventGenerateUniqueSkuExistingProduct() { $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( \Magento\Catalog\Model\ProductRepository::class @@ -22,12 +45,16 @@ public function testGenerateUniqueSkuExistingProduct() $product = $repository->get('simple'); $product->setId(null); $this->assertEquals('simple', $product->getSku()); + + $this->expectException('Magento\Framework\Exception\AlreadyExistsException'); $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); - $this->assertEquals('simple-1', $product->getSku()); + + $this->fail('Unique sku generation should be allowed only for product duplication.'); } /** * @param $product \Magento\Catalog\Model\Product + * * @dataProvider uniqueSkuDataProvider */ public function testGenerateUniqueSkuNotExistingProduct($product) @@ -42,22 +69,54 @@ public function testGenerateUniqueSkuNotExistingProduct($product) * @magentoAppArea adminhtml * @magentoDbIsolation enabled */ - public function testGenerateUniqueLongSku() + public function testGenerateUniqueLongSkuDuplicatedProduct() { $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( \Magento\Catalog\Model\ProductRepository::class ); $product = $repository->get('simple'); - $product->setSku('0123456789012345678901234567890123456789012345678901234567890123'); + $product->setSku('0123456789012345678901234567890123456789012345678901234567890124'); + $product->save(); + $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); + $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890124', $product->getSku()); /** @var \Magento\Catalog\Model\Product\Copier $copier */ - $copier = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( - \Magento\Catalog\Model\Product\Copier::class + $copier = $this->getCopier(); + $product2 = $copier->copy($product); + + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product2->getSku()); + $product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2); + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product2->getSku()); + + $product2->setId(null); + $product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2); + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-2', $product2->getSku()); + } + + /** + * Checks if generation of long unique sku is not allowed + * + * @magentoDataFixture Magento/Catalog/_files/product_simple.php + * @magentoAppArea adminhtml + * @magentoDbIsolation enabled + */ + public function testPreventGenerateUniqueLongSku() + { + $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\Catalog\Model\ProductRepository::class ); + $product = $repository->get('simple'); + $product->setSku('0123456789012345678901234567890123456789012345678901234567890123'); + + /** @var \Magento\Catalog\Model\Product\Copier $copier */ + $copier = $this->getCopier(); + $copier->copy($product); $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890123', $product->getSku()); + $this->expectException('Magento\Framework\Exception\AlreadyExistsException'); $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); - $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product->getSku()); + + $this->fail('Unique long sku generation should be allowed only for product duplication.'); } /** @@ -68,6 +127,7 @@ public function testGenerateUniqueLongSku() public function uniqueSkuDataProvider() { $product = $this->_getProduct(); + return [[$product]]; } @@ -107,6 +167,18 @@ protected function _getProduct() )->setStockData( ['use_config_manage_stock' => 1, 'qty' => 100, 'is_qty_decimal' => 0, 'is_in_stock' => 1] ); + return $product; } + + /** + * @return \Magento\Catalog\Model\Product\Copier + */ + protected function getCopier() + { + return \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( + \Magento\Catalog\Model\Product\Copier::class + ); + } + } From bc1bc09b0282748f08da84abd91ed8fbfa842fe7 Mon Sep 17 00:00:00 2001 From: wokisajn Date: Fri, 15 Jun 2018 14:10:54 +0200 Subject: [PATCH 4/4] Issue: 7121 - Before save without "else" --- .../Magento/Catalog/Model/Product/Attribute/Backend/Sku.php | 5 ++--- .../Catalog/Model/Product/Attribute/Backend/SkuTest.php | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php index d1739561ba502..c75654138c4e8 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -90,7 +90,7 @@ private function checkUniqueSku($object) 'Duplicated %attribute found: %value', [ 'attribute' => $attributeCode, - 'value' => $attributeValue, + 'value' => $attributeValue ] ) ); @@ -134,9 +134,8 @@ public function beforeSave($object) { if (true === $object->getIsDuplicate()) { $this->_generateUniqueSku($object); - } else { - $this->checkUniqueSku($object); } + $this->checkUniqueSku($object); return parent::beforeSave($object); } diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php index 9e022b71baf0d..357f97c9d27c3 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php @@ -180,5 +180,4 @@ protected function getCopier() \Magento\Catalog\Model\Product\Copier::class ); } - }