Allow discount_coupon on orders without the discount

I think this is a use case feature, however, I could also see it as a re-used feature if it was available. I have come across some extreme discount and promotion proposals. I think this extends the flexibility of the module without harming its core focus. After all, a coupon merely grants you access to something - why can it not simply grant you access to viewing discounts?

The concern I've had is that commerce_coupon_remove_coupon_from_order() is used to do garbage collection. We need to do this in Commerce because items can be added or removed from the cart and nullify the original conditions which allowed them to be placed. The order does not get refreshed once it leaves 'cart' state, unless it re-enters via canceling checkout.

A consideration I had is moving garbage collection to occur when use count is incremented, prior to checkout being completed. The end goal is for a end order to not have any cruft attached or skew coupon/discount usage. I'm suggesting this but feel its an improper approach and half endorse it. The context seems proper on an order when it has the 'cart' state, not before completing checkout.

Adding the option "Allow coupons to be added without discount applied" can allow bypass of the order refresh clean up. I'm just wondering the best steps to provide garbage collection if that option is enabled.

--------

Two cases instantly came to mind while writing this that past clients would have used this for:

1. Social media promotion "enter this code to reveal hidden discounts" drawing customers to the site.
2. Loyalty discounts to users. Instead of providing a pricing calculation a coupon of a limit (lets say 10) could be injected into a customer's order, allowing them limited discounts.

After discussion in IRC, caschbre brought up how this option could be stored and modified. We discussed having it be a setting on each coupon, providing a more robust control over how coupons operate (not all or nothing.) That way you can have special coupons but still normal coupons.

Saving this value could be simply tucking it into the entity's data attribute and pulling it out of the array when needed, much like Commerce Shipping does for its own line items.

@caschbre, check this out. commerce_coupon_form_submit() is the callback for the entity form, added it there. also made the order fresh conditional on coupon's setting. Not tested because I quick proof of concepted it to help bring this closer to solution, won't have much time tonight.

My IDE strips whitespace..so sorry about patch.

admin.inc, see addition of ->data below ->status

/*
 * Form submit callback: submit coupon entity form
 */
