Skip to content

Commit 02596b7

Browse files
committed
MAGETWO-67240: Duplicate and broken products appear after import
1 parent 96fca03 commit 02596b7

File tree

16 files changed

+311
-109
lines changed

16 files changed

+311
-109
lines changed

app/code/Magento/BundleImportExport/Model/Import/Product/Type/Bundle.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use \Magento\Framework\App\ObjectManager;
1212
use \Magento\Bundle\Model\Product\Price as BundlePrice;
1313
use \Magento\Catalog\Model\Product\Type\AbstractType;
14+
use Magento\CatalogImportExport\Model\Import\Product;
1415

1516
/**
1617
* Class Bundle
@@ -173,7 +174,7 @@ protected function parseSelections($rowData, $entityId)
173174
$rowData['bundle_values']
174175
);
175176
$selections = explode(
176-
\Magento\CatalogImportExport\Model\Import\Product::PSEUDO_MULTI_LINE_SEPARATOR,
177+
Product::PSEUDO_MULTI_LINE_SEPARATOR,
177178
$rowData['bundle_values']
178179
);
179180
foreach ($selections as $selection) {
@@ -343,7 +344,7 @@ public function saveData()
343344
$newSku = $this->_entityModel->getNewSku();
344345
while ($bunch = $this->_entityModel->getNextBunch()) {
345346
foreach ($bunch as $rowNum => $rowData) {
346-
$productData = $newSku[$rowData[\Magento\CatalogImportExport\Model\Import\Product::COL_SKU]];
347+
$productData = $newSku[strtolower($rowData[Product::COL_SKU])];
347348
$productIds[] = $productData[$this->getProductEntityLinkField()];
348349
}
349350
$this->deleteOptionsAndSelections($productIds);
@@ -355,7 +356,7 @@ public function saveData()
355356
if (!$this->_entityModel->isRowAllowedToImport($rowData, $rowNum)) {
356357
continue;
357358
}
358-
$productData = $newSku[$rowData[\Magento\CatalogImportExport\Model\Import\Product::COL_SKU]];
359+
$productData = $newSku[strtolower($rowData[Product::COL_SKU])];
359360
if ($this->_type != $productData['type_id']) {
360361
continue;
361362
}

app/code/Magento/BundleImportExport/Test/Unit/Model/Import/Product/Type/BundleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,11 @@ public function testSaveDataProvider()
299299
'bunch' => ['bundle_values' => 'value1', 'sku' => 'sku', 'name' => 'name'],
300300
'allowImport' => true
301301
],
302+
[
303+
'skus' => ['newSku' => ['sku' => ['sku' => 'SKU', 'entity_id' => 3, 'type_id' => 'bundle']]],
304+
'bunch' => ['bundle_values' => 'value1', 'sku' => 'SKU', 'name' => 'name'],
305+
'allowImport' => true
306+
],
302307
[
303308
'skus' => ['newSku' => ['sku' => ['sku' => 'sku', 'entity_id' => 3, 'type_id' => 'simple']]],
304309
'bunch' => ['bundle_values' => 'value1', 'sku' => 'sku', 'name' => 'name'],

app/code/Magento/CatalogImportExport/Model/Import/Product.php

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,7 @@ protected function _deleteProducts()
925925

926926
foreach ($bunch as $rowNum => $rowData) {
927927
if ($this->validateRow($rowData, $rowNum) && self::SCOPE_DEFAULT == $this->getRowScope($rowData)) {
928-
$idsToDelete[] = $this->_oldSku[$rowData[self::COL_SKU]]['entity_id'];
928+
$idsToDelete[] = $this->_oldSku[strtolower($rowData[self::COL_SKU])]['entity_id'];
929929
}
930930
}
931931
if ($idsToDelete) {
@@ -1115,7 +1115,7 @@ protected function _prepareRowForDb(array $rowData)
11151115

11161116
$lastSku = $rowData[self::COL_SKU];
11171117

1118-
if (isset($this->_oldSku[$lastSku])) {
1118+
if (isset($this->_oldSku[strtolower($lastSku)])) {
11191119
$newSku = $this->skuProcessor->getNewSku($lastSku);
11201120
$rowData[self::COL_ATTR_SET] = $newSku['attr_set_code'];
11211121
$rowData[self::COL_TYPE] = $newSku['type_id'];
@@ -1195,7 +1195,7 @@ protected function _saveLinks()
11951195
if (!empty($newSku)) {
11961196
$linkedId = $newSku['entity_id'];
11971197
} else {
1198-
$linkedId = $this->_oldSku[$linkedSku]['entity_id'];
1198+
$linkedId = $this->_oldSku[strtolower($linkedSku)]['entity_id'];
11991199
}
12001200

12011201
if ($linkedId == null) {
@@ -1396,7 +1396,7 @@ private function updateOldSku(array $newProducts)
13961396
$oldSkus = [];
13971397
foreach ($newProducts as $info) {
13981398
$typeId = $info['type_id'];
1399-
$sku = $info['sku'];
1399+
$sku = strtolower($info['sku']);
14001400
$oldSkus[$sku] = [
14011401
'type_id' => $typeId,
14021402
'attr_set_id' => $info['attribute_set_id'],
@@ -1455,7 +1455,7 @@ protected function getExistingImages($bunch)
14551455
}
14561456

14571457
$this->initMediaGalleryResources();
1458-
$productSKUs = array_map('strval', array_column($bunch, self::COL_SKU));
1458+
$productSKUs = array_map('strtolower', array_column($bunch, self::COL_SKU));
14591459
$select = $this->_connection->select()->from(
14601460
['mg' => $this->mediaGalleryTableName],
14611461
['value' => 'mg.value']
@@ -1582,7 +1582,7 @@ protected function _saveProducts()
15821582
}
15831583

15841584
// 1. Entity phase
1585-
if (isset($this->_oldSku[$rowSku])) {
1585+
if (isset($this->_oldSku[strtolower($rowSku)])) {
15861586
// existing row
15871587
if (isset($rowData['attribute_set_code'])) {
15881588
$attributeSetId = $this->catalogConfig->getAttributeSetId(
@@ -1606,14 +1606,14 @@ protected function _saveProducts()
16061606
$entityRowsUp[] = [
16071607
'updated_at' => (new \DateTime())->format(DateTime::DATETIME_PHP_FORMAT),
16081608
'attribute_set_id' => $attributeSetId,
1609-
$this->getProductEntityLinkField() => $this->_oldSku[$rowSku][$this->getProductEntityLinkField()]
1609+
$this->getProductEntityLinkField() => $this->_oldSku[strtolower($rowSku)][$this->getProductEntityLinkField()]
16101610
];
16111611
} else {
16121612
if (!$productLimit || $productsQty < $productLimit) {
16131613
$entityRowsIn[$rowSku] = [
16141614
'attribute_set_id' => $this->skuProcessor->getNewSku($rowSku)['attr_set_id'],
16151615
'type_id' => $this->skuProcessor->getNewSku($rowSku)['type_id'],
1616-
'sku' => $rowSku,
1616+
'sku' => $rowData[self::COL_SKU],
16171617
'has_options' => isset($rowData['has_options']) ? $rowData['has_options'] : 0,
16181618
'created_at' => (new \DateTime())->format(DateTime::DATETIME_PHP_FORMAT),
16191619
'updated_at' => (new \DateTime())->format(DateTime::DATETIME_PHP_FORMAT),
@@ -2192,7 +2192,8 @@ protected function _saveStockItem()
21922192
}
21932193

21942194
$row = [];
2195-
$row['product_id'] = $this->skuProcessor->getNewSku($rowData[self::COL_SKU])['entity_id'];
2195+
$sku = strtolower($rowData[self::COL_SKU]);
2196+
$row['product_id'] = $this->skuProcessor->getNewSku($sku)['entity_id'];
21962197
$productIdsToReindex[] = $row['product_id'];
21972198

21982199
$row['website_id'] = $this->stockConfiguration->getDefaultScopeId();
@@ -2209,7 +2210,7 @@ protected function _saveStockItem()
22092210
);
22102211

22112212
if ($this->stockConfiguration->isQty(
2212-
$this->skuProcessor->getNewSku($rowData[self::COL_SKU])['type_id']
2213+
$this->skuProcessor->getNewSku($sku)['type_id']
22132214
)) {
22142215
$stockItemDo->setData($row);
22152216
$row['is_in_stock'] = $this->stockStateProvider->verifyStock($stockItemDo);
@@ -2224,8 +2225,8 @@ protected function _saveStockItem()
22242225
} else {
22252226
$row['qty'] = 0;
22262227
}
2227-
if (!isset($stockData[$rowData[self::COL_SKU]])) {
2228-
$stockData[$rowData[self::COL_SKU]] = $row;
2228+
if (!isset($stockData[$sku])) {
2229+
$stockData[$sku] = $row;
22292230
}
22302231
}
22312232

@@ -2305,6 +2306,9 @@ public function getEntityTypeCode()
23052306
/**
23062307
* New products SKU data.
23072308
*
2309+
* Returns array of new products data with SKU as key. All SKU keys are in lowercase for avoiding creation of
2310+
* new products with the same SKU in different letter cases.
2311+
*
23082312
* @var string $sku
23092313
* @return array
23102314
*/
@@ -2326,6 +2330,9 @@ public function getNextBunch()
23262330
/**
23272331
* Existing products SKU getter.
23282332
*
2333+
* Returns array of existing products data with SKU as key. All SKU keys are in lowercase for avoiding creation of
2334+
* new products with the same SKU in different letter cases.
2335+
*
23292336
* @return array
23302337
*/
23312338
public function getOldSku()
@@ -2376,16 +2383,17 @@ public function validateRow(array $rowData, $rowNum)
23762383
$this->_validatedRows[$rowNum] = true;
23772384

