Skip to content

Commit 7ee9b1e

Browse files
dyushkindyushkin
committed
MC-18701: API Attribute Option update creates same value multiple times
1 parent 7d5f1b0 commit 7ee9b1e

File tree

4 files changed

+105
-81
lines changed

4 files changed

+105
-81
lines changed

‎app/code/Magento/Catalog/Model/Product/Attribute/OptionManagement.php

-11
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,6 @@ public function getItems($attributeCode)
4343
*/
4444
public function add($attributeCode, $option)
4545
{
46-
/** @var \Magento\Eav\Api\Data\AttributeOptionInterface[] $currentOptions */
47-
$currentOptions = $this->getItems($attributeCode);
48-
if (is_array($currentOptions)) {
49-
array_walk($currentOptions, function (&$attributeOption) {
50-
/** @var \Magento\Eav\Api\Data\AttributeOptionInterface $attributeOption */
51-
$attributeOption = $attributeOption->getLabel();
52-
});
53-
if (in_array($option->getLabel(), $currentOptions, true)) {
54-
return false;
55-
}
56-
}
5746
return $this->eavOptionManagement->add(
5847
\Magento\Catalog\Api\Data\ProductAttributeInterface::ENTITY_TYPE_CODE,
5948
$attributeCode,

‎app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php

+37-4
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
67

78
namespace Magento\Eav\Model\Entity\Attribute;
89

10+
use Magento\Eav\Api\Data\AttributeInterface as EavAttributeInterface;
911
use Magento\Framework\Exception\InputException;
1012
use Magento\Framework\Exception\NoSuchEntityException;
1113
use Magento\Framework\Exception\StateException;
@@ -64,6 +66,15 @@ public function add($entityType, $attributeCode, $option)
6466
}
6567
}
6668

69+
if (!$this->isAdminStoreLabelUnique($attribute, (string) $options['value'][$optionId][0])) {
70+
throw new InputException(
71+
__(
72+
'Admin store attribute option label "%1" is already exists.',
73+
$options['value'][$optionId][0]
74+
)
75+
);
76+
}
77+
6778
if ($option->getIsDefault()) {
6879
$attribute->setDefault([$optionId]);
6980
}
@@ -134,10 +145,10 @@ public function getItems($entityType, $attributeCode)
134145
/**
135146
* Validate option
136147
*
137-
* @param \Magento\Eav\Api\Data\AttributeInterface $attribute
148+
* @param EavAttributeInterface $attribute
138149
* @param int $optionId
139-
* @throws NoSuchEntityException
140150
* @return void
151+
*@throws NoSuchEntityException
141152
*/
142153
protected function validateOption($attribute, $optionId)
143154
{
@@ -167,13 +178,13 @@ private function getOptionId(\Magento\Eav\Api\Data\AttributeOptionInterface $opt
167178
* Set option value
168179
*
169180
* @param \Magento\Eav\Api\Data\AttributeOptionInterface $option
170-
* @param \Magento\Eav\Api\Data\AttributeInterface $attribute
181+
* @param EavAttributeInterface $attribute
171182
* @param string $optionLabel
172183
* @return void
173184
*/
174185
private function setOptionValue(
175186
\Magento\Eav\Api\Data\AttributeOptionInterface $option,
176-
\Magento\Eav\Api\Data\AttributeInterface $attribute,
187+
EavAttributeInterface $attribute,
177188
string $optionLabel
178189
) {
179190
$optionId = $attribute->getSource()->getOptionId($optionLabel);
@@ -188,4 +199,26 @@ private function setOptionValue(
188199
}
189200
}
190201
}
202+
203+
/**
204+
* Checks if the incoming admin store attribute option label is unique.
205+
*
206+
* @param EavAttributeInterface $attribute
207+
* @param string $adminStoreLabel
208+
* @return bool
209+
*/
210+
private function isAdminStoreLabelUnique(
211+
EavAttributeInterface $attribute,
212+
string $adminStoreLabel
213+
) :bool {
214+
$attribute->setStoreId(0);
215+
216+
foreach ($attribute->getSource()->toOptionArray() as $existingAttributeOption) {
217+
if ($existingAttributeOption['label'] === $adminStoreLabel) {
218+
return false;
219+
}
220+
}
221+
222+
return true;
223+
}
191224
}

‎app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php

+67-66
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@
66

77
namespace Magento\Eav\Test\Unit\Model\Entity\Attribute;
88

9+
use Magento\Eav\Api\Data\AttributeOptionInterface as EavAttributeOptionInterface;
10+
use Magento\Eav\Api\Data\AttributeOptionLabelInterface as EavAttributeOptionLabelInterface;
11+
use Magento\Eav\Model\Entity\Attribute\AbstractAttribute as EavAbstractAttribute;
12+
use Magento\Eav\Model\Entity\Attribute\Source\Table as EavAttributeSource;
13+
use PHPUnit\Framework\MockObject\MockObject as MockObject;
14+
915
class OptionManagementTest extends \PHPUnit\Framework\TestCase
1016
{
1117
/**
@@ -38,25 +44,9 @@ public function testAdd()
3844
{
3945
$entityType = 42;
4046
$attributeCode = 'atrCde';
41-
$optionMock = $this->getMockForAbstractClass(
42-
\Magento\Eav\Api\Data\AttributeOptionInterface::class,
43-
[],
44-
'',
45-
false,
46-
false,
47-
true,
48-
['getSourceLabels']
49-
);
50-
$attributeMock = $this->getMockForAbstractClass(
51-
\Magento\Framework\Model\AbstractModel::class,
52-
[],
53-
'',
54-
false,
55-
false,
56-
true,
57-
['usesSource', 'setDefault', 'setOption']
58-
);
59-
$labelMock = $this->createMock(\Magento\Eav\Api\Data\AttributeOptionLabelInterface::class);
47+
$attributeMock = $this->getAttribute();
48+
$optionMock = $this->getAttributeOption();
49+
$labelMock = $this->getAttributeOptionLabel();
6050
$option =
6151
['value' => [
6252
'id_new_option' => [
@@ -92,15 +82,7 @@ public function testAddWithEmptyAttributeCode()
9282
{
9383
$entityType = 42;
9484
$attributeCode = '';
95-
$optionMock = $this->getMockForAbstractClass(
96-
\Magento\Eav\Api\Data\AttributeOptionInterface::class,
97-
[],
98-
'',
99-
false,
100-
false,
101-
true,
102-
['getSourceLabels']
103-
);
85+
$optionMock = $this->getAttributeOption();
10486
$this->resourceModelMock->expects($this->never())->method('save');
10587
$this->model->add($entityType, $attributeCode, $optionMock);
10688
}
@@ -113,24 +95,8 @@ public function testAddWithWrongOptions()
11395
{
11496
$entityType = 42;
11597
$attributeCode = 'testAttribute';
116-
$optionMock = $this->getMockForAbstractClass(
117-
\Magento\Eav\Api\Data\AttributeOptionInterface::class,
118-
[],
119-
'',
120-
false,
121-
false,
122-
true,
123-
['getSourceLabels']
124-
);
125-
$attributeMock = $this->getMockForAbstractClass(
126-
\Magento\Framework\Model\AbstractModel::class,
127-
[],
128-
'',
129-
false,
130-
false,
131-
true,
132-
['usesSource', 'setDefault', 'setOption']
133-
);
98+
$attributeMock = $this->getAttribute();
99+
$optionMock = $this->getAttributeOption();
134100
$this->attributeRepositoryMock->expects($this->once())->method('get')->with($entityType, $attributeCode)
135101
->willReturn($attributeMock);
136102
$attributeMock->expects($this->once())->method('usesSource')->willReturn(false);
@@ -146,25 +112,9 @@ public function testAddWithCannotSaveException()
146112
{
147113
$entityType = 42;
148114
$attributeCode = 'atrCde';
149-
$optionMock = $this->getMockForAbstractClass(
150-
\Magento\Eav\Api\Data\AttributeOptionInterface::class,
151-
[],
152-
'',
153-
false,
154-
false,
155-
true,
156-
['getSourceLabels']
157-
);
158-
$attributeMock = $this->getMockForAbstractClass(
159-
\Magento\Framework\Model\AbstractModel::class,
160-
[],
161-
'',
162-
false,
163-
false,
164-
true,
165-
['usesSource', 'setDefault', 'setOption']
166-
);
167-
$labelMock = $this->createMock(\Magento\Eav\Api\Data\AttributeOptionLabelInterface::class);
115+
$optionMock = $this->getAttributeOption();
116+
$attributeMock = $this->getAttribute();
117+
$labelMock = $this->getAttributeOptionLabel();
168118
$option =
169119
['value' => [
170120
'id_new_option' => [
@@ -340,7 +290,7 @@ public function testGetItems()
340290
true,
341291
['getOptions']
342292
);
343-
$optionsMock = [$this->createMock(\Magento\Eav\Api\Data\AttributeOptionInterface::class)];
293+
$optionsMock = [$this->createMock(EavAttributeOptionInterface::class)];
344294
$this->attributeRepositoryMock->expects($this->once())->method('get')->with($entityType, $attributeCode)
345295
->willReturn($attributeMock);
346296
$attributeMock->expects($this->once())->method('getOptions')->willReturn($optionsMock);
@@ -380,4 +330,55 @@ public function testGetItemsWithEmptyAttributeCode()
380330
$attributeCode = '';
381331
$this->model->getItems($entityType, $attributeCode);
382332
}
333+
334+
/**
335+
* Returns attribute entity mock.
336+
*
337+
* @param array $attributeOptions attribute options for return
338+
* @return MockObject|EavAbstractAttribute
339+
*/
340+
private function getAttribute(array $attributeOptions = [])
341+
{
342+
$attribute = $this->getMockBuilder(EavAbstractAttribute::class)
343+
->disableOriginalConstructor()
344+
->setMethods(
345+
[
346+
'usesSource',
347+
'setDefault',
348+
'setOption',
349+
'setStoreId',
350+
'getSource',
351+
]
352+
)
353+
->getMock();
354+
$source = $this->getMockBuilder(EavAttributeSource::class)
355+
->disableOriginalConstructor()
356+
->getMock();
357+
358+
$attribute->method('getSource')->willReturn($source);
359+
$source->method('toOptionArray')->willReturn($attributeOptions);
360+
361+
return $attribute;
362+
}
363+
364+
/**
365+
* Return attribute option entity mock.
366+
*
367+
* @return MockObject|EavAttributeOptionInterface
368+
*/
369+
private function getAttributeOption()
370+
{
371+
return $this->getMockBuilder(EavAttributeOptionInterface::class)
372+
->setMethods(['getSourceLabels'])
373+
->getMockForAbstractClass();
374+
}
375+
376+
/**
377+
* @return MockObject|EavAttributeOptionLabelInterface
378+
*/
379+
private function getAttributeOptionLabel()
380+
{
381+
return $this->getMockBuilder(EavAttributeOptionLabelInterface::class)
382+
->getMockForAbstractClass();
383+
}
383384
}

‎app/code/Magento/Eav/i18n/en_US.csv

+1
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,4 @@ hello,hello
143143
"The value of attribute not valid","The value of attribute not valid"
144144
"EAV types and attributes","EAV types and attributes"
145145
"Entity types declaration cache","Entity types declaration cache"
146+
"Admin store attribute option label ""%1"" is already exists.","Admin store attribute option label ""%1"" is already exists."

0 commit comments

Comments
 (0)