Prestashop: Cart presenter totals wrong when shouldSeparateGifts is true

[Hlavtox]

Describe the bug
In order to display cart gifts correctly, CartPresenter->present must be called with shouldSeparateGifts as true. This was proposed in this PR: #8706.

To test, change bold to true:
classes/controller/FrontController.php - Line 490 - 'cart' => $this->cart_presenter->present($this->context->cart, true),

Now, this works until you put another pieces of the _product you got as a gift_ to cart.

1 - WRONG TOTALS
From then, cart totals are wrong, because the totals are calculated from wrong quantity variable.

Example
Product ABC with price 100.
You add 5 to cart manually.
You get 1 free because you got over XXX amount or applied a voucher.
Total products should be 600, but it is 1200.

2 - WRONG DISCOUNTS
Another bug - product gets discounted twice.

Additional information
PrestaShop version: 1.7.6.0.

[khouloudbelguith]

Hi @Hlavtox,

Thanks for your report.
Isn't the same issue reported here: https://github.com/PrestaShop/PrestaShop/issues/11722?

Thanks!

[Hlavtox]

Hi @khouloudbelguith,

I think its not. They do not have gift separation turned on. But this needs to be done somewhere in the future. I think it's maybe a right time to have gifts working after many years?

Try my suggestions and you will see. Product total is definitely a nonsense and is an easy fix.

[khouloudbelguith]

@Hlavtox, thanks for your feedback.

have gift separation turned on

What is the exact option enabled in this case? a cart rule option?

Thanks!

[Hlavtox]

@khouloudbelguith

1. Make a product ABC with price 100 including tax.
2. Make a cart rule, unlimited, without code, that gives that product for free if you order over 150.
3. Add like 5 pieces of that product to cart to apply it.
4. You will see the product and gift separated correctly, but totals on the right will be wrong.
5. Apply my fix in point 1, subtotal will be correct. (Even when gift separation will be turned off again).

Continuing - part 2:

1. Apply my fix in point 2.
2. Discounts will be correct, but found out later that it breaks other calculations, go on:
3. Make a cart rule, that discounts all orders by 50% if order is over 150.
4. In cart, and checkout, totals will be still correct, but after submitting order, only half of the gift price will be discounted.

[khouloudbelguith]

@Hlavtox, sorry but this bug is reproduced only if we apply this fix.
with PS1.7.6.1, we don't have gift separation

Is it right?

Thanks!

[Hlavtox]

@khouloudbelguith Yes, but if gifts should be working in this century, I expect this to be enabled in some version in the near future.

This worked correctly in 1.6 for years and in 1.7 it's not.

Can you ping the dev team? I saw something about remake of the calculation code.

[khouloudbelguith]

@Hlavtox,

if gifts should be working in this century, I expect this to be enabled in some version in the near future.

So, your issue is not reproduced and the old PR is not merged, it is just closed.

This worked correctly in 1.6 for years and in 1.7 it's not.

This issue: https://github.com/PrestaShop/PrestaShop/issues/9757 is added to our bug roadmap, so that it’s fixed with a PR reviewed by our devs & tested by QA team.

Can you ping the dev team? I saw something about remake of the calculation code.

If this issue is not reproduced, we can close this ticket.
About this issue: https://github.com/PrestaShop/PrestaShop/issues/9757
If you have already fixed it on your end or if you think you can do it, please do send us a pull request, so our devs will review your code.

Thanks!

[Hlavtox]

@khouloudbelguith

My issue is reproduced! The function is not behaving correctly. I can use it in my module and it will not work correctly.

When its not used in front end, it doesnt mean its not there.

Relating #9757 - Thats why I opened this thread. It could fix the gift issue finally. But its not perfect and I wanted to discuss it.

[khouloudbelguith]

@Hlavtox,

My issue is reproduced! The function is not behaving correctly. I can use it in my module and it will not work correctly.

but to reproduce your issue, we need to apply this fix first: https://github.com/PrestaShop/PrestaShop/pull/8706, which is incorrect, because this fix is not valid.

Thanks to provide me steps to reproduce the issue without applying the PR fix.

Thanks!

[Hlavtox]

@khouloudbelguith Do we need to do this discussion?

Okay then:

