Skip to content

Commit 4a3e02f

Browse files
author
Anton Khristiansen
authored
BB-7247: Incorrect value in totals in shopping list
- fixing bug with incorrect calculating with precision > 0
1 parent 6bee774 commit 4a3e02f

File tree

6 files changed

+110
-45
lines changed

6 files changed

+110
-45
lines changed

Diff for: ‎src/Oro/Bundle/PricingBundle/Provider/ProductPriceProvider.php

+27-12
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ public function getMatchedPrices(array $productsPriceCriteria, BasePriceList $pr
8383
$code = $productPriceCriteria->getProductUnit()->getCode();
8484
$quantity = $productPriceCriteria->getQuantity();
8585
$currency = $productPriceCriteria->getCurrency();
86+
$precision = $productPriceCriteria->getProductUnit()->getDefaultPrecision();
8687

8788
$productPrices = array_filter(
8889
$prices,
@@ -91,38 +92,52 @@ function (array $price) use ($id, $code, $currency) {
9192
}
9293
);
9394

94-
$price = $this->matchPriceByQuantity($productPrices, $quantity);
95-
96-
$result[$productPriceCriteria->getIdentifier()] = $price !== null ? Price::create($price, $currency) : null;
95+
list($price, $matchedQuantity) = $this->matchPriceByQuantity($productPrices, $quantity);
96+
if ($price !== null) {
97+
$result[$productPriceCriteria->getIdentifier()] = Price::create(
98+
$this->recalculatePricePerUnit($price, $matchedQuantity, $precision),
99+
$currency
100+
);
101+
} else {
102+
$result[$productPriceCriteria->getIdentifier()] = null;
103+
}
97104
}
98105

99106
return $result;
100107
}
101108

109+
/**
110+
* @param float $price
111+
* @param float $quantityPerAmount
112+
* @param int $precision
113+
* @return float
114+
*/
115+
protected function recalculatePricePerUnit($price, $quantityPerAmount, $precision)
116+
{
117+
return $precision > 0 ?
118+
$price / $quantityPerAmount :
119+
$price;
120+
}
121+
102122
/**
103123
* @param array $prices
104124
* @param float $expectedQuantity
105-
* @return float
125+
* @return array
106126
*/
107127
protected function matchPriceByQuantity(array $prices, $expectedQuantity)
108128
{
109129
$price = null;
110-
130+
$matchedQuantity = null;
111131
foreach ($prices as $productPrice) {
112132
$quantity = (float)$productPrice['quantity'];
113133

114-
if ($expectedQuantity < $quantity) {
115-
break;
116-
}
117-
118134
if ($expectedQuantity >= $quantity) {
119135
$price = (float)$productPrice['value'];
120-
} else {
121-
break;
136+
$matchedQuantity = $quantity;
122137
}
123138
}
124139

125-
return $price;
140+
return [$price, $matchedQuantity];
126141
}
127142

