Skip to content

Commit e56696e

Browse files
RuslanKostiv1RuslanKostiv1
committed
Merge remote-tracking branch 'origin/MAGETWO-86918' into PANDA-FIXES-2.2
2 parents 2bbf7e9 + 4e031d7 commit e56696e

File tree

3 files changed

+165
-25
lines changed

3 files changed

+165
-25
lines changed

‎app/code/Magento/ConfigurableProduct/Model/Product/SaveHandler.php

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
1111
use Magento\ConfigurableProduct\Model\ResourceModel\Product\Type\Configurable as ResourceModelConfigurable;
1212
use Magento\Framework\EntityManager\Operation\ExtensionInterface;
13+
use Magento\ConfigurableProduct\Api\Data\OptionInterface;
14+
use Magento\ConfigurableProduct\Model\Product\Type\Configurable\Attribute;
1315

1416
/**
1517
* Class SaveHandler
@@ -76,7 +78,7 @@ public function execute($entity, $arguments = [])
7678
}
7779

7880
/**
79-
* Save attributes for configurable product
81+
* Save only newly created attributes for configurable product
8082
*
8183
* @param ProductInterface $product
8284
* @param array $attributes
@@ -85,26 +87,57 @@ public function execute($entity, $arguments = [])
8587
private function saveConfigurableProductAttributes(ProductInterface $product, array $attributes)
8688
{
8789
$ids = [];
90+
$existingAttributeIds = [];
91+
foreach ($this->optionRepository->getList($product->getSku()) as $option) {
92+
$existingAttributeIds[$option->getAttributeId()] = $option;
93+
}
8894
/** @var \Magento\ConfigurableProduct\Model\Product\Type\Configurable\Attribute $attribute */
8995
foreach ($attributes as $attribute) {
90-
$attribute->setId(null);
91-
$ids[] = $this->optionRepository->save($product->getSku(), $attribute);
96+
if (!in_array($attribute->getAttributeId(), array_keys($existingAttributeIds))
97+
|| $this->isOptionChanged($existingAttributeIds[$attribute->getAttributeId()], $attribute)
98+
) {
99+
$attribute->setId(null);
100+
$ids[] = $this->optionRepository->save($product->getSku(), $attribute);
101+
}
92102
}
93-
94103
return $ids;
95104
}
96105

97106
/**
98-
* Remove product attributes
107+
* Remove product attributes which no longer used
99108
*
100109
* @param ProductInterface $product
101110
* @return void
102111
*/
103112
private function deleteConfigurableProductAttributes(ProductInterface $product)
104113
{
105-
$list = $this->optionRepository->getList($product->getSku());
106-
foreach ($list as $item) {
107-
$this->optionRepository->deleteById($product->getSku(), $item->getId());
114+
$newAttributeIds = [];
115+
foreach ($product->getExtensionAttributes()->getConfigurableProductOptions() as $option) {
116+
$newAttributeIds[$option->getAttributeId()] = $option;
117+
}
118+
foreach ($this->optionRepository->getList($product->getSku()) as $option) {
119+
if (!in_array($option->getAttributeId(), array_keys($newAttributeIds))
120+
|| $this->isOptionChanged($option, $newAttributeIds[$option->getAttributeId()])
121+
) {
122+
$this->optionRepository->deleteById($product->getSku(), $option->getId());
123+
}
124+
}
125+
}
126+
127+
/**
128+
* Check if existing option is changed
129+
*
130+
* @param OptionInterface $option
131+
* @param Attribute $attribute
132+
* @return bool
133+
*/
134+
private function isOptionChanged(OptionInterface $option, Attribute $attribute)
135+
{
136+
if ($option->getLabel() == $attribute->getLabel()
137+
&& $option->getPosition() == $attribute->getPosition()
138+
) {
139+
return false;
108140
}
141+
return true;
109142
}
110143
}

