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

Correctly display percentage in cart #11656

merged 1 commit into from Dec 14, 2018


None yet
5 participants
Copy link

jolelievre commented Dec 7, 2018

Questions Answers
Branch? develop
Description? Update Product::getProductProperties so that the cart displays the appropriate discount
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #9892
How to test? Set different rules of price on a product depending on the quantity Add it to your cart In the cart view when updating the quantity the price AND the discount percentage are updated

This change is Reviewable

@jolelievre jolelievre added this to the milestone Dec 7, 2018

@@ -4742,6 +4742,8 @@ public static function getProductProperties($id_lang, $row, Context $context = n
if (isset($row['quantity_wanted'])) {
// 'quantity_wanted' may very well be zero even if set
$quantity = max((int) $row['minimal_quantity'], (int) $row['quantity_wanted']);
} elseif (isset($row['cart_quantity'])) {
$quantity = max((int) $row['minimal_quantity'], (int) $row['cart_quantity']);
} else {
$quantity = (int) $row['minimal_quantity'];

This comment has been minimized.


PierreRambaud Dec 7, 2018


Could it be simpler to have

$quantity = max(
  (int) $row['minimal_quantity'],
  isset($row['quantity_wanted']) ? (int) $row['quantity_wanted'] : 0,
  isset($row['cart_quantity']) ? (int) $row['cart_quantity'] : 0,

This comment has been minimized.


PierreRambaud Dec 7, 2018


Or is there an importance about order. quantity_wanted before cart_quantity ?

This comment has been minimized.


jolelievre Dec 7, 2018


I'm actually not sure, since cart_quantity was not managed I think leaving quantity_approved the priority reduces the risk of side effects bug
Although I don't think they are used in the same use cases it is safer

@marionf marionf self-assigned this Dec 7, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Dec 12, 2018

@Quetzacoalt91 Quetzacoalt91 merged commit c1ada4c into PrestaShop:develop Dec 14, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
continuous-integration/travis-ci/pr The Travis CI build passed

This comment has been minimized.

Copy link

Quetzacoalt91 commented Dec 14, 2018

Thank you @jolelievre

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