New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Currency exchange fixes on product page #8283

Merged
merged 5 commits into from Sep 18, 2017

Conversation

Projects
None yet
5 participants
@tomlev
Member

tomlev commented Aug 30, 2017

Questions Answers
Branch? 1.7.2.x
Description? Fixes on FO product page : fix the displayed discount whith specific prices on another currency than default one, fix the displayed ecotax on another currency than default one
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-3275 http://forge.prestashop.com/browse/BOOM-3739
How to test? Discount calculation Add another currency than default one, with an exchange rate different from 1. In BO, on a product, set specific discount price (fixed one) with minimal quantity. Return on FO product page, switch currency from default one to the second one. Discount values are expected to be correctly converted. Ecotax exchange Now activate ecotax and set a not-null value on the product. Check on FO product page that the ecotax is correctly converted when changing currency.

Important guidelines

@prestonBot

This comment has been minimized.

Show comment
Hide comment
@prestonBot

prestonBot Aug 30, 2017

Collaborator

Hello tomlev!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

Collaborator

prestonBot commented Aug 30, 2017

Hello tomlev!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@@ -826,25 +836,30 @@ protected function formatQuantityDiscounts($specific_prices, $price, $tax_rate,
if ($row['price'] >= 0) {
// The price may be directly set
$cur_price = (!$row['reduction_tax'] ? $row['price'] : $row['price'] * (1 + $tax_rate / 100)) + (float) $ecotax_amount;
/** @var float $currentPriceDefaultCurrency current price with taxes in default currency */
$currentPriceDefaultCurrency = (!$row['reduction_tax'] ? $row['price'] : $row['price'] * (1 + $tax_rate / 100)) + (float) $ecotax_amount;

This comment has been minimized.

@eternoendless

eternoendless Sep 4, 2017

Member

Try using PrestaShop\Decimal\Number, we should start using it everywhere, especially when in places where we perform currency divisions.

@eternoendless

eternoendless Sep 4, 2017

Member

Try using PrestaShop\Decimal\Number, we should start using it everywhere, especially when in places where we perform currency divisions.

This comment has been minimized.

@tomlev

tomlev Sep 5, 2017

Member

I think this is out of current scope, here I simply renamed the var with a more meaningful name

@tomlev

tomlev Sep 5, 2017

Member

I think this is out of current scope, here I simply renamed the var with a more meaningful name

This comment has been minimized.

@eternoendless

eternoendless Sep 5, 2017

Member

After discussion, we'll do that later

@eternoendless

eternoendless Sep 5, 2017

Member

After discussion, we'll do that later

Show outdated Hide outdated tests/Unit/controller/FrontController/ProductControllerTest.php
Show outdated Hide outdated tests/Unit/controller/FrontController/ProductControllerTest.php

@PrestaShop PrestaShop deleted a comment from codacy-bot Sep 5, 2017

@PrestaShop PrestaShop deleted a comment from codacy-bot Sep 7, 2017

@vincentbz vincentbz added this to the 1.7.2.3 milestone Sep 11, 2017

comments have been adressed & tests pass ;)

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless
Member

eternoendless commented Sep 18, 2017

Thank you @tomlev

@eternoendless eternoendless merged commit 0c255f9 into PrestaShop:1.7.2.x Sep 18, 2017

2 checks passed

codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment