Coupon validation does not handle disabled coupons

We saw this as well, but for us it wasn't just the form; it was the fact that a discount attached to a disabled coupon would apply the discount, despite the disabled coupon status. Seems like a weird bug somewhere deeper in Coupons that is outside the form validation process. If I can find some time I will dig into this, but for now (for our issue at least) the trick is to just make sure your coupon is Active.

@paranojik: That's what I mean - when you have an Active Discount coupled to a Disabled coupon, the coupon doesn't need to be applied - the discount just seems to apply itself.

I'd say this change makes absolutely sense, no need to process the whole shebang if the coupon is disabled.

#6 Works for disabled coupons, the discount will not automatically be added to the product. As it would when you disable the coupon. Simple fix, seems to work good.

While the patch is +1; we should really have a test for this, too.

I have downloaded the latest dev version and applied this patch. I am still having the issue. When I look at a discount rule that has coupons, but none enabled it has no other conditions but to apply the discount to the order. Am I possibly missing something when I setup a discount and a coupon?

EDIT: It is possible something was lost in translation when I upgraded from 7.1 to 7.2.. E.g., the upgrade creates a $10 discount and references all coupons with a $10 discount there, when discounts should probably be created more robustly. I'm trying to avoid a situation where coupons end up disabled but the discount is still enabled and accidentally applied to all orders.

Can their be a way to add a "require coupon" feature to the discount? So the discount doesn't fire unless it finds one of the attached coupons (and the coupon is enabled of course). I can think of scenarios where it doesn't makes sense to disable a discount simply because all the referenced coupons are disabled. Especially since coupons now require a discount to be attached (and you don't want to lose your coupon codes).

EDIT: I believe the issue is that, once all the coupons on a discount have been disabled the "Coupon code for a particular discount has been added to an order" condition gets removed from the discount rule.

EDIT 2: "coupon_count->value()" counts the amount of coupons referenced by a discount. It's being used to determine whether to add the "Coupon code for a particular discount has been added to an order" condition to the discount rule. It requires that the coupon is enabled (status=1). I found the condition for the query on line 666 of the commerce_coupon.module and commented it out (disable coupons still count):

        $query->join('field_data_commerce_discount_reference', 'd', 'c.coupon_id=d.entity_id');
        $query
           //->condition('c.status', 1) // disabled coupons still count
           ->condition('d.commerce_discount_reference_target_id', $discount->discount_id)
           ->condition('d.deleted', 0)
           ->condition('d.entity_type', 'commerce_coupon');

This leaves the "Coupon code for a particular discount has been added to an order" condition on the discount rule even if all the coupons are disabled, which, in turn fixes the discount applying to orders when all referenced coupons are disabled. However, I hope this isn't complicating anything else.

I think @joegl found the root of the problem. The rules condition commerce_coupon_discount_coupon_codes_exist_on_order() checks if the discount uses coupons by calling $discount_wrapper->coupon_count->value(). This however does not take disabled coupons into account. So if the discount has only disabled coupons (and no other conditions) it will be applied on every order.

Patch is solving this by applying @joegl's fix. The patch also includes @paranojik's check.

Added test.

Test passes without the fix too, which is not good.

Test updated. This contains the fix, so test should pass.

This is just the test, so it should fail.

Hi All,
The current patch file works on 7x Branch,Thanks

root@asad-Vostro-3550:/var/www/html/git/commerce_coupon# git apply -v commerce_coupon-2642832-17-disabled_coupons_only_test.patch 
Checking patch commerce_coupon.test...
Applied patch commerce_coupon.test cleanly.
root@asad-Vostro-3550:/var/www/html/git/commerce_coupon# 

@AsadKamil You should use the patch in #16, not the one in #17.

BTW @AsadKamil 95% of your comments (https://www.drupal.org/user/3407474/track) are similar and wrong. You really should not say "patch file works" if you only applied it but did not test it. At least say only "applies cleanly".

We've been using this for a while now on production. Since this is based on paranojik's patch, I mark it as RTBC.

czigor which patch #? I'll commit.

mglaman patch #16. Thanks!

Committed, thanks!