EntityMetadataWrapperException for orders with deleted coupons

i can confirm this issue.
It's not only for Coupon cart form.
But for orders with deleted coupons, this crashes all pages with this order.
Maybe, besides we fix the code, we also need some deletion of the deleted coupon reference.

There is the first patch about this.

There 2 aspects that this patch tries to fix.

1. To get rid of the error messages
Do not use entity_metadata_wrapper for the order coupons values and try to use directly the coupon object instead.
2. Do not let people to delete the "in use" (referenced) coupons.
Currently is it possible to delete coupons form Discount edit forms.

For 2. i built a Helper function for Delete/Remove buttons.
commerce_coupon_delete_is_referenced_message()
This uses existing commerce_coupon_get_number_of_uses() to check the reference.
And use this helper function for both Coupon Delete page/form and Discount Coupons delete buttons.

Maybe there other places that some code could be "cleaned" in terms of using entity_metadata_wrapper approach.
But at least the current patch should solve the deletion issue.

What about cleaning "Unexisting" (deleted) coupons that are referenced in orders.

@vasike,

Thanks for the patch. We stumbled upon this issue today on one of our production websites where it affected the login of user accounts (resulting in a nasty fatal error).

After applying the patch the errors went away and everything is working correctly again. Major thanks for your effort on this one. Updating issue status to RTBC.

I tried this patch but still got the error - I also fixed up the tables manually (after backing up) - see https://www.drupal.org/node/2711961#comment-11151557.

I then tried https://www.drupal.org/node/1596594#comment-9006865 re entity module but still got issues with checking out on drupal ecommerce. Im using latest commerce_discount and latest commerce_coupon.

Now getting this error on payment page (only seems to be logged in users not anonymous)

Exception: Cannot save a usage transaction without a coupon id in CommerceCouponUsageTransactionEntityController->save() (line 66 of /xxxx/html/sites/all/modules/contrib/commerce_coupon/modules/usage/includes/commerce_coupon_usage.controller.inc).

Weird thing is im not even checking out using a coupon - just a simple purchase. Ive cleared caches etc but the data in the commerce_coupon and commerce_coupon_usage_transaction tables dont seem to match up with the stats being displayed for coupon usage

There is a spot in commerce_coupon_usage.module which uses the approach in the patch, and it causes a fatal EntityMetadataWrapper Exception when attempting to load $coupon_wrapper->commerce_coupon->coupon_id->value() on a coupon that no longer exists. For instance:

