Skip to content

Configurable Product Links Validation Bug when int value is 0 #29001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\ConfigurableProduct\Model\Plugin;

use Magento\Catalog\Api\Data\ProductInterface;
Expand Down Expand Up @@ -56,7 +59,7 @@ public function beforeSave(
ProductRepositoryInterface $subject,
ProductInterface $product,
$saveOptions = false
) {
): array {
$result[] = $product;
if ($product->getTypeId() !== Configurable::TYPE_CODE) {
return $result;
Expand Down Expand Up @@ -102,7 +105,7 @@ public function afterSave(
ProductInterface $result,
ProductInterface $product,
$saveOptions = false
) {
): ProductInterface {
if ($product->getTypeId() !== Configurable::TYPE_CODE) {
return $result;
}
Expand All @@ -120,19 +123,23 @@ public function afterSave(
* @throws InputException
* @throws NoSuchEntityException
*/
private function validateProductLinks(array $attributeCodes, array $linkIds)
private function validateProductLinks(array $attributeCodes, array $linkIds): void
{
$valueMap = [];
foreach ($linkIds as $productId) {
$variation = $this->productRepository->getById($productId);
$valueKey = '';
foreach ($attributeCodes as $attributeCode) {
if (!$variation->getData($attributeCode)) {
if ($variation->getData($attributeCode) === null) {
throw new InputException(
__('Product with id "%1" does not contain required attribute "%2".', $productId, $attributeCode)
__(
'Product with id "%1" does not contain required attribute "%2".',
$productId,
$attributeCode
)
);
}
$valueKey = $valueKey . $attributeCode . ':' . $variation->getData($attributeCode) . ';';
$valueKey .= $attributeCode . ':' . $variation->getData($attributeCode) . ';';
}
if (isset($valueMap[$valueKey])) {
throw new InputException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\ConfigurableProduct\Test\Unit\Model\Plugin;
Expand All @@ -18,6 +19,7 @@
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Magento\Framework\Exception\InputException;

/**
* Test for ProductRepositorySave plugin
Expand Down Expand Up @@ -71,7 +73,8 @@ class ProductRepositorySaveTest extends TestCase
*/
protected function setUp(): void
{
$this->productAttributeRepository = $this->getMockForAbstractClass(ProductAttributeRepositoryInterface::class);
$this->productAttributeRepository =
$this->getMockForAbstractClass(ProductAttributeRepositoryInterface::class);

$this->product = $this->getMockBuilder(Product::class)
->disableOriginalConstructor()
Expand Down Expand Up @@ -105,8 +108,10 @@ protected function setUp(): void

/**
* Validating the result after saving a configurable product
*
* @return void
*/
public function testBeforeSaveWhenProductIsSimple()
public function testBeforeSaveWhenProductIsSimple(): void
{
$this->product->expects(static::once())
->method('getTypeId')
Expand All @@ -122,8 +127,10 @@ public function testBeforeSaveWhenProductIsSimple()

/**
* Test saving a configurable product without attribute options
*
* @return void
*/
public function testBeforeSaveWithoutOptions()
public function testBeforeSaveWithoutOptions(): void
{
$this->product->expects(static::once())
->method('getTypeId')
Expand Down Expand Up @@ -151,10 +158,12 @@ public function testBeforeSaveWithoutOptions()

/**
* Test saving a configurable product with same set of attribute values
*
* @return void
*/
public function testBeforeSaveWithLinks()
public function testBeforeSaveWithLinks(): void
{
$this->expectException('Magento\Framework\Exception\InputException');
$this->expectException(InputException::class);
$this->expectExceptionMessage('Products "5" and "4" have the same set of attribute values.');
$links = [4, 5];
$this->product->expects(static::once())
Expand Down Expand Up @@ -191,10 +200,12 @@ public function testBeforeSaveWithLinks()

/**
* Test saving a configurable product with missing attribute
*
* @return void
*/
public function testBeforeSaveWithLinksWithMissingAttribute()
public function testBeforeSaveWithLinksWithMissingAttribute(): void
{
$this->expectException('Magento\Framework\Exception\InputException');
$this->expectException(InputException::class);
$this->expectExceptionMessage('Product with id "4" does not contain required attribute "color".');
$simpleProductId = 4;
$links = [$simpleProductId, 5];
Expand Down Expand Up @@ -239,17 +250,19 @@ public function testBeforeSaveWithLinksWithMissingAttribute()
$product->expects(static::once())
->method('getData')
->with($attributeCode)
->willReturn(false);
->willReturn(null);

$this->plugin->beforeSave($this->productRepository, $this->product);
}

/**
* Test saving a configurable product with duplicate attributes
*
* @return void
*/
public function testBeforeSaveWithLinksWithDuplicateAttributes()
public function testBeforeSaveWithLinksWithDuplicateAttributes(): void
{
$this->expectException('Magento\Framework\Exception\InputException');
$this->expectException(InputException::class);
$this->expectExceptionMessage('Products "5" and "4" have the same set of attribute values.');
$links = [4, 5];
$attributeCode = 'color';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,20 @@
namespace Magento\ConfigurableProduct\Api;

use Magento\Catalog\Api\Data\ProductInterface;
use Magento\Catalog\Api\ProductRepositoryInterface;
use Magento\Catalog\Model\Entity\Attribute;
use Magento\Eav\Model\Config;
use Magento\Eav\Model\ResourceModel\Entity\Attribute\Option\Collection;
use Magento\Framework\Api\ExtensibleDataInterface;
use Magento\Framework\ObjectManagerInterface;
use Magento\Framework\Webapi\Rest\Request;
use Magento\TestFramework\Helper\Bootstrap;
use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
use Magento\TestFramework\TestCase\WebapiAbstract;

/**
* Class ProductRepositoryTest for testing ConfigurableProduct integration
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class ProductRepositoryTest extends WebapiAbstract
{
Expand All @@ -28,17 +31,22 @@ class ProductRepositoryTest extends WebapiAbstract
/**
* @var Config
*/
protected $eavConfig;
private $eavConfig;

/**
* @var ObjectManagerInterface
*/
protected $objectManager;
private $objectManager;

/**
* @var Attribute
*/
protected $configurableAttribute;
private $configurableAttribute;

/**
* @var ProductRepositoryInterface
*/
private $productRepository;

/**
* @inheritdoc
Expand All @@ -47,6 +55,7 @@ protected function setUp(): void
{
$this->objectManager = Bootstrap::getObjectManager();
$this->eavConfig = $this->objectManager->get(Config::class);
$this->productRepository = $this->objectManager->get(ProductRepositoryInterface::class);
}

/**
Expand Down Expand Up @@ -164,6 +173,65 @@ public function testCreateConfigurableProduct()
$this->assertEquals([$productId1, $productId2], $resultConfigurableProductLinks);
}

/**
* Create configurable with simple which has zero attribute value
*
* @magentoApiDataFixture Magento/ConfigurableProduct/_files/configurable_attribute_with_source_model.php
* @magentoApiDataFixture Magento/Catalog/_files/product_simple.php
* @return void
*/
public function testCreateConfigurableProductWithZeroOptionValue(): void
{
$attributeCode = 'test_configurable_with_sm';
$attributeValue = 0;

$product = $this->productRepository->get('simple');
$product->setCustomAttribute($attributeCode, $attributeValue);
$this->productRepository->save($product);

$configurableAttribute = $this->eavConfig->getAttribute('catalog_product', $attributeCode);

$productData = [
'sku' => self::CONFIGURABLE_PRODUCT_SKU,
'name' => self::CONFIGURABLE_PRODUCT_SKU,
'type_id' => Configurable::TYPE_CODE,
'attribute_set_id' => 4,
'extension_attributes' => [
'configurable_product_options' => [
[
'attribute_id' => $configurableAttribute->getId(),
'label' => 'Test configurable with source model',
'values' => [
['value_index' => '0'],
],
],
],
'configurable_product_links' => [$product->getId()],
],
];

$response = $this->createProduct($productData);

$this->assertArrayHasKey(ProductInterface::SKU, $response);
$this->assertEquals(self::CONFIGURABLE_PRODUCT_SKU, $response[ProductInterface::SKU]);

$this->assertArrayHasKey(ProductInterface::TYPE_ID, $response);
$this->assertEquals('configurable', $response[ProductInterface::TYPE_ID]);

$this->assertArrayHasKey(ProductInterface::EXTENSION_ATTRIBUTES_KEY, $response);
$this->assertArrayHasKey(
'configurable_product_options',
$response[ProductInterface::EXTENSION_ATTRIBUTES_KEY]
);
$configurableProductOption =
current($response[ProductInterface::EXTENSION_ATTRIBUTES_KEY]['configurable_product_options']);

$this->assertArrayHasKey('attribute_id', $configurableProductOption);
$this->assertEquals($configurableAttribute->getId(), $configurableProductOption['attribute_id']);
$this->assertArrayHasKey('values', $configurableProductOption);
$this->assertEquals($attributeValue, $configurableProductOption['values'][0]['value_index']);
}

/**
* @magentoApiDataFixture Magento/ConfigurableProduct/_files/product_configurable.php
*/
Expand Down