Needs review
Project:
Commerce Coupon (D7)
Version:
7.x-2.x-dev
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
6 Nov 2015 at 19:27 UTC
Updated:
28 Mar 2019 at 15:19 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
afoster commentedI'd suggest this a more like a (high/critical) bug then a feature request, but I'll defer @rszrama 's opinion but this was our use case.
1. Create a 10% off discount.
2. Disable the coupon
3. Forget to remove the discount manually
4. Coupon applies to 100% of shoppers and everyone gets 10% off.
5. Random shoppers :) / Client :(
If this is difficult to fix or update then adding a message in stating you need to go a delete the related discount would be a reasonable temporary fix.
Comment #3
lunazoid commentedSame situation as #2. Was confused why a bunch of discounts were being applied to all orders, and realized that they had coupons added, but the coupons were all disabled. The discount was then applied as if it didn't have any coupons.
Comment #4
jeromewiley commentedIs there a way to expedite this request?
Another way of looking at this issue:
Please enable a 'possible configuration' where all Sellers on a Site are REQUIRED to use coupons in order to save a new Discount. The same module enhancement would also disable a discount when the last remaining coupon for it became disabled or expired.
To clarify:
Site A has 100 Seller Clients and thousands of customers. They enforce the configuration described above, and all 100 Sellers are required to create a Coupon in order to be able to save new Discounts.
Site B has 300 Seller Clients and also thousands of customers. They choose to allow "coupon-less" discounts, as they have thoroughly trained Seller Clients, and are not concerned about scenarios where discounts are applied to all products for all customers
Comment #5
potop commentedSeems like if we modify "coupon_count" property of Commerce Discount entity so it returns the number of all coupons (both active and inactive), both scenarios #2 and #3 gonna be fixed.
Would it have any unwanted implications?
Comment #6
torgospizzaWe also have this issue, though I have seen it manifest in two ways. The first is the same way as the OP, but more recently, the following:
1. Create a coupon and discount
2. Create a Rule that reacts to "Completing checkout" that checks "If the coupon in #1 was applied to the order". (In our case the Action was to create a Gift Card for customers at checkout.
3. Disable the coupon
The Rule in #2 will fire for ALL orders, even if they did not have the coupon applied to them. We were giving out several Gift Cards before someone alerted us to the bug.
Therefore I think this problem goes a bit deeper as the Rule invoked for checking a coupon's application is liable to return a false positive in our scenario. I haven't dug into why just yet but I will soon.
Comment #7
GoddamnNoise commentedI've just faced this bug too. This seems like a highly critical bug to me, as it can potentially cause massive losses to any Drupal Commerce store owner. The discount shouldn't be applied if it has coupon codes linked to it and all of them are disabled.
Comment #8
mellowtothemax commentedSame as #7. Coupons were disabled and discount was applied to all orders.
Comment #9
drupov commentedSame as #7.
Generally if a discount exists with no coupon code attached to it, it will get subtracted automatically. If you add a coupon to the discount, the user gets the discount only when he enters the coupon code. If the coupon is disabled (for any good reason) the discount starts applying automatically, as if the coupon is not attached to it.
Comment #10
joegl commentedI think is intertwined with this issue in commerce_discount: https://www.drupal.org/project/commerce_discount/issues/2595981
In my opinion it boils down to the people upgrading from versions of commerce_coupon before commerce_discount integration being unaware of the relationship between discounts and coupons, and that discounts, not coupons are the primary driver.
If you understand a discount stands on its own, and always applies to orders unless there is a condition, coupon or otherwise, it makes sense you'd need to attach a coupon first. But if you come from the coupon route, and are unfamiliar with discounts, you're going to run into the problem many have had; all their customers are getting discounts on everything because no coupons are attached!
I can't think of the best path forward either. I would posit because commerce_coupon has decided to leverage commerce_discount, it's up to commerce_coupon to handhold this portion. Maybe there can be a setting added somewhere, whether globally or on a discount-by-discount basis, which requires a discount to have a coupon for it to be Active? Or at the very least, when editing a discount without a coupon attached, a warning of some kind is displayed to the user: "This discount does not have a coupon attached and will apply to every order if active."
Because there are definitely many people who wants coupons, but not discounts.
Comment #11
joegl commentedI've created a patch which does the following:
- Adds a Coupon Settings Page
- Added two options to the Coupon Settings page:
1. Auto-disable discounts without active coupons (default to false)
2. Show active discounts without coupons warning (default to true)
- In hook_init() the patch checks if either of these settings are enabled. If it they are, it runs an EntityFieldQuery for active commerce_discounts, loops through them, and then runs another EFQ for active commerce_coupons attached to the discount.
- If an active discount does not have an active coupon attached to it, depending on the settings it will either:
1. Do nothing
2. With "Auto-disable discounts without active coupons" enabled, it will load the discount, set the status to 0, and save it. Which deactivates it
3. With "Show active discounts without coupons warning" enabled, it will spit out a warning message stating there are active discounts without active coupons attached, and then a link to either disable the message, or deactivate those discounts
I chose the approach and default settings because I wanted to work with all the following scenarios:
1. A site which wants discounts without coupons attached, and has just added the commerce_coupon module; it should not disable any of their discounts
2. A site which is upgrading to commerce_coupon 2.x, and has just added the commerce_discount integration; it should make the site owners aware of how discounts work when coupons aren't attached, and spits out a warning
I don't know if hook_init() is the right way to approach this. This might end up being too resource-intensive in the long run and could probably be made more efficient, built into the module better, etc.,
Comment #12
joegl commentedTyped all that up and forgot the patch :D
Comment #14
joegl commentedUpdated line 33 of the patch and added:
Should pass the tests now.
Comment #15
joegl commentedNot sure if bumping is appropriate but this has again caught another of our customers unaware resulting in mass discounts to every order. This is a huge problem in my opinion. I've applied the patch I created to the site (not sure why we didn't have it in place before -- that's on me).