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

Unify algorithm for calculating average tax rate for products in a cart #12014

Merged
merged 2 commits into from Jan 9, 2019

Conversation

Projects
None yet
4 participants
@eternoendless
Copy link
Member

eternoendless commented Jan 3, 2019

Questions Answers
Branch? develop
Description? There are currently two methods for calculating the average tax rate in a cart, Cart::getTaxesAverageUsed() and $cart->getAverageProductsTaxRate(). They used a different algorithm to calculate it, but it more or less ended up in "sum of all products without tax / sum of all taxes applied". This PR unifies the behavior and depreciates the static method.
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? n/a
How to test? Add many products to a cart, ideally with different tax rates, and with some reductions. In the order detail (FO), the reduction amount should be correct (or at least no different from before :trollface:)

This change is Reviewable

@eternoendless eternoendless added this to the 1.7.6.0 milestone Jan 3, 2019

@marionf marionf self-assigned this Jan 7, 2019

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Jan 8, 2019

@eternoendless

Product price tax excl: 10€
VAT 20%
Product price tax incl: 12€
Cart rule: -2,53€ tax excl (= -3,04 tax incl)

Before this PR, it's ok:

capture d ecran_882

With this PR, it's not ok:

capture d ecran_883

@eternoendless

This comment has been minimized.

Copy link
Member Author

eternoendless commented Jan 8, 2019

Fixed!

screenshot 2019-01-08 at 16 46 45

It turns out that getTaxesAverageUsed() returned a tax rate (eg '20' for 20%) while the other method returns a tax multiplier (eg. 0.2 for 20%).

@marionf marionf added QA ✔️ and removed waiting for QA labels Jan 8, 2019

@marionf marionf removed their assignment Jan 8, 2019

@Quetzacoalt91 Quetzacoalt91 merged commit a22ac95 into PrestaShop:develop Jan 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Jan 9, 2019

Thank you @eternoendless

@eternoendless eternoendless deleted the eternoendless:avg-tax-rate branch Jan 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment