Coupon clean-up prevents the use of the Free Shipping offer type

The free shipping offer is indeed problematic because of the reasons you stated. Even within the context of Commerce Discount it doesn't function completely right - the conditions for it only fire when you hit the shipping page form because it uses the collect-rates event and does not do any integrity checks for itself on cart refresh.

I've implemented a partial fix: where Commerce Coupon will not remove free shipping coupons at order refresh. This should allow you to add coupons that enable free shipping.

Here's the hitch: if some one removes the coupon, it will not recalculate unless for some reason they go back to the shipping page. This could result in some one removing the discount on the review page and getting the free shipping without using the coupon. Perhaps a workaround is to take the "remove" button off the review page view ..

I'm going to pass off fixing the underlying issue with the free shipping offer to the discount issue queue. Coupon module is designed only to act as a "gate" for discounts when they check their conditions, and if an offer does not check its conditions regularly enough then there is not much we can do from here.

Committed to dev: http://drupalcode.org/project/commerce_coupon.git/commit/490f9c7

@dpolant is there an existing issue with commerce_discount for the underlying problem?

Sorry, but this still needs work. This temporary solution works for the specific case of free shipping, but we're implementing another module that works with shipping discounts and it doesn't work there at all. Moreover, having to add exceptions for every kind of discount to this one area of the code isn't maintainable.

I'd suggest we add a hook here instead that gives modules a chance to control how the coupons are filtered, or gives them a chance to indicate whether or not the coupon still applies.

Can someone try this patch on for size?

This is against the latest dev rather than stacking other changes made for this issue into the patch.

Ugh, digging deeper the real issue lies with commerce_coupon_order_coupon_code_discounts(). That function can only return line item discounts, rather than any generic discount on the order. As a result, free shipping discounts and all other types of discounts that don't directly affect line items for the cart require special handling.

Isn't there a way for the coupon module to store the coupons on the order instead of relying on line items?

Please disregard my patch in #7 and try this instead.

This patch removes the special case logic for free shipping from both commerce_coupon_commerce_cart_order_refresh() of commerce_coupon.module and render() in commerce_coupon_handler_field_discount_value_display.inc in favor of using the new hook I've introduced for coupon clean-up.

