Skip to content

Commit 12482fc

Browse files
authored
Merge pull request #1 from ecomchallenger/issue-7121-warning-multiple-products-same-sku
ISSUE:7121 - warning when try to create multiple products with the same SKU (product duplication improvement and test)
2 parents f1c4662 + 545575d commit 12482fc

2 files changed

Lines changed: 100 additions & 12 deletions

File tree

  • app/code/Magento/Catalog/Model/Product/Attribute/Backend
  • dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend

app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*
1212
* @author Magento Core Team <core@magentocommerce.com>
1313
*/
14+
1415
namespace Magento\Catalog\Model\Product\Attribute\Backend;
1516

1617
use Magento\Catalog\Model\Product;
@@ -44,6 +45,7 @@ public function __construct(\Magento\Framework\Stdlib\StringUtils $string)
4445
* Validate SKU
4546
*
4647
* @param Product $object
48+
*
4749
* @return bool
4850
* @throws \Magento\Framework\Exception\LocalizedException
4951
* @throws \Magento\Framework\Exception\LocalizedException
@@ -53,22 +55,28 @@ public function validate($object)
5355
$attrCode = $this->getAttribute()->getAttributeCode();
5456
$value = $object->getData($attrCode);
5557
if ($this->getAttribute()->getIsRequired() && strlen($value) === 0) {
56-
throw new \Magento\Framework\Exception\LocalizedException(__('The value of attribute "%1" must be set', $attrCode));
58+
throw new \Magento\Framework\Exception\LocalizedException(
59+
__('The value of attribute "%1" must be set', $attrCode)
60+
);
5761
}
5862

5963
if ($this->string->strlen($object->getSku()) > self::SKU_MAX_LENGTH) {
6064
throw new \Magento\Framework\Exception\LocalizedException(
6165
__('SKU length should be %1 characters maximum.', self::SKU_MAX_LENGTH)
6266
);
6367
}
68+
6469
return true;
6570
}
6671

6772
/**
6873
* Check unique SKU for product
6974
*
7075
* @param Product $object
76+
*
7177
* @return void
78+
*
79+
* @throws AlreadyExistsException
7280
*/
7381
private function checkUniqueSku($object)
7482
{
@@ -82,7 +90,7 @@ private function checkUniqueSku($object)
8290
'Duplicated %attribute found: %value',
8391
[
8492
'attribute' => $attributeCode,
85-
'value' => $attributeValue
93+
'value' => $attributeValue,
8694
]
8795
)
8896
);
@@ -93,8 +101,8 @@ private function checkUniqueSku($object)
93101
* Generate and set unique SKU to product
94102
*
95103
* @param Product $object
104+
*
96105
* @return void
97-
* @deprecated
98106
*/
99107
protected function _generateUniqueSku($object)
100108
{
@@ -119,11 +127,17 @@ protected function _generateUniqueSku($object)
119127
* Make SKU unique before save
120128
*
121129
* @param Product $object
130+
*
122131
* @return $this
123132
*/
124133
public function beforeSave($object)
125134
{
126-
$this->checkUniqueSku($object);
135+
if (true === $object->getIsDuplicate()) {
136+
$this->_generateUniqueSku($object);
137+
} else {
138+
$this->checkUniqueSku($object);
139+
}
140+
127141
return parent::beforeSave($object);
128142
}
129143

@@ -132,6 +146,7 @@ public function beforeSave($object)
132146
*
133147
* @param \Magento\Eav\Model\Entity\Attribute\AbstractAttribute $attribute
134148
* @param Product $object
149+
*
135150
* @return int
136151
* @deprecated
137152
*/
@@ -153,6 +168,7 @@ protected function _getLastSimilarAttributeValueIncrement($attribute, $object)
153168
1
154169
);
155170
$data = $connection->fetchOne($select, $bind);
156-
return abs((int)str_replace($value, '', $data));
171+
172+
return abs((int) str_replace($value, '', $data));
157173
}
158174
}

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,31 +3,58 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Catalog\Model\Product\Attribute\Backend;
78