foreach ($order_wrapper->commerce_coupons as $coupon_wrapper) {
      $coupon_id = $coupon_wrapper->coupon_id->value();
...

It will fail with EntityMetadataWrapperException: Unable to get the data property coupon_id as the parent data structure is not set.

It seems the issue is that, while the coupon_id value exists in an internal cache, the coupon itself no longer exists and cannot be wrapped. For this particular instance, I thin we should determine whether the coupon is deleted first before attempting to wrap any values from the entity.

Here is what I did to the Usage submodule to make sure that usage transactions do not break on account of a coupon being missing/deleted:

@ -416,7 +416,15 @@ function commerce_coupon_usage_commerce_order_update($order) {
     // Add usage records for coupons present if coupon is present in current but
     // not original or if the original order was in a non-tracked status.
     foreach ($order_wrapper->commerce_coupons as $coupon_wrapper) {
-      $coupon_id = $coupon_wrapper->coupon_id->value();
+      // Make sure the coupon exists.
+      $coupon = $coupon_wrapper->value();
+      if (is_null($coupon)) {
+        return;
+      }
+      else {
+        $coupon_id = $coupon_wrapper->coupon_id->value();
+      }
+
       if (!$unchanged->commerce_coupons->value() || !in_array($coupon_id, $unchanged->commerce_coupons->raw()) || !in_array($unchanged->status->value(), $statuses)) {
         // Never write a record for the same order and coupon twice.
         if (commerce_coupon_usage_record_exists($order->order_id, $coupon_id)) {

Whether this is a deeper issue is a good question. Perhaps we shouldn't delete coupons at all, if deleting a coupon can result in "ghost" entities being wrapped (or attempted to be wrapped, at least) which results in this same error.

Per #9 this needs work.

So, I was getting an error in my Drupal installation and wrote this patch to remove orphan coupon references using the hook_commerce_cart_order_refresh() hook.

Basically, every time an invalid coupon code is detected, it's automatically removed from the order.

It works well for me, although it definitely can be improved and merged with #4

The patch in #11 no longer applies cleanly. Can you re-roll it against the latest dev?

What exactly is the issue here?

I can't reproduce an EntityMetadataWrapperException if the coupon redeem form is on the cart page (unlike it's stated in the issue description).

What I can reproduce is the following:
0. Enable commerce_coupon_usage.
1. Create an order discount and add a coupon to it.
2. Go to checkout, redeem this coupon.
3. Remove coupon. (It's only possible on the discount edit form, the direct coupon deletion form does not allow deleting referenced coupons.)
4. I can still move back and forth in checkout but when I try to place the order I get the EntityMetadataWrapperException. It's coming from commerce_coupon_usage_commerce_coupon_final_checkout_validate().

@vasike What do you mean by 'crashes al pages with this order'?

Just came across this issue.

@czigor

What exactly is the issue here?

As of my experience this scenario happens if a coupon is deleted (the coupon itself not the reference on the order) which was already attached to an order.
The order still references the deleted coupon and the entity metadata wrapper happily loads a non existing object but obviously fails subsequently to fetch properties - ending in the exception.

So between 2. and 3. of the reproduction steps add a step to delete the coupon in the backend.

@c0rtex

1. +++ b/web/sites/all/modules/contrib/commerce_coupon/commerce_coupon.module
   @@ -1360,7 +1360,14 @@ function commerce_coupon_commerce_cart_order_refresh($order_wrapper) {
   +      $order_wrapper->save();
   

   I think the hook_commerce_cart_order_refresh() handling takes care of saving if necessary. So we should remove that to avoid overhead.

Re-rolled patch against latest 7.x-2.x and removed the mentioned save.

I also stumbled upon the same exception, this time coming from commerce_coupon_usage_commerce_order_update(), in the first loop. It should normally be fixed with the attached patch.

+++ b/commerce_coupon.module
@@ -1360,7 +1360,12 @@ function commerce_coupon_commerce_cart_order_refresh($order_wrapper) {
+      unset($order_wrapper->commerce_coupons[$delta]);

Normally, we use the offsetUnset() method from the wrapper, that will work but the keys won't be renumbered, so maybe we just need to do this slight change to the patch.

New patch with the use of offsetUnset().

The patch above worked for me in a real-world scenario where we had coupons that were either deleted or (I think?) possibly disabled as part of Commerce Giftcard.

Before the patch the site would attempt to save the coupon and fail, but after the patch, the process worked without issue.

The problem occurs when you have an order with coupon and and then you delete coupon (disabling coupon is ok). Then if you try to run commerce_order_save($order); on that order you will get above mentioned error.

The problem originates from commerce_coupon_usage.module
line 418 where there is foreach ($order_wrapper->commerce_coupons as $coupon_wrapper) {
and is nicely ignored and cleaned up with latest patch.

But we need some consistency here. As when you try to delete coupon from coupon UI, you can't do that, so the same should be in discount UI.
Patch from #4 does that with commerce_coupon_delete_is_referenced_message helper function implemented into
commerce_coupon_form_commerce_discount_form_alter so users cant delete coupon from discount UI. Think that should be combined and then patched.

I combined patch #4 and patch #19 in one so we have both areas covered
1. No more EMW error on missing coupons
2. Not possible to delete coupons (in discount UI) that are referenced in orders without deleting orders first

Patch in #22 looks good.

I second patch #22. Works great, fixed this error for me that I was getting on the /cart and /checkout.

EntityMetadataWrapperException: Unable to get the data property coupon_id as the parent data structure is not set. in EntityStructureWrapper->getPropertyValue() (line 457 of /sites/all/modules/entity/includes/entity.wrapper.inc).

Confirming #22 patch worked for me to.

Patch #22 works for me too.

Patch #22 works great!