Prestashop: FO - The promo value is not correct in the cart summary & checkout summary

[khouloudbelguith]

Describe the bug
The promo value is not correct in the cart summary & checkout summary

To Reproduce
Steps to reproduce the behavior:

1. Go to the BO => Catalog => Discounts => Create a new cart rule
   1.1 has no code
   1.2 there is no condition
   1.3 As an acrtion=> Apply a discount Percent = 100% => Apply a discount to:"Cheapest product"
   

1. Go to the FO => Add to the cart demo_11 & demo_6
2. The cart rule will be applied to the demo_11
3. Go to the cart page
4. See error
   

With PS1.7.6.2 was ok

Additional information
PrestaShop version: develop
PHP version: 7.1

Edit:
Specs:
The discount should display the value of the discount and nor the percentage.
And if the value can't be retrieved for a technical reason, it should display the percentage.

[arouiadib]

Thank you, @khouloudbelguith, you addressed an issue that will probably resolve other bugs.

When "Apply discount to:" is set to "cheapest" in BO, the calculation now is = ( All Products Total excl Taxes * Reduction percentage ) / 100

but should be = ( Cheapest product price excl Taxes * Reduction percentage ) / 100

I think some functionalities are not yet added in develop:

* There should be a column in database that stores the configuration value of "Apply discount to:" for each cart rule. I unfortunately don't find it (but the value is correctly sent from BO, see picture )

* Retrieve this value from database inside cart voucher object.
* A function that checks if this value is equal to "cheapest" and apply the appropriate equation.
* Unit tests.

Any remarks? suggestions? Was this available in 1.6? Can i be assigned to this?

[MatShir]

We need to determine the right display between 100% and the amount of the discount ? And if it can be done? @PrestaShop/prestashop-product-team

[sam-pires]

Hello @arouiadib ,

Thanks for the feedback, really interesting 👍
As this issue is focused on the regression, it would be better to create another issue labeled "Improvement" with all the suggestions you listed in your post.
Can you do it? I can do it otherwise.

This will allow us to work on your suggestions in a different issue for the 1.7.8, and of course, you can be assigned to it 🙂 We would love it!

[arouiadib]

Hello @sam-pires,
Thank you for your answer.
I don't think I am able to create labelled issues, but I would really love to be assigned to this task once the corresponding one is created.

Regards,

[MatShir]

Specs:
The discount should display the value of the discount and not the percentage.
And if the value can't be retrieved for a technical reason, it should display the percentage.

[arouiadib]

Hello @MatShir ,
Thank you for the specs, they removed some of the confusion. But in current code these specs are respected (unlike in code source of 1.7.6.2 and as tested by @khouloudbelguith). The result is formatted as an amount but the problem comes from the calculation.

I have created a PR but I would love to get community feedback about some points:

1- The calculation should include selected quantity? i.e, a cart that contains more than one item of the cheapest product? is the reduction a percentage of unit price of the cheapest product?

2- the PR I created is a "white-flag" PR for me, because I have spend hours trying to create a cleaner and better solution to no avail. My one-line PR uses an existing method getContextualValue in CartRule.php. Does it go against migration principles? Better not to use methods from that class? Am I right? Additionally, the processing in that mothod almost exists in CartRuleCalculator also, but not easy to re-use ( as I see it addresses another purpose IMO, and re-factoring should be done carefully).

Another possible solution is use quantity from product list in Presenter, but it would be a second duplication for that processing, and maybe is not the role of cartPresenter?!

I think for a cleaner solution is to make a re-factoring in CartRuleCalculator. But more experienced people from the community will suggest me the right direction.

Thanks to all of you.

[MatShir]

Thanks a lot for your contribution @arouiadib

1- The calculation should include selected quantity? i.e, a cart that contains more than one item of the cheapest product? is the reduction a percentage of unit price of the cheapest product?

If a cart has 3 products with 2 same products as the cheapest, it applies the discount only for one of the cheapest products. So, the discount is based on the unit price tax included of the cheapest product.

2- the PR I created is a "white-flag" PR for me, because I have spend hours trying to create a cleaner and better solution to no avail. My one-line PR uses an existing method getContextualValue in CartRule.php. Does it go against migration principles? Better not to use methods from that class? Am I right? Additionally, the processing in that mothod almost exists in CartRuleCalculator also, but not easy to re-use ( as I see it addresses another purpose IMO, and re-factoring should be done carefully).

Another possible solution is use quantity from product list in Presenter, but it would be a second duplication for that processing, and maybe is not the role of cartPresenter?!

I copy and paste your comment in your PR for developers to answer you.

[arouiadib]

Hi @MatShir,

I have the same question but this time about the last two remaining types: "Specific product" and "Selection of products". Is the calculation based on unit price like in the case of "cheapest product"?

You can look at this draft for more details. In fact, I am trying to understand once and for all this problem of "unit vs final" price, before diving more in the solution of this current issue. If you take exactly the same example provided by @khouloudbelguith, and increase the quantity of cheapest specific product (or selection of products), discounts subtotal increases also, and this is a different problem. It happens also for "Specific product" and "Selection of products".

Do you think it is necessary to create a separate issue?

Again, big thanks :).With regards,

[MatShir]

@arouiadib, separating the issue will facilitate the code review and testing, one problem at a time. But one specs is enough :)
If you can write down every step to reproduce it

[arouiadib]

Thank you for your time. You are right @MatShir.
I have created the tickets with steps to reproduce issues: #17666 and #17665.

Since I feel, and kinda believe, these calculations should also be done using unit price ( like as discussed above concerning cheapest product ), here is a fix I suggest..

With regards,

[Progi1984]

Fixed by #17502