Skip to content

Commit 0c50659

Browse files
nikita-shcherbatykhnikita-shcherbatykh
committed
MAGETWO-73774: [2.3] Custom option price field disallows negative values #7333 
1 parent d5ad9dd commit 0c50659

File tree

13 files changed

+40
-76
lines changed

13 files changed

+40
-76
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ protected function validateOptionType(Option $option)
132132
*/
133133
protected function validateOptionValue(Option $option)
134134
{
135-
return $this->isInRange($option->getPriceType(), $this->priceTypes) && !$this->isNegative($option->getPrice());
135+
return $this->isInRange($option->getPriceType(), $this->priceTypes);
136136
}
137137

138138
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ protected function isValidOptionPrice($priceType, $price, $storeId)
8383
if (!$priceType && !$price) {
8484
return true;
8585
}
86-
if (!$this->isInRange($priceType, $this->priceTypes) || $this->isNegative($price)) {
86+
if (!$this->isInRange($priceType, $this->priceTypes)) {
8787
return false;
8888
}
8989

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

Lines changed: 7 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -60,31 +60,30 @@ public function isValidTitleDataProvider()
6060
{
6161
$mess = ['option required fields' => 'Missed values for option required fields'];
6262
return [
63-
['option_title', 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
64-
['option_title', 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 0]), [], true],
65-
[null, 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
66-
[null, 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 0]), $mess, false],
63+
['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
64+
['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 0]), [], true],
65+
[null, 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 1]), [], true],
66+
[null, 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 0]), $mess, false],
6767
];
6868
}
6969