128143
/**

Diff for: ‎src/Oro/Bundle/PricingBundle/Tests/Functional/Controller/AbstractAjaxProductPriceControllerTest.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,17 @@ public function getMatchingPriceActionDataProvider()
150150
'unit' => 'liter',
151151
'currency' => 'USD',
152152
'expected' => [
153-
'value' => 12.2,
153+
'value' => 1.22,
154154
'currency' => 'USD'
155155
]
156156
],
157157
[
158158
'product' => 'product-1',
159-
'qty' => 100,
159+
'qty' => 120,
160160
'unit' => 'liter',
161161
'currency' => 'USD',
162162
'expected' => [
163-
'value' => 12.2,
163+
'value' => 1.22,
164164
'currency' => 'USD'
165165
]
166166
]

Diff for: ‎src/Oro/Bundle/PricingBundle/Tests/Unit/Provider/ProductPriceProviderTest.php

+9-9
Original file line numberDiff line numberDiff line change
@@ -207,19 +207,18 @@ public function testGetMatchedPrices(
207207
public function getMatchedPricesDataProvider()
208208
{
209209
$currency = 'USD';
210-
$prodUnitQty1 = $this->getProductPriceCriteria(1, $currency);
211-
$prodUnitQty105 = $this->getProductPriceCriteria(10.5, $currency);
212-
$prodUnitQty50 = $this->getProductPriceCriteria(50, $currency);
213-
214-
$repositoryData = $this->getRepositoryData($prodUnitQty50);
210+
$prodUnitQty1 = $this->getProductPriceCriteria(1, $currency, 0);
211+
$prodUnitQty105 = $this->getProductPriceCriteria(10.5, $currency, 0);
212+
$prodUnitQty50 = $this->getProductPriceCriteria(50, $currency, 3);
215213

216214
return [
217215
[
218-
'productPriceCriteria' => [$prodUnitQty1, $prodUnitQty105],
219-
'repositoryData' => $repositoryData,
216+
'productPriceCriteria' => [$prodUnitQty1, $prodUnitQty105, $prodUnitQty50],
217+
'repositoryData' => $this->getRepositoryData($prodUnitQty50),
220218
'expectedData' => [
221219
$prodUnitQty1->getIdentifier() => null,
222220
$prodUnitQty105->getIdentifier() => Price::create(15, $currency),
221+
$prodUnitQty50->getIdentifier() => Price::create(6, $currency),
223222
]
224223
],
225224
];
@@ -230,12 +229,13 @@ public function getMatchedPricesDataProvider()
230229
* @param string $currency
231230
* @return ProductPriceCriteria
232231
*/
233-
protected function getProductPriceCriteria($quantity, $currency)
232+
protected function getProductPriceCriteria($quantity, $currency, $precision)
234233
{
235234
/** @var Product $product */
236235
$product = $this->getEntity('Oro\Bundle\ProductBundle\Entity\Product', 42);
237236

238237
$productUnit = new ProductUnit();
238+
$productUnit->setDefaultPrecision($precision);
239239
$productUnit->setCode('kg');
240240

241241
return new ProductPriceCriteria($product, $productUnit, $quantity, $currency);
@@ -269,7 +269,7 @@ protected function getRepositoryData(ProductPriceCriteria $productPriceCriteria)
269269
'id' => $product->getId(),
270270
'code' => $productUnit->getCode(),
271271
'quantity' => 20,
272-
'value' => 300,
272+
'value' => 120,
273273
'currency' => 'USD'
274274
],
275275
[

Diff for: ‎src/Oro/Bundle/PricingBundle/Tests/Unit/SubtotalProcessor/Provider/LineItemNotPricedSubtotalProviderTest.php

+67-17
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,18 @@ function ($value) {
6868
}
6969
)
7070
);
71-
$this->productPriceProvider = $this->getMockBuilder('Oro\Bundle\PricingBundle\Provider\ProductPriceProvider')
71+
72+
$this->productPriceProvider = $this
73+
->getMockBuilder('Oro\Bundle\PricingBundle\Provider\ProductPriceProvider')
7274
->disableOriginalConstructor()
7375
->getMock();
7476

7577
$this->doctrineHelper = $this->getMockBuilder('Oro\Bundle\EntityBundle\ORM\DoctrineHelper')
7678
->disableOriginalConstructor()
7779
->getMock();
7880

79-
$this->priceListTreeHandler = $this->getMockBuilder('Oro\Bundle\PricingBundle\Model\PriceListTreeHandler')
81+
$this->priceListTreeHandler = $this
82+
->getMockBuilder('Oro\Bundle\PricingBundle\Model\PriceListTreeHandler')
8083
->disableOriginalConstructor()
8184
->getMock();
8285

@@ -95,27 +98,42 @@ protected function tearDown()
9598
unset($this->translator, $this->provider);
9699
}
97100

98-
public function testGetSubtotal()
99-
{
100-
$value = 142.0;
101+
/**
102+
* @dataProvider getPriceDataProvider
103+
* @param float $value
104+
* @param string $identifier
105+
* @param float $defaultQuantity
106+
* @param float $quantity
107+
* @param float $expectedValue
108+
* @param int $precision
109+
* @param string $code
110+
*/
111+
public function testGetSubtotal(
112+
$value,
113+
$identifier,
114+
$defaultQuantity,
115+
$quantity,
116+
$expectedValue,
117+
$precision,
118+
$code
119+
) {
101120
$currency = 'USD';
102-
$identifier = '1-code-2-USD';
103121

104122
$this->translator->expects($this->once())
105123
->method('trans')
106124
->with(LineItemNotPricedSubtotalProvider::NAME . '.label')
107125
->willReturn('test');
108126

109127
$product = $this->prepareProduct();
110-
$productUnit = $this->prepareProductUnit();
128+
$productUnit = $this->prepareProductUnit($code, $precision);
111129
$this->prepareEntityManager($product, $productUnit);
112-
$this->preparePrice($value, $identifier);
130+
$this->preparePrice($value, $identifier, $defaultQuantity);
113131

114132
$entity = new EntityNotPricedStub();
115133
$lineItem = new LineItemNotPricedStub();
116134
$lineItem->setProduct($product);
117135
$lineItem->setProductUnit($productUnit);
118-
$lineItem->setQuantity(2);
136+
$lineItem->setQuantity($quantity);
119137

120138
$entity->addLineItem($lineItem);
121139

@@ -135,7 +153,7 @@ public function testGetSubtotal()
135153
$this->assertEquals('test', $subtotal->getLabel());
136154
$this->assertEquals($entity->getCurrency(), $subtotal->getCurrency());
137155
$this->assertInternalType('float', $subtotal->getAmount());
138-
$this->assertEquals($value * 2, $subtotal->getAmount());
156+
$this->assertEquals($expectedValue, (float)$subtotal->getAmount());
139157
$this->assertTrue($subtotal->isVisible());
140158
}
141159

@@ -178,15 +196,18 @@ public function testIsNotSupported()
178196
/**
179197
* @return ProductUnit|\PHPUnit_Framework_MockObject_MockObject
180198
*/
181-
protected function prepareProductUnit()
199+
protected function prepareProductUnit($code, $precision)
182200
{
183201
/** @var ProductUnit|\PHPUnit_Framework_MockObject_MockObject $productUnit */
184202
$productUnit = $this->getMockBuilder('Oro\Bundle\ProductBundle\Entity\ProductUnit')
185203
->disableOriginalConstructor()
186204
->getMock();
187205
$productUnit->expects($this->any())
188206
->method('getCode')
189-
->willReturn('code');
207+
->willReturn($code);
208+
$productUnit->expects($this->any())
209+
->method('getDefaultPrecision')
210+
->willReturn($precision);
190211

191212
return $productUnit;
192213
}
@@ -208,7 +229,7 @@ protected function prepareProduct()
208229
}
209230

