Redesign the address book page using Views

I'm aware of this behavior. When I redesigned the address book for the 6.x-2.x version I took the address book from Magento as an example. The first time when you enter an address there it get's also saved as both the default billing and default shipping address. When you edit one of them, also both are updated, as it is the same address.

Now that I think of it, it can indeed be confusing that the same address is presented twice and having one edited also updates the other.

I'm not in favor of saving the address to the database as two records. In most cases, the customer would only have one address and it's not a good practice to have the same information multiple times in the database.

A checkbox "My billing information is the same as my delivery information" sounds like a better option, although the code can become more complex.

Perhaps we should make a change the UI, so it becomes clear the default shipping address and the default billing address is the same address (when having one address). I'm not a hero in UI-designing. Maybe you have a good idea about it?

I didn't understand what you meant to say with "upon editing either default, only it's updated."

Let me present an opinion. Caveat: I haven't tried out the 7.x release, so this might not be relevant.

A customer arrives at checkout. I'm assuming that 7.x still has a feature that fills out the address with the customer's default address? The customer now edits the billing address (or the shipping address—it doesn't matter which). Has he created a new address or edited his default address?

You may have added a checkbox that lets the customer state whether this is a new address or an edit to an existing address. That was a feature I wanted to include but never got around to.

If the customer checks the billing = shipping address option, then it would make sense to point both to the same address in the UC Addresses database. If the option is not checked, then a change to one (and not the other) could create some real confusion for the customer if the two addresses remain linked. Once unlinked, the two should act as though they were independent.

The UC Addresses 6.x-1.x model was pretty simple and fairly fool-proof, perhaps even close to optimal. In checkout, you never had the opportunity to edit or delete an address, only to create new ones or select existing ones. Editing and deleting were features available only in the profile. If a billing or shipping address matched a record in the database, then they would point to that address. If they didn't, then a new address would be added.

To avoid adding a lot of mistyped addressed, UC Addresses 6.x-1.x would add an address only when the order had been confirmed.

This algorithm ensured that the customer never had more than one data record per unique address. The customer never had to decide at checkout time whether a change to an address field was an edit or an add. As I said, most of the time this resulted in optimal storage allocation. There are a few fringe cases where you wind up with an address you no longer need, but in this rare cases, you could go into your profile to clean up your addresses.

One final note: in 6.x-1.x, the profile code would not allow you to create or edit an address that would create a duplicate address in the database. This was necessary to support the above logic.

I hope this explanation proves somewhat helpful in deciding how to handle addresses at checkout.

Hi Tony, welcome back!

This issue is about the presentation of the address book (for example: user/1/addresses). Currently, when a customer adds his/her first address to the address book it will be marked as both default billing and default shipping. In the address book the address is presented twice; I had taken over this presentation model from Magento. When you edit the default billing address, the default shipping address will also be updated, because it is the same address. This could be confusing for customers as it now looks like they have two addresses, but instead they have only one.

In this example a customer has two addresses. One address is marked as both default shipping and default billing. When you click "Edit address" for the default billing address, you are also editing the default shipping address in this case. (To have a default shipping address that is different from the default billing address, the customer should mark an other address as the default shipping or the default billing but not both.)

@Sam-Inet
I don't like to have the two records setup as the default behavior for all users of this module. At least, not until we've thought well enough what's the best way to fix this issue.
To achieve the desired results, you can also implement the Ubercart Addresses "presave" hook to prevent the first address from being marked as both default billing and default shipping.
This hook implementation should create two records when a customer adds his/her first address:

/**
 * Implements hook_uc_addresses_address_presave().
 *
 * @param UcAddressesAddress $address
 *   The address object
 *
 * @return void
 */
function mymodule_uc_addresses_address_presave($address) {
  // Don't allow addresses to be both default billing and default shipping
  // when the customer creates his/her first address.
  if ($address->isDefault('billing') && $address->isDefault('shipping')) {
    // Check if the customer has any addresses
    $uid = $address->getUserId();
    $addresses_count = db_query("SELECT COUNT(aid) FROM {uc_addresses} WHERE uid = :uid", array(':uid' => $uid))->fetchField();
    if ($addresses_count < 1) {
      // The customer has no saved addresses yet, so this must be his/her first address.
      // Copy the address data and save it as a separate record.
      $address2 = $address->copyAddress();
      $address2->setAsDefault('shipping');
      $address2->save();
    }
  }
}

Hi, MegaChriz,

Thanks for the clarification—I should have read the bug report more carefully. OK, here is my new suggestion:

Analysys

• The database should never have two identical address records for a user.
• The user interface should reflect the actual system behavior.
• In the system, the two default addresses are just pointers to data records. They can both point to the same record.
• The current representation leads the user to believe that the default addresses are each represented by an address record.

Suggestion

• The user interface should display all address records in the same way. In other words, they should not be separated into default addresses and other addresses.
• The user should be presented with a method for selecting which address should be the default billing address and which should be the default shipping address

Some ideas for doing this:

• You could provide a "Make this address the default billing address" and "Make this address the default shipping address" function by each address. If an address was already a default, the function would be removed and replaced with "This is the default billing address" and/or "This is the default shipping address". I think this would add a lot of clutter to the interface.
• You could use something similar to your current layout, but the default addresses would not have an "Edit address" option. Instead there would be some way of designating an address as a default. This could be done with a drop-down selection or by adding a "Select a new default billing address" function and going to another page where all the addresses are listed with a radio button selection.
• The default addresses could still be listed on the left and all the addresses (not just "Other") would be listed on the right. This creates some duplications.

So, one way of doing this without changing your current display too much would be to replace the "Edit address" for the addresses on the left side with a "Select default billing/shipping address" drop-down, change "Other addresses" to "All addresses" and list the all addresses on the right.

I would not recommend creating duplicate address records. As I described above, the 6.x-1.x approach for checkout relied on not having duplicate records. The code went to some effort to prevent duplicates. And when a user is selecting an address, wouldn't they be confused if they were presented with the same address twice?

I think that fix would cause more problems than it would solve, mainly because it doesn't reflect the reality that the default addresses are pointers to addresses in the users address book. If the user wants to change a default to another one of their addresses, they wouldn't be able to select an existing address—or if they were able to do so, they could get back to the case where both defaults pointed to the same address. If they had to retype the address, then they would have one address as a default and the same exact address as an "other" (plus, it would be a pain). The code would get full of exceptions and awkward workarounds trying to hide the true implementation.

UcAddressesAddressBook.class.php, line ~266

      // Check if address belongs to this address book
      if ($address->getAddressBook() !== $this && $address->isOwned()) {
        throw new UcAddressesInvalidParameterException(t('Tried to add an address already in an other address book'));
      }

If that is doing what I think it's doing, e.g. blocking a Wife from entering a home address that a Husband has entered, then can we talk about removing that code block?

No, this ensures that addresses belonging to user A don't end up in the address book of user B. This situation should never occur, but if it occurs, we don't want to let it happen.

Also, does uc_addresses have a nice helper function like "user_save(NULL, $NewUser);" ? {yeah, I'm being lazy... no, I don't want to inject straight through SQL, shoot me...}

You can use the address book API to add and save new addresses. To fill in the address fields you need to make use of the method setField() or setMultipleFields(). There is no shortcut function for saving addresses. Using the API should be easy enough.

In reaction to point 9 in #6: "address names should be required".
You can always make the address name field a required field in a custom module. The easiest way to do that is by implementing hook_uc_addresses_address_field_alter().

You have some of the best documentation I've ever seen on drupal.org.

Thanks for your compliments. I took me more than a month to write it. Some credits should also go to Tony, who has reviewed the documentation before I published it on drupal.org. With this documentation I hoped to make it easier for potential co-maintainers to step in. It seems the link to the documentation is not so conspicuous. I should give this more attention on the project page.

(...) when I'm finished with the user migration script (...)

Interesting, I once started with writing an extension for Feeds for the new Ubercart Addresses version, but it became quickly outdated while developing Ubercart Addresses and that's why I didn't published this extension yet. To discuss this further we'd better continue in #1309546: Address import / Feeds integration.

To go back on topic:
Tony or Sam, do you want to create a graphical design of the address book? You two seem to have a good idea about how it should look like. I'll focus on other aspects in the mean time (I'm currently busy with #1424022: Upgrade from Ubercart Addresses 6.x-2.x).

Tagging. Before changing the layout of the address book in the code, we first need a graphical design. Tony or Sam, you may post a proposal here. I'll focus on other aspects first.

Hi,

Here's a quick wireframe using the marvelous wireframe tool "Pencil" (http://pencil.evolus.vn/en-US/Home.aspx). There's nothing fancy here. I'm guessing that the checkout screen address selection is still a drop-down, so the drop-downs here should match. The default address(es) can be identified by a title (as shown) or maybe by some words where "Delete address" would go.

The Pencil wireframe file is included if someone wants to play with this in Pencil.

Looks good. The only thing that I miss here is a submit button in the "Default addresses" fieldset.
When looking at the layout, I get the feeling we should make a default view (made with the Views module) of the address book. Views has a layout style to display items in a grid. Unfortunelately, Ubercart Addresses 7.x-1.x (and 6.x-2.x) does not integrate with Views yet (but this is planned, see #1391216: Views integration for the 6.x-2.x and 7.x-1.x version).

Well, Pencil won't install, FF just tries to save it..

Go to http://pencil.evolus.vn/en-US/Downloads/Application.aspx and download the stand-alone version. No need to clutter up Firefox with an extension that's more of an application.

Sam, from your sketchy diagram, I'm not sure I follow exactly what you are proposing, but if you are saying that the user only gets to see one address at a time, then I vote no. I would like to see all my addresses at a glance and not have to select them one at a time to view them.

Other notes:

• Adding "Edit" to the default addresses would re-create the original problem noted in this issue.
• The drop-down requires either a page load or Ajax coding in order to make a newly selected address appear.
• When a default address is selected from the drop-down, the Delete address link should not be displayed.
• When the default is changed (using what I assume is a checkbox), there needs to either be a submit button or an Ajax operation (so that the change is recorded and so that the display of default addresses is updated).

Here's an alternative UI proposal. One of the things I hate about web apps is how they make you wait for page loads for each little change you want to make. In my uc_product_keys module, I allow people to edit and delete multiple product keys or product key sequences. The approach I used there is fine for admins, but not great for regular users. Using Drupal AHAH coding is a real pain. In this proposal, most everything can be done with straight Javascript code (no Ajax calls). The Javascript handles all state changes and all the changes are sent to the server with a single POST.

Here's an example of the initial view. The default addresses are highlighted in green.

Here is the same page after the user has made a few changes. First, they clicked "Delete" on the "Mom" address. The background color changes to pink, the "Delete" changes to "Undelete" and the other links are grayed out. Then the user clicked on "Edit" for the "Work" address. Each field is changed to a input field (the address may actually expand to multiple text fields rather than the single text box shown) and the "Edit" turns into "Reset". Pressing "Reset" would restore the entry to its original settings and display.

In the final example, the user has taken "Aunti Ann" and clicked on "Default Shipping". The background color turns green, the "Delete" button is grayed out, the "Default Shipping" turns black and the "Default Shipping" on the "Home" address turns blue.

Pressing Submit would now submit all these changes. In uc_product_keys, I perform all the changes except the deletions. I saved them up for a confirmation page, which shows up next. If the user confirms, the items are deleted.

I don't really expect the first version to get this fancy—just food for thought. It's not as hard as it looks to support all this except that the AHAH code needed for state/country fields might create some problems. Also, the display fields are more compact than the edit fields of an address, so there could be some resizing weirdness.

I don't think you could create this with Views.

@Sam-Inet

but if moving to Views, why waste the effort?

Sometimes you have to see things first, before coming to new ideas.

@freixas
While the last proposal posted in #18 looks awesome, I agree with Sam it's looks like it will costs many hours to implement it like that. There is also one problem with editing the address entry in one field. How would we know which part of the address is what? For example, we can't be sure if the last name is "Koala" or "de Koala". And displaying all address fields separately will probably resize a lot (as you already said). But thanks for your proposals! Could bring up some more ideas.

#14 looks the easiest one to implement and I like the idea of having a default view, so site builders will get more control about how they want the address book to look. I guess #1391216: Views integration for the 6.x-2.x and 7.x-1.x version would need some more priority then, although I'm afraid I won't find the time to do that one quickly.

I leave this one active for now, maybe we'll get some more ideas.

The overlay module (included in Drupal core) sort of already can make the edit form appear in a popup. When changes are made within the popup the whole page is reloaded when the popup is closed, though.

Hm, no more ideas for two weeks now. Then it's time to decide which path we will go.

If no one has objections, the design posted in #14 will be implemented with one difference: the default addresses fieldset will be omitted, at least for the first version. The design will be build with the Views module, which will require that the module has integration with Views first. That means this issue is postponed until Views integration is ready.
Views integration is planned for after the first alpha release, thus that means this issue will be scheduled for the second alpha release.

Steps for fixing this issue:
1. Implement full Views integration: #1391216: Views integration for the 6.x-2.x and 7.x-1.x version.
2. Implement a default View for the address book.
3. Adjust the Ubercart Addresses templates, remove the address book template.
4. Adjust uc_addresses_theme(), remove link to address book template.

Updated issue title.

Now that #1391216: Views integration for the 6.x-2.x and 7.x-1.x version is nearly completed (one feature is still in the process of being backported to the 6.x-2.x version), we can finally continue on this issue.

Wow! I took me more than a year to come to this point. First I made sure Ubercart Addresses' base features worked properly. Second, I studied for three months to get Entity API integration right. And third, I took two months to implement all Views plugins that were needed to able to create an access controlled address book page with Views.

Well, here is a patch that tries to come close to the design posted in #14. I would like your feedback!

I've updated the issue summary, so it's clear what's the state of this issue.

I would like reviews as this is quite a significant change.

@Sam-Inet
Thanks for your reply and that you want to try it out. I took me some time to make up a downloadable test site, but here is the link:
https://code.google.com/p/uc-addresses/source/browse?repo=1433034
I had used this repo in the past for working on uc_addresses.

The link will be temporary available, which means I will probably remove the repo when this issue reaches the 'closed (fixed)' status.

Alternatively, you could also download the latest versions of Drupal, Ctools, Views, Token, Rules, Entity API and Ubercart. And the 7.x-1.x+25-dev version Ubercart Addresses with the patch in #26 applied and setup a test site your self.

As I want to create a new release soon and this is one of the last issues that prevents me from creating the new release, maybe it's better to leave the View disabled by default for now. Attached is a new patch with small changes + an automated test that basically does access checks for the address book View. For example, users that have permissions to view only their own addresses should have no access to the Views address book page of other users.

Manual testing

I have tested the design in several themes. In some themes there was an ugly border around each item in a single address listing, like in the Blue Masters theme:

I have fixed this by setting border of 0px in the CSS for .list-address and .list-address td and then it looks like this:

In addition of the patch in #26, this patch adds/changes the following:

• The default address book View is disabled by default.
• An automated test.
• CSS-rule for setting border to 0px inside an address item.
• address-item-inner is renamed in list-address-wrapper.

If this passes tests, I think it's ready for commit.

I have already started with backporting the patch in #30 to 6.x-2.x and under while I saw something I forgot to include: the message "No addresses have been saved." when someone has no addresses in his/her address book.

Attached patch adds this (in addition of the patch in #30).

Committed #31.

Moving to 6.x-2.x.

Patch for the 6.x-2.x version.

Committed #33.

Setting issue status back to the version it was originally reported for and mark it as fixed.
Any improvements for the default View could be handled in follow ups.