Skip to content

Commit

Permalink
Merge pull request #1461 from magento-qwerty/MAGETWO-71215-fix
Browse files Browse the repository at this point in the history
Fixed issues:
- MAGETWO-71215: [EAV] Junk attribute values created when scheduling new staging update
  • Loading branch information
dvoskoboinikov authored Sep 7, 2017
2 parents 6d61ccd + 80cd41d commit 097f8af
Show file tree
Hide file tree
Showing 4 changed files with 273 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Magento\Catalog\Model\Product\Initialization\Helper\ProductLinks;
use Magento\Catalog\Model\Product\Link\Resolver as LinkResolver;
use Magento\Framework\App\ObjectManager;
use Magento\Catalog\Controller\Adminhtml\Product\Initialization\Helper\AttributeFilter;

/**
* @api
Expand Down Expand Up @@ -84,6 +85,11 @@ class Helper
*/
private $linkTypeProvider;

/**
* @var AttributeFilter
*/
private $attributeFilter;

/**
* Constructor
*
Expand All @@ -97,6 +103,7 @@ class Helper
* @param \Magento\Catalog\Api\Data\ProductLinkInterfaceFactory|null $productLinkFactory
* @param \Magento\Catalog\Api\ProductRepositoryInterface|null $productRepository
* @param \Magento\Catalog\Model\Product\LinkTypeProvider|null $linkTypeProvider
* @param AttributeFilter|null $attributeFilter
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
Expand All @@ -109,7 +116,8 @@ public function __construct(
\Magento\Catalog\Api\Data\ProductCustomOptionInterfaceFactory $customOptionFactory = null,
\Magento\Catalog\Api\Data\ProductLinkInterfaceFactory $productLinkFactory = null,
\Magento\Catalog\Api\ProductRepositoryInterface $productRepository = null,
\Magento\Catalog\Model\Product\LinkTypeProvider $linkTypeProvider = null
\Magento\Catalog\Model\Product\LinkTypeProvider $linkTypeProvider = null,
AttributeFilter $attributeFilter = null
) {
$this->request = $request;
$this->storeManager = $storeManager;
Expand All @@ -125,6 +133,8 @@ public function __construct(
->get(\Magento\Catalog\Api\ProductRepositoryInterface::class);
$this->linkTypeProvider = $linkTypeProvider ?: \Magento\Framework\App\ObjectManager::getInstance()
->get(\Magento\Catalog\Model\Product\LinkTypeProvider::class);
$this->attributeFilter = $attributeFilter ?: \Magento\Framework\App\ObjectManager::getInstance()
->get(AttributeFilter::class);
}

/**
Expand Down Expand Up @@ -187,6 +197,11 @@ public function initializeFromData(\Magento\Catalog\Model\Product $product, arra
$productOptions = [];
}
$productData['tier_price'] = isset($productData['tier_price']) ? $productData['tier_price'] : [];

$useDefaults = (array)$this->request->getPost('use_default', []);

$productData = $this->attributeFilter->prepareProductAttributes($product, $productData, $useDefaults);

$product->addData($productData);

if ($wasLockedMedia) {
Expand All @@ -196,8 +211,6 @@ public function initializeFromData(\Magento\Catalog\Model\Product $product, arra
/**
* Check "Use Default Value" checkboxes values
*/
$useDefaults = (array)$this->request->getPost('use_default', []);

