Order administration: "copy address" feature

The path the address form is located is at
admin/store/orders/[order_id]/edit
where [order_id] is the ID of the order.

The file uc_addresses.ubercart.inc currently includes the code for generating the delivery/billing checkout panes and the ship_to/bill_to order panes. Ubercart Addresses has overridden the order panes by implementing hook_uc_order_pane_alter(). This hook implementation can be found in the uc_addresses.module file.

The address book select field is currently loaded with javascript as a separate form, thus it doesn't appear in the order pane callback uc_addresses_order_pane_address(). The original address book form uc_order_address_book_form() is defined in uc_order/uc_order.admin.inc. This form is altered by Ubercart Addresses in uc_addresses_form_uc_order_address_book_form_alter() in uc_addresses.module. Ubercart Addresses replaces the selectable addresses (that come from previous orders) by addresses from the address book.

The address book form should probably become part of the order pane callback uc_addresses_order_pane_address() just as is the case with the checkout pane callback uc_addresses_checkout_pane_address(). Parts of the form should be updated by the form API by using the #ajax property on the address book select field and the copy address checkbox.

I hope this information helps. If there are still some things unclear, feel free to ask.

But on the checkout page (with uc_quote module disabled, see #1421720: [$150 Bounty] Delivery information does not display in checkout) it's working with #ajax.

@MEsch, can you post (as a patch) what you got so far? I may be able better to track down the issue you ran into better in this case. On the checkout form, applying addresses from the address book works nicely, so I don't understand why yet this doesn't work in other forms.

I suppose you have adjusted uc_addresses_select_addresses() a little? This function now operates the "old way" when the context is "order_form", thus with JSON code as the key of each address. It should use address ID's when the address book select field is part of the order form itself.

This (uc_addresses.module, ± line 1111)

switch ($context) {
  case 'order_form':
  $key = drupal_json_encode($address->getRawFieldData());
    break;
  default:
    $key = $address->getId();
    break;
}

should be replaced by this:

$key = $address->getId();

when implementing this feature as described.

Thanks for posting the patch (sorry for my late reaction).

I had assumed that the hint

I'd like to have the order administration form work in a similar way as the checkout form.

would have been enough to know the checkout form already does the trick, and with the uc_addresses_address element (which I should document in the developer's guide). At the other hand, I know very little about how it exactly works on checkout. Most of the working mechanism is copied from Ubercart and some Ubercart code is still involved in the process. That fact makes it harder to fix this issue. But because it works at checkout, it should be possible to work elsewhere too.

I'll take a look at your code and see where exactly you "hit the wall".

I have take a look at the patch posted in #9 and I see a lot of wrappers in there which make it confusing for me what exactly happens there. There are not that much wrappers at the checkout form. I suppose it is used as attempts to fix the problem.

I have also made an attempt or at least a start to implement this. I took uc_addresses_checkout_pane_address() as an example as that is working completely. At first, I noticed the same issue as Jay Matwichuk and MEsch, that the address never did update. But when I added the following:

$form[$pane] = array(
  '#tree' => TRUE,
);

things began slowly sort of working. That piece of code made sure the field "select_address" was named "delivery[select_address]" in the form HTML instead of just "select_address", which is harder to track down because we deal with two address forms on one page (delivery and billing). I've also replaced $uc_type by $pane for consistency (that's what the implementation for checkout uses).

The attached patch is capable of updating the address form when selecting an address from the dropdown. To concentrate at one piece at a time, I left out the functionality for copying an address from one pane to the other for now. Unfortunately, there are still some things that aren't working:

• In some cases, the zone field is not correctly updated when changing the country, don't know yet why.
• When just saving the form, I'll get the error: "Fatal error: Nesting level too deep - recursive dependency? in /Websites/drupal/drupalsites/drupal7/sites/all/modules/contrib/ubercart/uc_order/uc_order.admin.inc on line 1065". I saw an error like this somewhere in the Ubercart issue queue too, thus this could be a bug in Ubercart.

I think the changes in the patch gets us closer to the solution, but we aren't there yet.

I've examined two of the problems noted #11.

The "Nesting level too deep" error

The "Nesting level too deep" error is caused in the uc_order module: it tries to compare the uc_addresses array from two order objects. In the $order->uc_addresses array live two UcAddressesAddress objects and because an UcAddressesAddress object is connected with an UcAddressesAddressBook object, there is some cross referrencing which is causing the "Nesting level too deep" error.

Zone field issues

For the issues with the zone field, I've found out that when the address form is updated via ajax, the address form element is processed twice (I've tested this with a single address form). This causes the construction of a new address object with an other ID, while it would work for us at best if the form API just takes the address object from the (form) cache. The address ID is used in the zone wrapper element to allow a form to have multiple address forms at a single page. If an address gets a new ID everytime a form is updated, well, then you ask for trouble. I seem to have worked around this problem in all places an address form is used and we can do this for the order form too, but it would be better to deal with the same address object everytime when an address form is updated via ajax.

