Cannot save address information in orders

Making some tests
- If I disable uc addresses, I can set and save the address fields with the order normally
- when I enable again uc addresses, changing the fields manually or selecting an address does not change anything. The data set when uc addresses was disabled are not affected.
- the behavior is the same if I edit an existing "normal" order (created by the user to be invoiced and shipped to)

so it is not specific to manual orders.

Thinking about it, I am wondering if this might be an untested non-functional feature as one can only notice in 2 circumstances that many do not encounter:

• Creating orders for other
• Modifying customer information on existing orders

Because everything works fine in checkout or addresses CRUD but not in order edition.

Looking at the code, I have tried to compare the processing of address panes in checkout and order.

in uc_addresses_order_pane_address() which does not save address info in order:

   case 'edit-process':
      $order_values = $order;
      $order = uc_order_load($order_values['order_id']);
      $address = $order->uc_addresses[$type];
      $changes = array();

      foreach ($order_values[$uc_type] as $key => $value) {
        // Set value
        $order_values[$key] = $value;

        // Check if the value was changed
        $fieldname = substr($key, strlen($uc_type) + 1);
        try {
          if ($address->getField($fieldname) != $value) {
            $changes[$key] = $value;
          }
        }
        catch (UcAddressesException $e) {
          // Ignore any Ubercart Addresses exceptions.
        }
      }

in uc_addresses_checkout_pane_address () which saves address info in order:

    case 'process':
      $uc_type = $type;
      if ($type == 'shipping') {
        $uc_type = 'delivery';
      }

      foreach ($values[$uc_type] as $fieldname => $value) {
        $order->{$fieldname} = $value;
      }

      $address = $values['uc_addresses_address'];
      if ($address->isNew() && !(isset($values['copy_address']) && $values['copy_address'])) {
        // Set flag that this address may be saved to the addressbook
        $address->save_address = TRUE;
      }
      if (isset($values['copy_address']) && $values['copy_address']) {
        $address->copy_address = TRUE;
      }
      else {
        $address->copy_address = FALSE;
      }
      $order->uc_addresses[$type] = $address;

      // Save address into session
      $_SESSION['uc_addresses_order'][$order->order_id][$type] = serialize($address);
      return TRUE;

In the first case, the " // Save address into session" thing is not present.
Well, I don't know the logic, so this might be irrelevant,

Well, I might have solved this issue by chance:

    case 'edit-process':
      $order_values = $order;
      $order = uc_order_load($order_values['order_id']);
      $address = $order->uc_addresses[$type];
      $changes = array();

      foreach ($order_values[$uc_type] as $key => $value) {
        // Set value
        $order_values[$key] = $value;

        // Check if the value was changed
        $fieldname = substr($key, strlen($uc_type) + 1);
        try {
          if ($address->getField($fieldname) != $value) {
            $changes[$key] = $value;
          }
        }
        catch (UcAddressesException $e) {
          // Ignore any Ubercart Addresses exceptions.
        }
      }
+      // Save address into session
+      $_SESSION['uc_addresses_order'][$order->order_id][$type] = serialize($address);
      return $changes;

This works now.

You are true, the change does not help.
It looks like it saves address data, but it is not. They only appear on order and invoice in the current session, but are not saved in the database.

I am lost...
I tried to disable any module that could conflict with no success.
Just thinking about my Pressflow distribution - could it have an effect?

Here is the xhprof output - this link centered on uc_addresses_order_pane_address calls:
http://xhprof.shareontheweb.com/?run=511f5f043bea2&symbol=uc_addresses_o...

Thank you very much for helping ùe sorting out this bad issue...

Could you please look at the code. It seems to me that there is something wrong.
I made extensive checks with dpm, commented below :

    case 'edit-process':
      $order_values = $order;
      $order = uc_order_load($order_values['order_id']);
      $address = $order->uc_addresses[$type];
      $changes = array();

dpm ($order_values);      // values in the form appear here i.e. the values from the selected addresses - correct
dpm ($order);                  // values from the database are here i.e. address fields are empty - correct
dpm ($address);             // reports "ucxf_pane_type (String, 14 characters ) extra_delivery"
dpm ($order_values[$uc_type]);   // the address value for the address pane being processed  - consistent with above
      foreach ($order_values[$uc_type] as $key => $value) {
        // Set value
        $order_values[$key] = $value;
dpm ($value);                 // The address values entered - correct
        // Check if the value was changed
        $fieldname = substr($key, strlen($uc_type) + 1);
dpm ($fieldname, "fieldname") ;    // the field name being checked - correct
dpm ($address->getField($fieldname));  // the exact same value than $value above!!! WRONG - this code seems to catch the changed values, not the one in the database (from $order)
        try {
          if ($address->getField($fieldname) != $value) {
            $changes[$key] = $value;
          }
        }
        catch (UcAddressesException $e) {
          // Ignore any Ubercart Addresses exceptions.
        }
      }
dpm($changes);    // of course, no change, so no values saved

      return $changes;

I tested this patch, but no success.
The saved address object still returns the changed field values.

$address = $saved_order->uc_addresses[$type];
returns

... (Object) UcAddressesAddress
ucxf_pane_type (String, 14 characters ) extra_delivery

and

$address->getField($fieldname)

returns values from changed order, not from saved order, the same as
$order_values[$key] = $value;

quite puzzling...

Actually, I don't get the logic.
I would think that the data from the saved order would be simply taken from the delivery_* and billing_* fields in $saved_order, those we see on the panes before we begin to select an other / new address or change fields manually.

Instead, this code takes a convoluted way, using the structure

uc_addresses (Array, 2 elements)
    shipping (Object) UcAddressesAddress
        ucxf_pane_type (String, 14 characters ) extra_delivery
    billing (Object) UcAddressesAddress
        ucxf_pane_type (String, 13 characters )

within $saved_order.

I really don't know what it gets from this indirect way, but there might be chances that this does not match what is in the panes, coming directly from the uc_order database....
As this is certainly intended, something might be corrupted, bringing data from the changed address instead of the saved order.

I still cannot figure out why taking this API way to just check the fields that are plainly exposed in $order.
I gave a try by simply checking the field values in $order (I reverted the $save_order patch):

    case 'edit-process':
      $order_values = $order;
      $order = uc_order_load($order_values['order_id']);
-      $address = $order->uc_addresses[$type];
      $changes = array();

      foreach ($order_values[$uc_type] as $key => $value) {
        // Set value
        $order_values[$key] = $value;

        // Check if the value was changed
-        $fieldname = substr($key, strlen($uc_type) + 1);
        try {
+         if ($order->$key != $value) {
-          if ($address->getField($fieldname) != $value) {
            $changes[$key] = $value;
          }
        }
        catch (UcAddressesException $e) {
          // Ignore any Ubercart Addresses exceptions.
        }
      }
      return $changes;

This works for now, I don(t now about possible drawbacks.

Thanks for your active support

Thanks MegaChriz, I am a little bit relaxed to have a temporary fix.
I set the issue to normal as we can now process our manual orders...

Yes, I will continue to investigate, but because I found a temporary fix, I moved to more urgent work.
Thank you very much for the detail hints on how to investigate.
So please forget it until I am back...
Downgraded to minor.

I tried to revert my changes, and it is working now.

I don't now if it is Ubercart or Drupal fault, but the coding is so clumsy that all this seems to work by chance...
I can see some functions be called 20 times when one should be exactly enough...