23782385
$rowScope = $this->getRowScope($rowData);
2386+
$sku = strtolower($rowData[self::COL_SKU]);
23792387

23802388
// BEHAVIOR_DELETE and BEHAVIOR_REPLACE use specific validation logic
23812389
if (Import::BEHAVIOR_REPLACE == $this->getBehavior()) {
2382-
if (self::SCOPE_DEFAULT == $rowScope && !isset($this->_oldSku[$rowData[self::COL_SKU]])) {
2390+
if (self::SCOPE_DEFAULT == $rowScope && !isset($this->_oldSku[$sku])) {
23832391
$this->addRowError(ValidatorInterface::ERROR_SKU_NOT_FOUND_FOR_DELETE, $rowNum);
23842392
return false;
23852393
}
23862394
}
23872395
if (Import::BEHAVIOR_DELETE == $this->getBehavior()) {
2388-
if (self::SCOPE_DEFAULT == $rowScope && !isset($this->_oldSku[$rowData[self::COL_SKU]])) {
2396+
if (self::SCOPE_DEFAULT == $rowScope && !isset($this->_oldSku[$sku])) {
23892397
$this->addRowError(ValidatorInterface::ERROR_SKU_NOT_FOUND_FOR_DELETE, $rowNum);
23902398
return false;
23912399
}
@@ -2398,10 +2406,9 @@ public function validateRow(array $rowData, $rowNum)
23982406
}
23992407
}
24002408

2401-
$sku = $rowData[self::COL_SKU];
2402-
if (null === $sku) {
2409+
if (null === $rowData[self::COL_SKU]) {
24032410
$this->addRowError(ValidatorInterface::ERROR_SKU_IS_EMPTY, $rowNum);
2404-
} elseif (false === $sku) {
2411+
} elseif (false === $rowData[self::COL_SKU]) {
24052412
$this->addRowError(ValidatorInterface::ERROR_ROW_IS_ORPHAN, $rowNum);
24062413
} elseif (self::SCOPE_STORE == $rowScope
24072414
&& !$this->storeResolver->getStoreCodeToId($rowData[self::COL_STORE])
@@ -2412,8 +2419,6 @@ public function validateRow(array $rowData, $rowNum)
24122419
// SKU is specified, row is SCOPE_DEFAULT, new product block begins
24132420
$this->_processedEntitiesCount++;
24142421

2415-
$sku = $rowData[self::COL_SKU];
2416-
24172422
if (isset($this->_oldSku[$sku]) && Import::BEHAVIOR_REPLACE !== $this->getBehavior()) {
24182423
// can we get all necessary data from existent DB product?
24192424
// check for supported type of existing product
@@ -2486,9 +2491,9 @@ public function validateRow(array $rowData, $rowNum)
24862491
$productUrlSuffix = $this->getProductUrlSuffix($storeId);
24872492
$urlPath = $urlKey . $productUrlSuffix;
24882493
if (empty($this->urlKeys[$storeId][$urlPath])
2489-
|| ($this->urlKeys[$storeId][$urlPath] == $rowData[self::COL_SKU])
2494+
|| ($this->urlKeys[$storeId][$urlPath] == $sku)
24902495
) {
2491-
$this->urlKeys[$storeId][$urlPath] = $rowData[self::COL_SKU];
2496+
$this->urlKeys[$storeId][$urlPath] = $sku;
24922497
$this->rowNumbers[$storeId][$urlPath] = $rowNum;
24932498
} else {
24942499
$message = sprintf(

app/code/Magento/CatalogImportExport/Model/Import/Product/SkuProcessor.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ public function reloadOldSkus()
113113
*/
114114
public function addNewSku($sku, $data)
115115
{
116+
$sku = strtolower($sku);
116117
$this->newSkus[$sku] = $data;
117118
return $this;
118119
}
@@ -125,6 +126,7 @@ public function addNewSku($sku, $data)
125126
*/
126127
public function setNewSkuData($sku, $key, $data)
127128
{
129+
$sku = strtolower($sku);
128130
if (isset($this->newSkus[$sku])) {
129131
$this->newSkus[$sku][$key] = $data;
130132
}
@@ -138,6 +140,7 @@ public function setNewSkuData($sku, $key, $data)
138140
public function getNewSku($sku = null)
139141
{
140142
if ($sku !== null) {
143+
$sku = strtolower($sku);
141144
return isset($this->newSkus[$sku]) ? $this->newSkus[$sku] : null;
142145
}
143146
return $this->newSkus;
@@ -157,7 +160,7 @@ protected function _getSkus()
157160
}
158161
foreach ($this->productFactory->create()->getProductEntitiesInfo($columns) as $info) {
159162
$typeId = $info['type_id'];
160-
$sku = $info['sku'];
163+
$sku = strtolower($info['sku']);
161164
$oldSkus[$sku] = [
162165
'type_id' => $typeId,
163166
'attr_set_id' => $info['attribute_set_id'],

app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/SuperProductsSku.php

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,20 @@
66
namespace Magento\CatalogImportExport\Model\Import\Product\Validator;
77

88
use Magento\CatalogImportExport\Model\Import\Product\RowValidatorInterface;
9+
use Magento\CatalogImportExport\Model\Import\Product\SkuProcessor;
910

1011
class SuperProductsSku extends AbstractImportValidator implements RowValidatorInterface
1112
{
1213
/**
13-
* @var \Magento\CatalogImportExport\Model\Import\Product\SkuProcessor
14+
* @var SkuProcessor
1415
*/
1516
protected $skuProcessor;
1617

1718
/**
18-
* @param \Magento\CatalogImportExport\Model\Import\Product\SkuProcessor $skuProcessor
19+
* @param SkuProcessor $skuProcessor
1920
*/
2021
public function __construct(
21-
\Magento\CatalogImportExport\Model\Import\Product\SkuProcessor $skuProcessor
22+
SkuProcessor $skuProcessor
2223
) {
2324
$this->skuProcessor = $skuProcessor;
2425
}
@@ -30,13 +31,14 @@ public function isValid($value)
3031
{
3132
$this->_clearMessages();
3233
$oldSku = $this->skuProcessor->getOldSkus();
33-
if (!empty($value['_super_products_sku']) && (!isset(
34-
$oldSku[$value['_super_products_sku']]
35-
) && $this->skuProcessor->getNewSku($value['_super_products_sku']) === null
36-
)
37-
) {
38-
$this->_addMessages([self::ERROR_SUPER_PRODUCTS_SKU_NOT_FOUND]);
39-
return false;
34+
if (!empty($value['_super_products_sku'])) {
35+
$superSku = strtolower($value['_super_products_sku']);
36+
if (!isset($oldSku[$superSku])
37+
&& $this->skuProcessor->getNewSku($superSku) === null
38+
) {
39+
$this->_addMessages([self::ERROR_SUPER_PRODUCTS_SKU_NOT_FOUND]);
40+
return false;
41+
}
4042
}
4143
return true;
4244
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\CatalogImportExport\Test\Unit\Model\Import\Product\Validator;
7+
8+
use Magento\CatalogImportExport\Model\Import\Product\SkuProcessor;
9+
use Magento\CatalogImportExport\Model\Import\Product\Validator\SuperProductsSku;
10+
use PHPUnit_Framework_MockObject_MockObject as Mock;
11+
12+
/**
13+
* Test for SuperProductsSku
14+
*
15+
* @see SuperProductsSku
16+
*/
17+
class SuperProductsSkuTest extends \PHPUnit_Framework_TestCase
18+
{
19+
/**
20+
* @var SkuProcessor|Mock
21+
*/
22+
private $skuProcessorMock;
23+
24+
/**
25+
* @var SuperProductsSku
26+
*/
27+
private $model;
28+
29+
protected function setUp()
30+
{
31+
$this->skuProcessorMock = $this->getMockBuilder(SkuProcessor::class)
32+
->disableOriginalConstructor()
33+
->getMock();
34+
35+
$this->model = new SuperProductsSku($this->skuProcessorMock);
36+
}
37+
38+
/**
39+
* @param array $value
40+
* @param array $oldSkus
41+
* @param bool $hasNewSku
42+
* @param bool $expectedResult
43+
* @dataProvider isValidDataProvider
44+
*/
45+
public function testIsValid(array $value, array $oldSkus, $hasNewSku = false, $expectedResult = true)
46+
{
47+
$this->skuProcessorMock->expects($this->once())
48+
->method('getOldSkus')
49+
->willReturn($oldSkus);
50+
51+
if ($hasNewSku) {
52+
$this->skuProcessorMock->expects($this->once())
53+
->method('getNewSku')
54+
->willReturn('someNewSku');
55+
}
56+
57+
$this->assertEquals($expectedResult, $this->model->isValid($value));
58+
}
59+
60+
/**
61+
* @return array
62+
*/
63+
public function isValidDataProvider()
64+
{
65+
return [
66+
[
67+
[],
68+
[],
69+
],
70+
[
71+
[],
72+
['sku1' => []]
73+
],
74+
[
75+
['_super_products_sku' => 'SKU1'],
76+
['sku2' => []],
77+
false,
78+
false
79+
],
80+
[
81+
['_super_products_sku' => 'SKU1'],
82+
['sku2' => []],
83+
true,
84+
true
85+
],
86+
[
87+
['_super_products_sku' => 'SKU1'],
88+
['sku1' => []],
89+
false,
90+
true
91+
],
92+
];
93+
}
94+
}

0 commit comments

Comments
 (0)