210231
/**
211-
* @param Product$product
232+
* @param Product $product
212233
* @param ProductUnit $productUnit
213234
*/
214235
protected function prepareEntityManager(Product $product, ProductUnit $productUnit)
@@ -229,20 +250,49 @@ protected function prepareEntityManager(Product $product, ProductUnit $productUn
229250
}
230251

231252
/**
232-
* @param float $value
233-
* @param string $identifier
253+
* @param $value
254+
* @param $identifier
255+
* @param $defaultQuantity
234256
*/
235-
protected function preparePrice($value, $identifier)
257+
protected function preparePrice($value, $identifier, $defaultQuantity)
236258
{
237259
/** @var Price|\PHPUnit_Framework_MockObject_MockObject $price */
238260
$price = $this->getMockBuilder('Oro\Bundle\CurrencyBundle\Entity\Price')
239261
->disableOriginalConstructor()
240262
->getMock();
241263
$price->expects($this->any())
242264
->method('getValue')
243-
->willReturn($value);
265+
->willReturn($value / $defaultQuantity);
266+
244267
$this->productPriceProvider->expects($this->any())
245268
->method('getMatchedPrices')
246269
->willReturn([$identifier => $price]);
247270
}
271+
272+
/**
273+
* @return array
274+
*/
275+
public function getPriceDataProvider()
276+
{
277+
return [
278+
'kilogram' => [
279+
'value' => 25.0,
280+
'identifier' => '1-kg-3-USD',
281+
'defaultQuantity' => 0.5,
282+
'quantity' => 3,
283+
'expectedValue' => 150,
284+
'precision' => 3,
285+
'code' => 'kg'
286+
],
287+
'item' => [
288+
'value' => 142.0,
289+
'identifier' => '1-item-2-USD',
290+
'defaultQuantity' => 1,
291+
'quantity' => 2,
292+
'expectedValue' => 284,
293+
'precision' => 0,
294+
'code' => 'item'
295+
],
296+
];
297+
}
248298
}

Diff for: ‎src/Oro/Bundle/ProductBundle/Resources/translations/messages.en.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,8 @@ oro:
311311
short: kg
312312
short_plural: kgs
313313
value:
314-
full: '{0} none|{1} %count% kilogram|]1,Inf] %count% kilograms'
315-
short: '{0} none|{1} %count% kg|]1,Inf] %count% kg'
314+
full: '{0} %count% kilogram|{1} %count% kilogram|]1,Inf] %count% kilograms'
315+
short: '{0}%count% kg|{1} %count% kg|]1,Inf] %count% kg'
316316

317317
product_unit.piece:
318318
label:

Diff for: ‎src/Oro/Bundle/ShoppingListBundle/Tests/Functional/Controller/Frontend/AjaxLineItemControllerTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ public function addProductFromViewDataProvider()
117117
[
118118
'product' => LoadProductData::PRODUCT_2,
119119
'unit' => 'product_unit.liter',
120-
'quantity' => 14,
121-
'expectedSubtotals' => ['EUR' => 1573, 'USD' => 1611.8],
120+
'quantity' => 15,
121+
'expectedSubtotals' => ['EUR' => 1573, 'USD' => 1456.25],
122122
],
123123
[
124124
'product' => LoadProductData::PRODUCT_1,

0 commit comments

Comments
 (0)