We could use a developer here who has a deep understanding of the Form API and especially knows what exactly the Form API does when forms are processed/constructed via ajax using the #ajax property.

I've opened the issue #1471500: Issues with the uc_addresses_address element to work on the zone field issues reported in #12. This is probably the same issue as Jay Matwichuk encountered in #4.

The "Nesting level too deep" error can be still handled here.

I've committed some changes to the 7.x-1.x branch. The patch posted in #11 still applies when using patch -p1 < uc-addresses-order-form-1424038-11.patch, but not when using git apply uc-addresses-order-form-1424038-11.patch.

@Sam-Inet
The patch isn't finished yet. It's intended to be used as a starting point for fixing this issue. Before we can seriously continue with this one, #1471500: Issues with the uc_addresses_address element need to be reviewed and fixed.

#1471500: Issues with the uc_addresses_address element is done now. We can now continue with fixing this issue.

Here is a new patch to be used as a starting point for fixing this issue. The differences with the patch posted in #11 is it's now in line with the changes done in the uc_addresses_address element (see also #1471500: Issues with the uc_addresses_address element).
It also fixes one of the errors noted in #14 (about undefined properties).
In my installation, selecting an address is working fine and somehow I don't get the "nested level too deep" error anymore (as reported in #12). I think this is no longer an issue since the fundamental changes in the uc_addresses_address element. It can also because I didn't do a wide test yet (with modules turned on and off).

The next steps for fixing this issue:
- Implement "copy address". This should not work exactly the same as on checkout. It should be possible to copy addresses in both directions without collapsing the forms.
- The address selector should be "hidden" by default. That is because the label for a selectable address can be very long and that can cause (very) wide address panes (see image).
- The automated tests should be updated (I will work on that part when the two first bullets are done).
- The fix should be tested with several configurations (for example: with uc_quote module turned on and off)

Marked issue #1529960: Addresses not saving when order created through backend as a duplicate of this one.

@st455
Ubercart Addresses 6.x-2.x and 7.x-1.x have support for adding extra address fields through the field handler api. I'm working on an example module to demonstrate that. Currently, the example module's documentation is a bit short and it also demonstrates only one type of field (a simple text field) and that's why I hadn't published it yet. However, I think I will include it with the Ubercart Addresses package this week, as it's quite useful to have an example, although it less good documented.
There is no UI for adding extra address fields, it's only possible to do so via the API. For Ubercart Addresses 6.x-2.x, Extra Fields Pane delivered that UI (in the form of supporting only a few field types), but I haven't had the time yet to port that module to Drupal 7. Ideally, the UI would be delivered by the Drupal 7 field API (or maybe it should even replace the Ubercart Addresses field handler API), but that is an idea for the far future. At this point, I'm not even sure if I will ever come to that point.

I'm currently working on this issue; I'm taking the steps as noted in #17. As it less easy as I had expected, I will commit the patch in #17 soon as it fixes also the now broken order administration section.

Patch in #17 is committed:
http://drupalcode.org/project/uc_addresses.git/commit/e070d2a
http://drupalcode.org/project/uc_addresses.git/commit/4170010

In two commits this time, I got interrupted when doing changes and forgot to remove debug code and doing a code review when I did the first commit.

New problem detected:
- When an order is created for a customer with no addresses in the address book, but with previous orders, selecting an address will result into errors: the familiar "Illegal choice" error (zone issues).

So, to sum up what needs to be done before this issue is fixed:
- Implement "copy address". Should work just like Ubercart one's except it should be done via Ajax.
- Hide the address selector by default, just like Ubercart does.
- Fix selecting addresses for customers with no addresses in the address book.
- Update automated tests (partly done now).
- Testing with several configurations.

Edit: duplicated comment.

@st455
There is no relation between the uc_addresses and uc_orders table. Addresses for an order should be stored in the uc_orders table.
I have added an example module to the 7.x-1.x version that demonstrates a way of adding address fields that will work with both the address book and Ubercart orders. Hopefully, you will find the answers there. If not, please open a new issue if you have further questions about extra address fields.

The following patch fixes the following for this issue:
- The return of the "copy address" feature, but now implemented via Drupal Ajax Framework/Drupal Form API.
- The address selector is hidden by default (just as is the case in Ubercart). Also done with Ajax technics.
- Selecting addresses from previous orders should work again.

Since this patch changes a lot of code, please test if it doesn't introduce any new bugs or rare side effects.