‎app/code/Magento/ConfigurableProduct/Test/Unit/Model/Product/SaveHandlerTest.php

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ public function testExecuteWithInvalidProductType()
8888
public function testExecuteWithEmptyExtensionAttributes()
8989
{
9090
$sku = 'test';
91+
$configurableProductLinks = [1, 2, 3];
9192
$product = $this->getMockBuilder(Product::class)
9293
->disableOriginalConstructor()
9394
->setMethods(['getTypeId', 'getExtensionAttributes', 'getSku'])
@@ -105,16 +106,16 @@ public function testExecuteWithEmptyExtensionAttributes()
105106
->disableOriginalConstructor()
106107
->getMockForAbstractClass();
107108

108-
$product->expects(static::once())
109+
$product->expects(static::atLeastOnce())
109110
->method('getExtensionAttributes')
110111
->willReturn($extensionAttributes);
111112

112-
$extensionAttributes->expects(static::exactly(2))
113+
$extensionAttributes->expects(static::atLeastOnce())
113114
->method('getConfigurableProductOptions')
114115
->willReturn([]);
115-
$extensionAttributes->expects(static::once())
116+
$extensionAttributes->expects(static::atLeastOnce())
116117
->method('getConfigurableProductLinks')
117-
->willReturn([]);
118+
->willReturn($configurableProductLinks);
118119

119120
$this->optionRepository->expects(static::once())
120121
->method('getList')
@@ -133,7 +134,10 @@ public function testExecuteWithEmptyExtensionAttributes()
133134
public function testExecute()
134135
{
135136
$sku = 'config-1';
136-
$id = 25;
137+
$idOld = 25;
138+
$idNew = 26;
139+
$attributeIdOld = 11;
140+
$attributeIdNew = 22;
137141
$configurableProductLinks = [1, 2, 3];
138142

139143
$product = $this->getMockBuilder(Product::class)
@@ -143,7 +147,7 @@ public function testExecute()
143147
$product->expects(static::once())
144148
->method('getTypeId')
145149
->willReturn(ConfigurableModel::TYPE_CODE);
146-
$product->expects(static::exactly(3))
150+
$product->expects(static::exactly(4))
147151
->method('getSku')
148152
->willReturn($sku);
149153

@@ -156,30 +160,36 @@ public function testExecute()
156160
->method('getExtensionAttributes')
157161
->willReturn($extensionAttributes);
158162

159-
$attribute = $this->getMockBuilder(Attribute::class)
163+
$attributeNew = $this->getMockBuilder(Attribute::class)
160164
->disableOriginalConstructor()
161165
->setMethods(['getAttributeId', 'loadByProductAndAttribute', 'setId', 'getId'])
162166
->getMock();
163-
$this->processSaveOptions($attribute, $sku, $id);
164-
165-
$option = $this->getMockForAbstractClass(OptionInterface::class);
166-
$option->expects(static::once())
167+
$attributeNew->expects(static::atLeastOnce())
168+
->method('getAttributeId')
169+
->willReturn($attributeIdNew);
170+
$this->processSaveOptions($attributeNew, $sku, $idNew);
171+
172+
$optionOld = $this->getMockForAbstractClass(OptionInterface::class);
173+
$optionOld->expects(static::atLeastOnce())
174+
->method('getAttributeId')
175+
->willReturn($attributeIdOld);
176+
$optionOld->expects(static::atLeastOnce())
167177
->method('getId')
168-
->willReturn($id);
178+
->willReturn($idOld);
169179

170-
$list = [$option];
171-
$this->optionRepository->expects(static::once())
180+
$list = [$optionOld];
181+
$this->optionRepository->expects(static::atLeastOnce())
172182
->method('getList')
173183
->with($sku)
174184
->willReturn($list);
175185
$this->optionRepository->expects(static::once())
176186
->method('deleteById')
177-
->with($sku, $id);
187+
->with($sku, $idOld);
178188

179189
$configurableAttributes = [
180-
$attribute
190+
$attributeNew
181191
];
182-
$extensionAttributes->expects(static::exactly(2))
192+
$extensionAttributes->expects(static::atLeastOnce())
183193
->method('getConfigurableProductOptions')
184194
->willReturn($configurableAttributes);
185195

‎dev/tests/integration/testsuite/Magento/ConfigurableProduct/Model/Product/SaveHandlerTest.php

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
// @codingStandardsIgnoreFile
8+
9+
namespace Magento\ConfigurableProduct\Model\Product;
10+
11+
use Magento\Catalog\Api\ProductRepositoryInterface;
12+
use Magento\Catalog\Model\Product;
13+
use Magento\TestFramework\Helper\Bootstrap;
14+
use Magento\ConfigurableProduct\Model\ResourceModel\Product\Type\Configurable as ConfigurableResource;
15+
16+
/**
17+
* @magentoAppIsolation enabled
18+
* @magentoDataFixture Magento/ConfigurableProduct/_files/product_configurable.php
19+
*/
20+
class SaveHandlerTest extends \PHPUnit\Framework\TestCase
21+
{
22+
/**
23+
* Object under test
24+
*
25+
* @var SaveHandler
26+
*/
27+
private $handler;
28+
29+
/**
30+
* @var Product
31+
*/
32+
private $product;
33+
34+
/**
35+
* @var ProductRepositoryInterface
36+
*/
37+
private $productRepository;
38+
39+
/**
40+
* @var ConfigurableResource
41+
*/
42+
private $resource;
43+
44+
/**
45+
* @inheritdoc
46+
*/
47+
protected function setUp()
48+
{
49+
$this->productRepository = Bootstrap::getObjectManager()
50+
->create(ProductRepositoryInterface::class);
51+
$this->product = $this->productRepository->get('configurable');
52+
$this->resource = Bootstrap::getObjectManager()->create(ConfigurableResource::class);
53+
$this->handler = Bootstrap::getObjectManager()
54+
->create(SaveHandler::class);
55+
}
56+
57+
public function testExecuteWithConfigurableProductLinksChanged()
58+
{
59+
$childrenIds = $this->product->getTypeInstance()
60+
->getChildrenIds($this->product->getId());
61+
$newChildrenIds = [reset($childrenIds[0])];
62+
$product = $this->productRepository->getById($this->product->getId());
63+
$extensionAttributes = $product->getExtensionAttributes();
64+
$extensionAttributes->setConfigurableProductLinks($newChildrenIds);
65+
$product->setExtensionAttributes($extensionAttributes);
66+
$this->handler->execute($product);
67+
$childrenIds = $this->product->getTypeInstance()
68+
->getChildrenIds($this->product->getId());
69+
$savedChildrenIds = [reset($childrenIds[0])];
70+
self::assertEquals($newChildrenIds, $savedChildrenIds);
71+
}
72+
73+
public function testExecuteWithConfigurableProductLinksNotChanged()
74+
{
75+
$childrenIds = $this->product->getTypeInstance()
76+
->getChildrenIds($this->product->getId())[0];
77+
$product = $this->productRepository->getById($this->product->getId());
78+
$extensionAttributes = $product->getExtensionAttributes();
79+
$extensionAttributes->setConfigurableProductLinks($childrenIds);
80+
$product->setExtensionAttributes($extensionAttributes);
81+
$oldProductLinks = $this->getCurrentProductLinks();
82+
$this->handler->execute($product);
83+
$newProductLinks = $this->getCurrentProductLinks();
84+
self::assertEquals($oldProductLinks, $newProductLinks);
85+
}
86+
87+
/**
88+
* @return array
89+
*/
90+
private function getCurrentProductLinks()
91+
{
92+
$select = $this->resource->getConnection()->select()->from(
93+
['l' => $this->resource->getMainTable()]
94+
);
95+
return $this->resource->getConnection()->fetchAll($select);
96+
}
97+
}

0 commit comments

Comments
 (0)