From 4a3e02fcefdda71c1f57e570542b75c4a5d7cee8 Mon Sep 17 00:00:00 2001 From: Anton Khristiansen Date: Tue, 28 Mar 2017 19:53:27 +0300 Subject: [PATCH] BB-7247: Incorrect value in totals in shopping list - fixing bug with incorrect calculating with precision > 0 --- .../Provider/ProductPriceProvider.php | 39 ++++++--- ...AbstractAjaxProductPriceControllerTest.php | 6 +- .../Provider/ProductPriceProviderTest.php | 18 ++-- .../LineItemNotPricedSubtotalProviderTest.php | 84 +++++++++++++++---- .../Resources/translations/messages.en.yml | 4 +- .../Frontend/AjaxLineItemControllerTest.php | 4 +- 6 files changed, 110 insertions(+), 45 deletions(-) diff --git a/src/Oro/Bundle/PricingBundle/Provider/ProductPriceProvider.php b/src/Oro/Bundle/PricingBundle/Provider/ProductPriceProvider.php index e348df51a74..ebca4d2c063 100644 --- a/src/Oro/Bundle/PricingBundle/Provider/ProductPriceProvider.php +++ b/src/Oro/Bundle/PricingBundle/Provider/ProductPriceProvider.php @@ -83,6 +83,7 @@ public function getMatchedPrices(array $productsPriceCriteria, BasePriceList $pr $code = $productPriceCriteria->getProductUnit()->getCode(); $quantity = $productPriceCriteria->getQuantity(); $currency = $productPriceCriteria->getCurrency(); + $precision = $productPriceCriteria->getProductUnit()->getDefaultPrecision(); $productPrices = array_filter( $prices, @@ -91,38 +92,52 @@ function (array $price) use ($id, $code, $currency) { } ); - $price = $this->matchPriceByQuantity($productPrices, $quantity); - - $result[$productPriceCriteria->getIdentifier()] = $price !== null ? Price::create($price, $currency) : null; + list($price, $matchedQuantity) = $this->matchPriceByQuantity($productPrices, $quantity); + if ($price !== null) { + $result[$productPriceCriteria->getIdentifier()] = Price::create( + $this->recalculatePricePerUnit($price, $matchedQuantity, $precision), + $currency + ); + } else { + $result[$productPriceCriteria->getIdentifier()] = null; + } } return $result; } + /** + * @param float $price + * @param float $quantityPerAmount + * @param int $precision + * @return float + */ + protected function recalculatePricePerUnit($price, $quantityPerAmount, $precision) + { + return $precision > 0 ? + $price / $quantityPerAmount : + $price; + } + /** * @param array $prices * @param float $expectedQuantity - * @return float + * @return array */ protected function matchPriceByQuantity(array $prices, $expectedQuantity) { $price = null; - + $matchedQuantity = null; foreach ($prices as $productPrice) { $quantity = (float)$productPrice['quantity']; - if ($expectedQuantity < $quantity) { - break; - } - if ($expectedQuantity >= $quantity) { $price = (float)$productPrice['value']; - } else { - break; + $matchedQuantity = $quantity; } } - return $price; + return [$price, $matchedQuantity]; } /** diff --git a/src/Oro/Bundle/PricingBundle/Tests/Functional/Controller/AbstractAjaxProductPriceControllerTest.php b/src/Oro/Bundle/PricingBundle/Tests/Functional/Controller/AbstractAjaxProductPriceControllerTest.php index 13cf56de684..324cf758b32 100644 --- a/src/Oro/Bundle/PricingBundle/Tests/Functional/Controller/AbstractAjaxProductPriceControllerTest.php +++ b/src/Oro/Bundle/PricingBundle/Tests/Functional/Controller/AbstractAjaxProductPriceControllerTest.php @@ -150,17 +150,17 @@ public function getMatchingPriceActionDataProvider() 'unit' => 'liter', 'currency' => 'USD', 'expected' => [ - 'value' => 12.2, + 'value' => 1.22, 'currency' => 'USD' ] ], [ 'product' => 'product-1', - 'qty' => 100, + 'qty' => 120, 'unit' => 'liter', 'currency' => 'USD', 'expected' => [ - 'value' => 12.2, + 'value' => 1.22, 'currency' => 'USD' ] ] diff --git a/src/Oro/Bundle/PricingBundle/Tests/Unit/Provider/ProductPriceProviderTest.php b/src/Oro/Bundle/PricingBundle/Tests/Unit/Provider/ProductPriceProviderTest.php index d9e6ab5b30f..5b02d6932d0 100644 --- a/src/Oro/Bundle/PricingBundle/Tests/Unit/Provider/ProductPriceProviderTest.php +++ b/src/Oro/Bundle/PricingBundle/Tests/Unit/Provider/ProductPriceProviderTest.php @@ -207,19 +207,18 @@ public function testGetMatchedPrices( public function getMatchedPricesDataProvider() { $currency = 'USD'; - $prodUnitQty1 = $this->getProductPriceCriteria(1, $currency); - $prodUnitQty105 = $this->getProductPriceCriteria(10.5, $currency); - $prodUnitQty50 = $this->getProductPriceCriteria(50, $currency); - - $repositoryData = $this->getRepositoryData($prodUnitQty50); + $prodUnitQty1 = $this->getProductPriceCriteria(1, $currency, 0); + $prodUnitQty105 = $this->getProductPriceCriteria(10.5, $currency, 0); + $prodUnitQty50 = $this->getProductPriceCriteria(50, $currency, 3); return [ [ - 'productPriceCriteria' => [$prodUnitQty1, $prodUnitQty105], - 'repositoryData' => $repositoryData, + 'productPriceCriteria' => [$prodUnitQty1, $prodUnitQty105, $prodUnitQty50], + 'repositoryData' => $this->getRepositoryData($prodUnitQty50), 'expectedData' => [ $prodUnitQty1->getIdentifier() => null, $prodUnitQty105->getIdentifier() => Price::create(15, $currency), + $prodUnitQty50->getIdentifier() => Price::create(6, $currency), ] ], ]; @@ -230,12 +229,13 @@ public function getMatchedPricesDataProvider() * @param string $currency * @return ProductPriceCriteria */ - protected function getProductPriceCriteria($quantity, $currency) + protected function getProductPriceCriteria($quantity, $currency, $precision) { /** @var Product $product */ $product = $this->getEntity('Oro\Bundle\ProductBundle\Entity\Product', 42); $productUnit = new ProductUnit(); + $productUnit->setDefaultPrecision($precision); $productUnit->setCode('kg'); return new ProductPriceCriteria($product, $productUnit, $quantity, $currency); @@ -269,7 +269,7 @@ protected function getRepositoryData(ProductPriceCriteria $productPriceCriteria) 'id' => $product->getId(), 'code' => $productUnit->getCode(), 'quantity' => 20, - 'value' => 300, + 'value' => 120, 'currency' => 'USD' ], [ diff --git a/src/Oro/Bundle/PricingBundle/Tests/Unit/SubtotalProcessor/Provider/LineItemNotPricedSubtotalProviderTest.php b/src/Oro/Bundle/PricingBundle/Tests/Unit/SubtotalProcessor/Provider/LineItemNotPricedSubtotalProviderTest.php index 690693fd6f7..56ecc13203c 100644 --- a/src/Oro/Bundle/PricingBundle/Tests/Unit/SubtotalProcessor/Provider/LineItemNotPricedSubtotalProviderTest.php +++ b/src/Oro/Bundle/PricingBundle/Tests/Unit/SubtotalProcessor/Provider/LineItemNotPricedSubtotalProviderTest.php @@ -68,7 +68,9 @@ function ($value) { } ) ); - $this->productPriceProvider = $this->getMockBuilder('Oro\Bundle\PricingBundle\Provider\ProductPriceProvider') + + $this->productPriceProvider = $this + ->getMockBuilder('Oro\Bundle\PricingBundle\Provider\ProductPriceProvider') ->disableOriginalConstructor() ->getMock(); @@ -76,7 +78,8 @@ function ($value) { ->disableOriginalConstructor() ->getMock(); - $this->priceListTreeHandler = $this->getMockBuilder('Oro\Bundle\PricingBundle\Model\PriceListTreeHandler') + $this->priceListTreeHandler = $this + ->getMockBuilder('Oro\Bundle\PricingBundle\Model\PriceListTreeHandler') ->disableOriginalConstructor() ->getMock(); @@ -95,11 +98,26 @@ protected function tearDown() unset($this->translator, $this->provider); } - public function testGetSubtotal() - { - $value = 142.0; + /** + * @dataProvider getPriceDataProvider + * @param float $value + * @param string $identifier + * @param float $defaultQuantity + * @param float $quantity + * @param float $expectedValue + * @param int $precision + * @param string $code + */ + public function testGetSubtotal( + $value, + $identifier, + $defaultQuantity, + $quantity, + $expectedValue, + $precision, + $code + ) { $currency = 'USD'; - $identifier = '1-code-2-USD'; $this->translator->expects($this->once()) ->method('trans') @@ -107,15 +125,15 @@ public function testGetSubtotal() ->willReturn('test'); $product = $this->prepareProduct(); - $productUnit = $this->prepareProductUnit(); + $productUnit = $this->prepareProductUnit($code, $precision); $this->prepareEntityManager($product, $productUnit); - $this->preparePrice($value, $identifier); + $this->preparePrice($value, $identifier, $defaultQuantity); $entity = new EntityNotPricedStub(); $lineItem = new LineItemNotPricedStub(); $lineItem->setProduct($product); $lineItem->setProductUnit($productUnit); - $lineItem->setQuantity(2); + $lineItem->setQuantity($quantity); $entity->addLineItem($lineItem); @@ -135,7 +153,7 @@ public function testGetSubtotal() $this->assertEquals('test', $subtotal->getLabel()); $this->assertEquals($entity->getCurrency(), $subtotal->getCurrency()); $this->assertInternalType('float', $subtotal->getAmount()); - $this->assertEquals($value * 2, $subtotal->getAmount()); + $this->assertEquals($expectedValue, (float)$subtotal->getAmount()); $this->assertTrue($subtotal->isVisible()); } @@ -178,7 +196,7 @@ public function testIsNotSupported() /** * @return ProductUnit|\PHPUnit_Framework_MockObject_MockObject */ - protected function prepareProductUnit() + protected function prepareProductUnit($code, $precision) { /** @var ProductUnit|\PHPUnit_Framework_MockObject_MockObject $productUnit */ $productUnit = $this->getMockBuilder('Oro\Bundle\ProductBundle\Entity\ProductUnit') @@ -186,7 +204,10 @@ protected function prepareProductUnit() ->getMock(); $productUnit->expects($this->any()) ->method('getCode') - ->willReturn('code'); + ->willReturn($code); + $productUnit->expects($this->any()) + ->method('getDefaultPrecision') + ->willReturn($precision); return $productUnit; } @@ -208,7 +229,7 @@ protected function prepareProduct() } /** - * @param Product$product + * @param Product $product * @param ProductUnit $productUnit */ protected function prepareEntityManager(Product $product, ProductUnit $productUnit) @@ -229,10 +250,11 @@ protected function prepareEntityManager(Product $product, ProductUnit $productUn } /** - * @param float $value - * @param string $identifier + * @param $value + * @param $identifier + * @param $defaultQuantity */ - protected function preparePrice($value, $identifier) + protected function preparePrice($value, $identifier, $defaultQuantity) { /** @var Price|\PHPUnit_Framework_MockObject_MockObject $price */ $price = $this->getMockBuilder('Oro\Bundle\CurrencyBundle\Entity\Price') @@ -240,9 +262,37 @@ protected function preparePrice($value, $identifier) ->getMock(); $price->expects($this->any()) ->method('getValue') - ->willReturn($value); + ->willReturn($value / $defaultQuantity); + $this->productPriceProvider->expects($this->any()) ->method('getMatchedPrices') ->willReturn([$identifier => $price]); } + + /** + * @return array + */ + public function getPriceDataProvider() + { + return [ + 'kilogram' => [ + 'value' => 25.0, + 'identifier' => '1-kg-3-USD', + 'defaultQuantity' => 0.5, + 'quantity' => 3, + 'expectedValue' => 150, + 'precision' => 3, + 'code' => 'kg' + ], + 'item' => [ + 'value' => 142.0, + 'identifier' => '1-item-2-USD', + 'defaultQuantity' => 1, + 'quantity' => 2, + 'expectedValue' => 284, + 'precision' => 0, + 'code' => 'item' + ], + ]; + } } diff --git a/src/Oro/Bundle/ProductBundle/Resources/translations/messages.en.yml b/src/Oro/Bundle/ProductBundle/Resources/translations/messages.en.yml index f8fae98b256..6bb321ed692 100644 --- a/src/Oro/Bundle/ProductBundle/Resources/translations/messages.en.yml +++ b/src/Oro/Bundle/ProductBundle/Resources/translations/messages.en.yml @@ -311,8 +311,8 @@ oro: short: kg short_plural: kgs value: - full: '{0} none|{1} %count% kilogram|]1,Inf] %count% kilograms' - short: '{0} none|{1} %count% kg|]1,Inf] %count% kg' + full: '{0} %count% kilogram|{1} %count% kilogram|]1,Inf] %count% kilograms' + short: '{0}%count% kg|{1} %count% kg|]1,Inf] %count% kg' product_unit.piece: label: diff --git a/src/Oro/Bundle/ShoppingListBundle/Tests/Functional/Controller/Frontend/AjaxLineItemControllerTest.php b/src/Oro/Bundle/ShoppingListBundle/Tests/Functional/Controller/Frontend/AjaxLineItemControllerTest.php index 8735848f91d..07615a97e2c 100644 --- a/src/Oro/Bundle/ShoppingListBundle/Tests/Functional/Controller/Frontend/AjaxLineItemControllerTest.php +++ b/src/Oro/Bundle/ShoppingListBundle/Tests/Functional/Controller/Frontend/AjaxLineItemControllerTest.php @@ -117,8 +117,8 @@ public function addProductFromViewDataProvider() [ 'product' => LoadProductData::PRODUCT_2, 'unit' => 'product_unit.liter', - 'quantity' => 14, - 'expectedSubtotals' => ['EUR' => 1573, 'USD' => 1611.8], + 'quantity' => 15, + 'expectedSubtotals' => ['EUR' => 1573, 'USD' => 1456.25], ], [ 'product' => LoadProductData::PRODUCT_1,