Skip to content

Commit cd3b244

Browse files
VinaiVinai
committed
Allow construction of products with custom_attributes
This patch does two things: 1. Currently it is not possible to pass an array with `custom_attributes` to the `\Magento\Catalog\Model\Product` constructor (it causes a fatal error). The reason is because the `filterCustomAttribute` and `eavConfig` arguments are assigned after the call to `parent::__construct`. However, the properties are used during the `parent::__construct` calls. The flow of execution is as follows: Product::__construct -> Catalog\Model\AbstractModel::__construct Catalog\Model\AbstractModel::__construct -> AbstractExtensibleModel::__construct AbstractExtensibleModel::__construct -> AbstractExtensibleModel::filterCustomAttributes AbstractExtensibleModel::filterCustomAttributes -> AbstractExtensibleModel::getCustomAttributesCodes ...which is overridden by Product::getCustomAttributesCodes getCustomAttributesCodes expectes the `filterCustomAttribute` and `eavConfig` properties to be set if `custom_attributes` are present in `$data`, but they are still null because the `Product::__construct` method has not yet completed. The fix this PR applies is to assign the properties before the call to `parent::__construct`. The bug and fix are covered by the integration test: `\Magento\Catalog\Model\ProductTest::testConstructionWithCustomAttributesMapInData` 2. The method `AbstractExtensibleModel::filterCustomAttribute` expects the `custom_attributes` in `$data` to be a simple map from codes to values, e.g. `['category_ids => '1,2']`. However, the method `\Magento\Framework\Reflection\DataObjectProcessor::buildOutputDataArray` generates a numerically indexed custom attributes array, where each custom attribute is a sub-array with a `attribute_code` and `value` record. This PR allows passing such an `custom_attributes` array into the `Product` model constructor. Currently it would be ignored, but with this patch the code checks if `custom_attributes` is numerically indexed, and if so, flattens the sub-arrays into the expected map format. To illustrate the difference of the `custom_attributes` array formats: Map: [ 'custom_attributes' => [ 'category_ids' => '1,2', 'tax_class_id' => '3', ] ] Numerically indexed array of sub-arrays: [ 'custom_attributes' => [ [ 'attribute_code' => 'category_ids', 'value' => '1,2' ], [ 'attribute_code' => 'tax_class_id', 'value' => '3' ], ] ] This improvement is covered by the integration test `\Magento\Catalog\Model\ProductTest::testConstructionWithCustomAttributesArrayInData`
1 parent 70d2287 commit cd3b244

File tree

3 files changed

+83
-19
lines changed

3 files changed

+83
-19
lines changed

‎app/code/Magento/Catalog/Model/Product.php

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,9 @@ public function __construct(
472472
$this->mediaGalleryEntryConverterPool = $mediaGalleryEntryConverterPool;
473473
$this->dataObjectHelper = $dataObjectHelper;
474474
$this->joinProcessor = $joinProcessor;
475+
$this->eavConfig = $config ?? ObjectManager::getInstance()->get(\Magento\Eav\Model\Config::class);
476+
$this->filterCustomAttribute = $filterCustomAttribute
477+
?? ObjectManager::getInstance()->get(FilterProductCustomAttribute::class);
475478
parent::__construct(
476479
$context,
477480
$registry,
@@ -482,9 +485,6 @@ public function __construct(
482485
$resourceCollection,
483486
$data
484487
);
485-
$this->eavConfig = $config ?? ObjectManager::getInstance()->get(\Magento\Eav\Model\Config::class);
486-
$this->filterCustomAttribute = $filterCustomAttribute
487-
?? ObjectManager::getInstance()->get(FilterProductCustomAttribute::class);
488488
}
489489

490490
/**
@@ -835,12 +835,13 @@ public function getStoreIds()
835835
if (!$this->isObjectNew() && $this->_storeManager->isSingleStoreMode()) {
836836
$websiteIds = array_keys($websiteIds);
837837
}
838-
foreach ($websiteIds as $websiteId) {
839-
$websiteStores = $this->_storeManager->getWebsite($websiteId)->getStoreIds();
840-
foreach ($websiteStores as $websiteStore) {
841-
$storeIds []= $websiteStore;
842-
}
843-
}
838+
$websiteStoreIds = array_map(
839+
function ($websiteId): array {
840+
return $this->_storeManager->getWebsite($websiteId)->getStoreIds();
841+
},
842+
$websiteIds
843+
);
844+
$storeIds = array_merge($storeIds, ...$websiteStoreIds);
844845
}
845846
$this->setStoreIds($storeIds);
846847
}

‎dev/tests/integration/testsuite/Magento/Catalog/Model/ProductTest.php

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88

99
namespace Magento\Catalog\Model;
1010

11+
use Magento\Eav\Model\Config as EavConfig;
1112
use Magento\Framework\App\Filesystem\DirectoryList;
13+
use Magento\TestFramework\ObjectManager;
1214

1315
/**
1416
* Tests product model:
@@ -49,8 +51,8 @@ protected function setUp()
4951
}
5052

5153
/**
52-
* @throws \Magento\Framework\Exception\FileSystemException
5354
* @return void
55+
* @throws \Magento\Framework\Exception\FileSystemException
5456
*/
5557
public static function tearDownAfterClass()
5658
{
@@ -307,9 +309,9 @@ public function testIsSalable()
307309
$this->_model = $this->productRepository->get('simple');
308310

309311
// fixture
310-
$this->assertTrue((bool)$this->_model->isSalable());
311-
$this->assertTrue((bool)$this->_model->isSaleable());
312-
$this->assertTrue((bool)$this->_model->isAvailable());
312+
$this->assertTrue((bool) $this->_model->isSalable());
313+
$this->assertTrue((bool) $this->_model->isSaleable());
314+
$this->assertTrue((bool) $this->_model->isAvailable());
313315
$this->assertTrue($this->_model->isInStock());
314316
}
315317

@@ -324,9 +326,9 @@ public function testIsNotSalableWhenStatusDisabled()
324326
$this->_model = $this->productRepository->get('simple');
325327

326328
$this->_model->setStatus(0);
327-
$this->assertFalse((bool)$this->_model->isSalable());
328-
$this->assertFalse((bool)$this->_model->isSaleable());
329-
$this->assertFalse((bool)$this->_model->isAvailable());
329+
$this->assertFalse((bool) $this->_model->isSalable());
330+
$this->assertFalse((bool) $this->_model->isSaleable());
331+
$this->assertFalse((bool) $this->_model->isAvailable());
330332
$this->assertFalse($this->_model->isInStock());
331333
}
332334