7070
/**
7171
* @param $title
7272
* @param $type
7373
* @param $priceType
74-
* @param $price
7574
* @param $product
7675
* @param $messages
7776
* @param $result
7877
* @dataProvider isValidTitleDataProvider
7978
*/
80-
public function testIsValidTitle($title, $type, $priceType, $price, $product, $messages, $result)
79+
public function testIsValidTitle($title, $type, $priceType, $product, $messages, $result)
8180
{
82-
$methods = ['getTitle', 'getType', 'getPriceType', 'getPrice', '__wakeup', 'getProduct'];
81+
$methods = ['getTitle', 'getType', 'getPriceType', '__wakeup', 'getProduct'];
8382
$valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods);
8483
$valueMock->expects($this->once())->method('getTitle')->will($this->returnValue($title));
8584
$valueMock->expects($this->any())->method('getType')->will($this->returnValue($type));
8685
$valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType));
87-
$valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
86+
// $valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
8887
$valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product));
8988
$this->assertEquals($result, $this->validator->isValid($valueMock));
9089
$this->assertEquals($messages, $this->validator->getMessages());
@@ -124,41 +123,4 @@ public function testIsValidFail($product)
124123
$this->assertFalse($this->validator->isValid($valueMock));
125124
$this->assertEquals($messages, $this->validator->getMessages());
126125
}
127-
128-
/**
129-
* Data provider for testValidationNegativePrice
130-
* @return array
131-
*/
132-
public function validationNegativePriceDataProvider()
133-
{
134-
return [
135-
['option_title', 'name 1.1', 'fixed', -12, new \Magento\Framework\DataObject(['store_id' => 1])],
136-
['option_title', 'name 1.1', 'fixed', -12, new \Magento\Framework\DataObject(['store_id' => 0])],
137-
];
138-
}
139-
140-
/**
141-
* @param $title
142-
* @param $type
143-
* @param $priceType
144-
* @param $price
145-
* @param $product
146-
* @dataProvider validationNegativePriceDataProvider
147-
*/
148-
public function testValidationNegativePrice($title, $type, $priceType, $price, $product)
149-
{
150-
$methods = ['getTitle', 'getType', 'getPriceType', 'getPrice', '__wakeup', 'getProduct'];
151-
$valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods);
152-
$valueMock->expects($this->once())->method('getTitle')->will($this->returnValue($title));
153-
$valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue($type));
154-
$valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType));
155-
$valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price));
156-
$valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product));
157-
158-
$messages = [
159-
'option values' => 'Invalid option value',
160-
];
161-
$this->assertFalse($this->validator->isValid($valueMock));
162-
$this->assertEquals($messages, $this->validator->getMessages());
163-
}
164126
}

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ public function testIsValidSuccess()
5858
{
5959
$this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title'));
6060
$this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1'));
61-
$this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed'));
62-
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
61+
$this->valueMock->method('getPriceType')
62+
->willReturn('fixed');
63+
$this->valueMock->method('getPrice')
64+
->willReturn(10);
6365
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10));
6466
$this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(15));
6567
$this->assertEmpty($this->validator->getMessages());
@@ -70,8 +72,10 @@ public function testIsValidWithNegativeImageSize()
7072
{
7173
$this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title'));
7274
$this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1'));
73-
$this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed'));
74-
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
75+
$this->valueMock->method('getPriceType')
76+
->willReturn('fixed');
77+
$this->valueMock->method('getPrice')
78+
->willReturn(10);
7579
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(-10));
7680
$this->valueMock->expects($this->never())->method('getImageSizeY');
7781
$messages = [
@@ -85,8 +89,10 @@ public function testIsValidWithNegativeImageSizeY()
8589
{
8690
$this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title'));
8791
$this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1'));
88-
$this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed'));
89-
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
92+
$this->valueMock->method('getPriceType')
93+
->willReturn('fixed');
94+
$this->valueMock->method('getPrice')
95+
->willReturn(10);
9096
$this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10));
9197
$this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(-10));
9298
$messages = [

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public function isValidSuccessDataProvider()
9090
]
9191
],
9292
[
93-
false,
93+
true,
9494
[
9595
'title' => 'Some Title',
9696
'price_type' => 'fixed',
@@ -163,7 +163,6 @@ public function testIsValidateWithInvalidData($priceType, $price, $title)
163163
public function isValidateWithInvalidDataDataProvider()
164164
{
165165
return [
166-
'invalid_price' => ['fixed', -10, 'Title'],
167166
'invalid_price_type' => ['some_value', '10', 'Title'],
168167
'empty_title' => ['fixed', 10, null]
169168
];

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ public function testIsValidSuccess()
5858
{
5959
$this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title'));
6060
$this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1'));
61-
$this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed'));
62-
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
61+
$this->valueMock->method('getPriceType')
62+
->willReturn('fixed');
63+
$this->valueMock->method('getPrice')
64+
->willReturn(10);
6365
$this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(10));
6466
$this->assertTrue($this->validator->isValid($this->valueMock));
6567
$this->assertEmpty($this->validator->getMessages());
@@ -69,8 +71,10 @@ public function testIsValidWithNegativeMaxCharacters()
6971
{
7072
$this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title'));
7173
$this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1'));
72-
$this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed'));
73-
$this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10));
74+
$this->valueMock->method('getPriceType')
75+
->willReturn('fixed');
76+
$this->valueMock->method('getPrice')
77+
->willReturn(10);
7478
$this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(-10));
7579
$messages = [
7680
'option values' => 'Invalid option value',

‎app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/CustomOptions.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -923,7 +923,7 @@ protected function getPriceFieldConfig($sortOrder)
923923
'addbeforePool' => $this->productOptionsPrice->prefixesToOptionArray(),
924924
'sortOrder' => $sortOrder,
925925
'validation' => [
926-
'validate-zero-or-greater' => true
926+
'validate-number' => true
927927
],
928928
],
929929
],

‎dev/tests/api-functional/testsuite/Magento/Catalog/Api/_files/product_options.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
'type' => 'field',
1111
'sort_order' => 1,
1212
'is_require' => 1,
13-
'price' => 10,
13+
'price' => -10,
1414
'price_type' => 'fixed',
1515
'sku' => 'sku1',
1616
'max_characters' => 10,

‎dev/tests/api-functional/testsuite/Magento/Catalog/Api/_files/product_options_negative.php

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,6 @@
1515
'sku' => 'sku1',
1616
'max_characters' => 10,
1717
],
18-
'negative_price' => [
19-
'title' => 'area option',
20-
'type' => 'area',
21-
'sort_order' => 2,
22-
'is_require' => 0,
23-
'price' => -20,
24-
'price_type' => 'percent',
25-
'sku' => 'sku2',
26-
'max_characters' => 20,
27-
28-
],
2918
'negative_value_of_image_size' => [
3019
'title' => 'file option',
3120
'type' => 'file',

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public function testGetMinPrice(): void
9696
$collection->load();
9797
/** @var \Magento\Catalog\Model\Product $product */
9898
$product = $collection->getFirstItem();
99-
$this->assertEquals(333, $product->getData('min_price'));
99+
$this->assertEquals(323, $product->getData('min_price'));
100100
}
101101

102102
/**

0 commit comments

Comments
 (0)