Skip to content

Commit f8aedef

Browse files
author
Sergey Shvets
committed
MAGETWO-91771: Comma special character in cart price rule condition value results in incorrect rule
1 parent 8c227d5 commit f8aedef

File tree

6 files changed

+161
-19
lines changed

6 files changed

+161
-19
lines changed

‎app/code/Magento/ConfigurableProduct/Test/Unit/Block/Product/View/Type/ConfigurableTest.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\ConfigurableProduct\Test\Unit\Block\Product\View\Type;
78

89
use Magento\Customer\Model\Session;
@@ -174,7 +175,7 @@ protected function setUp()
174175
*
175176
* @return array
176177
*/
177-
public function cacheKeyProvider() : array
178+
public function cacheKeyProvider(): array
178179
{
179180
return [
180181
'without_currency_and_customer_group' => [
@@ -313,11 +314,7 @@ public function testGetJsonConfig()
313314
$this->localeFormat->expects($this->atLeastOnce())->method('getPriceFormat')->willReturn([]);
314315
$this->localeFormat->expects($this->any())
315316
->method('getNumber')
316-
->willReturnMap([
317-
[$amount, $amount],
318-
[$priceQty, $priceQty],
319-
[$percentage, $percentage],
320-
]);
317+
->willReturnArgument(0);
321318

322319
$this->variationPricesMock->expects($this->once())
323320
->method('getFormattedPrices')

‎app/code/Magento/Rule/Model/Condition/AbstractCondition.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ public function isArrayOperatorType()
380380
}
381381

382382
/**
383-
* @return array
383+
* @return mixed
384384
*/
385385
public function getValue()
386386
{

‎app/code/Magento/SalesRule/Model/Rule/Condition/Product.php

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\SalesRule\Model\Rule\Condition;
78

89
/**
@@ -51,10 +52,17 @@ public function validate(\Magento\Framework\Model\AbstractModel $model)
5152

5253
$attrCode = $this->getAttribute();
5354

54-
if ('category_ids' == $attrCode) {
55+
if ($attrCode === 'category_ids') {
5556
return $this->validateAttribute($this->_getAvailableInCategories($product->getId()));
5657
}
5758

59+
if ($attrCode === 'quote_item_price') {
60+
$numericOperations = $this->getDefaultOperatorInputByType()['numeric'];
61+
if (in_array($this->getOperator(), $numericOperations)) {
62+
$this->setData('value', $this->getFormattedPrice($this->getValue()));
63+
}
64+
}
65+
5866
return parent::validate($product);
5967
}
6068

@@ -79,4 +87,23 @@ public function getValueElementChooserUrl()
7987
}
8088
return $url !== false ? $this->_backendData->getUrl($url) : '';
8189
}
90+
91+
/**
92+
* @param string $value
93+
* @return float|null
94+
*/
95+
private function getFormattedPrice($value)
96+
{
97+
$value = preg_replace('/[^0-9^\^.,-]/m', '', $value);
98+
99+
/**
100+
* If the comma is the third symbol in the number, we consider it to be a decimal separator
101+
*/
102+
$separatorComa = strpos($value, ',');
103+
$separatorDot = strpos($value, '.');
104+
if ($separatorComa !== false && $separatorDot === false && preg_match('/,\d{3}$/m', $value) === 1) {
105+
$value .= '.00';
106+
}
107+
return $this->_localeFormat->getNumber($value);
108+
}
82109
}

‎app/code/Magento/SalesRule/Test/Unit/Model/Rule/Condition/ProductTest.php

Lines changed: 91 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\SalesRule\Test\Unit\Model\Rule\Condition;
78

9+
use Magento\Directory\Model\CurrencyFactory;
10+
use Magento\Framework\App\ScopeResolverInterface;
811
use \Magento\Framework\DB\Adapter\AdapterInterface;
912
use \Magento\Framework\DB\Select;
10-
use \Magento\Framework\Model\AbstractModel;
13+
use Magento\Framework\Locale\Format;
14+
use Magento\Framework\Locale\ResolverInterface;
1115
use Magento\Quote\Model\Quote\Item\AbstractItem;
1216
use \Magento\Rule\Model\Condition\Context;
1317
use \Magento\Backend\Helper\Data;
@@ -50,8 +54,8 @@ class ProductTest extends \PHPUnit\Framework\TestCase
5054
/** @var Collection|\PHPUnit_Framework_MockObject_MockObject */
5155
protected $collectionMock;
5256

53-
/** @var FormatInterface|\PHPUnit_Framework_MockObject_MockObject */
54-
protected $formatMock;
57+
/** @var FormatInterface */
58+
protected $format;
5559

5660
/** @var AttributeLoaderInterface|\PHPUnit_Framework_MockObject_MockObject */
5761
protected $attributeLoaderInterfaceMock;
@@ -130,8 +134,12 @@ protected function setUp()
130134
$this->collectionMock = $this->getMockBuilder(Collection::class)
131135
->disableOriginalConstructor()
132136
->getMock();
133-
$this->formatMock = $this->getMockBuilder(FormatInterface::class)
134-
->getMockForAbstractClass();
137+
$this->format = new Format(
138+
$this->getMockBuilder(ScopeResolverInterface::class)->disableOriginalConstructor()->getMock(),
139+
$this->getMockBuilder(ResolverInterface::class)->disableOriginalConstructor()->getMock(),
140+
$this->getMockBuilder(CurrencyFactory::class)->disableOriginalConstructor()->getMock()
141+
);
142+
135143
$this->model = new SalesRuleProduct(
136144
$this->contextMock,
137145
$this->backendHelperMock,
@@ -140,7 +148,7 @@ protected function setUp()
140148
$this->productRepositoryMock,
141149
$this->productMock,
142150
$this->collectionMock,
143-
$this->formatMock
151+
$this->format
144152
);
145153
}
146154

@@ -231,4 +239,81 @@ public function testValidateCategoriesIgnoresVisibility()
231239

232240
$this->model->validate($item);
233241
}
242+
243+
/**
244+
* @param boolean $isValid
245+
* @param string $conditionValue
246+
* @param string $operator
247+
* @param double $productPrice
248+
* @dataProvider localisationProvider
249+
*/
250+
public function testQuoteLocaleFormatPrice($isValid, $conditionValue, $operator = '>=', $productPrice = 2000.00)
251+
{
252+
$attr = $this->getMockBuilder(\Magento\Framework\Model\ResourceModel\Db\AbstractDb::class)
253+
->disableOriginalConstructor()
254+
->setMethods(['getAttribute'])
255+
->getMockForAbstractClass();
256+
257+
$attr->expects($this->any())
258+
->method('getAttribute')
259+
->willReturn('');
260+
261+
/* @var \Magento\Catalog\Model\Product|\PHPUnit_Framework_MockObject_MockObject $product */
262+
$product = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
263+
->disableOriginalConstructor()
264+
->setMethods(['setQuoteItemPrice', 'getResource', 'hasData', 'getData',])
265+
->getMock();
266+
267+
$product->expects($this->any())
268+
->method('setQuoteItemPrice')
269+
->willReturnSelf();
270+
271+
$product->expects($this->any())
272+
->method('getResource')
273+
->willReturn($attr);
274+
275+
$product->expects($this->any())
276+
->method('hasData')
277+
->willReturn(true);
278+
279+
$product->expects($this->any())
280+
->method('getData')
281+
->with('quote_item_price')
282+
->willReturn($productPrice);
283+
284+
/* @var AbstractItem|\PHPUnit_Framework_MockObject_MockObject $item */
285+
$item = $this->getMockBuilder(AbstractItem::class)
286+
->disableOriginalConstructor()
287+
->setMethods(['getPrice', 'getProduct',])
288+
->getMockForAbstractClass();
289+
290+
$item->expects($this->any())
291+
->method('getPrice')
292+
->willReturn($productPrice);
293+
294+
$item->expects($this->any())
295+
->method('getProduct')
296+
->willReturn($product);
297+
298+
$this->model->setAttribute('quote_item_price')
299+
->setOperator($operator);
300+
301+
$this->assertEquals($isValid, $this->model->setValue($conditionValue)->validate($item));
302+
}
303+
304+
/**
305+
* DataProvider for testQuoteLocaleFormatPrice
306+
*
307+
* @return array
308+
*/
309+
public function localisationProvider(): array
310+
{
311+
return [
312+
'number' => [true, 500.01],
313+
'locale' => [true, '1,500.03'],
314+
'operation' => [true, '1,500.03', '!='],
315+
'stringOperation' => [false, '1,500.03', '{}'],
316+
'smallPrice' => [false, '1,500.03', '>=', 1000],
317+
];
318+
}
234319
}

