When creating a "manual order", i.e. an order created by an admin for another user through admin/store/orders/create, I cannot attach to the order any address information
I can
- fill up the shipping an billing address fields manually
- select addresses in address book and have the fields filled up accordingly
- copy information form shipping to billing and vice-versa

However, when saving the order, the address data are not saved, all the fields are blanked whatever is the method for filling these data.
This fails silently, nothing in logs, no script error reported in the browser.

This is very annoying, making impossible to process an order.

I really cannot disable all modules and try one by one to find a possible conflict - more than 400 in this application. I will do my best to investigate, but at this point I don't know where to look.

Thanks for help

Files: 
CommentFileSizeAuthor
#7 uc_addresses6-uc_order-fix-1917666-7.patch632 bytesMegaChriz
PASSED: [[SimpleTest]]: [MySQL] 2,331 pass(es).
[ View ]

Comments

jvieille’s picture

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)

jvieille’s picture

Title:Cannot save address information in manual orders» Cannot save address information in orders

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:

<?php
  
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:

<?php
   
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,

jvieille’s picture

Status:Active» Needs review

Well, I might have solved this issue by chance:

<?php
   
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.

MegaChriz’s picture

I've tried to reproduce the issue you have on my local install with the latest dev of Ubercart Addresses installed and Ubercart 6.x-2.11, but I didn't encounter the same problem as you. Also, editing address values on the order admin pages is covered by the automated tests and these are passing (though only editing the billing first and billing last name is tested).

I really cannot disable all modules and try one by one to find a possible conflict - more than 400 in this application. I will do my best to investigate, but at this point I don't know where to look.

You could take a copy of your site to your local environment and try to debug it there. This way you'll keep the "real" site in a working state. Then, disable all modules except Ubercart Addresses and dependencies. If you can confirm Ubercart Addresses works in this state, you could start by enabling modules that extend Ubercart. Because you have a lot of modules, I would say you enable multiple at once (instead of one by one). This may get you there faster.

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,

During the checkout process, the address is saved into session, so it could be added to the address book later, when the customer completes checkout. In theory, other modules have a chance to add extra data to this address, which they can use when the address is saved. They could also prevent the address from being saved this way (again in theory, not tested).
I agree that the code comment is not very helpful. I'll add it to my to do list to make a more clearer description of why that happens there.

Thanks for trying to fix the issue. Save the address into session when editing the order seems like a workaround and it will fill up the database extra. I would like to know what's the cause of this issue, so I know what exactly I fix.

jvieille’s picture

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...

jvieille’s picture

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

<?php
   
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;
?>
MegaChriz’s picture

StatusFileSize
new632 bytes
PASSED: [[SimpleTest]]: [MySQL] 2,331 pass(es).
[ View ]

Thanks for your help on trying to debug this. That's much appreciated.

Thanks to the code you posted in #6, I now see something that doesn't look right:

<?php
$order_values
= $order;
$order = uc_order_load($order_values['order_id']);
?>

In the second line $order get's overwritten! I have suspicions that that could be the cause of this issue, though I have not tested it.

Attached is a patch that alters the name of the variable. Let me know if this helps.

jvieille’s picture

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...

jvieille’s picture

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.

MegaChriz’s picture

Just thinking about my Pressflow distribution - could it have an effect?

I have installed the Pressflow distribution, but I still couldn't reproduce the issue with it.

(getting from UcAddressesAddress object)

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....

Ubercart Addresses 6.x-2.x (and 7.x-1.x) has an API for adding extra address fields (Extra Fields Pane makes use of that API, for example). Values of extra address fields are expected within the UcAddressesAddress object. This object has private members you will not see with dpm() (appearantly). You could try dprint_r(); die(); instead.

Could not find the time today to dig in this deeper. Hopefully I will have later this week. On Thursdays I usually have some more time.

MegaChriz’s picture

I also find print_r_tree() a handy debug function:
http://www.php.net/manual/en/function.print-r.php#90759

jvieille’s picture

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):

<?php
   
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

MegaChriz’s picture

I still cannot figure out why taking this API way to just check the fields that are plainly exposed in $order.

With the field handler API other modules can add extra address fields for uc_addresses. These fields are shown on address forms, like in the order administration for example, and saved within a UcAddressesAddress object. Values for extra fields (added by other modules) may not always exists directly on the order object, so that's why the check is done on the UcAddressesAddress object instead.

I'll try to come up with a list of things you can try to narrow down the list of possible causes later today.

jvieille’s picture

Priority:Critical» Normal

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...

MegaChriz’s picture

A list of possible causes:

  • Environment (PHP/MySQL/Apache versions/settings)
    I ought it unlikely, but differences in environment could theorically lead to different results.
    • Try to install a new Drupal website on the same environment with only Ubercart Addresses, the "required" Ubercart core modules and dependencies enabled. Check if you have the same issue when creating or editing an order. If not, then the issue is probably not caused by the environment.
    • Which version PHP version do you use?
    • Which database connection and version do you use?
  • Files of module(s) are changed/patched
    It can happen that if you applied patches to Ubercart Addresses or Ubercart that some other piece gets broken. It's also possible a part of a patch failed to apply, which can lead to unexpected results.
    • Download the latest 6.x-2.x-dev of Ubercart Addresses and Ubercart 6.x-2.11 (even if you already done before). Replace the existing module folders on the site the problem is on (this may be a copy of the site). Check if you have the same issue when creating or editing an order. If not, then the issue is caused by (unofficial) changes to the modules.
  • Two copies of the module are on the same install
    It can happen that when you move module folders around, a module folder accidently ends up in the folder of an other module. The website may load an other version of the module than you might expect in this case.
    • Go to the available updates page in your website (admin/reports/updates) to check which version of Ubercart Addresses and Ubercart are loaded. It should be Ubercart Addresses 6.x-2.0-alpha2+8-dev and Ubercart 6.x-2.11.
    • Modify code in both Ubercart Addresses and Ubercart (for example a simple die(); in the .module-file) and check if these modifications have effect on the website.
  • Module conflict
    Two or more modules affect the same piece of functionality and do not work (well) together. I ought this possible cause to be the most likely.
    • I assume you already narrowed down this possible cause. If not, then try to disable all modules except the one that Ubercart Addresses needs to function. If you don't have the same issue when creating or editing an order, then the issue is likely to be a module conflict. It can still be caused by a bug in Ubercart Addresses, though (but a so called "sleeping" one that wakes up when a particular other module is used).
  • Alternate settings (or misconfiguration)
    Similar as the previous one. Ubercart Addresses and/or Ubercart is setup in a way that is not tested and I didn't thought of. I ought this possible cause to be less likely.
    • Try to reset all settings to the default and see if the problem still exists. Since this may take a lot of time to test, you'd better only try this when all other cases noted above are counted out.

This is what I can think of so far. Hopefully, this will bring us closer to the cause of the issue.

MegaChriz’s picture

Status:Needs review» Postponed (maintainer needs more info)

jvieille, are you still interested to search for the cause of this issue? Else, I'm afraid I have to close this issue as 'cannot reproduce' and then your issue will probably come back if you later update the module to a newer version. See #15 for things you can try to narrow down the list of possible causes.

jvieille’s picture

Priority:Normal» Minor

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.

jvieille’s picture

Status:Postponed (maintainer needs more info)» Closed (cannot reproduce)

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...