89
/**
910
* Test class for \Magento\Catalog\Model\Product\Attribute\Backend\Sku.
11+
*
1012
* @magentoAppArea adminhtml
1113
*/
1214
class SkuTest extends \PHPUnit\Framework\TestCase
1315
{
1416
/**
1517
* @magentoDataFixture Magento/Catalog/_files/product_simple.php
1618
*/
17-
public function testGenerateUniqueSkuExistingProduct()
19+
public function testGenerateUniqueSkuDuplicatedProduct()
20+
{
21+
$repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
22+
\Magento\Catalog\Model\ProductRepository::class
23+
);
24+
$product = $repository->get('simple');
25+
/** @var \Magento\Catalog\Model\Product\Copier $copier */
26+
$copier = $this->getCopier();
27+
/** @var \Magento\Catalog\Model\Product; $product2 */
28+
$product2 = $copier->copy($product);
29+
30+
$this->assertEquals('simple', $product->getSku());
31+
$product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2);
32+
$this->assertEquals('simple-1', $product2->getSku());
33+
}
34+
35+
/**
36+
* Checks if generation of unique sku is not allowed
37+
*
38+
* @magentoDataFixture Magento/Catalog/_files/product_simple.php
39+
*/
40+
public function testPreventGenerateUniqueSkuExistingProduct()
1841
{
1942
$repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
2043
\Magento\Catalog\Model\ProductRepository::class
2144
);
2245
$product = $repository->get('simple');
2346
$product->setId(null);
2447
$this->assertEquals('simple', $product->getSku());
48+
49+
$this->expectException('Magento\Framework\Exception\AlreadyExistsException');
2550
$product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product);
26-
$this->assertEquals('simple-1', $product->getSku());
51+
52+
$this->fail('Unique sku generation should be allowed only for product duplication.');
2753
}
2854

2955
/**
3056
* @param $product \Magento\Catalog\Model\Product
57+
*
3158
* @dataProvider uniqueSkuDataProvider
3259
*/
3360
public function testGenerateUniqueSkuNotExistingProduct($product)
@@ -42,22 +69,54 @@ public function testGenerateUniqueSkuNotExistingProduct($product)
4269
* @magentoAppArea adminhtml
4370
* @magentoDbIsolation enabled
4471
*/
45-
public function testGenerateUniqueLongSku()
72+
public function testGenerateUniqueLongSkuDuplicatedProduct()
4673
{
4774
$repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
4875
\Magento\Catalog\Model\ProductRepository::class
4976
);
5077
$product = $repository->get('simple');
51-
$product->setSku('0123456789012345678901234567890123456789012345678901234567890123');
78+
$product->setSku('0123456789012345678901234567890123456789012345678901234567890124');
79+
$product->save();
80+
$product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product);
81+
$this->assertEquals('0123456789012345678901234567890123456789012345678901234567890124', $product->getSku());
5282

5383
/** @var \Magento\Catalog\Model\Product\Copier $copier */
54-
$copier = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
55-
\Magento\Catalog\Model\Product\Copier::class
84+
$copier = $this->getCopier();
85+
$product2 = $copier->copy($product);
86+
87+
$this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product2->getSku());
88+
$product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2);
89+
$this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product2->getSku());
90+
91+
$product2->setId(null);
92+
$product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2);
93+
$this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-2', $product2->getSku());
94+
}
95+
96+
/**
97+
* Checks if generation of long unique sku is not allowed
98+
*
99+
* @magentoDataFixture Magento/Catalog/_files/product_simple.php
100+
* @magentoAppArea adminhtml
101+
* @magentoDbIsolation enabled
102+
*/
103+
public function testPreventGenerateUniqueLongSku()
104+
{
105+
$repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
106+
\Magento\Catalog\Model\ProductRepository::class
56107
);
108+
$product = $repository->get('simple');
109+
$product->setSku('0123456789012345678901234567890123456789012345678901234567890123');
110+
111+
/** @var \Magento\Catalog\Model\Product\Copier $copier */
112+
$copier = $this->getCopier();
113+
57114
$copier->copy($product);
58115
$this->assertEquals('0123456789012345678901234567890123456789012345678901234567890123', $product->getSku());
116+
$this->expectException('Magento\Framework\Exception\AlreadyExistsException');
59117
$product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product);
60-
$this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product->getSku());
118+
119+
$this->fail('Unique long sku generation should be allowed only for product duplication.');
61120
}
62121

63122
/**
@@ -68,6 +127,7 @@ public function testGenerateUniqueLongSku()
68127
public function uniqueSkuDataProvider()
69128
{
70129
$product = $this->_getProduct();
130+
71131
return [[$product]];
72132
}
73133

@@ -107,6 +167,18 @@ protected function _getProduct()
107167
)->setStockData(
108168
['use_config_manage_stock' => 1, 'qty' => 100, 'is_qty_decimal' => 0, 'is_in_stock' => 1]
109169
);
170+
110171
return $product;
111172
}
173+
174+
/**
175+
* @return \Magento\Catalog\Model\Product\Copier
176+
*/
177+
protected function getCopier()
178+
{
179+
return \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
180+
\Magento\Catalog\Model\Product\Copier::class
181+
);
182+
}
183+
112184
}

0 commit comments

Comments
 (0)