@@ -585,7 +587,7 @@ public function testGetOptions()
585587
continue;
586588
}
587589
foreach ($option->getValues() as $value) {
588-
$this->assertEquals($expectedValue[$value->getSku()], (float)$value->getPrice());
590+
$this->assertEquals($expectedValue[$value->getSku()], (float) $value->getPrice());
589591
}
590592
}
591593
}
@@ -632,4 +634,40 @@ public function productWithBackordersDataProvider(): array
632634
[1, 1, true],
633635
];
634636
}
637+
638+
public function testConstructionWithCustomAttributesMapInData()
639+
{
640+
$data = [
641+
'custom_attributes' => [
642+
'tax_class_id' => '3',
643+
'category_ids' => '1,2'
644+
],
645+
];
646+
647+
/** @var Product $product */
648+
$product = ObjectManager::getInstance()->create(Product::class, ['data' => $data]);
649+
$this->assertSame($product->getCustomAttribute('tax_class_id')->getValue(), '3');
650+
$this->assertSame($product->getCustomAttribute('category_ids')->getValue(), '1,2');
651+
}
652+
653+
public function testConstructionWithCustomAttributesArrayInData()
654+
{
655+
$data = [
656+
'custom_attributes' => [
657+
[
658+
'attribute_code' => 'tax_class_id',
659+
'value' => '3'
660+
],
661+
[
662+
'attribute_code' => 'category_ids',
663+
'value' => '1,2'
664+
]
665+
],
666+
];
667+
668+
/** @var Product $product */
669+
$product = ObjectManager::getInstance()->create(Product::class, ['data' => $data]);
670+
$this->assertSame($product->getCustomAttribute('tax_class_id')->getValue(), '3');
671+
$this->assertSame($product->getCustomAttribute('category_ids')->getValue(), '1,2');
672+
}
635673
}

‎lib/internal/Magento/Framework/Model/AbstractExtensibleModel.php

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,28 @@ public function __construct(
7474
}
7575
}
7676

77+
/**
78+
* Convert the custom attributes array format to map format
79+
*
80+
* The method \Magento\Framework\Reflection\DataObjectProcessor::buildOutputDataArray generates a custom_attributes
81+
* array representation where each custom attribute is a sub-array with a `attribute_code and value key.
82+
* This method maps such an array to the plain code => value map format exprected by filterCustomAttributes
83+
*
84+
* @param array[] $customAttributesData
85+
* @return array
86+
*/
87+
private function flattenCustomAttributesArrayToMap(array $customAttributesData): array
88+
{
89+
return array_reduce(
90+
$customAttributesData,
91+
function (array $acc, array $customAttribute): array {
92+
$acc[$customAttribute['attribute_code']] = $customAttribute['value'];
93+
return $acc;
94+
},
95+
[]
96+
);
97+
}
98+
7799
/**
78100
* Verify custom attributes set on $data and unset if not a valid custom attribute
79101
*
@@ -85,9 +107,12 @@ protected function filterCustomAttributes($data)
85107
if (empty($data[self::CUSTOM_ATTRIBUTES])) {
86108
return $data;
87109
}
88-
$customAttributesCodes = $this->getCustomAttributesCodes();
110+
if (isset($data[self::CUSTOM_ATTRIBUTES][0])) {
111+
$data[self::CUSTOM_ATTRIBUTES] = $this->flattenCustomAttributesArrayToMap($data[self::CUSTOM_ATTRIBUTES]);
112+
}
113+
$customAttributesCodes = $this->getCustomAttributesCodes();
89114
$data[self::CUSTOM_ATTRIBUTES] = array_intersect_key(
90-
(array)$data[self::CUSTOM_ATTRIBUTES],
115+
(array) $data[self::CUSTOM_ATTRIBUTES],
91116
array_flip($customAttributesCodes)
92117
);
93118
foreach ($data[self::CUSTOM_ATTRIBUTES] as $code => $value) {

0 commit comments

Comments
 (0)