‎lib/internal/Magento/Framework/Locale/Format.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public function getNumber($value)
6565
}
6666

6767
//trim spaces and apostrophes
68-
$value = str_replace(['\'', ' '], '', $value);
68+
$value = preg_replace('/[^0-9^\^.,-]/m', '', $value);
6969

7070
$separatorComa = strpos($value, ',');
7171
$separatorDot = strpos($value, '.');

‎lib/internal/Magento/Framework/Locale/Test/Unit/FormatTest.php

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ protected function setUp()
4141
$this->scope = $this->getMockBuilder(\Magento\Framework\App\ScopeInterface::class)
4242
->setMethods(['getCurrentCurrency'])
4343
->getMockForAbstractClass();
44-
$this->scope->expects($this->once())
45-
->method('getCurrentCurrency')
46-
->willReturn($this->currency);
44+
4745
$this->scopeResolver = $this->getMockBuilder(\Magento\Framework\App\ScopeResolverInterface::class)
4846
->setMethods(['getScope'])
4947
->getMockForAbstractClass();
@@ -52,6 +50,8 @@ protected function setUp()
5250
->willReturn($this->scope);
5351
$this->localeResolver = $this->getMockBuilder(\Magento\Framework\Locale\ResolverInterface::class)
5452
->getMock();
53+
54+
/** @var \Magento\Directory\Model\CurrencyFactory|\PHPUnit_Framework_MockObject_MockObject $currencyFactory */
5555
$currencyFactory = $this->getMockBuilder(\Magento\Directory\Model\CurrencyFactory::class)
5656
->getMock();
5757

