Skip to content
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

Fix logic and display of customer's cart rules #16638

Merged
merged 2 commits into from Dec 6, 2019

Conversation

@Hlavtox
Copy link
Contributor

Hlavtox commented Nov 30, 2019

Questions Answers
Branch? develop
Description? This fixes a million-times reported issues of customers seeing cart rules they should not see.
Type? bug fix / improvement
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #10766 #13002
How to test? See below

What's wrong
There is a bug in getCustomerCartRules function. The function accidentaly includes ALL CART RULES THAT HAVE SOME KIND OF GROUP RESTRICTION.

If you set some kind of group restriction on a cart rule, it group_restriction in database to 1. Then it stores the rules in a separate table.

Now, no matter what parameters you specify when calling the function, this part of code gets done. Does the cart rule have any kind of group restriction? It gets included. Even the generic ones, without a code.
$sql .= ' UNION (SELECT ' . $sql_part1 . ' WHERE cr.group_restriction = 1 ' . $sql_part2 . ')';

Removing this part of code restores the correct function and logic.

The logic works fine, because it's done later in the code - near // Remove cart rule that does not match the customer groups

Why it's important
Imagine you have X of vouchers, that reduce amount of order by X percent. They are unlimited by all means and have a code. You have them ready to be told to customers if you need to persuade them about ordering. Now (if conditions are met), every customers sees all of that "hidden vouchers" and can use it. Do you realise how crucial it can be if some merchants don't know about it?

How to test

  • Make a customer A. Customer group 3.
  • Make a new customer group ID 4, for example "Business Customers". Do NOT assign the customer to it.
  • Make a cart rule - limited to customer A, no group restrictions, voucher code ABCDE, discount order by 10 %.
  • Make a cart rule - limited to groups 1,2,3 (not for 4), voucher code EFGHI, discount order by 10 %.
  • Make a cart rule - no restrictions on customers or groups, voucher code JKLMN, discount order by 10 %.

Results - before a fix
ABCDE - Visible - OK
EFGHI - Visible - WRONG
JKLMN - Not visible - OK

Results - before a fix
ABCDE - Visible - OK
EFGHI - Not visible - OK
JKLMN - Not visible - OK


This change is Reviewable

Hlavtox added 2 commits Nov 30, 2019
@Hlavtox Hlavtox requested a review from PrestaShop/prestashop-core-developers as a code owner Nov 30, 2019
@PierreRambaud PierreRambaud added this to the 1.7.7.0 milestone Dec 2, 2019
@Robin-Fischer-PS Robin-Fischer-PS self-assigned this Dec 5, 2019
@Progi1984 Progi1984 merged commit 596b192 into PrestaShop:develop Dec 6, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@Progi1984

This comment has been minimized.

Copy link
Contributor

Progi1984 commented Dec 6, 2019

Thanks @Hlavtox

@Hlavtox Hlavtox deleted the Hlavtox:patch-5 branch Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.