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 99d5016f5cdb9..08455430ccac8 100644 --- a/app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php +++ b/app/code/Magento/Catalog/Model/Product/Option/Validator/DefaultValidator.php @@ -28,13 +28,20 @@ class DefaultValidator extends \Magento\Framework\Validator\AbstractValidator */ protected $priceTypes; + /** + * @var \Magento\Framework\Locale\FormatInterface + */ + private $localeFormat; + /** * @param \Magento\Catalog\Model\ProductOptions\ConfigInterface $productOptionConfig * @param \Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig + * @param \Magento\Framework\Locale\FormatInterface|null $localeFormat */ public function __construct( \Magento\Catalog\Model\ProductOptions\ConfigInterface $productOptionConfig, - \Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig + \Magento\Catalog\Model\Config\Source\Product\Options\Price $priceConfig, + \Magento\Framework\Locale\FormatInterface $localeFormat = null ) { foreach ($productOptionConfig->getAll() as $option) { foreach ($option['types'] as $type) { @@ -45,6 +52,9 @@ public function __construct( foreach ($priceConfig->toOptionArray() as $item) { $this->priceTypes[] = $item['value']; } + + $this->localeFormat = $localeFormat ?: \Magento\Framework\App\ObjectManager::getInstance() + ->get(\Magento\Framework\Locale\FormatInterface::class); } /** @@ -137,11 +147,11 @@ protected function validateOptionType(Option $option) */ protected function validateOptionValue(Option $option) { - return $this->isInRange($option->getPriceType(), $this->priceTypes); + return $this->isInRange($option->getPriceType(), $this->priceTypes) && $this->isNumber($option->getPrice()); } /** - * Check whether value is empty + * Check whether the value is empty * * @param mixed $value * @return bool @@ -152,7 +162,7 @@ protected function isEmpty($value) } /** - * Check whether value is in range + * Check whether the value is in range * * @param string $value * @param array $range @@ -164,13 +174,24 @@ protected function isInRange($value, array $range) } /** - * Check whether value is not negative + * Check whether the value is negative * * @param string $value * @return bool */ protected function isNegative($value) { - return (int) $value < 0; + return $this->localeFormat->getNumber($value) < 0; + } + + /** + * Check whether the value is a number + * + * @param string $value + * @return bool + */ + public function isNumber($value) + { + return is_numeric($this->localeFormat->getNumber($value)); } } 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 44756890b6ed7..209531f599811 100644 --- a/app/code/Magento/Catalog/Model/Product/Option/Validator/Select.php +++ b/app/code/Magento/Catalog/Model/Product/Option/Validator/Select.php @@ -8,6 +8,9 @@ use Magento\Catalog\Model\Product\Option; +/** + * Select validator class + */ class Select extends DefaultValidator { /** @@ -83,7 +86,7 @@ protected function isValidOptionPrice($priceType, $price, $storeId) if (!$priceType && !$price) { return true; } - if (!$this->isInRange($priceType, $this->priceTypes)) { + if (!$this->isInRange($priceType, $this->priceTypes) || !$this->isNumber($price)) { 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 da6b790fedfa6..7c2ec8abb768a 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 @@ -18,6 +18,11 @@ class DefaultValidatorTest extends \PHPUnit\Framework\TestCase */ protected $valueMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + protected $localeFormatMock; + /** * @inheritdoc */ @@ -26,6 +31,8 @@ protected function setUp() $configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class); $storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); $priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock); + $this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class); + $config = [ [ 'label' => 'group label 1', @@ -51,7 +58,8 @@ protected function setUp() $configMock->expects($this->once())->method('getAll')->will($this->returnValue($config)); $this->validator = new \Magento\Catalog\Model\Product\Option\Validator\DefaultValidator( $configMock, - $priceConfigMock + $priceConfigMock, + $this->localeFormatMock ); } @@ -63,10 +71,10 @@ public function isValidTitleDataProvider() { $mess = ['option required fields' => 'Missed values for option required fields']; return [ - ['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], + ['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], ]; } @@ -79,15 +87,18 @@ public function isValidTitleDataProvider() * @param bool $result * @dataProvider isValidTitleDataProvider */ - public function testIsValidTitle($title, $type, $priceType, $product, $messages, $result) + public function testIsValidTitle($title, $type, $priceType, $price, $product, $messages, $result) { - $methods = ['getTitle', 'getType', 'getPriceType', '__wakeup', 'getProduct']; + $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->once())->method('getPrice')->will($this->returnValue($price)); $valueMock->expects($this->once())->method('getProduct')->will($this->returnValue($product)); + + $this->localeFormatMock->expects($this->once())->method('getNumber')->will($this->returnValue($price)); + $this->assertEquals($result, $this->validator->isValid($valueMock)); $this->assertEquals($messages, $this->validator->getMessages()); } @@ -126,4 +137,43 @@ public function testIsValidFail($product) $this->assertFalse($this->validator->isValid($valueMock)); $this->assertEquals($messages, $this->validator->getMessages()); } + + /** + * Data provider for testValidationNegativePrice + * @return array + */ + public function validationPriceDataProvider() + { + 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', 12, new \Magento\Framework\DataObject(['store_id' => 1])], + ['option_title', 'name 1.1', 'fixed', 12, new \Magento\Framework\DataObject(['store_id' => 0])] + ]; + } + + /** + * @param $title + * @param $type + * @param $priceType + * @param $price + * @param $product + * @dataProvider validationPriceDataProvider + */ + public function testValidationPrice($title, $type, $priceType, $price, $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->once())->method('getProduct')->will($this->returnValue($product)); + + $this->localeFormatMock->expects($this->once())->method('getNumber')->will($this->returnValue($price)); + + $messages = []; + $this->assertTrue($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 2de993c075514..e688da1c6aa16 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 @@ -18,6 +18,11 @@ class FileTest extends \PHPUnit\Framework\TestCase */ protected $valueMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + protected $localeFormatMock; + /** * @inheritdoc */ @@ -26,6 +31,8 @@ protected function setUp() $configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class); $storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); $priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock); + $this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class); + $config = [ [ 'label' => 'group label 1', @@ -53,7 +60,8 @@ protected function setUp() $this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods); $this->validator = new \Magento\Catalog\Model\Product\Option\Validator\File( $configMock, - $priceConfigMock + $priceConfigMock, + $this->localeFormatMock ); } @@ -70,6 +78,15 @@ public function testIsValidSuccess() ->willReturn(10); $this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(15)); + $this->localeFormatMock->expects($this->at(0)) + ->method('getNumber') + ->with($this->equalTo(10)) + ->will($this->returnValue(10)); + $this->localeFormatMock + ->expects($this->at(2)) + ->method('getNumber') + ->with($this->equalTo(15)) + ->will($this->returnValue(15)); $this->assertEmpty($this->validator->getMessages()); $this->assertTrue($this->validator->isValid($this->valueMock)); } @@ -87,6 +104,16 @@ public function testIsValidWithNegativeImageSize() ->willReturn(10); $this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(-10)); $this->valueMock->expects($this->never())->method('getImageSizeY'); + $this->localeFormatMock->expects($this->at(0)) + ->method('getNumber') + ->with($this->equalTo(10)) + ->will($this->returnValue(10)); + $this->localeFormatMock + ->expects($this->at(1)) + ->method('getNumber') + ->with($this->equalTo(-10)) + ->will($this->returnValue(-10)); + $messages = [ 'option values' => 'Invalid option value', ]; @@ -107,6 +134,15 @@ public function testIsValidWithNegativeImageSizeY() ->willReturn(10); $this->valueMock->expects($this->once())->method('getImageSizeX')->will($this->returnValue(10)); $this->valueMock->expects($this->once())->method('getImageSizeY')->will($this->returnValue(-10)); + $this->localeFormatMock->expects($this->at(0)) + ->method('getNumber') + ->with($this->equalTo(10)) + ->will($this->returnValue(10)); + $this->localeFormatMock + ->expects($this->at(2)) + ->method('getNumber') + ->with($this->equalTo(-10)) + ->will($this->returnValue(-10)); $messages = [ 'option values' => 'Invalid option value', ]; 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 b97783edf856c..7fad5592a2d21 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 @@ -18,6 +18,11 @@ class SelectTest extends \PHPUnit\Framework\TestCase */ protected $valueMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + protected $localeFormatMock; + /** * @inheritdoc */ @@ -26,6 +31,7 @@ protected function setUp() $configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class); $storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); $priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock); + $this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class); $config = [ [ 'label' => 'group label 1', @@ -53,7 +59,8 @@ protected function setUp() $this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods, []); $this->validator = new \Magento\Catalog\Model\Product\Option\Validator\Select( $configMock, - $priceConfigMock + $priceConfigMock, + $this->localeFormatMock ); } @@ -69,6 +76,12 @@ public function testIsValidSuccess($expectedResult, array $value) $this->valueMock->expects($this->never())->method('getPriceType'); $this->valueMock->expects($this->never())->method('getPrice'); $this->valueMock->expects($this->any())->method('getData')->with('values')->will($this->returnValue([$value])); + if (isset($value['price'])) { + $this->localeFormatMock + ->expects($this->once()) + ->method('getNumber') + ->will($this->returnValue($value['price'])); + } $this->assertEquals($expectedResult, $this->validator->isValid($this->valueMock)); } @@ -117,6 +130,7 @@ public function testIsValidateWithInvalidOptionValues() ->method('getData') ->with('values') ->will($this->returnValue('invalid_data')); + $messages = [ 'option values' => 'Invalid option value', ]; @@ -159,6 +173,7 @@ public function testIsValidateWithInvalidData($priceType, $price, $title) $this->valueMock->expects($this->never())->method('getPriceType'); $this->valueMock->expects($this->never())->method('getPrice'); $this->valueMock->expects($this->any())->method('getData')->with('values')->will($this->returnValue([$value])); + $this->localeFormatMock->expects($this->any())->method('getNumber')->will($this->returnValue($price)); $messages = [ 'option values' => 'Invalid option value', ]; 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 4881154728ddc..a3e6189f74925 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 @@ -18,6 +18,11 @@ class TextTest extends \PHPUnit\Framework\TestCase */ protected $valueMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + protected $localeFormatMock; + /** * @inheritdoc */ @@ -26,6 +31,7 @@ protected function setUp() $configMock = $this->createMock(\Magento\Catalog\Model\ProductOptions\ConfigInterface::class); $storeManagerMock = $this->createMock(\Magento\Store\Model\StoreManagerInterface::class); $priceConfigMock = new \Magento\Catalog\Model\Config\Source\Product\Options\Price($storeManagerMock); + $this->localeFormatMock = $this->createMock(\Magento\Framework\Locale\FormatInterface::class); $config = [ [ 'label' => 'group label 1', @@ -53,7 +59,8 @@ protected function setUp() $this->valueMock = $this->createPartialMock(\Magento\Catalog\Model\Product\Option::class, $methods); $this->validator = new \Magento\Catalog\Model\Product\Option\Validator\Text( $configMock, - $priceConfigMock + $priceConfigMock, + $this->localeFormatMock ); } @@ -69,6 +76,10 @@ public function testIsValidSuccess() $this->valueMock->method('getPrice') ->willReturn(10); $this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(10)); + $this->localeFormatMock->expects($this->exactly(2)) + ->method('getNumber') + ->with($this->equalTo(10)) + ->will($this->returnValue(10)); $this->assertTrue($this->validator->isValid($this->valueMock)); $this->assertEmpty($this->validator->getMessages()); } @@ -85,6 +96,15 @@ public function testIsValidWithNegativeMaxCharacters() $this->valueMock->method('getPrice') ->willReturn(10); $this->valueMock->expects($this->once())->method('getMaxCharacters')->will($this->returnValue(-10)); + $this->localeFormatMock->expects($this->at(0)) + ->method('getNumber') + ->with($this->equalTo(10)) + ->will($this->returnValue(10)); + $this->localeFormatMock + ->expects($this->at(1)) + ->method('getNumber') + ->with($this->equalTo(-10)) + ->will($this->returnValue(-10)); $messages = [ 'option values' => 'Invalid option value', ];