Skip to content

Commit 3151fbc

Browse files
magento-engcom-teammagento-engcom-team
committed
ENGCOM-1862: [Bugfix] #7333 Unable to set negative custom option fixed price in admin view. #15267
- Merge Pull Request #15267 from dverkade/magento2:2.3-Fix-7333-negative-value-for-custom-option - Merged commits: 1. dc4de31 2. f7aab57 3. 9a8e7af 4. 04caefa 5. 8b7f1f2 6. d43a52f 7. d66329a 8. 08ea73c 9. 6baf928 10. 4927e56
2 parents 9cb874f + 4927e56 commit 3151fbc

File tree

6 files changed

+163
-18
lines changed

6 files changed

+163
-18
lines changed

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

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,20 @@ class DefaultValidator extends \Magento\Framework\Validator\AbstractValidator
2828
*/
2929
protected $priceTypes;
3030

31+
/**
32+
* @var \Magento\Framework\Locale\FormatInterface
33+
*/
34+
private $localeFormat;
35+
3136
/**
3237
* @param \Magento\Catalog\Model\ProductOptions\ConfigInterface $productOptionConfig
3338
* @param \Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig
39+
* @param \Magento\Framework\Locale\FormatInterface|null $localeFormat
3440
*/
3541
public function __construct(
3642
\Magento\Catalog\Model\ProductOptions\ConfigInterface $productOptionConfig,
37-
\Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig
43+
\Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig,
44+
\Magento\Framework\Locale\FormatInterface $localeFormat = null
3845
) {
3946
foreach ($productOptionConfig->getAll() as $option) {
4047
foreach ($option['types'] as $type) {
@@ -45,6 +52,9 @@ public function __construct(
4552
foreach ($priceConfig->toOptionArray() as $item) {
4653
$this->priceTypes[] = $item['value'];
4754
}
55+
56+
$this->localeFormat = $localeFormat ?: \Magento\Framework\App\ObjectManager::getInstance()
57+
->get(\Magento\Framework\Locale\FormatInterface::class);
4858
}
4959

5060
/**
@@ -137,11 +147,11 @@ protected function validateOptionType(Option $option)
137147
*/
138148
protected function validateOptionValue(Option $option)
139149
{
140-
return $this->isInRange($option->getPriceType(), $this->priceTypes);
150+
return $this->isInRange($option->getPriceType(), $this->priceTypes) && $this->isNumber($option->getPrice());
141151
}
142152

143153
/**
144-
* Check whether value is empty
154+
* Check whether the value is empty
145155
*
146156
* @param mixed $value
147157
* @return bool
@@ -152,7 +162,7 @@ protected function isEmpty($value)
152162
}
153163

154164
/**
155-
* Check whether value is in range
165+
* Check whether the value is in range
156166
*
157167
* @param string $value
158168
* @param array $range
@@ -164,13 +174,24 @@ protected function isInRange($value, array $range)
164174
}
165175

166176
/**
167-
* Check whether value is not negative
177+
* Check whether the value is negative
168178
*
169179
* @param string $value
170180
* @return bool
171181
*/
172182
protected function isNegative($value)
173183
{
174-
return (int) $value < 0;
184+
return $this->localeFormat->getNumber($value) < 0;
185+
}
186+
187+
/**
188+
* Check whether the value is a number
189+
*
190+
* @param string $value
191+
* @return bool
192+
*/
193+
public function isNumber($value)
194+
{
195+
return is_numeric($this->localeFormat->getNumber($value));
175196
}
176197
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
use Magento\Catalog\Model\Product\Option;
1010

11+
/**
12+
* Select validator class
13+
*/
1114
class Select extends DefaultValidator
1215
{
1316
/**
@@ -83,7 +86,7 @@ protected function isValidOptionPrice($priceType, $price, $storeId)
8386
if (!$priceType && !$price) {
8487
return true;
8588
}
86-
if (!$this->isInRange($priceType, $this->priceTypes)) {
89+
if (!$this->isInRange($priceType, $this->priceTypes) || !$this->isNumber($price)) {
8790
return false;
8891
}
8992

‎app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/DefaultValidatorTest.php

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ class DefaultValidatorTest extends \PHPUnit\Framework\TestCase
1818
*/
1919
protected $valueMock;
2020

21+
/**
22+
* @var \PHPUnit_Framework_MockObject_MockObject
23+
*/
24+
protected $localeFormatMock;
25+
2126
/**
2227
* @inheritdoc
2328
*/
@@ -26,6 +31,8 @@ protected function setUp()
2631
$configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class);
2732
$storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
2833
$priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock);
34+
$this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class);
35+
2936
$config = [
3037
[
3138
'label' => 'group label 1',
@@ -51,7 +58,8 @@ protected function setUp()
5158
$configMock->expects($this->once())->method('getAll')->will($this->returnValue($config));
5259
$this->validator = new \Magento\Catalog\Model\Product\Option\Validator\DefaultValidator(
5360
$configMock,
54-
$priceConfigMock
61+
$priceConfigMock,
62+
$this->localeFormatMock
5563
);
5664
}
5765

@@ -63,10 +71,10 @@ public function isValidTitleDataProvider()
6371
{
6472
$mess = ['option required fields' => 'Missed values for option required fields'];
6573
return [
66-
['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
67-
['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 0]), [], true],
68-
[null, 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
69-
[null, 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 0]), $mess, false],
74+
['option_title', 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
75+
['option_title', 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 0]), [], true],
76+
[null, 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
77+
[null, 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 0]), $mess, false],
7078
];
7179
}
7280

@@ -79,15 +87,18 @@ public function isValidTitleDataProvider()
7987
* @param bool $result
8088
* @dataProvider isValidTitleDataProvider
8189
*/
82-
public function testIsValidTitle($title, $type, $priceType, $product, $messages, $result)
90+
public function testIsValidTitle($title, $type, $priceType, $price, $product, $messages, $result)
8391
{
84-
$methods = ['getTitle', 'getType', 'getPriceType', '__wakeup', 'getProduct'];
92+
$methods = ['getTitle', 'getType', 'getPriceType', 'getPrice', '__wakeup', 'getProduct'];
8593
$valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods);
8694
$valueMock->expects($this->once())->method('getTitle')->will($this->returnValue($title));
8795
$valueMock->expects($this->any())->method('getType')->will($this->returnValue($type));
8896
$valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType));
89-
// $valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
97+
$valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
9098
$valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product));
99+
100+
$this->localeFormatMock->expects($this->once())->method('getNumber')->will($this->returnValue($price));
101+
91102
$this->assertEquals($result, $this->validator->isValid($valueMock));
92103
$this->assertEquals($messages, $this->validator->getMessages());
93104
}
@@ -126,4 +137,43 @@ public function testIsValidFail($product)
126137
$this->assertFalse($this->validator->isValid($valueMock));
127138
$this->assertEquals($messages, $this->validator->getMessages());
128139
}
140+
141+
/**
142+
* Data provider for testValidationNegativePrice
143+
* @return array
144+
*/
145+
public function validationPriceDataProvider()
146+
{
147+
return [
148+
['option_title', 'name 1.1', 'fixed', -12, new \Magento\Framework\DataObject(['store_id' => 1])],
149+
['option_title', 'name 1.1', 'fixed', -12, new \Magento\Framework\DataObject(['store_id' => 0])],
150+
['option_title', 'name 1.1', 'fixed', 12, new \Magento\Framework\DataObject(['store_id' => 1])],
151+
['option_title', 'name 1.1', 'fixed', 12, new \Magento\Framework\DataObject(['store_id' => 0])]
152+
];
153+
}
154+
155+
/**
156+
* @param $title
157+
* @param $type
158+
* @param $priceType
159+
* @param $price
160+
* @param $product
161+
* @dataProvider validationPriceDataProvider
162+
*/
163+
public function testValidationPrice($title, $type, $priceType, $price, $product)
164+
{
165+
$methods = ['getTitle', 'getType', 'getPriceType', 'getPrice', '__wakeup', 'getProduct'];
166+
$valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods);
167+
$valueMock->expects($this->once())->method('getTitle')->will($this->returnValue($title));
168+
$valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue($type));
169+
$valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType));
170+
$valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
171+
$valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product));
172+
173+
$this->localeFormatMock->expects($this->once())->method('getNumber')->will($this->returnValue($price));
174+
175+
$messages = [];
176+
$this->assertTrue($this->validator->isValid($valueMock));
177+
$this->assertEquals($messages, $this->validator->getMessages());
178+
}
129179
}

‎app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/FileTest.php

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ class FileTest extends \PHPUnit\Framework\TestCase
1818
*/
1919
protected $valueMock;
2020

21+
/**
22+
* @var \PHPUnit_Framework_MockObject_MockObject
23+
*/
24+
protected $localeFormatMock;
25+
2126
/**
2227
* @inheritdoc
2328
*/
@@ -26,6 +31,8 @@ protected function setUp()
2631
$configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class);
2732
$storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
2833
$priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock);
34+
$this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class);
35+
2936
$config = [
3037
[
3138
'label' => 'group label 1',
@@ -53,7 +60,8 @@ protected function setUp()
5360
$this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods);
5461
$this->validator = new \Magento\Catalog\Model\Product\Option\Validator\File(
5562
$configMock,
56-
$priceConfigMock
63+
$priceConfigMock,
64+
$this->localeFormatMock
5765
);
5866
}
5967

@@ -70,6 +78,15 @@ public function testIsValidSuccess()
7078
->willReturn(10);
7179
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10));
7280
$this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(15));
81+
$this->localeFormatMock->expects($this->at(0))
82+
->method('getNumber')
83+
->with($this->equalTo(10))
84+
->will($this->returnValue(10));
85+
$this->localeFormatMock
86+
->expects($this->at(2))
87+
->method('getNumber')
88+
->with($this->equalTo(15))
89+
->will($this->returnValue(15));
7390
$this->assertEmpty($this->validator->getMessages());
7491
$this->assertTrue($this->validator->isValid($this->valueMock));
7592
}
@@ -87,6 +104,16 @@ public function testIsValidWithNegativeImageSize()
87104
->willReturn(10);
88105
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(-10));
89106
$this->valueMock->expects($this->never())->method('getImageSizeY');
107+
$this->localeFormatMock->expects($this->at(0))
108+
->method('getNumber')
109+
->with($this->equalTo(10))
110+
->will($this->returnValue(10));
111+
$this->localeFormatMock
112+
->expects($this->at(1))
113+
->method('getNumber')
114+
->with($this->equalTo(-10))
115+
->will($this->returnValue(-10));
116+
90117
$messages = [
91118
'option values' => 'Invalid option value',
92119
];
@@ -107,6 +134,15 @@ public function testIsValidWithNegativeImageSizeY()
107134
->willReturn(10);
108135
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10));
109136
$this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(-10));
137+
$this->localeFormatMock->expects($this->at(0))
138+
->method('getNumber')
139+
->with($this->equalTo(10))
140+
->will($this->returnValue(10));
141+
$this->localeFormatMock
142+
->expects($this->at(2))
143+
->method('getNumber')
144+
->with($this->equalTo(-10))
145+
->will($this->returnValue(-10));
110146
$messages = [
111147
'option values' => 'Invalid option value',
112148
];

‎app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/SelectTest.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ class SelectTest extends \PHPUnit\Framework\TestCase
1818
*/
1919
protected $valueMock;
2020

21+
/**
22+
* @var \PHPUnit_Framework_MockObject_MockObject
23+
*/
24+
protected $localeFormatMock;
25+
2126
/**
2227
* @inheritdoc
2328
*/
@@ -26,6 +31,7 @@ protected function setUp()
2631
$configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class);
2732
$storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
2833
$priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock);
34+
$this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class);
2935
$config = [
3036
[
3137
'label' => 'group label 1',
@@ -53,7 +59,8 @@ protected function setUp()
5359
$this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods, []);
5460
$this->validator = new \Magento\Catalog\Model\Product\Option\Validator\Select(
5561
$configMock,
56-
$priceConfigMock
62+
$priceConfigMock,
63+
$this->localeFormatMock
5764
);
5865
}
5966

@@ -69,6 +76,12 @@ public function testIsValidSuccess($expectedResult, array $value)
6976
$this->valueMock->expects($this->never())->method('getPriceType');
7077
$this->valueMock->expects($this->never())->method('getPrice');
7178
$this->valueMock->expects($this->any())->method('getData')->with('values')->will($this->returnValue([$value]));
79+
if (isset($value['price'])) {
80+
$this->localeFormatMock
81+
->expects($this->once())
82+
->method('getNumber')
83+
->will($this->returnValue($value['price']));
84+
}
7285
$this->assertEquals($expectedResult, $this->validator->isValid($this->valueMock));
7386
}
7487

@@ -117,6 +130,7 @@ public function testIsValidateWithInvalidOptionValues()
117130
->method('getData')
118131
->with('values')
119132
->will($this->returnValue('invalid_data'));
133+
120134
$messages = [
121135
'option values' => 'Invalid option value',
122136
];
@@ -159,6 +173,7 @@ public function testIsValidateWithInvalidData($priceType, $price, $title)
159173
$this->valueMock->expects($this->never())->method('getPriceType');
160174
$this->valueMock->expects($this->never())->method('getPrice');
161175
$this->valueMock->expects($this->any())->method('getData')->with('values')->will($this->returnValue([$value]));
176+
$this->localeFormatMock->expects($this->any())->method('getNumber')->will($this->returnValue($price));
162177
$messages = [
163178
'option values' => 'Invalid option value',
164179
];

0 commit comments

Comments
 (0)