diff --git a/app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php b/app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php index 1e5c7f76d829b..d1fe4c570dc75 100644 --- a/app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php +++ b/app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php @@ -132,7 +132,7 @@ protected function validateOptionType(Option $option) */ protected function validateOptionValue(Option $option) { - return $this->isInRange($option->getPriceType(), $this->priceTypes) && !$this->isNegative($option->getPrice()); + return $this->isInRange($option->getPriceType(), $this->priceTypes); } /** diff --git a/app/code/Magento/Catalog/Model/Product/Option/Validator/Select.php b/app/code/Magento/Catalog/Model/Product/Option/Validator/Select.php index f04ab497e1d4f..44756890b6ed7 100644 --- a/app/code/Magento/Catalog/Model/Product/Option/Validator/Select.php +++ b/app/code/Magento/Catalog/Model/Product/Option/Validator/Select.php @@ -83,7 +83,7 @@ protected function isValidOptionPrice($priceType, $price, $storeId) if (!$priceType && !$price) { return true; } - if (!$this->isInRange($priceType, $this->priceTypes) || $this->isNegative($price)) { + if (!$this->isInRange($priceType, $this->priceTypes)) { return false; } diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/DefaultValidatorTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/DefaultValidatorTest.php index 1eb5f1a2dacd2..086af60994d8d 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/DefaultValidatorTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/DefaultValidatorTest.php @@ -60,10 +60,10 @@ public function isValidTitleDataProvider() { $mess = ['option required fields' => 'Missed values for option required fields']; return [ - ['option_title', 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 1]), [], true], - ['option_title', 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 0]), [], true], - [null, 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 1]), [], true], - [null, 'name 1.1', 'fixed', 10, new \Magento\Framework\DataObject(['store_id' => 0]), $mess, false], + ['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 1]), [], true], + ['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 0]), [], true], + [null, 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 1]), [], true], + [null, 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 0]), $mess, false], ]; } @@ -71,20 +71,19 @@ public function isValidTitleDataProvider() * @param $title * @param $type * @param $priceType - * @param $price * @param $product * @param $messages * @param $result * @dataProvider isValidTitleDataProvider */ - public function testIsValidTitle($title, $type, $priceType, $price, $product, $messages, $result) + public function testIsValidTitle($title, $type, $priceType, $product, $messages, $result) { $methods = ['getTitle', 'getType', 'getPriceType', 'getPrice', '__wakeup', 'getProduct']; $valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods); $valueMock->expects($this->once())->method('getTitle')->will($this->returnValue($title)); $valueMock->expects($this->any())->method('getType')->will($this->returnValue($type)); $valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType)); - $valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price)); + $valueMock->expects($this->never())->method('getPrice'); $valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product)); $this->assertEquals($result, $this->validator->isValid($valueMock)); $this->assertEquals($messages, $this->validator->getMessages()); @@ -126,37 +125,72 @@ public function testIsValidFail($product) } /** - * Data provider for testValidationNegativePrice + * Data provider for testValidPriceTypeIsValid * @return array */ - public function validationNegativePriceDataProvider() + public function validPriceTypeIsValidDataProvider() { return [ - ['option_title', 'name 1.1', 'fixed', -12, new \Magento\Framework\DataObject(['store_id' => 1])], - ['option_title', 'name 1.1', 'fixed', -12, new \Magento\Framework\DataObject(['store_id' => 0])], + ['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 1])], + ['option_title', 'name 1.1', 'fixed', new \Magento\Framework\DataObject(['store_id' => 0])], ]; } /** + * + * * @param $title * @param $type * @param $priceType - * @param $price * @param $product - * @dataProvider validationNegativePriceDataProvider + * @dataProvider validPriceTypeIsValidDataProvider */ - public function testValidationNegativePrice($title, $type, $priceType, $price, $product) + public function testValidPriceTypeIsValid($title, $type, $priceType, $product) { $methods = ['getTitle', 'getType', 'getPriceType', 'getPrice', '__wakeup', 'getProduct']; $valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods); $valueMock->expects($this->once())->method('getTitle')->will($this->returnValue($title)); $valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue($type)); $valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType)); - $valueMock->expects($this->once())->method('getPrice')->will($this->returnValue($price)); + $valueMock->expects($this->never())->method('getPrice'); + $valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product)); + + $messages = []; + $this->assertTrue($this->validator->isValid($valueMock)); + $this->assertEquals($messages, $this->validator->getMessages()); + } + + /** + * Data provider for testInvalidPriceTypeIsFail + * @return array + */ + public function invalidPriceTypeIsFailDataProvider() + { + return [ + ['option_title', 'name 1.1', 'notexisting', new \Magento\Framework\DataObject(['store_id' => 1])], + ['option_title', 'name 1.1', 'wrongtype', new \Magento\Framework\DataObject(['store_id' => 0])], + ]; + } + + /** + * @param $title + * @param $type + * @param $priceType + * @param $product + * @dataProvider invalidPriceTypeIsFailDataProvider + */ + public function testInvalidPriceTypeIsFail($title, $type, $priceType, $product) + { + $methods = ['getTitle', 'getType', 'getPriceType', 'getPrice', '__wakeup', 'getProduct']; + $valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods); + $valueMock->expects($this->once())->method('getTitle')->will($this->returnValue($title)); + $valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue($type)); + $valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue($priceType)); + $valueMock->expects($this->never())->method('getPrice'); $valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product)); $messages = [ - 'option values' => 'Invalid option value', + 'option values' => 'Invalid option value' ]; $this->assertFalse($this->validator->isValid($valueMock)); $this->assertEquals($messages, $this->validator->getMessages()); diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/FileTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/FileTest.php index 3c06db0e7ce5f..557ef7ef930dc 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/FileTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/FileTest.php @@ -59,7 +59,7 @@ public function testIsValidSuccess() $this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title')); $this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1')); $this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed')); - $this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10)); + $this->valueMock->expects($this->never())->method('getPrice')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(15)); $this->assertEmpty($this->validator->getMessages()); @@ -71,7 +71,7 @@ public function testIsValidWithNegativeImageSize() $this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title')); $this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1')); $this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed')); - $this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10)); + $this->valueMock->expects($this->never())->method('getPrice')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(-10)); $this->valueMock->expects($this->never())->method('getImageSizeY'); $messages = [ @@ -86,7 +86,7 @@ public function testIsValidWithNegativeImageSizeY() $this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title')); $this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1')); $this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed')); - $this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10)); + $this->valueMock->expects($this->never())->method('getPrice')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(-10)); $messages = [ diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/SelectTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/SelectTest.php index 046ee703c850e..924893bce23a4 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/SelectTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/SelectTest.php @@ -87,7 +87,7 @@ public function isValidSuccessDataProvider() ] ], [ - false, + true, [ 'title' => 'Some Title', 'price_type' => 'fixed', @@ -157,7 +157,6 @@ public function testIsValidateWithInvalidData($priceType, $price, $title) public function isValidateWithInvalidDataDataProvider() { return [ - 'invalid_price' => ['fixed', -10, 'Title'], 'invalid_price_type' => ['some_value', '10', 'Title'], 'empty_title' => ['fixed', 10, null] ]; diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/TextTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/TextTest.php index cf31d67817684..1405e4faaace5 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/TextTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Validator/TextTest.php @@ -59,7 +59,7 @@ public function testIsValidSuccess() $this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title')); $this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1')); $this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed')); - $this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10)); + $this->valueMock->expects($this->never())->method('getPrice')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(10)); $this->assertTrue($this->validator->isValid($this->valueMock)); $this->assertEmpty($this->validator->getMessages()); @@ -70,7 +70,7 @@ public function testIsValidWithNegativeMaxCharacters() $this->valueMock->expects($this->once())->method('getTitle')->will($this->returnValue('option_title')); $this->valueMock->expects($this->exactly(2))->method('getType')->will($this->returnValue('name 1.1')); $this->valueMock->expects($this->once())->method('getPriceType')->will($this->returnValue('fixed')); - $this->valueMock->expects($this->once())->method('getPrice')->will($this->returnValue(10)); + $this->valueMock->expects($this->never())->method('getPrice')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(-10)); $messages = [ 'option values' => 'Invalid option value', diff --git a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/CustomOptions.php b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/CustomOptions.php index 7995926d27de5..2b75c4926516f 100755 --- a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/CustomOptions.php +++ b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/CustomOptions.php @@ -923,7 +923,7 @@ protected function getPriceFieldConfig($sortOrder) 'addbeforePool' => $this->productOptionsPrice->prefixesToOptionArray(), 'sortOrder' => $sortOrder, 'validation' => [ - 'validate-zero-or-greater' => true + 'validate-zero-or-greater' => false ], ], ], diff --git a/app/code/Magento/Catalog/view/base/web/js/price-options.js b/app/code/Magento/Catalog/view/base/web/js/price-options.js index ceeea4c878622..e86f48c93aca8 100644 --- a/app/code/Magento/Catalog/view/base/web/js/price-options.js +++ b/app/code/Magento/Catalog/view/base/web/js/price-options.js @@ -20,8 +20,10 @@ define([ optionConfig: {}, optionHandlers: {}, optionTemplate: '<%= data.label %>' + - '<% if (data.finalPrice.value) { %>' + + '<% if (data.finalPrice.value > 0) { %>' + ' +<%- data.finalPrice.formatted %>' + + '<% } else if (data.finalPrice.value) { %>' + + ' <%- data.finalPrice.formatted %>' + '<% } %>', controlContainer: 'dd' };