function commerce_coupon_form_submit(&$form, &$form_state) {
  $coupon = &$form_state['commerce_coupon'];
  $coupon->code = !empty($form_state['values']['code']) ? $form_state['values']['code'] : commerce_coupon_generate_coupon_code($coupon->type);
  $coupon->status = $form_state['values']['status'];
  $coupon->data['apply_without_discount'] = $form_state['values']['apply_without_discount'];

.module (refresh improved)

      // Remove invalid coupons, unless spciefied not to.
      if (!$coupon->data['apply_without_discount']) {
        commerce_coupon_remove_coupon_from_order($order, $coupon, FALSE);
      }

.module (your form area, improved a bit)

  $form['apply_without_discount'] = array(
    '#type' => 'checkbox',
    '#title' => t('Apply without Discount'),
    '#default_value' => isset($coupon->data['apply_without_discount']) ? $coupon->$data['apply_without_discount'] : 0,
  );

+++ b/commerce_coupon.module
@@ -1733,6 +1735,12 @@ function commerce_coupon_attach_coupon_entity_form(&$form, &$form_state, $coupon
@@ -1920,6 +1928,17 @@ function commerce_coupon_commerce_checkout_form_review_validate(&$form, &$form_s

@@ -1920,6 +1928,17 @@ function commerce_coupon_commerce_checkout_form_review_validate(&$form, &$form_s
+    foreach ($order_wrapper->commerce_coupons->value() as $coupon) {
+      // Invalidate coupons that exist on the order without their discount
+      // present, meaning an inline condition on the discount was not satisfied.
+      // Free shipping discounts often do not apply immediately. For this reason,
+      // do not remove coupon codes that exclusively grant free shipping.
+      if ($coupon && $coupon->type == 'discount_coupon' && !commerce_coupon_order_coupon_code_discounts($coupon->code, $order) && !_commerce_coupon_free_shipping_single_discount($coupon)) {
+        // Remove coupons without discounts based on coupon setting.
+        commerce_coupon_remove_coupon_from_order($order, $coupon, FALSE);
+      }
+    }
+

This is an existing hook, but my only question is: is that clean up logic copy & pasted from order refresh logic? Should this patch move the logic into a helper function and then call the helper function at both locations? That could make the process more manageable and future proof on any changes in logic.

Well, I think I'm just splitting hairs then :)

I'll test sometime this week on my local dev

+++ b/includes/commerce_coupon.admin.inc
@@ -139,6 +139,7 @@ function commerce_coupon_form_submit(&$form, &$form_state) {
+  $coupon->data['apply_without_discount'] = $form_state['values']['apply_without_discount'];

$entity->data actually comes as a serialized array. Value never gets set properly on entity. I'm having some issues getting it to work properly.

Finals for this class wrap up today, I'll test more tonight or tomorrow.

The real problem here is that the entity controller doesn't automatically unserialize the data property for us, like line items do. Opened issue here: #2280505: Support automatic entity data unserialization

If we can get a patch written for that it'd make this process much simpler to manage.

Thanks for working on this @caschbre! One of the best features when we were using Ubercart on D6 was the ability to include a coupon code in our marketing emails, so that a user could hit a catalog URL (like e.g., rifftrax.com/catalog?coupon=PROMO1) and the coupon would be attached to their session, but of course the discount would only apply to a product once/if it was added to their cart. It seems like this issue gets us closer to having this same functionality in Commerce, so I am looking forward to testing it out.

The client @caschbre and I are working for identified an issue with this patch...the "Apply Without Discount" property saves correctly if an admin is adding/editing a coupon on the coupon form, but it does not save if the coupon is being edited inline elsewhere, for example, while adding/editing a discount.

This updated patch relocates the logic for saving this property from commerce_coupon_form_submit(), which is only called when editing a coupon directly, to commerce_coupon_form_attach_coupons_validate(), which is invoked on any occasion.

After further testing, we discovered that commerce_coupon_form_attach_coupons_validate() isn't actually invoked when saving a coupon directly from the coupon form. Attached is a patch that combines the best of #21 and #24 to cover all cases.

Patch in #25 seems to work great. I'm looking forward to using it in production.

Testing further, I'm wondering if we should introduce an option that allows a coupon to be stripped from the order at checkout if no discounts apply? It seems kind of weird to have a coupon in the Coupons View/list but have the Value be blank.

It's definitely a nice feature to be able to apply the coupon in this way (especially with links in email blasts!) but I do think there should be some consideration into whether we want a no-discounts coupon to still be sticking around once the user has made it to Checkout.

EDIT: Upon even more testing, I think this patch has either created or revealed a problem with Discounts, which is that a Product Discount can be removed from an order after it has been calculated and added as price components. The coupon remains attached but the Discount reference gets removed from an Order, sometimes resulting in the erroneous removal of the coupon as well.

Twice now I've had to check the "apply without discounts" checkbox for coupons that used to work without issue. So again I'm not sure if this is a regression or in fact we are getting to the route of a larger issue with how Discounts alters line items and discount references when the discount is a product-level discount.

Yeah that makes sense. Also I may just not have understood how coupons work when they are part of a product discount. The order_refresh stage applies the discount as a price component but removes the reference to the discount itself, but keeps the coupon intact. Which I guess makes some sense :)

Anyways I am a fan of this change. I think we could address the concern of "orphaned coupons" later on.

Putting back to needs review because this always depended on a patch, so it couldn't be RTBC. BUT! The dependant patch has been committed #2280505: Support automatic entity data unserialization. Could those involved test against latest -dev?

Also. I've committed a base test class. A test would be even better. #2614392: Add tests! Start with basic UI tests..

Hmm, something in the latest dev has definitely broke something. When saving a coupon with the "Apply without discounts" box checked, I'm receiving a warning: "Illegal string offset: apply_without_discounts in commerce_coupon_form_attach_coupons_validate() line 1819"

Which is this:

$coupon->data['apply_without_discount'] = $coupon_values['apply_without_discount'];

Not sure how to best resolve it. But the warning has the effect of leaving the "Apply without discount" value being unchanged.

EDIT to add: I can't reproduce this locally, of course. Argh. (Maybe because we're using Memcache/Entitycache on the live servers?)

Looks like setting the data array if one doesn't already exist is the solution:

if (!isset($coupon->data)) {
  $coupon->data = array();
}
$coupon->data['apply_without_discount'] = (int) $coupon_values['apply_without_discount'];

Not sure if there is a more elegant way to do this but it appears to be necessary.

EDIT: Upon further inspection it seems that mainly doing this through the Discounts form (with the inline coupon form) also has some issues there. When doing it right through the coupon form (from the list of coupons) it also seems to work. Will try to investigate further, but for me I think it finally saved properly when I edited the coupon from the Coupon Edit form (and not the Discount edit form).

Does this look to be an issue where the controller needs to ensure this is an array on load if empty? Would need to look again at other controllers. Or is it an implementation issue of the inline coupon and discount form?

@mglaman: I'm not actually sure. Even with my fix, I'm now getting this warning:
Creating default object from empty value in commerce_coupon_form_attach_coupons_validate()

The object in question is $coupon, so I'm not sure if that means the form is no longer passing the actual object in through its values or what. I'll try to keep digging if I can. Luckily, saving does seem to work through the inline form now!

Marking as needs work for tests, also we should alter commerce_coupon_discount_coupon_codes_exist_on_order() as I think that'd allow product discounts to render properly.

Add an isset() check during refresh in case data attribute is missing.

Saw a patch but the status is the same. Setting to Needs Review.

It would also be interesting to add a section that does this:

 if (!empty($query_parameters['coupon'])) {

that way a user can just add ?coupon to the end of a URL to apply the coupon to their session. This is something currently you'd need to implement in a custom module.

#36 Working for me so far.

I am having issue where I am applying a correct coupon(product discount) to an order, but the conditions will fail because the wrong product is in the order.

Thus the coupon / discount is not applied > Correct !

But however I am seeing a message: "Your shopping cart has been updated."

This is not correct, because it has not been updated, because it is actually a wrong "discount code".

Does anyone else have this issue ?

@mschudders: No, I haven't experienced that issue. Does this only occur when you have applied one of the patches from this issue? Can you verify that reverting the patch resolves the behavior you're seeing?

Well some strange things have started to happen. The serialized data being saved in the database is in a strange format so that when unserialized, it is not an array. Therefore, $coupon->data['apply_without_discount'] returns NULL.

I believe because of this, the coupon is not being applied to the order.

Updating patch to run against 7.x-2.0.

Also added $coupon->data = array(); to get data to be serialized properly.