In addition, this patch fixes commerce_coupon_code_is_in_order() so that it actually works (I'm guessing this is an old hold-over from 1.x). This function is now used in commerce_coupon_order_coupon_code_discounts() to verify that the coupon actually exists on the order in question.

Please test and let me know if this works.

Sorry, This patch not working in my side. Module enable and disable functioning works fine. No errors and warnings. But nothing happens what we expected functioning. Please let me know GuyPaddock. Thankq

When you say "nothing happens when we expected", can you elaborate? What workflow?

Just ran it through its paces and don't see a difference with the way free shipping is handled -- the patch doesn't appear to break that existing workflow.

Can you tell me what type of discount you're trying that's not working properly? Have you confirmed that the workflow works without the patch and fails with the patch?

hi GuyPaddock,
After i am applying patch it works fine. no errors and warnings.It does not break the workflow also. My workflow is creating coupon for free shipping who entered coupon code they will have provided by shipping service free is applicable on order discount. Details shown in above image. Here i am using discount and coupon 2.x module. What i am expected means when user applied coupon code then he will get free shipping service only, other services will be disappeared in shipping service page. Is it possible with this patch. please let me know. Thankq.

Your use case is definitely unrelated to this patch; I just changed the issue title to help clarify this. This patch is only intent on fixing problems with the way that Commerce cleans-up coupons on the order when it doesn't think that that they apply any longer.

To do what you want, I see two options (you may have other options, but these are just the two I can think of right now):

1. You could find or write a module that hides shipping methods when the free shipping discount applies to the order. (You may need to apply my patch if you're implementing your own type of coupon / offer to make this happen, but otherwise my patch doesn't apply here.)
2. You could configure each of the conditions on the rules for the other shipping methods (under admin/commerce/config/shipping) so that conditions on the free shipping method only pass when the order has a free shipping discount, and the conditions on the other methods only pass where there is NOT a free shipping discount. Tedious if you have a lot of shipping methods, but it wouldn't require writing any code if I'm correct.

hi GuyPaddock,
Thankyou for detailed justification. problem is solved with changing rules. It doesn't require any code. Follow second option by adding condition to the each of the shipping method and its working fine.

Could you pls give clearly information about this patch. Thankq

@GuyPaddock
Patch needs re-roll on the latest dev so we can test.

My issue is #2593251: "Unable to redeem coupon." when I use "% off of shipping"

About to re-roll and test.

Updated patch attached. This was more than a re-roll because it interacted with some functions that no longer are included in commerce_coupon.

Note that free shipping coupons have worked since Dan posted the two hard-coded free-shipping conditions in this module. So the best outcome of this patch is to support free shipping coupons in a way that other modules can implement a similar "wait and see" if the order discount gets applied.

To test:

1. Install Drupal + Commerce Kickstart 1.40 (or latest stable)
2. Download Commerce Shipping, Commerce Flat Rate, Commerce Discount, Commerce Coupon
   • drush dl commerce_shipping commerce_flat_rate commerce_discount commerce_coupon-2.x
3. Enable Modules
   • drush en commerce_shipping_ui commerce_flat_rate commerce_coupon -y
4. /admin/commerce/config/shipping/services/flat-rate/add - Add an Expensive Flat Rate
5. /admin/commerce/discounts/add - Add a Free Shipping Discount (make sure to add a coupon condition at the bottom!)
6. Buy things and checkout!
7. Without patch, I can add a coupon, select a shipping service, and in review the shipping service is free.
8. With patch, I can do the same thing.

Please test and update the status as you have a chance!

This patch also addresses percent_off_shipping and shipping_upgrade discount types. Thanks to #2593251: "Unable to redeem coupon." when I use "% off of shipping" for point out another kind of discount that fails. This patch makes all three kinds of shipping discounts work as coupons with one hook implementation.

Coupons can reference multiple discounts. I would expect the coupon to be removed unless ALL of its discounts existed on the order. Currently that's not the case. Updated the patch to fix this. Also simplified commerce_coupon_cleanup_process_invoke().

The label for a free shipping discount was an empty string for me. It's because the discount isn't being detected in commerce_coupon_order_discount_ids(). Specifically this line

if (!empty($component['price']['data']['discount_name']))

I'm not sure if this is a commerce_discount issue or commerce_coupon because in commerce_discount_free_shipping_service() it's setting

$discount_price = array(
  'amount' => -$discount_amount,
  'currency_code' => $free_service_wrapper->commerce_unit_price->currency_code->value(),
  'data' => array(
    'discount_component_title' => check_plain($discount->component_title),
  ),
);

and that's what the component for my free shipping service looks like.

Also, I thought implementing the hook this patch creates to support all the shipping discount offers was incorrect because that should be the responsibility of commerce_shipping. Than I noticed that there's a lot of shipping support in both commerce_discount and commerce_coupon. Is there a legitimate reason it's done this way? Maybe there isn't a hook to define a new discount offer type?

Maybe there isn't a hook to define a new discount offer type?

There is, hook_commerce_discount_offer_type_info(). We're using it for a special discount offer type.

Free shipping discounts were something Ryan was just working on recently. You may want to look at #2560773: Add support for "Free or reduced shipping" offers.

EDIT to add, sorry if this is stuff you already know about :) Just wanted to mention it since I remember seeing Ryan's issues and patches for Free Shipping so recently!

BTW we're experiencing a problem related to this issue, so I'm testing the latest patch.

Setting to needs work. Here are my findings:

- Getting an error "undefined variable: $result", probably because that var is returned outside of the condition if ($coupon->type == 'discount_coupon')
- Also pretty sure that in the function commerce_coupon_cleanup_process_invoke() we should specifically check if (!$eligible === FALSE) rather than if (!$eligible) because in the event that no modules return a value then $eligible may not be defined and this could throw more errors.

So unfortunately it seems like this patch did not solve anything for us, (having coupons removed during Checkout), but I do think the change itself is needed.

So I cleaned up commerce_coupon_commerce_coupon_cleanup_process() which should take of your undefined variable: $result error.

In commerce_coupon_cleanup_process_invoke() we foreach() over module_invoke_all() which returns an empty array() if no modules implement the hook. In PHP it's safe to foreach over an empty array().

Updated patch with interdiff provided.

When using commerce_discount_date, validation is not working. When using commerce_coupon_date, validation is working.