A strange issue this whereby the more products you add to the order (via the the admin order edit screen) the slower the AJAX response becomes, until you hit around 9 or 10 products at which point the request times out.

After some debugging I figured out the size of the form being built was growing exponentially with each product that was added, as the form was being pushed into the cache_form table this was causing speed issues as we end up pushing around several MB worth of form data ...

I've traced this down to the ship_to and bill_to order panes in the uc_order.order_pane.inc file - each time the form is being built, all the form_state['values'] are being added to the #default_value for the form element, when actually I *think* we should just be setting the default_value to be $form_state['values']['ship_to'] (or bill_to) ...

For example, I think

$form['ship_to'] = array(
        '#type' => 'uc_address',
        '#default_value' => isset($form_state['values']) ? $form_state['values'] : $order,
        '#required' => FALSE,
        '#attributes' => array('class' => array('uc-store-address-field')),
        '#key_prefix' => 'delivery',
);

should actually be

$form['ship_to'] = array(
        '#type' => 'uc_address',
        '#default_value' => isset($form_state['values']) ? $form_state['values']['ship_to'] : $order,
        '#required' => FALSE,
        '#attributes' => array('class' => array('uc-store-address-field')),
        '#key_prefix' => 'delivery',
 );

This at least stops the form from taking over the world, and it seems to not have broken the rest of the order handling :)

CommentFileSizeAuthor
#3 1347650_order_form_values.patch1.09 KBIsland Usurper

Comments

longwave’s picture

Assigned: Unassigned » longwave

I have a suspicion that the checkout form address panes suffer from the same issue.

Island Usurper’s picture

$form_state['values']['ship_to'] shouldn't have anything in it, because that form doesn't have #tree = TRUE. Now, maybe setting the #default_value to be the given form values isn't that important, but I think it's good to realize what that change actually does. If we always set it to be the $order instead, does that cause problems?

Island Usurper’s picture

Status: Active » Needs review
StatusFileSize
new1.09 KB

Since the checkout form just uses the $order object, I don't think it would hurt anything. Changing addresses still works as expected. Hooray for objects being treated as references.

longwave’s picture

Assigned: longwave » Unassigned
Status: Needs review » Fixed

Committed #3.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.