foreach ($useDefaults as $attributeCode => $useDefaultState) {
if ($useDefaultState) {
$product->setData($attributeCode, null);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Catalog\Controller\Adminhtml\Product\Initialization\Helper;

use \Magento\Catalog\Model\Product;

/**
* Class provides functionality to check and filter data came with product form.
*
* The main goal is to avoid database population with empty(null) attribute values.
*/
class AttributeFilter
{
/**
* Method provides product data check and its further filtration.
*
* Filtration helps us to avoid unnecessary empty product data to be saved.
* Empty data will be preserved only if user explicitly set it.
*
* @param Product $product
* @param array $productData
* @param array $useDefaults
* @return array
*/
public function prepareProductAttributes(Product $product, array $productData, array $useDefaults)
{
foreach ($productData as $attribute => $value) {
$considerUseDefaultsAttribute = !isset($useDefaults[$attribute]) || $useDefaults[$attribute] === "1";
if ($value === '' && $considerUseDefaultsAttribute) {
/** @var $product Product */
if ((bool)$product->getData($attribute) === (bool)$value) {
unset($productData[$attribute]);
}
}
}
return $productData;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\Initialization\Helper;

use Magento\Catalog\Controller\Adminhtml\Product\Initialization\Helper\AttributeFilter;
use Magento\Catalog\Model\Product;

class AttributeFilterTest extends \PHPUnit\Framework\TestCase
{
/**
* @var AttributeFilter
*/
protected $model;

/**
* @var \PHPUnit_Framework_MockObject_MockObject
*/
protected $objectManagerMock;

/**
* @var Product|\PHPUnit_Framework_MockObject_MockObject
*/
protected $productMock;

protected function setUp()
{
$objectHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
$this->model = $objectHelper->getObject(AttributeFilter::class);
}

/**
* @param array $requestProductData
* @param array $useDefaults
* @param array $expectedProductData
* @param array $initialProductData
* @dataProvider setupInputDataProvider
*/
public function testPrepareProductAttributes(
$requestProductData,
$useDefaults,
$expectedProductData,
$initialProductData
) {
$productMockMap = $this->getMockBuilder(Product::class)
->disableOriginalConstructor()
->setMethods(['getData'])
->getMock();

if (!empty($initialProductData)) {
$productMockMap->expects($this->any())->method('getData')->willReturnMap($initialProductData);
}

$actualProductData = $this->model->prepareProductAttributes($productMockMap, $requestProductData, $useDefaults);
$this->assertEquals($expectedProductData, $actualProductData);
}

/**
* @return array
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function setupInputDataProvider()
{
return [
'create_new_product' => [
'productData' => [
'name' => 'testName',
'sku' => 'testSku',
'price' => '100',
'description' => ''
],
'useDefaults' => [],
'expectedProductData' => [
'name' => 'testName',
'sku' => 'testSku',
'price' => '100'
],
'initialProductData' => []
],
'update_product_without_use_defaults' => [
'productData' => [
'name' => 'testName2',
'sku' => 'testSku2',
'price' => '101',
'description' => '',
'special_price' => null
],
'useDefaults' => [],
'expectedProductData' => [
'name' => 'testName2',
'sku' => 'testSku2',
'price' => '101',
'special_price' => null
],
'initialProductData' => [
['name', 'testName2'],
['sku', 'testSku2'],
['price', '101'],
['special_price', null]
]
],
'update_product_without_use_defaults_2' => [
'productData' => [
'name' => 'testName2',
'sku' => 'testSku2',
'price' => '101',
'description' => 'updated description',
'special_price' => null
],
'useDefaults' => [],
'expectedProductData' => [
'name' => 'testName2',
'sku' => 'testSku2',
'price' => '101',
'description' => 'updated description',
'special_price' => null
],
'initialProductData' => [
['name', 'testName2'],
['sku', 'testSku2'],
['price', '101'],
['special_price', null]
]
],
'update_product_with_use_defaults' => [
'productData' => [
'name' => 'testName2',
'sku' => 'testSku2',
'price' => '101',
'description' => '',
'special_price' => null
],
'useDefaults' => [
'description' => '0'
],
'expectedProductData' => [
'name' => 'testName2',
'sku' => 'testSku2',
'price' => '101',
'special_price' => null,
'description' => ''
],
'initialProductData' => [
['name', 'testName2'],
['sku', 'testSku2'],
['price', '101'],
['special_price', null],
['description', 'descr text']
]
],
'update_product_with_use_defaults_2' => [
'requestProductData' => [
'name' => 'testName3',
'sku' => 'testSku3',
'price' => '103',
'description' => 'descr modified',
'special_price' => '100'
],
'useDefaults' => [
'description' => '0'
],
'expectedProductData' => [
'name' => 'testName3',
'sku' => 'testSku3',
'price' => '103',
'special_price' => '100',
'description' => 'descr modified'
],
'initialProductData' => [
['name', null,'testName2'],
['sku', null, 'testSku2'],
['price', null, '101'],
['description', null, 'descr text']
]
],
'update_product_with_use_defaults_3' => [
'requestProductData' => [
'name' => 'testName3',
'sku' => 'testSku3',
'price' => '103',
'special_price' => '100'
],
'useDefaults' => [
'description' => '1'
],
'expectedProductData' => [
'name' => 'testName3',
'sku' => 'testSku3',
'price' => '103',
'special_price' => '100',
],
'initialProductData' => [
['name', null,'testName2'],
['sku', null, 'testSku2'],
['price', null, '101'],
['description', null, 'descr text']
]
],
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Magento\Catalog\Model\Product\LinkTypeProvider;
use Magento\Catalog\Api\Data\ProductLinkTypeInterface;
use Magento\Catalog\Model\ProductLink\Link as ProductLink;
use Magento\Catalog\Controller\Adminhtml\Product\Initialization\Helper\AttributeFilter;

/**
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
Expand Down Expand Up @@ -89,6 +90,11 @@ class HelperTest extends \PHPUnit\Framework\TestCase
*/
protected $productLinksMock;

/**
* @var AttributeFilter|\PHPUnit_Framework_MockObject_MockObject
*/
protected $attributeFilterMock;

protected function setUp()
{
$this->objectManager = new ObjectManager($this);
Expand Down Expand Up @@ -134,6 +140,10 @@ protected function setUp()
$this->productLinksMock->expects($this->any())
->method('initializeLinks')
->willReturn($this->productMock);
$this->attributeFilterMock = $this->getMockBuilder(AttributeFilter::class)
->setMethods(['prepareProductAttributes'])
->disableOriginalConstructor()
->getMock();

$this->helper = $this->objectManager->getObject(
Helper::class,
Expand All @@ -146,6 +156,7 @@ protected function setUp()
'productLinkFactory' => $this->productLinkFactoryMock,
'productRepository' => $this->productRepositoryMock,
'linkTypeProvider' => $this->linkTypeProviderMock,
'attributeFilter' => $this->attributeFilterMock
]
);

Expand Down Expand Up @@ -269,6 +280,8 @@ public function testInitialize(
->getMock();
});

$this->attributeFilterMock->expects($this->any())->method('prepareProductAttributes')->willReturnArgument(1);

$this->assertEquals($this->productMock, $this->helper->initialize($this->productMock));
$this->assertEquals($expWebsiteIds, $this->productMock->getDataByKey('website_ids'));

Expand Down

0 comments on commit 097f8af

Please sign in to comment.