Skip to content
This repository was archived by the owner on Apr 29, 2019. It is now read-only.

Commit 9bfb67e

Browse files
magento-engcom-teammagento-engcom-team
authored
🔃 [EngCom] Public Pull Requests - 2.3-develop
Accepted Public Pull Requests: - magento/magento2#16838: [Forwardport] Correctly save Product Custom Option values (by @JeroenVanLeusden) - magento/magento2#16829: [Forwardport] Fix responsive tables showing broken heading (by @ronak2ram) - magento/magento2#16834: [Forwardport] Smallest codestyle fix in Option/Type/Text.php (by @mage2pratik) - magento/magento2#16817: [Forwardport] Fix zero price simple failed to resolve as default (by @mage2pratik) - magento/magento2#16808: [forwardport] fix #16764 Rating Star issue on Product detail Page. (by @Karlasa) - magento/magento2#16803: [Forwardport] magento/magento2#16685 Updated security issues details (by @quisse) - magento/magento2#16792: [Forwardport] Add sort order to user agent rules table headers (by @JRhyne) - magento/magento2#16413: [Forwardport] Fixing a Mistype Error (by @tiagosampaio) - magento/magento2#16767: [Forwardport] Improve attribute checking (by @FreekVandeursen) - magento-engcom/import-export-improvements#110: magento-engcom/import-export-improvements#44: Update the sample customer csv (by @dmanners) - magento-engcom/import-export-improvements#112: magento-engcom/import-export-improvements#88: Set store id on import product category initialization to 0 (by @pogster) - magento-engcom/import-export-improvements#113: magento-engcom/import-export-improvements#30: Validation should fail when a required field is supplied but is empty after trimming (by @pogster) Fixed GitHub Issues: - magento/magento2#5067: Custom option values do not save correctly (reported by @samtay) has been fixed in magento/magento2#16838 by @JeroenVanLeusden in 2.3-develop branch Related commits: 1. 955fbf2 2. db95d8a - magento/magento2#16764: Rating Star issue on Product detail Page. (reported by @Sathishkumar8731) has been fixed in magento/magento2#16808 by @Karlasa in 2.3-develop branch Related commits: 1. 1dd2017 - magento/magento2#16703: User Agent Rules table headers do match content of rows. (reported by @JRhyne) has been fixed in magento/magento2#16792 by @JRhyne in 2.3-develop branch Related commits: 1. 0430946
2 parents 5a186a3 + f73b119 commit 9bfb67e

File tree

21 files changed

+212
-70
lines changed

21 files changed

