From c6d8494fea1dd7655de8fd6ed96993c00433624c Mon Sep 17 00:00:00 2001 From: kalexeyev Date: Mon, 4 Sep 2017 18:49:43 +0300 Subject: [PATCH] MAGETWO-71215: [EAV] Junk attribute values created when scheduling new staging update --- .../Product/Initialization/Helper.php | 19 +- .../Initialization/Helper/AttributeFilter.php | 41 ++++ .../Helper/AttributeFilterTest.php | 203 ++++++++++++++++++ .../Product/Initialization/HelperTest.php | 13 ++ 4 files changed, 273 insertions(+), 3 deletions(-) create mode 100644 app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilter.php create mode 100644 app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilterTest.php diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper.php index 17a6bf6a189d1..beb6f2b13bcfe 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper.php @@ -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 @@ -84,6 +85,11 @@ class Helper */ private $linkTypeProvider; + /** + * @var AttributeFilter + */ + private $attributeFilter; + /** * Constructor * @@ -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( @@ -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; @@ -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); } /** @@ -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) { @@ -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); diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilter.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilter.php new file mode 100644 index 0000000000000..237168282afae --- /dev/null +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilter.php @@ -0,0 +1,41 @@ + $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; + } +} diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilterTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilterTest.php new file mode 100644 index 0000000000000..28617addc6d27 --- /dev/null +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/AttributeFilterTest.php @@ -0,0 +1,203 @@ +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'] + ] + ], + ]; + } +} diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/HelperTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/HelperTest.php index c164c281a4e53..dce3f5886d1a8 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/HelperTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Initialization/HelperTest.php @@ -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) @@ -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); @@ -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, @@ -146,6 +156,7 @@ protected function setUp() 'productLinkFactory' => $this->productLinkFactoryMock, 'productRepository' => $this->productRepositoryMock, 'linkTypeProvider' => $this->linkTypeProviderMock, + 'attributeFilter' => $this->attributeFilterMock ] ); @@ -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'));