@pthurmond
The records in the uc_addresses table are pure used for the customer's address book and thus it make no sense to link them with the uc_orders table. Addresses for the Ubercart order are saved in the uc_orders table. This is the default Ubercart behavior and Ubercart expects the addresses to be there.

Hm, including images to a patch doesn't seem to work. Here's a new patch. The images are attached separately.

Again, this are the fixes:
- The return of the "copy address" feature, but now implemented via Drupal Ajax Framework/Drupal Form API.
- The address selector is hidden by default (just as is the case in Ubercart). Also done with Ajax technics.
- Selecting addresses from previous orders should work again.

Please test if there no new bugs introduced or other side effects.

Here's a new patch, with a few changes. The "copy address" feature didn't work at all in the last patch, because I accidentally removed a function that was still used as the ajax callback of the copy address function. For the rest some little fixes/cleanup to the address select field element, that's introduced to fix selecting addresses from previous orders. I would like to separate these two fixes, but since their so tight well together now it will take more time than I can spend now. I'm happy to hear opinions.

The dependency should be "uc_store", which is one of the modules Ubercart Addresses depends on. But "uc_store" is not the name of the project. Changing the dependency to "ubercart" wouldn't be right because that's not the name of a module Ubercart Addresses depends on. As I read the issue you posted a link to, it looks like Drush tries to download the dependencies, but doesn't have the sufficient information to do so, because drupal.org doesn't seem to provide enough information for Drush. I guess the easiest to do is to just downloading the dependencies manually. I have not much experience with Drush, but maybe Drush doesn't complain anymore when all the dependency modules are present?

I have committed a part of #29. This part introduced a new form element called "uc_addresses_address_select" and is there to fix the issue of selecting addresses from previous orders. I have moved this part of the issue, which is related but actually a different issue in #1701324: Selecting addresses from previous orders fails.

Here is a new patch, a sort of "re-roll" of #29. There are no new changes, just the fix for #1701324: Selecting addresses from previous orders fails has been removed from it.

The images folder in images.zip should be placed in the root of the uc_addresses module folder.

Removed the tags. The last piece for the order administration feature - the "copy address" feature - will be implemented later.

@jvieille
This issue is about selecting addresses from the address book in the order administration for the 7.x-1.x version. This works differently than for the 6.x-2.x version. For the 7.x-1.x version, Drupal's AJAX framework is used to apply addresses. In the 6.x-2.x version this is done with just javascript.

One piece that is missing for this issue is the ability to copy addresses from the shipping to the billing pane (and vice versa). This feature is originally provided by Ubercart, but Ubercart's version doesn't work with extra address fields as it uses "plain" javascript and only cares for the fields it knows. That's why the feature is removed at the moment with Ubercart Addresses installed. It should be replaced by a solution using Drupal's AJAX framework, but I haven't been able to figure out a neat solution. The patch in #33 needs extra CSS in order to style the copy button similar as in Ubercart, but this CSS may be to aggressive: hard to override by themers. A solution using a simple link is not supported by Drupal's AJAX framework, because there wouldn't be a fallback when javascript is disabled.

I never liked the fix in #33 where it was needed to add images to uc_addresses that are already present in Ubercart. I thought a long time how to come up with a nicer fix that doesn't require the extra images. I finally came up the idea to fix the problem the way the Rules module did: on some places in Rules UI, a smaller button is shown:

The following patch implements the copy address button, but doesn't include all the features from the patch in #33.
An overview of the changes:

• A copy address button is placed below the address book select field.
• The code checks if it expects the other pane to be present and only shows the copy address button if this is the case. For example: if the bill_to pane is disabled, no copy address button is shown in the ship_to pane.
• Order panes are renamed from "delivery" and "billing" to "ship_to" and "bill_to" respectively, just as usual in Ubercart.
• CSS is added to style the copy address button the same as in the Rules UI.

Plus, compared to the patch in #33:

• CSS for styling the copy address button as an image has been removed.
• Images are removed.
• The address selector is always shown.
• The button for showing/hiding the address selector is removed.

This is how the address panes on the order admin screen now look like:

This needs some manual testing to ensure all address related features still work (address selecting, zone selecting, copying addresses, etc.). But let first see if it passes the automated tests.

In addition of the patch in #37, this patch adjusts the order test case: it informs the test that the element in the form has changed from "billing" to "bill_to".
No other changes were made.

I manually tested address selecting, zone selecting and copying addresses and didn't encounter issues with it. I even tested together with Extra Fields Pane. In combination with that module copying addresses doesn't work properly (extra field values aren't copied over), but that should be fixed in Extra Fields Pane which will I do later.

Committed #39.

A patch for Extra Fields Pane is available in #1997842: Adjust order administration. It will be committed after a new release of Ubercart Addresses 7.x-1.x is made.