From 01f1f157dc2f17a0bbefcfe9ba717c96d788fb53 Mon Sep 17 00:00:00 2001 From: Gareth James Date: Tue, 18 Jun 2019 16:17:52 +0100 Subject: [PATCH 1/8] Reworked query in getAttributeRawValue so that it returns a store specific value even if there is no default value --- .../Model/ResourceModel/AbstractResource.php | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php index 3d7f863b7c0d3..0cd38b72b3da9 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php @@ -560,15 +560,19 @@ public function getAttributeRawValue($entityId, $attribute, $store) $store = (int) $store; if ($typedAttributes) { foreach ($typedAttributes as $table => $_attributes) { + $defaultJoinCondition = [ + $connection->quoteInto('default_value.attribute_id IN (?)', array_keys($_attributes)), + "default_value.{$this->getLinkField()} = e.{$this->getLinkField()}", + 'default_value.store_id = 0', + ]; + $select = $connection->select() - ->from(['default_value' => $table], ['attribute_id']) - ->join( - ['e' => $this->getTable($this->getEntityTable())], - 'e.' . $this->getLinkField() . ' = ' . 'default_value.' . $this->getLinkField(), - '' - )->where('default_value.attribute_id IN (?)', array_keys($_attributes)) - ->where("e.entity_id = :entity_id") - ->where('default_value.store_id = ?', 0); + ->from(['e' => $this->getTable($this->getEntityTable())], []) + ->joinLeft( + ['default_value' => $table], + implode(' AND ', $defaultJoinCondition), + [] + )->where("e.entity_id = :entity_id"); $bind = ['entity_id' => $entityId]; @@ -578,6 +582,11 @@ public function getAttributeRawValue($entityId, $attribute, $store) 'default_value.value', 'store_value.value' ); + $attributeIdExpr = $connection->getCheckSql( + 'store_value.attribute_id IS NULL', + 'default_value.attribute_id', + 'store_value.attribute_id' + ); $joinCondition = [ $connection->quoteInto('store_value.attribute_id IN (?)', array_keys($_attributes)), "store_value.{$this->getLinkField()} = e.{$this->getLinkField()}", @@ -587,18 +596,23 @@ public function getAttributeRawValue($entityId, $attribute, $store) $select->joinLeft( ['store_value' => $table], implode(' AND ', $joinCondition), - ['attr_value' => $valueExpr] + ['attribute_id' => $attributeIdExpr, 'attr_value' => $valueExpr] ); $bind['store_id'] = $store; } else { - $select->columns(['attr_value' => 'value'], 'default_value'); + $select->columns( + ['attribute_id' => 'attribute_id', 'attr_value' => 'value'], + 'default_value' + ); } $result = $connection->fetchPairs($select, $bind); foreach ($result as $attrId => $value) { - $attrCode = $typedAttributes[$table][$attrId]; - $attributesData[$attrCode] = $value; + if ($attrId !== '') { + $attrCode = $typedAttributes[$table][$attrId]; + $attributesData[$attrCode] = $value; + } } } } From 64184b3bf28b2a88ac691ca651d219ed74cf94ef Mon Sep 17 00:00:00 2001 From: Gareth James Date: Sun, 23 Jun 2019 13:59:29 +0100 Subject: [PATCH 2/8] Addedtests to cover store/default scenarios with getAttributeRawValue --- .../Model/ResourceModel/ProductTest.php | 85 +++++++++++++++++++ ...mple_with_custom_store_scope_attribute.php | 72 ++++++++++++++++ ..._custom_store_scope_attribute_rollback.php | 41 +++++++++ 3 files changed, 198 insertions(+) mode change 100644 => 100755 dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php create mode 100755 dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php create mode 100755 dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute_rollback.php diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php old mode 100644 new mode 100755 index 476f01eb277df..f073993f9b653 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php @@ -9,6 +9,10 @@ use Magento\Framework\ObjectManagerInterface; use Magento\TestFramework\Helper\Bootstrap; use PHPUnit\Framework\TestCase; +use Magento\Framework\Exception\NoSuchEntityException; +use Magento\Framework\Exception\CouldNotSaveException; +use Magento\Framework\Exception\InputException; +use Magento\Framework\Exception\StateException; class ProductTest extends TestCase { @@ -53,6 +57,87 @@ public function testGetAttributeRawValue() self::assertEquals($product->getName(), $actual); } + /** + * @magentoAppArea adminhtml + * @magentoDataFixture Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php + * @throws NoSuchEntityException + * @throws CouldNotSaveException + * @throws InputException + * @throws StateException + */ + public function testGetAttributeRawValueGetDefault() + { + $product = $this->productRepository->get('simple_with_store_scoped_custom_attribute', true, 0, true); + $product->setCustomAttribute('store_scoped_attribute_code', 'default_value'); + $this->productRepository->save($product); + + $actual = $this->model->getAttributeRawValue($product->getId(), 'store_scoped_attribute_code', 1); + $this->assertEquals('default_value', $actual); + } + + /** + * @magentoAppArea adminhtml + * @magentoDataFixture Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php + * @throws NoSuchEntityException + * @throws CouldNotSaveException + * @throws InputException + * @throws StateException + */ + public function testGetAttributeRawValueGetStoreSpecificValueNoDefault() + { + $product = $this->productRepository->get('simple_with_store_scoped_custom_attribute', true, 0, true); + $product->setCustomAttribute('store_scoped_attribute_code', null); + $this->productRepository->save($product); + + $product = $this->productRepository->get('simple_with_store_scoped_custom_attribute', true, 1, true); + $product->setCustomAttribute('store_scoped_attribute_code', 'store_value'); + $this->productRepository->save($product); + + $actual = $this->model->getAttributeRawValue($product->getId(), 'store_scoped_attribute_code', 1); + $this->assertEquals('store_value', $actual); + } + + /** + * @magentoAppArea adminhtml + * @magentoDataFixture Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php + * @throws NoSuchEntityException + * @throws CouldNotSaveException + * @throws InputException + * @throws StateException + */ + public function testGetAttributeRawValueGetStoreSpecificValueWithDefault() + { + $product = $this->productRepository->get('simple_with_store_scoped_custom_attribute', true, 0, true); + $product->setCustomAttribute('store_scoped_attribute_code', 'default_value'); + $this->productRepository->save($product); + + $product = $this->productRepository->get('simple_with_store_scoped_custom_attribute', true, 1, true); + $product->setCustomAttribute('store_scoped_attribute_code', 'store_value'); + $this->productRepository->save($product); + + $actual = $this->model->getAttributeRawValue($product->getId(), 'store_scoped_attribute_code', 1); + $this->assertEquals('store_value', $actual); + } + + /** + * @magentoAppArea adminhtml + * @magentoDataFixture Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php + * @throws NoSuchEntityException + * @throws CouldNotSaveException + * @throws InputException + * @throws StateException + * @throws NoSuchEntityException + */ + public function testGetAttributeRawValueGetStoreValueFallbackToDefault() + { + $product = $this->productRepository->get('simple_with_store_scoped_custom_attribute', true, 0, true); + $product->setCustomAttribute('store_scoped_attribute_code', 'default_value'); + $this->productRepository->save($product); + + $actual = $this->model->getAttributeRawValue($product->getId(), 'store_scoped_attribute_code', 1); + $this->assertEquals('default_value', $actual); + } + /** * @magentoAppArea adminhtml * @magentoDataFixture Magento/Catalog/_files/product_special_price.php diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php new file mode 100755 index 0000000000000..183d531f947e8 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute.php @@ -0,0 +1,72 @@ +get(ProductRepositoryInterface::class); +/** @var ProductFactory $productFactory */ +$productFactory = $objectManager->get(ProductFactory::class); +/** @var ProductAttributeRepositoryInterface $attributeRepository */ +$attributeRepository = $objectManager->get(ProductAttributeRepositoryInterface::class); + + +/** @var $installer CategorySetup */ +$installer = $objectManager->create(CategorySetup::class); +$entityModel = $objectManager->create(Entity::class); +$attributeSetId = $installer->getAttributeSetId(Product::ENTITY, 'Default'); +$entityTypeId = $entityModel->setType(Product::ENTITY) + ->getTypeId(); +$groupId = $installer->getDefaultAttributeGroupId($entityTypeId, $attributeSetId); + +/** @var ProductAttributeInterface $attribute */ +$attribute = $objectManager->create(ProductAttributeInterface::class); + +$attribute->setAttributeCode('store_scoped_attribute_code') + ->setEntityTypeId($entityTypeId) + ->setIsVisible(true) + ->setFrontendInput('text') + ->setIsFilterable(1) + ->setIsUserDefined(1) + ->setUsedInProductListing(1) + ->setBackendType('varchar') + ->setIsUsedInGrid(1) + ->setIsVisibleInGrid(1) + ->setIsFilterableInGrid(1) + ->setFrontendLabel('nobody cares') + ->setAttributeGroupId($groupId) + ->setAttributeSetId(4); + +$attributeRepository->save($attribute); + +$product = $productFactory->create() + ->setTypeId(Type::TYPE_SIMPLE) + ->setAttributeSetId(4) + ->setName('Simple With Store Scoped Custom Attribute') + ->setSku('simple_with_store_scoped_custom_attribute') + ->setPrice(100) + ->setVisibility(1) + ->setStockData( + [ + 'use_config_manage_stock' => 1, + 'qty' => 100, + 'is_in_stock' => 1, + ] + ) + ->setStatus(1); +$product->setCustomAttribute('store_scoped_attribute_code', 'default_value'); +$productRepository->save($product); diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute_rollback.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute_rollback.php new file mode 100755 index 0000000000000..54c832dd6a6ff --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_custom_store_scope_attribute_rollback.php @@ -0,0 +1,41 @@ +get(ProductRepositoryInterface::class); +/** @var ProductAttributeRepositoryInterface $attributeRepository */ +$attributeRepository = $objectManager->get(ProductAttributeRepositoryInterface::class); +/** @var Registry $registry */ +$registry = $objectManager->get(Registry::class); + +$registry->unregister('isSecureArea'); +$registry->register('isSecureArea', true); + +try { + /** @var \Magento\Catalog\Api\Data\ProductInterface $product */ + $product = $productRepository->get('simple_with_store_scoped_custom_attribute'); + $productRepository->delete($product); +} catch (NoSuchEntityException $e) { +} + +try { + /** @var \Magento\Catalog\Api\Data\ProductAttributeInterface $attribute */ + $attribute = $attributeRepository->get('store_scoped_attribute_code'); + $attributeRepository->delete($attribute); +} catch (NoSuchEntityException $e) { +} + +$registry->unregister('isSecureArea'); +$registry->register('isSecureArea', false); From 35c5185bdae84cae7ac5008098ae0d2d893cf05b Mon Sep 17 00:00:00 2001 From: Gareth James Date: Sun, 23 Jun 2019 18:02:10 +0100 Subject: [PATCH 3/8] minorcomment tweak to prevent testLiveCode code sniff test from failing --- .../Catalog/Model/ResourceModel/AbstractResource.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) mode change 100644 => 100755 app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php diff --git a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php old mode 100644 new mode 100755 index 0cd38b72b3da9..0399d022acb17 --- a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php @@ -466,9 +466,9 @@ protected function _getOrigObject($object) * Checks also attribute's store scope: * We should insert on duplicate key update values if we unchecked 'STORE VIEW' checkbox in store view. * - * @param AbstractAttribute $attribute - * @param mixed $value New value of the attribute. - * @param array &$origData + * @param AbstractAttribute $attribute + * @param mixed $value New value of the attribute. + * @param array &$origData * @return bool */ protected function _canUpdateAttribute(AbstractAttribute $attribute, $value, array &$origData) From 9ae33c855dd68bbdfcfe4148facaaa9c0c4b30a1 Mon Sep 17 00:00:00 2001 From: Gareth James Date: Mon, 24 Jun 2019 22:44:20 +0100 Subject: [PATCH 4/8] Revert "minorcomment tweak to prevent testLiveCode code sniff test from failing" This reverts commit 35c5185bdae84cae7ac5008098ae0d2d893cf05b. --- .../Catalog/Model/ResourceModel/AbstractResource.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) mode change 100755 => 100644 app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php diff --git a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php old mode 100755 new mode 100644 index 0399d022acb17..0cd38b72b3da9 --- a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php @@ -466,9 +466,9 @@ protected function _getOrigObject($object) * Checks also attribute's store scope: * We should insert on duplicate key update values if we unchecked 'STORE VIEW' checkbox in store view. * - * @param AbstractAttribute $attribute - * @param mixed $value New value of the attribute. - * @param array &$origData + * @param AbstractAttribute $attribute + * @param mixed $value New value of the attribute. + * @param array &$origData * @return bool */ protected function _canUpdateAttribute(AbstractAttribute $attribute, $value, array &$origData) From 8c4bfa62528235b651a06e9a24f4a8a0b3a5f7f9 Mon Sep 17 00:00:00 2001 From: Gareth James Date: Mon, 24 Jun 2019 22:48:52 +0100 Subject: [PATCH 5/8] Removing & from param for _canUpdateAttribute as it's breaking static tests --- .../Magento/Catalog/Model/ResourceModel/AbstractResource.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php index 0cd38b72b3da9..2680b84653e41 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php @@ -468,7 +468,7 @@ protected function _getOrigObject($object) * * @param AbstractAttribute $attribute * @param mixed $value New value of the attribute. - * @param array &$origData + * @param array $origData * @return bool */ protected function _canUpdateAttribute(AbstractAttribute $attribute, $value, array &$origData) From 7b0e528a316af06dac9d6327db25ed53065a0893 Mon Sep 17 00:00:00 2001 From: Gareth James Date: Mon, 24 Jun 2019 23:40:12 +0100 Subject: [PATCH 6/8] Added class comment ProductTest to remove static test warning --- .../Magento/Catalog/Model/ResourceModel/ProductTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php index f073993f9b653..e218c508b7d3e 100755 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/ProductTest.php @@ -14,6 +14,12 @@ use Magento\Framework\Exception\InputException; use Magento\Framework\Exception\StateException; +/** + * Tests product resource model + * + * @see \Magento\Catalog\Model\ResourceModel\Product + * @see \Magento\Catalog\Model\ResourceModel\AbstractResource + */ class ProductTest extends TestCase { /** From c34f83cb30a5960a62dbeff562fdbf7215d616a0 Mon Sep 17 00:00:00 2001 From: Gareth James Date: Mon, 24 Jun 2019 23:41:01 +0100 Subject: [PATCH 7/8] Changed usage of sizeof to count to remove usage of discouraged sizeof function in getAttributeRawValue --- .../Magento/Catalog/Model/ResourceModel/AbstractResource.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php index 2680b84653e41..b7665c176dfad 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php @@ -617,7 +617,7 @@ public function getAttributeRawValue($entityId, $attribute, $store) } } - if (is_array($attributesData) && sizeof($attributesData) == 1) { + if (is_array($attributesData) && count($attributesData) == 1) { $attributesData = array_shift($attributesData); } From 4d380bf488e027047833d270bb7d816d78d0f43f Mon Sep 17 00:00:00 2001 From: Gareth James Date: Wed, 31 Jul 2019 00:37:25 +0100 Subject: [PATCH 8/8] Added phpcs disable of Magento2.Classes.AbstractApi to Magento\Catalog\Model\ResourceModel\AbstractResource for static tests --- .../Magento/Catalog/Model/ResourceModel/AbstractResource.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php index b7665c176dfad..3946be32184ec 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/AbstractResource.php @@ -15,6 +15,7 @@ /** * Catalog entity abstract model * + * phpcs:disable Magento2.Classes.AbstractApi * @api * * @SuppressWarnings(PHPMD.CouplingBetweenObjects)