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

add total_shipping_tax_excl , incl in email data #11397

Merged
merged 2 commits into from Nov 20, 2018

Conversation

Projects
None yet
6 participants
@sitte
Contributor

sitte commented Nov 14, 2018

Is your feature request related to a problem? Please describe.
No way to use total shipping tax excl in order_conf email.

Describe the solution you'd like
In Shop where everything is tax excluded its inconsistency

Describe alternatives you've considered
overriding is not easy/imposible and total_shipping_tax_excl and total_shipping_tax_incl are right in class

Questions Answers
Branch? develop
Description? Add total_shipping_tax_excl, incl in email data.
Type? bug fix
Category? CO
BC breaks? Does it break backward compatibility? no
Deprecations? Does it deprecate an existing feature? no
Fixed ticket? #11396
How to test? Use {total_shipping_tax_excl} in order_conf email.

This change is Reviewable

add total_shipping_tax_excl , incl in email data
>Is your feature request related to a problem? Please describe.
No way to use total shipping tax excl in order_conf email.

>Describe the solution you'd like
In Shop where everything is tax excluded its inconsistency

>Describe alternatives you've considered
overriding is not easy/imposible and total_shipping_tax_excl and total_shipping_tax_incl are right in class
@prestonBot

This comment has been minimized.

Collaborator

prestonBot commented Nov 14, 2018

Hello @sitte!

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

@@ -817,6 +817,8 @@ public function validateOrder(
'{total_products}' => Tools::displayPrice(Product::getTaxCalculationMethod() == PS_TAX_EXC ? $order->total_products : $order->total_products_wt, $this->context->currency, false),
'{total_discounts}' => Tools::displayPrice($order->total_discounts, $this->context->currency, false),
'{total_shipping}' => Tools::displayPrice($order->total_shipping, $this->context->currency, false),
'{total_shipping_tax_excl}' => Tools::displayPrice($order->total_shipping_tax_excl, $this->context->currency, false),
'{total_shipping_tax_incl}' => Tools::displayPrice($order->total_shipping_tax_incl, $this->context->currency, false),

This comment has been minimized.

@MathiasReker

MathiasReker Nov 15, 2018

Contributor

Use space instead of tabs. Remove blank space in the end.

This comment has been minimized.

@sitte

sitte Nov 15, 2018

Contributor

Sorry, I fixed spaces on begining and end of lines

@MathiasReker

This comment has been minimized.

Contributor

MathiasReker commented Nov 15, 2018

Please update the table:

Branch? 	develop
Description? 	Add total_shipping_tax_excl, incl in email data.
Type? 	Bug fix
Category? 	CO
BC breaks? 	Does it break backward compatibility? no
Deprecations? 	Does it deprecate an existing feature? no
Fixed ticket? 	https://github.com/PrestaShop/PrestaShop/issues/11396
How to test? 	Use total shipping tax excl in order_conf email.

@prestonBot prestonBot added the develop label Nov 15, 2018

@sitte

This comment has been minimized.

Contributor

sitte commented Nov 15, 2018

I filled "update table" as You sugested

@marionf marionf self-assigned this Nov 20, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Nov 20, 2018

@mickaelandrieu

This comment has been minimized.

Contributor

mickaelandrieu commented Nov 20, 2018

LGTM, thanks everyone for the contributions, the reviews and the QA :)

@mickaelandrieu mickaelandrieu added this to the 1.7.6.0 milestone Nov 20, 2018

@mickaelandrieu mickaelandrieu merged commit 7ba06c2 into PrestaShop:develop Nov 20, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. 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