Skip to content

Commit 4bb9242

Browse files
serhii-balkoserhii-balko
committed
MC-32411: Linking a simple product to a configurable product does not rollback when the API request fails.
1 parent d69c242 commit 4bb9242

File tree

3 files changed

+122
-83
lines changed

3 files changed

+122
-83
lines changed

‎app/code/Magento/ConfigurableProduct/Model/Plugin/ProductRepositorySave.php

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@
55
*/
66
namespace Magento\ConfigurableProduct\Model\Plugin;
77

8-
use Magento\Catalog\Model\ProductFactory;
98
use Magento\Catalog\Api\Data\ProductInterface;
9+
use Magento\Catalog\Model\ProductRepository;
1010
use Magento\Framework\Exception\InputException;
1111
use Magento\Catalog\Api\ProductRepositoryInterface;
12-
use Magento\Framework\Exception\CouldNotSaveException;
1312
use Magento\ConfigurableProduct\Api\Data\OptionInterface;
1413
use Magento\Catalog\Api\ProductAttributeRepositoryInterface;
1514
use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
15+
use Magento\Framework\Exception\NoSuchEntityException;
1616

17+
/**
18+
* Plugin to validate product links of configurable product and reset configurable attributes
19+
*/
1720
class ProductRepositorySave
1821
{
1922
/**
@@ -22,46 +25,45 @@ class ProductRepositorySave
2225
private $productAttributeRepository;
2326

2427
/**
25-
* @var ProductFactory
28+
* @var ProductRepositoryInterface
2629
*/
27-
private $productFactory;
30+
private $productRepository;
2831

2932
/**
3033
* @param ProductAttributeRepositoryInterface $productAttributeRepository
31-
* @param ProductFactory $productFactory
34+
* @param ProductRepositoryInterface $productRepository
3235
*/
3336
public function __construct(
3437
ProductAttributeRepositoryInterface $productAttributeRepository,
35-
ProductFactory $productFactory
38+
ProductRepositoryInterface $productRepository
3639
) {
3740
$this->productAttributeRepository = $productAttributeRepository;
38-
$this->productFactory = $productFactory;
41+
$this->productRepository = $productRepository;
3942
}
4043

4144
/**
42-
* Validate product links and reset configurable attributes to configurable product
45+
* Validate product links of configurable product
4346
*
4447
* @param ProductRepositoryInterface $subject
45-
* @param ProductInterface $result
4648
* @param ProductInterface $product
4749
* @param bool $saveOptions
48-
* @return ProductInterface
49-
* @throws CouldNotSaveException
50+
* @return array
5051
* @throws InputException
52+
* @throws NoSuchEntityException
5153
*
5254
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
5355
*/
54-
public function afterSave(
56+
public function beforeSave(
5557
ProductRepositoryInterface $subject,
56-
ProductInterface $result,
5758
ProductInterface $product,
5859
$saveOptions = false
5960
) {
61+
$result[] = $product;
6062
if ($product->getTypeId() !== Configurable::TYPE_CODE) {
6163
return $result;
6264
}
6365

64-
$extensionAttributes = $result->getExtensionAttributes();
66+
$extensionAttributes = $product->getExtensionAttributes();
6567
if ($extensionAttributes === null) {
6668
return $result;
6769
}
@@ -81,23 +83,49 @@ public function afterSave(
8183
$attributeCodes[] = $attributeCode;
8284
}
8385
$this->validateProductLinks($attributeCodes, $configurableLinks);
86+
87+
return $result;
88+
}
89+
90+
/**
91+
* Reset configurable attributes to configurable product
92+
*
93+
* @param ProductRepositoryInterface $subject
94+
* @param ProductInterface $result
95+
* @param ProductInterface $product
96+
* @param bool $saveOptions
97+
* @return ProductInterface
98+
*
99+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
100+
*/
101+
public function afterSave(
102+
ProductRepositoryInterface $subject,
103+
ProductInterface $result,
104+
ProductInterface $product,
105+
$saveOptions = false
106+
) {
107+
if ($product->getTypeId() !== Configurable::TYPE_CODE) {
108+
return $result;
109+
}
84110
$result->getTypeInstance()->resetConfigurableAttributes($product);
85111

86112
return $result;
87113
}
88114

89115
/**
116+
* Validate product links
117+
*
90118
* @param array $attributeCodes
91119
* @param array $linkIds
92-
* @return $this
120+
* @return void
93121
* @throws InputException
122+
* @throws NoSuchEntityException
94123
*/
95124
private function validateProductLinks(array $attributeCodes, array $linkIds)
96125
{
97126
$valueMap = [];
98-
99127
foreach ($linkIds as $productId) {
100-
$variation = $this->productFactory->create()->load($productId);
128+
$variation = $this->productRepository->getById($productId);
101129
$valueKey = '';
102130
foreach ($attributeCodes as $attributeCode) {
103131
if (!$variation->getData($attributeCode)) {

‎app/code/Magento/ConfigurableProduct/Test/Unit/Model/Plugin/ProductRepositorySaveTest.php

Lines changed: 48 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,26 @@
99
use Magento\Catalog\Api\ProductAttributeRepositoryInterface;
1010
use Magento\Catalog\Api\ProductRepositoryInterface;
1111
use Magento\Catalog\Model\Product;
12-
use Magento\Catalog\Model\ProductFactory;
1312
use Magento\ConfigurableProduct\Api\Data\OptionInterface;
1413
use Magento\ConfigurableProduct\Model\Plugin\ProductRepositorySave;
1514
use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
1615
use Magento\ConfigurableProduct\Test\Unit\Model\Product\ProductExtensionAttributes;
1716
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
18-
use PHPUnit_Framework_MockObject_MockObject as MockObject;
17+
use PHPUnit\Framework\MockObject\MockObject;
18+
use PHPUnit\Framework\TestCase;
1919

2020
/**
21-
* Class ProductRepositorySaveTest
21+
* Test for ProductRepositorySave plugin
22+
*
2223
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2324
*/
24-
class ProductRepositorySaveTest extends \PHPUnit\Framework\TestCase
25+
class ProductRepositorySaveTest extends TestCase
2526
{
2627
/**
2728
* @var ProductAttributeRepositoryInterface|MockObject
2829
*/
2930
private $productAttributeRepository;
3031

31-
/**
32-
* @var ProductFactory|MockObject
33-
*/
34-
private $productFactory;
35-
3632
/**
3733
* @var Product|MockObject
3834
*/
@@ -68,15 +64,13 @@ class ProductRepositorySaveTest extends \PHPUnit\Framework\TestCase
6864
*/
6965
private $plugin;
7066

67+
/**
68+
* @inheritdoc
69+
*/
7170
protected function setUp()
7271
{
7372
$this->productAttributeRepository = $this->getMockForAbstractClass(ProductAttributeRepositoryInterface::class);
7473

75-
$this->productFactory = $this->getMockBuilder(ProductFactory::class)
76-
->disableOriginalConstructor()
77-
->setMethods(['create'])
78-
->getMock();
79-
8074
$this->product = $this->getMockBuilder(Product::class)
8175
->disableOriginalConstructor()
8276
->setMethods(['getTypeId', 'getExtensionAttributes'])
@@ -102,12 +96,15 @@ protected function setUp()
10296
ProductRepositorySave::class,
10397
[
10498
'productAttributeRepository' => $this->productAttributeRepository,
105-
'productFactory' => $this->productFactory
99+
'productRepository' => $this->productRepository
106100
]
107101
);
108102
}
109103

110-
public function testAfterSaveWhenProductIsSimple()
104+
/**
105+
* Validating the result after saving a configurable product
106+
*/
107+
public function testBeforeSaveWhenProductIsSimple()
111108
{
112109
$this->product->expects(static::once())
113110
->method('getTypeId')
@@ -116,18 +113,21 @@ public function testAfterSaveWhenProductIsSimple()
116113
->method('getExtensionAttributes');
117114

118115
$this->assertEquals(
119-
$this->result,
120-
$this->plugin->afterSave($this->productRepository, $this->result, $this->product)
116+
$this->product,
117+
$this->plugin->beforeSave($this->productRepository, $this->product)[0]
121118
);
122119
}
123120

124-
public function testAfterSaveWithoutOptions()
121+
/**
122+
* Test saving a configurable product without attribute options
123+
*/
124+
public function testBeforeSaveWithoutOptions()
125125
{
126126
$this->product->expects(static::once())
127127
->method('getTypeId')
128128
->willReturn(Configurable::TYPE_CODE);
129129

130-
$this->result->expects(static::once())
130+
$this->product->expects(static::once())
131131
->method('getExtensionAttributes')
132132
->willReturn($this->extensionAttributes);
133133

@@ -142,23 +142,25 @@ public function testAfterSaveWithoutOptions()
142142
->method('get');
143143

144144
$this->assertEquals(
145-
$this->result,
146-
$this->plugin->afterSave($this->productRepository, $this->result, $this->product)
145+
$this->product,
146+
$this->plugin->beforeSave($this->productRepository, $this->product)[0]
147147
);
148148
}
149149

150150
/**
151+
* Test saving a configurable product with same set of attribute values
152+
*
151153
* @expectedException \Magento\Framework\Exception\InputException
152154
* @expectedExceptionMessage Products "5" and "4" have the same set of attribute values.
153155
*/
154-
public function testAfterSaveWithLinks()
156+
public function testBeforeSaveWithLinks()
155157
{
156158
$links = [4, 5];
157159
$this->product->expects(static::once())
158160
->method('getTypeId')
159161
->willReturn(Configurable::TYPE_CODE);
160162

161-
$this->result->expects(static::once())
163+
$this->product->expects(static::once())
162164
->method('getExtensionAttributes')
163165
->willReturn($this->extensionAttributes);
164166
$this->extensionAttributes->expects(static::once())
@@ -173,27 +175,26 @@ public function testAfterSaveWithLinks()
173175

174176
$product = $this->getMockBuilder(Product::class)
175177
->disableOriginalConstructor()
176-
->setMethods(['load', 'getData', '__wakeup'])
178+
->setMethods(['getData', '__wakeup'])
177179
->getMock();
178180

179-
$this->productFactory->expects(static::exactly(2))
180-
->method('create')
181+
$this->productRepository->expects(static::exactly(2))
182+
->method('getById')
181183
->willReturn($product);
182184

183-
$product->expects(static::exactly(2))
184-
->method('load')
185-
->willReturnSelf();
186185
$product->expects(static::never())
187186
->method('getData');
188187

189-
$this->plugin->afterSave($this->productRepository, $this->result, $this->product);
188+
$this->plugin->beforeSave($this->productRepository, $this->product);
190189
}
191190

192191
/**
192+
* Test saving a configurable product with missing attribute
193+
*
193194
* @expectedException \Magento\Framework\Exception\InputException
194195
* @expectedExceptionMessage Product with id "4" does not contain required attribute "color".
195196
*/
196-
public function testAfterSaveWithLinksWithMissingAttribute()
197+
public function testBeforeSaveWithLinksWithMissingAttribute()
197198
{
198199
$simpleProductId = 4;
199200
$links = [$simpleProductId, 5];
@@ -208,7 +209,7 @@ public function testAfterSaveWithLinksWithMissingAttribute()
208209
->method('getTypeId')
209210
->willReturn(Configurable::TYPE_CODE);
210211

211-
$this->result->expects(static::once())
212+
$this->product->expects(static::once())
212213
->method('getExtensionAttributes')
213214
->willReturn($this->extensionAttributes);
214215
$this->extensionAttributes->expects(static::once())
@@ -228,29 +229,28 @@ public function testAfterSaveWithLinksWithMissingAttribute()
228229

229230
$product = $this->getMockBuilder(Product::class)
230231
->disableOriginalConstructor()
231-
->setMethods(['load', 'getData', '__wakeup'])
232+
->setMethods(['getData', '__wakeup'])
232233
->getMock();
233234

234-
$this->productFactory->expects(static::once())
235-
->method('create')
235+
$this->productRepository->expects(static::once())
236+
->method('getById')
236237
->willReturn($product);
237-
$product->expects(static::once())
238-
->method('load')
239-
->with($simpleProductId)
240-
->willReturnSelf();
238+
241239
$product->expects(static::once())
242240
->method('getData')
243241
->with($attributeCode)
244242
->willReturn(false);
245243

246-
$this->plugin->afterSave($this->productRepository, $this->result, $this->product);
244+
$this->plugin->beforeSave($this->productRepository, $this->product);
247245
}
248246

249247
/**
248+
* Test saving a configurable product with duplicate attributes
249+
*
250250
* @expectedException \Magento\Framework\Exception\InputException
251251
* @expectedExceptionMessage Products "5" and "4" have the same set of attribute values.
252252
*/
253-
public function testAfterSaveWithLinksWithDuplicateAttributes()
253+
public function testBeforeSaveWithLinksWithDuplicateAttributes()
254254
{
255255
$links = [4, 5];
256256
$attributeCode = 'color';
@@ -264,7 +264,7 @@ public function testAfterSaveWithLinksWithDuplicateAttributes()
264264
->method('getTypeId')
265265
->willReturn(Configurable::TYPE_CODE);
266266

267-
$this->result->expects(static::once())
267+
$this->product->expects(static::once())
268268
->method('getExtensionAttributes')
269269
->willReturn($this->extensionAttributes);
270270
$this->extensionAttributes->expects(static::once())
@@ -284,20 +284,18 @@ public function testAfterSaveWithLinksWithDuplicateAttributes()
284284

285285
$product = $this->getMockBuilder(Product::class)
286286
->disableOriginalConstructor()
287-
->setMethods(['load', 'getData', '__wakeup'])
287+
->setMethods(['getData', '__wakeup'])
288288
->getMock();
289289

290-
$this->productFactory->expects(static::exactly(2))
291-
->method('create')
290+
$this->productRepository->expects(static::exactly(2))
291+
->method('getById')
292292
->willReturn($product);
293-
$product->expects(static::exactly(2))
294-
->method('load')
295-
->willReturnSelf();
293+
296294
$product->expects(static::exactly(4))
297295
->method('getData')
298296
->with($attributeCode)
299297
->willReturn($attributeId);
300298

301-
$this->plugin->afterSave($this->productRepository, $this->result, $this->product);
299+
$this->plugin->beforeSave($this->productRepository, $this->product);
302300
}
303301
}

0 commit comments

Comments
 (0)