Issues with the uc_addresses_address element [#1471500]

The form element "uc_addresses_address" is used to construct an address form element. When the form element is processed, it will present a collection of address fields. An UcAddressesAddress instance will be associated with this form element and it's values will be used to prefill the address fields.

The problem

There is a problem with the associated address instance when the address form is updated via AJAX. If there's no address instance assigned in the form definition, uc_addresses_process_address_field() automatically assigns a new address. But when the form gets restructed via an AJAX call, a new address is assigned again. This results in having an address assigned with an other ID. This causes issues with the zone field, which uses the address ID in it's wrapper div, so multiple address forms at a single page should can be used.

The proposed solution

I think attaching the address ID (as a hidden field) to the form would be a good idea. The function uc_addresses_process_address_field() should then have to look for an address ID when it doesn't receive an address. The attached address should be saved in the form state too, so it gets cached and is retrieveable when the form gets restructed.

After this issue is fixed, it should be easier to fix #1424038: Order administration: "copy address" feature.

Comment	File	Size	Author
#3	uc-addresses-address-element-fix-1471500-3.patch	21.49 KB	MegaChriz
#3
#1	uc-addresses-address-element-fix-1471500-1.patch	20.09 KB	MegaChriz
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

MegaChriz CreditAttribution: MegaChriz commented 7 March 2012 at 15:50

Status:

Active

» Needs review

File	Size
uc-addresses-address-element-fix-1471500-1.patch	20.09 KB

I think this patch should fix all zone issues, but it's possible it breaks compatibility with other Ubercart modules. The most important difference is that #parents property of address fields is no longer overridden, while overriding does happen in Ubercart in uc_store_process_address_field() in uc_store.module (± line 410):

    // Set common values for all address fields.
    $label = uc_get_field_name($base_field);
    $element[$field] = $subelement + array(
      '#title' => $label ? $label : '&nbsp;',
      '#default_value' => $value[$field],
      '#parents' => array_merge(array_slice($element['#parents'], 0, -1), array($field)),
      '#pre_render' => array('uc_store_pre_render_address_field'),
      '#access' => $element['#hidden'] ? FALSE : uc_address_field_enabled($base_field),
      '#required' => $element['#required'] ? uc_address_field_required($base_field) : FALSE,
      '#weight' => (isset($weight[$base_field])) ? $weight[$base_field] : 0,
    );

The patch removes '#parents' => array_merge(array_slice($element['#parents'], 0, -1), array($prefix . $fieldname)), in order to make the address ID traceable in $element['#value'], else $element['#value'] is empty.

On other thing to note is that it breaks the order administration test case, but the order administration feature doesn't work like it should anyway and is being worked on in #1424038: Order administration: "copy address" feature.

I leave it here as a patch to review for the moment, but I will commit it if I don't find any other issues with it.

Comment #2

Michael-IDA CreditAttribution: Michael-IDA commented 9 March 2012 at 17:33

Hate doing multiple patches...

[uc_addresses]$ patch -p1 < uc-quote-incompatibility-fix-1421720-27.patch
patching file handlers/ubercart.handlers.inc
patching file uc_addresses.install
patching file uc_addresses.module
[uc_addresses]$ patch -p1 < uc-addresses-order-form-1424038-11.patch
patching file uc_addresses.module
Hunk #1 succeeded at 1132 (offset 24 lines).
patching file uc_addresses.ubercart.inc
[uc_addresses]$ patch -p1 < uc-addresses-address-element-fix-1471500-1.patch
patching file handlers/UcAddressesFieldHandler.class.php
patching file handlers/ubercart.handlers.inc
Hunk #1 FAILED at 107.
Hunk #2 succeeded at 146 (offset 9 lines).
1 out of 2 hunks FAILED -- saving rejects to file handlers/ubercart.handlers.inc.rej
patching file templates/uc-addresses-form.tpl.php
patching file tests/UcAddressesTestCase.class.php
patching file tests/uc_addresses.checkout.test
patching file tests/uc_addresses.order.test
patching file tests/uc_addresses.register.test
patching file uc_addresses.module
patching file uc_addresses.pages.inc
patching file uc_addresses.ubercart.inc

working on all three, post more later,
Sam

Comment #3

MegaChriz CreditAttribution: MegaChriz commented 12 March 2012 at 10:08

File	Size
uc-addresses-address-element-fix-1471500-3.patch	21.49 KB

I've updated the patch, so that it works with the latest revision. I've found no (new) issues with it so far.

Comment #4

MegaChriz CreditAttribution: MegaChriz commented 21 March 2012 at 13:20

Status:

Needs review

» Fixed

I've done some testing in combination with uc_quote, and shipping costs are still updating, thus the changes do not affect uc_quote functionality. But it may conflict with other Ubercart modules I've not tested.

The patch posted in #3 has been committed with one slightly modification: the form element for "debug purposes" has been commented out again. Now we can continue with #1424038: Order administration: "copy address" feature.