@@ -69,6 +69,10 @@ protected function setUp()
6969
*/
7070
public function testGetPriceFormat($localeCode, $expectedResult)
7171
{
72+
$this->scope->expects($this->once())
73+
->method('getCurrentCurrency')
74+
->willReturn($this->currency);
75+
7276
$result = $this->formatModel->getPriceFormat($localeCode);
7377
$intersection = array_intersect_assoc($result, $expectedResult);
7478
$this->assertCount(count($expectedResult), $intersection);
@@ -83,7 +87,36 @@ public function getPriceFormatDataProvider()
8387
['en_US', ['decimalSymbol' => '.', 'groupSymbol' => ',']],
8488
['de_DE', ['decimalSymbol' => ',', 'groupSymbol' => '.']],
8589
['de_CH', ['decimalSymbol' => '.', 'groupSymbol' => '\'']],
86-
['uk_UA', ['decimalSymbol' => ',', 'groupSymbol' => ' ']]
90+
['uk_UA', ['decimalSymbol' => ',', 'groupSymbol' => ' ']]
91+
];
92+
}
93+
94+
/**
95+
* @param float | null $expected
96+
* @param string|float|int $value
97+
* @dataProvider provideNumbers
98+
*/
99+
public function testGetNumber($value, $expected)
100+
{
101+
$this->assertEquals($expected, $this->formatModel->getNumber($value));
102+
}
103+
104+
/**
105+
* @return array
106+
*/
107+
public function provideNumbers(): array
108+
{
109+
return [
110+
[' 2345.4356,1234', 23454356.1234],
111+
['+23,3452.123', 233452.123],
112+
['12343', 12343],
113+
['-9456km', -9456],
114+
['0', 0],
115+
['2 054,10', 2054.1],
116+
['2046,45', 2046.45],
117+
['2 054.52', 2054.52],
118+
['2,46 GB', 2.46],
119+
['2,054.00', 2054],
87120
];
88121
}
89122
}

0 commit comments

Comments
 (0)