+212
-70
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ To learn more about issue gate labels click [here](https://github.com/magento/ma
5858

5959
<h2>Reporting security issues</h2>
6060

61-
To report security vulnerabilities in Magento software or web sites, please e-mail <a href="mailto:[email protected]">[email protected]</a>. Please do not report security issues using GitHub. Be sure to encrypt your e-mail with our <a href="https://info2.magento.com/rs/magentoenterprise/images/security_at_magento.asc">encryption key</a> if it includes sensitive information. Learn more about reporting security issues <a href="https://magento.com/security/reporting-magento-security-issue">here</a>.
61+
To report security vulnerabilities in Magento software or web sites, please create a Bugcrowd researcher account <a href="https://bugcrowd.com/magento">there</a> to submit and follow-up your issue. Learn more about reporting security issues <a href="https://magento.com/security/reporting-magento-security-issue">here</a>.
6262

6363
Stay up-to-date on the latest security news and patches for Magento by signing up for <a href="https://magento.com/security/sign-up">Security Alert Notifications</a>.
6464

‎app/code/Magento/Backend/view/adminhtml/ui_component/design_config_form.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292
</select>
9393
</formElements>
9494
</field>
95-
<actionDelete template="Magento_Backend/dynamic-rows/cells/action-delete" sortOrder="50">
95+
<actionDelete template="Magento_Backend/dynamic-rows/cells/action-delete">
9696
<argument name="data" xsi:type="array">
9797
<item name="config" xsi:type="array">
9898
<item name="fit" xsi:type="boolean">false</item>

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

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

99
use Magento\Catalog\Api\Data\ProductCustomOptionInterface;
1010
use Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface;
11+
use Magento\Catalog\Api\Data\ProductCustomOptionValuesInterfaceFactory;
1112
use Magento\Catalog\Api\Data\ProductInterface;
1213
use Magento\Catalog\Model\Product;
1314
use Magento\Catalog\Model\ResourceModel\Product\Option\Value\Collection;
@@ -102,6 +103,11 @@ class Option extends AbstractExtensibleModel implements ProductCustomOptionInter
102103
*/
103104
private $metadataPool;
104105

106+
/**
107+
* @var ProductCustomOptionValuesInterfaceFactory
108+
*/
109+
private $customOptionValuesFactory;
110+
105111
/**
106112
* @param \Magento\Framework\Model\Context $context
107113
* @param \Magento\Framework\Registry $registry
@@ -114,6 +120,7 @@ class Option extends AbstractExtensibleModel implements ProductCustomOptionInter
114120
* @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
115121
* @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
116122
* @param array $data
123+
* @param ProductCustomOptionValuesInterfaceFactory|null $customOptionValuesFactory
117124
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
118125
*/
119126
public function __construct(
@@ -127,12 +134,16 @@ public function __construct(
127134
Option\Validator\Pool $validatorPool,
128135
\Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
129136
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
130-
array $data = []
137+
array $data = [],
138+
ProductCustomOptionValuesInterfaceFactory $customOptionValuesFactory = null
131139
) {
132140
$this->productOptionValue = $productOptionValue;
133141
$this->optionTypeFactory = $optionFactory;
134142
$this->validatorPool = $validatorPool;
135143
$this->string = $string;
144+
$this->customOptionValuesFactory = $customOptionValuesFactory ?:
145+
\Magento\Framework\App\ObjectManager::getInstance()->get(ProductCustomOptionValuesInterfaceFactory::class);
146+
136147
parent::__construct(
137148
$context,
138149
$registry,
@@ -390,20 +401,21 @@ public function beforeSave()
390401
*/
391402
public function afterSave()
392403
{
393-
$this->getValueInstance()->unsetValues();
394404
$values = $this->getValues() ?: $this->getData('values');
395405
if (is_array($values)) {
396406
foreach ($values as $value) {
397-
if ($value instanceof \Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface) {
407+
if ($value instanceof ProductCustomOptionValuesInterface) {
398408
$data = $value->getData();
399409
} else {
400410
$data = $value;
401411
}
402-
$this->getValueInstance()->addValue($data);
403-
}
404412

405-
$this->getValueInstance()->setOption($this)->saveValues();
406-
} elseif ($this->getGroupByType($this->getType()) == self::OPTION_GROUP_SELECT) {
413+
$this->customOptionValuesFactory->create()
414+
->addValue($data)
415+
->setOption($this)
416+
->saveValues();
417+
}
418+
} elseif ($this->getGroupByType($this->getType()) === self::OPTION_GROUP_SELECT) {
407419
throw new LocalizedException(__('Select type options required values rows.'));
408420
}
409421

@@ -804,7 +816,7 @@ public function setImageSizeY($imageSizeY)
804816
}
805817

806818
/**
807-
* @param \Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface[] $values
819+
* @param ProductCustomOptionValuesInterface[] $values
808820
* @return $this
809821
*/
810822
public function setValues(array $values = null)

‎app/code/Magento/Catalog/Model/Product/Option/Type/Text.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function validateUserValue($values)
8484
*/
8585
public function prepareForCart()
8686
{
87-
if ($this->getIsValid() && strlen($this->getUserValue()) > 0) {
87+
if ($this->getIsValid() && ($this->getUserValue() !== '')) {
8888
return $this->getUserValue();
8989
} else {
9090
return null;

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class Value extends AbstractModel implements \Magento\Catalog\Api\Data\ProductCu
7676
* @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
7777
* @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
7878
* @param array $data
79+
* @param CustomOptionPriceCalculator|null $customOptionPriceCalculator
7980
*/
8081
public function __construct(
8182
\Magento\Framework\Model\Context $context,
@@ -89,6 +90,7 @@ public function __construct(
8990
$this->_valueCollectionFactory = $valueCollectionFactory;
9091
$this->customOptionPriceCalculator = $customOptionPriceCalculator
9192
?? \Magento\Framework\App\ObjectManager::getInstance()->get(CustomOptionPriceCalculator::class);
93+
9294
parent::__construct(
9395
$context,
9496
$registry,
@@ -201,19 +203,15 @@ public function getProduct()
201203
*/
202204
public function saveValues()
203205
{
206+
$option = $this->getOption();
207+
204208
foreach ($this->getValues() as $value) {
205209
$this->isDeleted(false);
206-
$this->setData(
207-
$value
208-
)->setData(
209-
'option_id',
210-
$this->getOption()->getId()
211-
)->setData(
212-
'store_id',
213-
$this->getOption()->getStoreId()
214-
);
215-
216-
if ($this->getData('is_delete') == '1') {
210+
$this->setData($value)
211+
->setData('option_id', $option->getId())
212+
->setData('store_id', $option->getStoreId());
213+
214+
if ((bool) $this->getData('is_delete') === true) {
217215
if ($this->getId()) {
218216
$this->deleteValues($this->getId());
219217
$this->delete();
@@ -222,7 +220,7 @@ public function saveValues()
222220
$this->save();
223221
}
224222
}
225-
//eof foreach()
223+
226224
return $this;
227225
}
228226

‎app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ protected function initCategories()
7575
$collection->addAttributeToSelect('name')
7676
->addAttributeToSelect('url_key')
7777
->addAttributeToSelect('url_path');
78+
$collection->setStoreId(\Magento\Store\Model\Store::DEFAULT_STORE_ID);
7879
/* @var $collection \Magento\Catalog\Model\ResourceModel\Category\Collection */
7980
foreach ($collection as $category) {
8081
$structure = explode(self::DELIMITER_CATEGORY, $category->getPath());

‎app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ abstract class AbstractType
2828
*/
2929
public static $commonAttributesCache = [];
3030

31+
/**
32+
* Maintain a list of invisible attributes
33+
*
34+
* @var array
35+
*/
36+
public static $invAttributesCache = [];
37+
3138
/**
3239
* Attribute Code to Id cache
3340
*
@@ -278,7 +285,14 @@ protected function _initAttributes()
278285
}
279286
}
280287
foreach ($absentKeys as $attributeSetName => $attributeIds) {
281-
$this->attachAttributesById($attributeSetName, $attributeIds);
288+
$unknownAttributeIds = array_diff(
289+
$attributeIds,
290+
array_keys(self::$commonAttributesCache),
291+
self::$invAttributesCache
292+
);
293+
if ($unknownAttributeIds || $this->_forcedAttributesCodes) {
294+
$this->attachAttributesById($attributeSetName, $attributeIds);
295+
}
282296
}
283297
foreach ($entityAttributes as $attributeRow) {
284298
if (isset(self::$commonAttributesCache[$attributeRow['attribute_id']])) {
@@ -303,37 +317,45 @@ protected function _initAttributes()
303317
protected function attachAttributesById($attributeSetName, $attributeIds)
304318
{
305319
foreach ($this->_prodAttrColFac->create()->addFieldToFilter(
306-
'main_table.attribute_id',
307-
['in' => $attributeIds]
320+
['main_table.attribute_id', 'main_table.attribute_code'],
321+
[
322+
['in' => $attributeIds],
323+
['in' => $this->_forcedAttributesCodes]
324+
]
308325
) as $attribute) {
309326
$attributeCode = $attribute->getAttributeCode();
310327
$attributeId = $attribute->getId();
311328

312329
if ($attribute->getIsVisible() || in_array($attributeCode, $this->_forcedAttributesCodes)) {
313-
self::$commonAttributesCache[$attributeId] = [
314-
'id' => $attributeId,
315-
'code' => $attributeCode,
316-
'is_global' => $attribute->getIsGlobal(),
317-
'is_required' => $attribute->getIsRequired(),
318-
'is_unique' => $attribute->getIsUnique(),
319-
'frontend_label' => $attribute->getFrontendLabel(),
320-
'is_static' => $attribute->isStatic(),
321-
'apply_to' => $attribute->getApplyTo(),
322-
'type' => \Magento\ImportExport\Model\Import::getAttributeType($attribute),
323-
'default_value' => strlen(
324-
$attribute->getDefaultValue()
325-
) ? $attribute->getDefaultValue() : null,
326-
'options' => $this->_entityModel->getAttributeOptions(
327-
$attribute,
328-
$this->_indexValueAttributes
329-
),
330-
];
330+
if (!isset(self::$commonAttributesCache[$attributeId])) {
331+
self::$commonAttributesCache[$attributeId] = [
332+
'id' => $attributeId,
333+
'code' => $attributeCode,
334+
'is_global' => $attribute->getIsGlobal(),
335+
'is_required' => $attribute->getIsRequired(),
336+
'is_unique' => $attribute->getIsUnique(),
337+
'frontend_label' => $attribute->getFrontendLabel(),
338+
'is_static' => $attribute->isStatic(),
339+
'apply_to' => $attribute->getApplyTo(),
340+
'type' => \Magento\ImportExport\Model\Import::getAttributeType($attribute),
341+
'default_value' => strlen(
342+
$attribute->getDefaultValue()
343+
) ? $attribute->getDefaultValue() : null,
344+
'options' => $this->_entityModel->getAttributeOptions(
345+
$attribute,
346+
$this->_indexValueAttributes
347+
),
348+
];
349+
}
350+
331351
self::$attributeCodeToId[$attributeCode] = $attributeId;
332352
$this->_addAttributeParams(
333353
$attributeSetName,
334354
self::$commonAttributesCache[$attributeId],
335355
$attribute
336356
);
357+
} else {
358+
self::$invAttributesCache[] = $attributeId;
337359
}
338360
}
339361
}

‎app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Type/AbstractTypeTest.php

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,28 @@ protected function setUp()
134134
->expects($this->any())
135135
->method('addFieldToFilter')
136136
->with(
137-
'main_table.attribute_id',
138-
['in' => ['attribute_id', 'boolean_attribute']]
137+
['main_table.attribute_id', 'main_table.attribute_code'],
138+
[
139+
[
140+
'in' =>
141+
[
142+
'attribute_id',
143+
'boolean_attribute',
144+
],
145+
],
146+
[
147+
'in' =>
148+
[
149+
'related_tgtr_position_behavior',
150+
'related_tgtr_position_limit',
151+
'upsell_tgtr_position_behavior',
152+
'upsell_tgtr_position_limit',
153+
'thumbnail_label',
154+
'small_image_label',
155+
'image_label',
156+
],
157+
],
158+
]
139159
)
140160
->willReturn([$attribute1, $attribute2]);
141161

‎app/code/Magento/ConfigurableProduct/Pricing/Price/ConfigurablePriceResolver.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public function resolvePrice(\Magento\Framework\Pricing\SaleableInterface $produ
6464

6565
foreach ($this->lowestPriceOptionsProvider->getProducts($product) as $subProduct) {
6666
$productPrice = $this->priceResolver->resolvePrice($subProduct);
67-
$price = $price ? min($price, $productPrice) : $productPrice;
67+
$price = isset($price) ? min($price, $productPrice) : $productPrice;
6868
}
6969

7070
return (float)$price;

‎app/code/Magento/ConfigurableProduct/Test/Unit/Pricing/Price/ConfigurablePriceResolverTest.php

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,31 @@ protected function setUp()
5555
* situation: one product is supplying the price, which could be a price of zero (0)
5656
*
5757
* @dataProvider resolvePriceDataProvider
58+
*
59+
* @param $variantPrices
60+
* @param $expectedPrice
5861
*/
59-
public function testResolvePrice($expectedValue)
62+
public function testResolvePrice($variantPrices, $expectedPrice)
6063
{
61-
$price = $expectedValue;
62-
6364
$product = $this->getMockBuilder(
6465
\Magento\Catalog\Model\Product::class
6566
)->disableOriginalConstructor()->getMock();
6667

6768
$product->expects($this->never())->method('getSku');
6869

69-
$this->lowestPriceOptionsProvider->expects($this->once())->method('getProducts')->willReturn([$product]);
70-
$this->priceResolver->expects($this->once())
70+
$products = array_map(function () {
71+
return $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
72+
->disableOriginalConstructor()
73+
->getMock();
74+
}, $variantPrices);
75+
76+
$this->lowestPriceOptionsProvider->expects($this->once())->method('getProducts')->willReturn($products);
77+
$this->priceResolver
7178
->method('resolvePrice')
72-
->with($product)
73-
->willReturn($price);
79+
->willReturnOnConsecutiveCalls(...$variantPrices);
7480

75-
$this->assertEquals($expectedValue, $this->resolver->resolvePrice($product));
81+
$actualPrice = $this->resolver->resolvePrice($product);
82+
self::assertSame($expectedPrice, $actualPrice);
7683
}
7784

7885
/**
@@ -81,8 +88,40 @@ public function testResolvePrice($expectedValue)
8188
public function resolvePriceDataProvider()
8289
{
8390
return [
84-
'price of zero' => [0.00],
85-
'price of five' => [5],
91+
'Single variant at price 0.00 (float), should return 0.00 (float)' => [
92+
$variantPrices = [
93+
0.00,
94+
],
95+
$expectedPrice = 0.00,
96+
],
97+
'Single variant at price 5 (integer), should return 5.00 (float)' => [
98+
$variantPrices = [
99+
5,
100+
],
101+
$expectedPrice = 5.00,
102+
],
103+
'Single variants at price null (null), should return 0.00 (float)' => [
104+
$variantPrices = [
105+
null,
106+
],
107+
$expectedPrice = 0.00,
108+
],
109+
'Multiple variants at price 0, 10, 20, should return 0.00 (float)' => [
110+
$variantPrices = [
111+
0,
112+
10,
113+
20,
114+
],
115+
$expectedPrice = 0.00,
116+
],
117+
'Multiple variants at price 10, 0, 20, should return 0.00 (float)' => [
118+
$variantPrices = [
119+
10,
120+
0,
121+
20,
122+
],
123+
$expectedPrice = 0.00,
124+
],
86125
];
87126
}
88127
}

0 commit comments

Comments
 (0)