Skip to content

Commit 2ee1c96

Browse files
zakdmazakdma
committed
Merge remote-tracking branch 'origin/MAGETWO-90316' into 2.3-develop-pr14
2 parents ace5fce + b6e289b commit 2ee1c96

File tree

3 files changed

+169
-26
lines changed

3 files changed

+169
-26
lines changed

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

Lines changed: 42 additions & 9 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,35 +78,66 @@ 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
8385
* @return array
8486
*/
85-
private function saveConfigurableProductAttributes(ProductInterface $product, array $attributes)
87+
private function saveConfigurableProductAttributes(ProductInterface $product, array $attributes): array
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
}
93103

94104
return $ids;
95105
}
96106

97107
/**
98-
* Remove product attributes
108+
* Remove product attributes which no longer used.
99109
*
100110
* @param ProductInterface $product
101111
* @return void
102112
*/
103-
private function deleteConfigurableProductAttributes(ProductInterface $product)
113+
private function deleteConfigurableProductAttributes(ProductInterface $product): void
114+
{
115+
$newAttributeIds = [];
116+
foreach ($product->getExtensionAttributes()->getConfigurableProductOptions() as $option) {
117+
$newAttributeIds[$option->getAttributeId()] = $option;
118+
}
119+
foreach ($this->optionRepository->getList($product->getSku()) as $option) {
120+
if (!in_array($option->getAttributeId(), array_keys($newAttributeIds))
121+
|| $this->isOptionChanged($option, $newAttributeIds[$option->getAttributeId()])
122+
) {
123+
$this->optionRepository->deleteById($product->getSku(), $option->getId());
124+
}
125+
}
126+
}
127+
128+
/**
129+
* Check if existing option is changed.
130+
*
131+
* @param OptionInterface $option
132+
* @param Attribute $attribute
133+
* @return bool
134+
*/
135+
private function isOptionChanged(OptionInterface $option, Attribute $attribute): bool
104136
{
105-
$list = $this->optionRepository->getList($product->getSku());
106-
foreach ($list as $item) {
107-
$this->optionRepository->deleteById($product->getSku(), $item->getId());
137+
if ($option->getLabel() == $attribute->getLabel() && $option->getPosition() == $attribute->getPosition()) {
138+
return false;
108139
}
140+
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: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\ConfigurableProduct\Model\Product;
9+
10+
use Magento\Catalog\Api\ProductRepositoryInterface;
11+
use Magento\Catalog\Model\Product;
12+
use Magento\TestFramework\Helper\Bootstrap;
13+
use Magento\ConfigurableProduct\Model\ResourceModel\Product\Type\Configurable as ConfigurableResource;
14+
15+
/**
16+
* Tests for \Magento\ConfigurableProduct\Model\Product\SaveHandler.
17+
*
18+
* @magentoAppIsolation enabled
19+
* @magentoDataFixture Magento/ConfigurableProduct/_files/product_configurable.php
20+
*/
21+
class SaveHandlerTest extends \PHPUnit\Framework\TestCase
22+
{
23+
/**
24+
* Object under test
25+
*
26+
* @var SaveHandler
27+
*/
28+
private $handler;
29+
30+
/**
31+
* @var Product
32+
*/
33+
private $product;
34+
35+
/**
36+
* @var ProductRepositoryInterface
37+
*/
38+
private $productRepository;
39+
40+
/**
41+
* @var ConfigurableResource
42+
*/
43+
private $resource;
44+
45+
/**
46+
* @inheritdoc
47+
*/
48+
protected function setUp()
49+
{
50+
$this->productRepository = Bootstrap::getObjectManager()->create(ProductRepositoryInterface::class);
51+
$this->product = $this->productRepository->get('configurable');
52+
$this->resource = Bootstrap::getObjectManager()->create(ConfigurableResource::class);
53+
$this->handler = Bootstrap::getObjectManager()->create(SaveHandler::class);
54+
}
55+
56+
/**
57+
* @return void
58+
*/
59+
public function testExecuteWithConfigurableProductLinksChanged(): void
60+
{
61+
$childrenIds = $this->product->getTypeInstance()->getChildrenIds($this->product->getId());
62+
$newChildrenIds = [reset($childrenIds[0])];
63+
$product = $this->productRepository->getById($this->product->getId());
64+
$extensionAttributes = $product->getExtensionAttributes();
65+
$extensionAttributes->setConfigurableProductLinks($newChildrenIds);
66+
$product->setExtensionAttributes($extensionAttributes);
67+
$this->handler->execute($product);
68+
$childrenIds = $this->product->getTypeInstance()->getChildrenIds($this->product->getId());
69+
$savedChildrenIds = [reset($childrenIds[0])];
70+
71+
self::assertEquals($newChildrenIds, $savedChildrenIds);
72+
}
73+
74+
/**
75+
* @return void
76+
*/
77+
public function testExecuteWithConfigurableProductLinksNotChanged(): void
78+
{
79+
$childrenIds = $this->product->getTypeInstance()->getChildrenIds($this->product->getId())[0];
80+
$product = $this->productRepository->getById($this->product->getId());
81+
$extensionAttributes = $product->getExtensionAttributes();
82+
$extensionAttributes->setConfigurableProductLinks($childrenIds);
83+
$product->setExtensionAttributes($extensionAttributes);
84+
$oldProductLinks = $this->getCurrentProductLinks();
85+
$this->handler->execute($product);
86+
$newProductLinks = $this->getCurrentProductLinks();
87+
88+
self::assertEquals($oldProductLinks, $newProductLinks);
89+
}
90+
91+
/**
92+
* @return array
93+
*/
94+
private function getCurrentProductLinks()
95+
{
96+
$select = $this->resource->getConnection()->select()->from(['l' => $this->resource->getMainTable()]);
97+
98+
return $this->resource->getConnection()->fetchAll($select);
99+
}
100+
}

0 commit comments

Comments
 (0)