1. Make a module that will show contents of cart.
2. Call CartPresenter->present($cart, true);
3. Echo wrong totals.

[khouloudbelguith]

@Hlavtox, thanks for your feedback.
I just created an override of the FrontController.php class

<?php

class FrontController extends FrontControllerCore
{
    protected function assignGeneralPurposeVariables()
    {
        $templateVars = array(
            'cart' => $this->cart_presenter->present($this->context->cart,true),
            'currency' => $this->getTemplateVarCurrency(),
            'customer' => $this->getTemplateVarCustomer(),
            'language' => $this->objectPresenter->present($this->context->language),
            'page' => $this->getTemplateVarPage(),
            'shop' => $this->getTemplateVarShop(),
            'urls' => $this->getTemplateVarUrls(),
            'configuration' => $this->getTemplateVarConfiguration(),
            'field_required' => $this->context->customer->validateFieldsRequiredDatabase(),
            'breadcrumb' => $this->getBreadcrumb(),
            'link' => $this->context->link,
            'time' => time(),
            'static_token' => Tools::getToken(false),
            'token' => Tools::getToken(),
        );

        $modulesVariables = Hook::exec('actionFrontControllerSetVariables', [], null, true);

        if (is_array($modulesVariables)) {
            foreach ($modulesVariables as $moduleName => $variables) {
                $templateVars['modules'][$moduleName] = $variables;
            }
        }

        $this->context->smarty->assign($templateVars);

        Media::addJsDef(array(
            'prestashop' => $this->buildFrontEndObject($templateVars),
        ));
    }
}

I manage to reproduce the issue with PS1.7.6.2build1 & PS1.7.6.1 & PS1.7.5.2.
I’ll add this to the debug roadmap.
Thanks!

[Hlavtox]

@khouloudbelguith Super, now you see the error. Maybe it's intentionally off.

Now try my fixes.

/src/Core/Cart/CartRow.php
Change $rowData['cart_quantity'] to $rowData['quantity'] everywhere in the file. 4 times total.
This will fix product subtotal.

/src/Core/Cart/CartRuleCalculator.php
The code causing the problem is in this file. If there is a cart rule with free gift, it goes through whole cart and IF the cart row matches the product in the voucher, it discounts the price of the product.

But there are two lines of the same product.

I added a check to see if this product was already discounted and it seems to work 100% with other discounts. (Polishing needs to be done, I didn't take attribute in to account).

Change this:

    // Free gift
        if ((int) $cartRule->gift_product) {
            foreach ($this->cartRows as $cartRow) {
                $product = $cartRow->getRowData();
                if ($product['id_product'] == $cartRule->gift_product
                    && ($product['id_product_attribute'] == $cartRule->gift_product_attribute
                        || !(int) $cartRule->gift_product_attribute)
                ) {
                    $cartRuleData->addDiscountApplied($cartRow->getFinalUnitPrice());
                    $cartRow->applyFlatDiscount($cartRow->getFinalUnitPrice());
                }
            }
        }

To this:

    // Free gift
        if ((int) $cartRule->gift_product) {
            $already_discounted = array();
            foreach ($this->cartRows as $cartRow) {
                $product = $cartRow->getRowData();
                if ($product['id_product'] == $cartRule->gift_product
                && !isset($already_discounted[$product['id_product']])
                    && ($product['id_product_attribute'] == $cartRule->gift_product_attribute
                        || !(int) $cartRule->gift_product_attribute)
                ) {
                    $already_discounted[$product['id_product']] = $product['id_product'];
                    $cartRuleData->addDiscountApplied($cartRow->getFinalUnitPrice());
                    $cartRow->applyFlatDiscount($cartRow->getFinalUnitPrice());
                }
            }
        }

[khouloudbelguith]

Hi @Hlavtox,

Thanks a lot for your feedback.
Thanks to make a pull request on GitHub with your code suggestion?
https://github.com/PrestaShop/PrestaShop/tree/develop
So you can discuss with our devs about this code fix.
Thank you!

[Hlavtox]

@khouloudbelguith PR opened with improved code #16573

[marionf]

Hello @Hlavtox is this issue fixed ?

[Hlavtox]

@marionf No, realized it was already open when I made a new one.

[marionf]

You mean it's a duplicate ?