From 4bb1097650b0225067f02bfb6a5b4fe08e1f56be Mon Sep 17 00:00:00 2001 From: Mike Bijnsdorp Date: Mon, 8 Nov 2021 15:27:47 +0100 Subject: [PATCH 1/5] Fix adding an option with label '0' Fix adding an attribute option to a product with the label '0' --- .../Magento/Eav/Model/Entity/Attribute/OptionManagement.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php b/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php index e99f4395953ad..fc47df9050ecf 100644 --- a/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php +++ b/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php @@ -93,8 +93,8 @@ public function update( if (empty($optionId)) { throw new InputException(__('The option id is empty. Enter the value and try again.')); } - $label = trim($option->getLabel() ?: ''); - if (empty($label)) { + $label = trim((string) $option->getLabel()); + if ($label === '') { throw new InputException(__('The attribute option label is empty. Enter the value and try again.')); } if ($attribute->getSource()->getOptionText($optionId) === false) { From 41abde5e3e28139555703479792a7a3acdb0a469 Mon Sep 17 00:00:00 2001 From: Mike Bijnsdorp Date: Wed, 10 Nov 2021 16:47:04 +0100 Subject: [PATCH 2/5] Update unit test to include a dataprovider for option label --- .../Entity/Attribute/OptionManagementTest.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php b/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php index b96b1e26696cd..12a1c6b02e7a2 100644 --- a/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php +++ b/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php @@ -59,13 +59,14 @@ protected function setUp(): void /** * Test to add attribute option + * + * @dataProvider optionLabelDataProvider */ - public function testAdd() + public function testAdd($label) { $entityType = 42; $storeId = 4; $attributeCode = 'atrCde'; - $label = 'optionLabel'; $storeLabel = 'labelLabel'; $sortOder = 'optionSortOrder'; $option = [ @@ -121,6 +122,17 @@ public function testAdd() ); } + /** + * @return array + */ + public function optionLabelDataProvider(): array + { + return [ + ['optionLabel'], + ['0'] + ]; + } + /** * Test to add attribute option with empty attribute code */ From 02f7fa05e04c5874ea85a9b00cbb8e3b10d13fc9 Mon Sep 17 00:00:00 2001 From: Mike Bijnsdorp Date: Wed, 10 Nov 2021 22:29:28 +0100 Subject: [PATCH 3/5] Apply label change to add method --- .../Magento/Eav/Model/Entity/Attribute/OptionManagement.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php b/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php index fc47df9050ecf..1a5cd9b19f251 100644 --- a/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php +++ b/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php @@ -60,8 +60,8 @@ public function add($entityType, $attributeCode, $option) { $attribute = $this->loadAttribute($entityType, (string)$attributeCode); - $label = trim($option->getLabel() ?: ''); - if (empty($label)) { + $label = trim((string) $option->getLabel()); + if ($label === '') { throw new InputException(__('The attribute option label is empty. Enter the value and try again.')); } From e3d09f704c9935c627c222ac38abc4a9fb9e2477 Mon Sep 17 00:00:00 2001 From: Mike Bijnsdorp Date: Thu, 11 Nov 2021 19:23:28 +0100 Subject: [PATCH 4/5] Add test for update method in OptionManagement Also removed an unnecessary alias --- .../Entity/Attribute/OptionManagementTest.php | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php b/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php index 12a1c6b02e7a2..998c9f61ef308 100644 --- a/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php +++ b/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php @@ -7,6 +7,7 @@ namespace Magento\Eav\Test\Unit\Model\Entity\Attribute; +use Magento\Catalog\Model\Product; use Magento\Eav\Api\Data\AttributeOptionInterface as EavAttributeOptionInterface; use Magento\Eav\Api\Data\AttributeOptionLabelInterface as EavAttributeOptionLabelInterface; use Magento\Eav\Model\AttributeRepository; @@ -18,7 +19,7 @@ use Magento\Framework\Exception\InputException; use Magento\Framework\Exception\NoSuchEntityException; use Magento\Framework\Exception\StateException; -use PHPUnit\Framework\MockObject\MockObject as MockObject; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; /** @@ -228,6 +229,74 @@ public function testAddWithCannotSaveException() $this->model->add($entityType, $attributeCode, $optionMock); } + /** + * Test to update attribute option + * + * @dataProvider optionLabelDataProvider + */ + public function testUpdate($label) + { + $entityType = Product::ENTITY; + $storeId = 4; + $attributeCode = 'atrCde'; + $storeLabel = 'labelLabel'; + $sortOder = 'optionSortOrder'; + $optionId = 10; + $option = [ + 'value' => [ + $optionId => [ + 0 => $label, + $storeId => $storeLabel, + ], + ], + 'order' => [ + $optionId => $sortOder, + ] + ]; + + $optionMock = $this->getAttributeOption(); + $labelMock = $this->getAttributeOptionLabel(); + /** @var SourceInterface|MockObject $sourceMock */ + $sourceMock = $this->createMock(EavAttributeSource::class); + + $sourceMock->expects($this->once()) + ->method('getOptionText') + ->with($optionId) + ->willReturn($label); + + $sourceMock->expects($this->once()) + ->method('getOptionId') + ->with($label) + ->willReturn($optionId); + + /** @var EavAbstractAttribute|MockObject $attributeMock */ + $attributeMock = $this->getMockBuilder(EavAbstractAttribute::class) + ->disableOriginalConstructor() + ->addMethods(['setOption']) + ->onlyMethods(['usesSource', 'getSource']) + ->getMock(); + $attributeMock->method('usesSource')->willReturn(true); + $attributeMock->expects($this->once())->method('setOption')->with($option); + $attributeMock->method('getSource')->willReturn($sourceMock); + + $this->attributeRepositoryMock->expects($this->once()) + ->method('get') + ->with($entityType, $attributeCode) + ->willReturn($attributeMock); + $optionMock->method('getLabel')->willReturn($label); + $optionMock->method('getSortOrder')->willReturn($sortOder); + $optionMock->method('getIsDefault')->willReturn(true); + $optionMock->method('getStoreLabels')->willReturn([$labelMock]); + $labelMock->method('getStoreId')->willReturn($storeId); + $labelMock->method('getLabel')->willReturn($storeLabel); + $this->resourceModelMock->expects($this->once())->method('save')->with($attributeMock); + + $this->assertEquals( + true, + $this->model->update($entityType, $attributeCode, $optionId, $optionMock) + ); + } + /** * Test to delete attribute option */ From 2a96c5e06b2277cd7b5c9ee8e87a4ef2805afb40 Mon Sep 17 00:00:00 2001 From: Ihor Sviziev Date: Fri, 12 Nov 2021 09:50:16 +0200 Subject: [PATCH 5/5] Fix adding an option with label '0' Apply suggestions from code review --- .../Magento/Eav/Model/Entity/Attribute/OptionManagement.php | 4 ++-- .../Unit/Model/Entity/Attribute/OptionManagementTest.php | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php b/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php index 1a5cd9b19f251..ecee1c6ab6361 100644 --- a/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php +++ b/app/code/Magento/Eav/Model/Entity/Attribute/OptionManagement.php @@ -60,7 +60,7 @@ public function add($entityType, $attributeCode, $option) { $attribute = $this->loadAttribute($entityType, (string)$attributeCode); - $label = trim((string) $option->getLabel()); + $label = trim((string)$option->getLabel()); if ($label === '') { throw new InputException(__('The attribute option label is empty. Enter the value and try again.')); } @@ -93,7 +93,7 @@ public function update( if (empty($optionId)) { throw new InputException(__('The option id is empty. Enter the value and try again.')); } - $label = trim((string) $option->getLabel()); + $label = trim((string)$option->getLabel()); if ($label === '') { throw new InputException(__('The attribute option label is empty. Enter the value and try again.')); } diff --git a/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php b/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php index 998c9f61ef308..7b554a19fc281 100644 --- a/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php +++ b/app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/OptionManagementTest.php @@ -61,9 +61,10 @@ protected function setUp(): void /** * Test to add attribute option * + * @param string $label * @dataProvider optionLabelDataProvider */ - public function testAdd($label) + public function testAdd(string $label): void { $entityType = 42; $storeId = 4; @@ -232,9 +233,10 @@ public function testAddWithCannotSaveException() /** * Test to update attribute option * + * @param string $label * @dataProvider optionLabelDataProvider */ - public function testUpdate($label) + public function testUpdate(string $label): void { $entityType = Product::ENTITY; $storeId = 4;