Prevent addresses from being removed from orders when Ajax enabled buttons are used

I am encountering the same issue. It also occurs when adding a shipping line item.

Marking as critical since there is loss of data on user input.

We are also experiencing this bug.

I've been investigating this and I can confirm this is a regression introduced in commerce 1.16, and in my opinion, this seems to be a critical issue due to sometimes the editor gets a 500 error, or some others could delete addresses.

What I've compared is both versions working in the same installation with the following structure:

modules>contrib
    - commerce
    - commerce1_15
    - commerce1_16

the commerce directory is a symbolic link pointing 1.15 or 1.16, depending on what I want to verify.

The steps to reproduce I do, in our custom, but simple, commerce installation are:

1. admin/commerce/orders/123123/edit
2. add a new line item
3. Save order

After this, I get the line item correctly added with commerce 1.15, but the following error with 1.16:

EntityMetadataWrapperException: Uncaught exception 'EntityMetadataWrapperException' with message 'Unknown data property commerce_customer_address.' in xxx/sites/all/modules/contrib/entity/includes/entity.wrapper.inc:354
in EntityStructureWrapper::getPropertyInfo called at xxx/sites/all/modules/contrib/entity/includes/entity.wrapper.inc (411)
in EntityStructureWrapper::get called at xxx/sites/all/modules/contrib/entity/includes/entity.wrapper.inc (432)

The error is in the commerce_order.forms.inc:285

// Notify field widgets.
  field_attach_submit('commerce_order', $order, $form, $form_state);

Before the execution of this function, the $order object has the commerce_customer_address field, but after it, it's removed, so then both addresses are empty

And inside the field_attach_submit, the error happens in:

  _field_invoke_default('extract_form_values', $entity_type, $entity, $form, $form_state, $options);

I've made a diff between the two order objects that are sent into the field_attach_submit and here is the only difference, the Commerce 1.16 seems to add/activate the revision:

diff Order\ 1.15.txt Order\ 1.16.txt 
1c1
< Order 1.15
---
> Order 1.16
286c286
<    'created' => '1637131493',
---
>    'created' => 1637131493,
437a438
>    'revision' => true,

Don't know if this makes sense or not, but I couldn't find anything different or strange in the new code added. I'm a bit lost yet on what is happening.

It appears as though this regression was introduced in https://www.drupal.org/node/2274833. I narrowed down the issue that causes the EntityMetadataWrapperException when adding a line item via the admin order edit form to the handling of empty profiles in commerce/modules/customer/commerce_customer.module. Reverting to the 7.x-1.15 version of commerce_customer.module, and more specifically just the code block on lines 1001-1008, allows us to to add line items on the admin order edit form and also prevents the removal of the billing and shipping info from the order.

Consider this only as a possible workaround for this issue until the maintainers can get into it.

diff --git a/modules/customer/commerce_customer.module b/modules/customer/commerce_customer.module
index c0dd3b046e..77b6a5de32 100644
--- a/modules/customer/commerce_customer.module
+++ b/modules/customer/commerce_customer.module
@@ -998,14 +998,7 @@ function commerce_customer_field_widget_form(&$form, &$form_state, $field, $inst
     $profiles = commerce_customer_profile_load_multiple($profile_ids);
 
     if (empty($profiles)) {
-      if (!empty($form_state['add_profile_' . $profile_type['type']])) {
-        // If the "Add profile" button is clicked, create a new profile.
-        array_push($profiles, commerce_customer_profile_new($profile_type['type']));
-      }
-      else {
-        // Otherwise add an empty array to make the "Add profile" button show up.
-        array_push($profiles, array());
-      }
+      $profiles[0] = commerce_customer_profile_new($profile_type['type']);
     }
 
     // Update the base form element array to use the proper validate function.

Steve, thanks for digging into this, and terribly sorry to see the problem rear its head. It appears to be an issue with the way the new code interacts with the fact that the Ajax process uses #limit_validation_errors, causing form input for the profile reference field to be ignored. Apparently that's leading to the unsetting of the reference field values. Going to be digging into a solution to roll a new release ASAP. (Brant from Bluespark brought this to my attention ... lovely that it didn't turn up in a year of testing / use in our customer's production site. 🤦🏻‍♂️)

Also, fwiw, this happens even if you don't add a line item. It's the use of an Ajax button anywhere on the edit form that causes the data loss.

My hunch was correct, and the issue is resolved by changing the way the widget form and custom element validate handler function when processing the widget during a button's "limited validation errors" submission. The attached patch will let you add new line items just fine without causing data loss, and I'd appreciate a +1 review.

That said ... fiddling with this turned up another issue with the way the form is built. I discovered a scenario where adding two new profiles on an order edit form and canceling one will result in a loss of context, causing the second cancel button click to fail. At first I thought I needed to make the "Cancel" buttons more specific (i.e. via unique #value labels), but that didn't appear to resolve the matter. You can see that represented in the current patch. I'm not sure I'll leave that in once I find the ultimate solution ... it's related to this fix, so I'm inclined to solve the matter in this ticket, but if I can't sort it out this weekend, we'll commit just the data loss bug fix and worry about the cancelation UX fix in a follow-up.

Alright, one last patch that I think resolves all issues.

I couldn't make everything work without differentiating the button text. I don't mind being more specific in the UI, but I know this impacts translations. Better to have an ironclad UX imo, so in testing, please abuse the Ajax enabled buttons on the order edit form to see if you can produce a scenario where the wrong form elements are replaced post-Ajax.

The reason this was failing is the custom element validate handler for profiles was testing a false negative condition when it checked to see if the validation was occurring for a related Ajax operation. It only checked to see if a property existed in the $triggering_element without checking to ensure the $triggering_element was within the scope of the particular customer profile reference $element being validated.

After correcting for that, the $form no longer loses the customer profile specific cancel button, the absence of which was causing Drupal to wrongly assume the "Add line item" button had been submitted. We've seen this in other contexts where Drupal will just default to the first submit button on a page if it can't find an exact match. Not sure the rationale, but it's certainly tricky to trace down!

I gave #12 a good workout and it fixes both the original reported issue as well as the line item issue in #6. Thank you, Ryan!

Thanks, Steve. Committed!