Skip to content

ISSUE:7121 - warning when try to create multiple products with the sa… #15317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
*
* @author Magento Core Team <core@magentocommerce.com>
*/

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
{
Expand Down Expand Up @@ -43,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
Expand All @@ -52,21 +55,53 @@ 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) {
throw new \Magento\Framework\Exception\LocalizedException(
__('SKU length should be %1 characters maximum.', self::SKU_MAX_LENGTH)
);
}

return true;
}

/**
* Check unique SKU for product
*
* @param Product $object
*
* @return void
*
* @throws AlreadyExistsException
*/
private function checkUniqueSku($object)
{
$attribute = $this->getAttribute();
$entity = $attribute->getEntity();
$attributeCode = $attribute->getAttributeCode();
$attributeValue = $object->getData($attributeCode);
while (!$entity->checkAttributeUniqueValue($attribute, $object)) {
throw new AlreadyExistsException(
__(
'Duplicated %attribute found: %value',
[
'attribute' => $attributeCode,
'value' => $attributeValue
]
)
);
}
}

/**
* Generate and set unique SKU to product
*
* @param Product $object
*
* @return void
*/
protected function _generateUniqueSku($object)
Expand All @@ -92,11 +127,16 @@ protected function _generateUniqueSku($object)
* Make SKU unique before save
*
* @param Product $object
*
* @return $this
*/
public function beforeSave($object)
{
$this->_generateUniqueSku($object);
if (true === $object->getIsDuplicate()) {
$this->_generateUniqueSku($object);
}
$this->checkUniqueSku($object);

return parent::beforeSave($object);
}

Expand All @@ -105,7 +145,9 @@ public function beforeSave($object)
*
* @param \Magento\Eav\Model\Entity\Attribute\AbstractAttribute $attribute
* @param Product $object
*
* @return int
* @deprecated
*/
protected function _getLastSimilarAttributeValueIncrement($attribute, $object)
{
Expand All @@ -125,6 +167,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));
}
}
17 changes: 17 additions & 0 deletions app/code/Magento/Catalog/Setup/UpgradeSchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/Catalog/etc/module.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_Catalog" setup_version="2.2.5">
<module name="Magento_Catalog" setup_version="2.2.6">
<sequence>
<module name="Magento_Eav"/>
<module name="Magento_Cms"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,58 @@
* 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
{
/**
* @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
);
$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)
Expand All @@ -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.');
}

/**
Expand All @@ -68,6 +127,7 @@ public function testGenerateUniqueLongSku()
public function uniqueSkuDataProvider()
{
$product = $this->_getProduct();

return [[$product]];
}

Expand Down Expand Up @@ -107,6 +167,17 @@ 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
);
}
}