From aa2a9a3318d559c5cd27e7c39a67fe1b69502213 Mon Sep 17 00:00:00 2001 From: Federico Rivollier Date: Wed, 28 Feb 2018 00:54:06 -0300 Subject: [PATCH 1/9] WIP: Removed logic for generating unique SKU when trying to save an existing one --- .../Model/Product/Attribute/Backend/Sku.php | 57 ++----------------- 1 file changed, 6 insertions(+), 51 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 a652d0ef902..04ced8d5e27 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -60,32 +60,16 @@ public function validate($object) __('SKU length should be %1 characters maximum.', self::SKU_MAX_LENGTH) ); } - return true; - } - /** - * Generate and set unique SKU to product - * - * @param Product $object - * @return void - */ - 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); + if (!$entity->checkAttributeUniqueValue($attribute, $object)) { + throw new \Magento\Framework\Exception\LocalizedException( + __('SKU is already in use.') + ); } + + return true; } /** @@ -96,35 +80,6 @@ protected function _generateUniqueSku($object) */ public function beforeSave($object) { - $this->_generateUniqueSku($object); return parent::beforeSave($object); } - - /** - * Return increment needed for SKU uniqueness - * - * @param \Magento\Eav\Model\Entity\Attribute\AbstractAttribute $attribute - * @param Product $object - * @return int - */ - protected function _getLastSimilarAttributeValueIncrement($attribute, $object) - { - $connection = $this->getAttribute()->getEntity()->getConnection(); - $select = $connection->select(); - $value = $object->getData($attribute->getAttributeCode()); - $bind = ['attribute_code' => trim($value) . '-%']; - - $select->from( - $this->getTable(), - $attribute->getAttributeCode() - )->where( - $attribute->getAttributeCode() . ' LIKE :attribute_code' - )->order( - ['entity_id DESC', $attribute->getAttributeCode() . ' ASC'] - )->limit( - 1 - ); - $data = $connection->fetchOne($select, $bind); - return abs((int)str_replace($value, '', $data)); - } } From 51171db1afc9032f24076afaab67960757bbe092 Mon Sep 17 00:00:00 2001 From: David Manners Date: Wed, 30 May 2018 10:37:16 +0000 Subject: [PATCH 2/9] magento-engcom/import-export-improvements#101: update functional test to expect the newer exception message --- .../Test/Constraint/AssertProductUrlDuplicateErrorMessage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php index 0741a226cea..7d18c0f994d 100644 --- a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php +++ b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php @@ -19,7 +19,7 @@ class AssertProductUrlDuplicateErrorMessage extends AbstractConstraint /** * Text title of the error message to be checked. */ - const ERROR_MESSAGE_TITLE = 'The value specified in the URL Key field would generate a URL that already exists.'; + const ERROR_MESSAGE_TITLE = 'SKU is already in use.'; /** * Assert that success message is displayed after product save. From d353d01b57eded387275f7e27dbe7078051ea0d7 Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sat, 11 Aug 2018 23:37:41 +0200 Subject: [PATCH 3/9] magento-engcom/import-export-improvements#101: Re-add SKU generation in case product is duplicated. Duplicate needs a SKU to be saved. --- .../Model/Product/Attribute/Backend/Sku.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) 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 04ced8d5e27..6412a7e2e0b 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -72,6 +72,31 @@ public function validate($object) return true; } + /** + * Generate and set unique SKU to product + * + * @param Product $object + * @return void + */ + 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 * @@ -80,6 +105,37 @@ public function validate($object) */ public function beforeSave($object) { + if ($object->getIsDuplicate()) { + $this->_generateUniqueSku($object); + } return parent::beforeSave($object); } + + /** + * Return increment needed for SKU uniqueness + * + * @param \Magento\Eav\Model\Entity\Attribute\AbstractAttribute $attribute + * @param Product $object + * @return int + */ + protected function _getLastSimilarAttributeValueIncrement($attribute, $object) + { + $connection = $this->getAttribute()->getEntity()->getConnection(); + $select = $connection->select(); + $value = $object->getData($attribute->getAttributeCode()); + $bind = ['attribute_code' => trim($value) . '-%']; + + $select->from( + $this->getTable(), + $attribute->getAttributeCode() + )->where( + $attribute->getAttributeCode() . ' LIKE :attribute_code' + )->order( + ['entity_id DESC', $attribute->getAttributeCode() . ' ASC'] + )->limit( + 1 + ); + $data = $connection->fetchOne($select, $bind); + return abs((int)str_replace($value, '', $data)); + } } From ec142afee3edab9f91b2be2828fe2b97bc22921d Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sat, 18 Aug 2018 19:48:20 +0200 Subject: [PATCH 4/9] magento-engcom/import-export-improvements#101: Adjust tests to new behaviour. --- .../Product/Attribute/Backend/SkuTest.php | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) 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 f5579198978..5938499b53d 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 @@ -14,18 +14,34 @@ class SkuTest extends \PHPUnit\Framework\TestCase /** * @magentoDataFixture Magento/Catalog/_files/product_simple.php */ - public function testGenerateUniqueSkuExistingProduct() + public function testGenerateUniqueSkuExistingProductDuplication() { $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( \Magento\Catalog\Model\ProductRepository::class ); $product = $repository->get('simple'); $product->setId(null); + $product->setIsDuplicate(true); $this->assertEquals('simple', $product->getSku()); $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); $this->assertEquals('simple-1', $product->getSku()); } + /** + * @magentoDataFixture Magento/Catalog/_files/product_simple.php + */ + public function testGenerateUniqueSkuExistingProductNoDuplication() + { + $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\Catalog\Model\ProductRepository::class + ); + $product = $repository->get('simple'); + $product->setId(null); + $this->assertEquals('simple', $product->getSku()); + $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); + $this->assertEquals('simple', $product->getSku()); + } + /** * @param $product \Magento\Catalog\Model\Product * @dataProvider uniqueSkuDataProvider @@ -54,10 +70,14 @@ public function testGenerateUniqueLongSku() $copier = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( \Magento\Catalog\Model\Product\Copier::class ); - $copier->copy($product); + $copy = $copier->copy($product); $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890123', $product->getSku()); $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); - $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product->getSku()); + $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890123', $product->getSku()); + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $copy->getSku()); + + $copy->getResource()->getAttribute('sku')->getBackend()->beforeSave($copy); + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-2', $copy->getSku()); } /** From 3d6c47453772030d9a2b2d6875ebd1592d21c731 Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sat, 18 Aug 2018 21:41:11 +0200 Subject: [PATCH 5/9] magento-engcom/import-export-improvements#101: setIsDuplicate(true) to ensure cloned products get valid SKUs. --- .../Magento/CatalogRule/Model/Indexer/IndexerBuilderTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/CatalogRule/Model/Indexer/IndexerBuilderTest.php b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/Indexer/IndexerBuilderTest.php index 13f6e5ae0e8..a1b8e1662af 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogRule/Model/Indexer/IndexerBuilderTest.php +++ b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/Indexer/IndexerBuilderTest.php @@ -137,10 +137,11 @@ protected function prepareProducts() $this->product->setStoreId(0)->setData('test_attribute', 'test_attribute_value')->save(); $this->productSecond = clone $this->product; - $this->productSecond->setId(null)->setUrlKey('product-second')->save(); + $this->productSecond->setId(null)->setUrlKey('product-second')->setIsDuplicate(true)->save(); $this->productThird = clone $this->product; $this->productThird->setId(null) ->setUrlKey('product-third') + ->setIsDuplicate(true) ->setData('test_attribute', 'NO_test_attribute_value') ->save(); } From 1326f9bac7ae6675b36c1b8f5b444afe93c1f1ab Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Sat, 18 Aug 2018 21:55:21 +0200 Subject: [PATCH 6/9] magento-engcom/import-export-improvements#101: Undo error message change. Does not fit the test and the constraint anymore this way. Test needs to be adjusted to result in expected error. --- .../Test/Constraint/AssertProductUrlDuplicateErrorMessage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php index 7d18c0f994d..0741a226cea 100644 --- a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php +++ b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/Constraint/AssertProductUrlDuplicateErrorMessage.php @@ -19,7 +19,7 @@ class AssertProductUrlDuplicateErrorMessage extends AbstractConstraint /** * Text title of the error message to be checked. */ - const ERROR_MESSAGE_TITLE = 'SKU is already in use.'; + const ERROR_MESSAGE_TITLE = 'The value specified in the URL Key field would generate a URL that already exists.'; /** * Assert that success message is displayed after product save. From 3d05ec77a8a439f719f8d0edd00a6926b8721dfb Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Tue, 11 Sep 2018 19:36:17 +0200 Subject: [PATCH 7/9] magento-engcom/import-export-improvements#101: Test adjustments to new behaviour. A product's SKU won't be changed by beforeSave anymore. Only a duplicated product will receive a changed SKU on its creation. --- .../Catalog/Model/Product/Attribute/Backend/SkuTest.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) 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 5938499b53d..63b64e102e5 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 @@ -64,7 +64,7 @@ public function testGenerateUniqueLongSku() \Magento\Catalog\Model\ProductRepository::class ); $product = $repository->get('simple'); - $product->setSku('0123456789012345678901234567890123456789012345678901234567890123'); + $product->setSku('0123456789012345678901234567890123456789012345678901234567890123')->save(); /** @var \Magento\Catalog\Model\Product\Copier $copier */ $copier = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( @@ -72,12 +72,7 @@ public function testGenerateUniqueLongSku() ); $copy = $copier->copy($product); $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890123', $product->getSku()); - $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); - $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890123', $product->getSku()); $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $copy->getSku()); - - $copy->getResource()->getAttribute('sku')->getBackend()->beforeSave($copy); - $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-2', $copy->getSku()); } /** From 2368e8f690b09f4452dc3534f35c4003ebd3715e Mon Sep 17 00:00:00 2001 From: carstenpfeifer Date: Tue, 23 Oct 2018 16:00:25 +0200 Subject: [PATCH 8/9] magento-engcom/import-export-improvements#101: Change test case to use two different products with the same URL key to prevent 'SKU already exists' error. --- .../CreateDuplicateUrlProductEntity.php | 18 +++++++++++------- .../CreateDuplicateUrlProductEntity.xml | 8 +++++++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.php b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.php index 2cfaecd8efe..591a0d8eded 100644 --- a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.php +++ b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.php @@ -68,6 +68,7 @@ public function __prepare(Category $category) * Run create product simple entity test. * * @param CatalogProductSimple $product + * @param CatalogProductSimple $product2 * @param Category $category * @param CatalogProductIndex $productGrid * @param CatalogProductNew $newProductPage @@ -77,6 +78,7 @@ public function __prepare(Category $category) */ public function testCreate( CatalogProductSimple $product, + CatalogProductSimple $product2, Category $category, CatalogProductIndex $productGrid, CatalogProductNew $newProductPage, @@ -92,13 +94,15 @@ public function testCreate( ['configData' => $this->configData, 'flushCache' => $this->flushCache] )->run(); - for ($index = 0; $index < 2; $index++) { - // Duplicate product - $productGrid->open(); - $productGrid->getGridPageActionBlock()->addProduct('simple'); - $newProductPage->getProductForm()->fill($product, null, $category); - $newProductPage->getFormPageActions()->save(); - } + $productGrid->open(); + $productGrid->getGridPageActionBlock()->addProduct('simple'); + $newProductPage->getProductForm()->fill($product, null, $category); + $newProductPage->getFormPageActions()->save(); + + $productGrid->open(); + $productGrid->getGridPageActionBlock()->addProduct('simple'); + $newProductPage->getProductForm()->fill($product2, null, $category); + $newProductPage->getFormPageActions()->save(); return ['product' => $product]; } diff --git a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.xml b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.xml index 1116821f756..17f3b75e70a 100644 --- a/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.xml +++ b/dev/tests/functional/tests/app/Magento/CatalogUrlRewrite/Test/TestCase/CreateDuplicateUrlProductEntity.xml @@ -9,12 +9,18 @@ test_type:acceptance_test, test_type:extended_acceptance_test, severity:S1 - simple-product-%isolation% + simple-product-samekey Simple Product %isolation% simple_sku_%isolation% 10000 50 657 + simple-product-samekey + Simple Product %isolation% + simple_sku_%isolation% + 10000 + 50 + 657 From f26354846646e80d0c063ab399ee7cea38e4d2df Mon Sep 17 00:00:00 2001 From: David Manners Date: Wed, 24 Oct 2018 18:24:54 +0200 Subject: [PATCH 9/9] magento-engcom/import-export-improvements#101: file clean-up Clean up the header of the class to fit with the static-tests. --- .../Catalog/Model/Product/Attribute/Backend/Sku.php | 7 +++---- 1 file changed, 3 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 6412a7e2e0b..44404710eb2 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -3,16 +3,15 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +namespace Magento\Catalog\Model\Product\Attribute\Backend; + +use Magento\Catalog\Model\Product; /** * Catalog product SKU backend attribute model * * @author Magento Core Team */ -namespace Magento\Catalog\Model\Product\Attribute\Backend; - -use Magento\Catalog\Model\Product; - class Sku extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend { /**