Skip to content

Commit f7aab57

Browse files
dverkadedverkade
committed
magento/magento2##7333: Unable to set negative custom option fixed price in admin view.
 - Fixed unit tests - Fixed integration tests - Changed defaultvalidator to have the locale format interface. Because a value entered in the backoffice of Magento is passed as the value like "40,000.00", which needs to be converted to a float in order to do a valid check on it.
1 parent dc4de31 commit f7aab57

File tree

5 files changed

+101
-7
lines changed

5 files changed

+101
-7
lines changed

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,20 @@ class DefaultValidator extends \Magento\Framework\Validator\AbstractValidator
2525
*/
2626
protected $priceTypes;
2727

28+
/**
29+
* @var \Magento\Framework\Locale\FormatInterface
30+
*/
31+
private $localeFormat;
32+
2833
/**
2934
* @param \Magento\Catalog\Model\ProductOptions\ConfigInterface $productOptionConfig
3035
* @param \Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig
36+
* @param \Magento\Framework\Locale\FormatInterface|null $localeFormat
3137
*/
3238
public function __construct(
3339
\Magento\Catalog\Model\ProductOptions\ConfigInterface $productOptionConfig,
34-
\Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig
40+
\Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig,
41+
\Magento\Framework\Locale\FormatInterface $localeFormat = null
3542
) {
3643
foreach ($productOptionConfig->getAll() as $option) {
3744
foreach ($option['types'] as $type) {
@@ -42,6 +49,9 @@ public function __construct(
4249
foreach ($priceConfig->toOptionArray() as $item) {
4350
$this->priceTypes[] = $item['value'];
4451
}
52+
53+
$this->localeFormat = $localeFormat ?: \Magento\Framework\App\ObjectManager::getInstance()
54+
->get(\Magento\Framework\Locale\FormatInterface::class);
4555
}
4656

4757
/**
@@ -166,7 +176,7 @@ protected function isInRange($value, array $range)
166176
*/
167177
protected function isNegative($value)
168178
{
169-
return intval($value) < 0;
179+
return $this->localeFormat->getNumber($value) < 0;
170180
}
171181

172182
/**
@@ -177,6 +187,6 @@ protected function isNegative($value)
177187
*/
178188
protected function isNumber($value)
179189
{
180-
return is_numeric($value);
190+
return is_numeric($this->localeFormat->getNumber($value));
181191
}
182192
}

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

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

21+
/**
22+
* @var \PHPUnit_Framework_MockObject_MockObject
23+
*/
24+
protected $localeFormatMock;
25+
2126
protected function setUp()
2227
{
2328
$configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class);
2429
$storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
2530
$priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock);
31+
$this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class);
32+
2633
$config = [
2734
[
2835
'label' => 'group label 1',
@@ -48,7 +55,8 @@ protected function setUp()
4855
$configMock->expects($this->once())->method('getAll')->will($this->returnValue($config));
4956
$this->validator = new \Magento\Catalog\Model\Product\Option\Validator\DefaultValidator(
5057
$configMock,
51-
$priceConfigMock
58+
$priceConfigMock,
59+
$this->localeFormatMock
5260
);
5361
}
5462

@@ -86,6 +94,9 @@ public function testIsValidTitle($title, $type, $priceType, $price, $product, $m
8694
$valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType));
8795
$valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
8896
$valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product));
97+
98+
$this->localeFormatMock->expects($this->once())->method('getNumber')->will($this->returnValue($price));
99+
89100
$this->assertEquals($result, $this->validator->isValid($valueMock));
90101
$this->assertEquals($messages, $this->validator->getMessages());
91102
}
@@ -157,6 +168,8 @@ public function testValidationPrice($title, $type, $priceType, $price, $product)
157168
$valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
158169
$valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product));
159170

171+
$this->localeFormatMock->expects($this->once())->method('getNumber')->will($this->returnValue($price));
172+
160173
$messages = [];
161174
$this->assertTrue($this->validator->isValid($valueMock));
162175
$this->assertEquals($messages, $this->validator->getMessages());

‎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,11 +18,18 @@ class FileTest extends \PHPUnit\Framework\TestCase
1818
*/
1919
protected $valueMock;
2020

21+
/**
22+
* @var \PHPUnit_Framework_MockObject_MockObject
23+
*/
24+
protected $localeFormatMock;
25+
2126
protected function setUp()
2227
{
2328
$configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class);
2429
$storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
2530
$priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock);
31+
$this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class);
32+
2633
$config = [
2734
[
2835
'label' => 'group label 1',
@@ -50,7 +57,8 @@ protected function setUp()
5057
$this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods);
5158
$this->validator = new \Magento\Catalog\Model\Product\Option\Validator\File(
5259
$configMock,
53-
$priceConfigMock
60+
$priceConfigMock,
61+
$this->localeFormatMock
5462
);
5563
}
5664

@@ -62,6 +70,15 @@ public function testIsValidSuccess()
6270
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
6371
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10));
6472
$this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(15));
73+
$this->localeFormatMock->expects($this->at(0))
74+
->method('getNumber')
75+
->with($this->equalTo(10))
76+
->will($this->returnValue(10));
77+
$this->localeFormatMock
78+
->expects($this->at(2))
79+
->method('getNumber')
80+
->with($this->equalTo(15))
81+
->will($this->returnValue(15));
6582
$this->assertEmpty($this->validator->getMessages());
6683
$this->assertTrue($this->validator->isValid($this->valueMock));
6784
}
@@ -74,6 +91,16 @@ public function testIsValidWithNegativeImageSize()
7491
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
7592
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(-10));
7693
$this->valueMock->expects($this->never())->method('getImageSizeY');
94+
$this->localeFormatMock->expects($this->at(0))
95+
->method('getNumber')
96+
->with($this->equalTo(10))
97+
->will($this->returnValue(10));
98+
$this->localeFormatMock
99+
->expects($this->at(1))
100+
->method('getNumber')
101+
->with($this->equalTo(-10))
102+
->will($this->returnValue(-10));
103+
77104
$messages = [
78105
'option values' => 'Invalid option value',
79106
];
@@ -89,6 +116,15 @@ public function testIsValidWithNegativeImageSizeY()
89116
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
90117
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10));
91118
$this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(-10));
119+
$this->localeFormatMock->expects($this->at(0))
120+
->method('getNumber')
121+
->with($this->equalTo(10))
122+
->will($this->returnValue(10));
123+
$this->localeFormatMock
124+
->expects($this->at(2))
125+
->method('getNumber')
126+
->with($this->equalTo(-10))
127+
->will($this->returnValue(-10));
92128
$messages = [
93129
'option values' => 'Invalid option value',
94130
];

‎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,11 +18,17 @@ class SelectTest extends \PHPUnit\Framework\TestCase
1818
*/
1919
protected $valueMock;
2020

21+
/**
22+
* @var \PHPUnit_Framework_MockObject_MockObject
23+
*/
24+
protected $localeFormatMock;
25+
2126
protected function setUp()
2227
{
2328
$configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class);
2429
$storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
2530
$priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock);
31+
$this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class);
2632
$config = [
2733
[
2834
'label' => 'group label 1',
@@ -50,7 +56,8 @@ protected function setUp()
5056
$this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods, []);
5157
$this->validator = new \Magento\Catalog\Model\Product\Option\Validator\Select(
5258
$configMock,
53-
$priceConfigMock
59+
$priceConfigMock,
60+
$this->localeFormatMock
5461
);
5562
}
5663

@@ -66,6 +73,12 @@ public function testIsValidSuccess($expectedResult, array $value)
6673
$this->valueMock->expects($this->never())->method('getPriceType');
6774
$this->valueMock->expects($this->never())->method('getPrice');
6875
$this->valueMock->expects($this->any())->method('getData')->with('values')->will($this->returnValue([$value]));
76+
if (isset($value['price'])) {
77+
$this->localeFormatMock
78+
->expects($this->once())
79+
->method('getNumber')
80+
->will($this->returnValue($value['price']));
81+
}
6982
$this->assertEquals($expectedResult, $this->validator->isValid($this->valueMock));
7083
}
7184

@@ -108,6 +121,7 @@ public function testIsValidateWithInvalidOptionValues()
108121
->method('getData')
109122
->with('values')
110123
->will($this->returnValue('invalid_data'));
124+
111125
$messages = [
112126
'option values' => 'Invalid option value',
113127
];
@@ -147,6 +161,7 @@ public function testIsValidateWithInvalidData($priceType, $price, $title)
147161
$this->valueMock->expects($this->never())->method('getPriceType');
148162
$this->valueMock->expects($this->never())->method('getPrice');
149163
$this->valueMock->expects($this->any())->method('getData')->with('values')->will($this->returnValue([$value]));
164+
$this->localeFormatMock->expects($this->any())->method('getNumber')->will($this->returnValue($price));
150165
$messages = [
151166
'option values' => 'Invalid option value',
152167
];

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

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

21+
/**
22+
* @var \PHPUnit_Framework_MockObject_MockObject
23+
*/
24+
protected $localeFormatMock;
25+
2126
protected function setUp()
2227
{
2328
$configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class);
2429
$storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class);
2530
$priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock);
31+
$this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class);
2632
$config = [
2733
[
2834
'label' => 'group label 1',
@@ -50,7 +56,8 @@ protected function setUp()
5056
$this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods);
5157
$this->validator = new \Magento\Catalog\Model\Product\Option\Validator\Text(
5258
$configMock,
53-
$priceConfigMock
59+
$priceConfigMock,
60+
$this->localeFormatMock
5461
);
5562
}
5663

@@ -61,6 +68,10 @@ public function testIsValidSuccess()
6168
$this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed'));
6269
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
6370
$this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(10));
71+
$this->localeFormatMock->expects($this->exactly(2))
72+
->method('getNumber')
73+
->with($this->equalTo(10))
74+
->will($this->returnValue(10));
6475
$this->assertTrue($this->validator->isValid($this->valueMock));
6576
$this->assertEmpty($this->validator->getMessages());
6677
}
@@ -72,6 +83,15 @@ public function testIsValidWithNegativeMaxCharacters()
7283
$this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed'));
7384
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
7485
$this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(-10));
86+
$this->localeFormatMock->expects($this->at(0))
87+
->method('getNumber')
88+
->with($this->equalTo(10))
89+
->will($this->returnValue(10));
90+
$this->localeFormatMock
91+
->expects($this->at(1))
92+
->method('getNumber')
93+
->with($this->equalTo(-10))
94+
->will($this->returnValue(-10));
7595
$messages = [
7696
'option values' => 'Invalid option value',
7797
];

0 commit comments

Comments
 (0)