Hello, guys
I am new to the commerce module, when i download the newest module which release in data 2012-Sep-22. I find one issue.
It will create duplicate billing information items if i check the checkbox "copy chipping information to billing information" and did not choose the existing address file.
Since many duplicate billing information items is annoying. Is it possible do not create a new billing information item when i check the checkbox and didn't choose any exist billing information item?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

Component: Developer experience » Checkout
Assigned: Gaofengzzz » Unassigned
Category: bug » support
Status: Needs work » Closed (works as designed)

If I'm not mistaken, what you're describing is the default behavior of Commerce, but I can't really tell. Yes, a new profile will be created each trip through checkout unless you're using Commerce Addressbook, but in my experience (corroborated by some quick local testing) you should not end up with multiple customer profiles from a single trip through checkout, even if you go back and forth toggling and untoggling the address copy checkbox as you go.

I'm moving this to "works as designed" unless you have some set of steps I can follow to reproduce the issue on a single trip through the checkout form.

brephraim’s picture

Status: Closed (works as designed) » Active

I am also having this issue, but with Commerce Addressbook installed.

To recap:

A customer has an existing billing address and an existing shipping address stored in Commerce Addressbook. The customer places a new order, selects their old billing address, and has the checkbox selected to use it for their shipping address. Meanwhile, they still have the --Choose-- option selected on the shipping address dropdown box. Now, even if the selected billing address matches the shipping address already stored, it will create a duplicate shipping address entry.

While this isn't critical, I'm sure it could become quite annoying for a customer.

rszrama’s picture

Can I get a screenshot? It sounds like the Addressbook module will just need to be updated once Commerce 1.4 comes out to disable the address copy option if an address from the addressbook is used.

bojanz’s picture

Addressbook will definitely need an update, nothing was done to make it compatible with the new feature in -dev.

brephraim’s picture

FileSize
50.06 KB
56.93 KB

Submit the order with the configuration show in step1.jpg, yielding the results in result.jpg on the next order.

rszrama’s picture

Title: It will create duplicate billing information items if you check the checkbox "copy chipping information to billing information". » Update after Commerce 1.4 to work alongside customer profile copying
Project: Commerce Core » Commerce Addressbook
Version: 7.x-1.x-dev » 7.x-2.x-dev
Component: Checkout » Code
Category: support » task

Specifically, if the customer has chosen to use an existing address, we should suppress profile copying if possible to prevent it saving a duplicate profile. It should be possible through the #ajax refresh to disable the copying checkbox entirely.

B-Prod’s picture

I just created a patch to hide the select box when the "same address" option is checked: Issue #1427264 Comment 12.

Still have to avoid duplicates.

rszrama’s picture

I don't think it's enough to simply hide this form element. We should likely just remove it entirely when the pane form is rebuilt and nullify its value. Not entirely sure; haven't played with the combination yet.

B-Prod’s picture

You totally right, since this still causes some troubles with existing profile: the duplicate address is stored on the existing profile defined by the address book select box, instead of been added to a new profile.

I modified this on the original issue rather than here (comment 13).

rszrama’s picture

Why don't we just close that one as "won't fix" since the Shipping module doesn't have that functionality any more and continue on in this one?

brephraim’s picture

#9: Unless I am missing something, the whole point is that the duplicate address should NOT be added to a new profile at all.

jsacksick’s picture

I'm not sure this issue really belongs to commerce addressbook as there's actually no code that deals with commerce customer profile copying.

B-Prod’s picture

Project: Commerce Addressbook » Ice Commerce
Version: 7.x-2.x-dev »

I am afraid the problem seems complicated, but not really dedicated to Addressbook module, but to the Commerce Customer module.

When using the "same address" checkbox, the source profile is used (no duplication), but the other profile is created from the source profile, so is duplicated if it existed already.

The module should check if a profile contains the same data than the source profile before creating a new one.

rszrama’s picture

Project: Ice Commerce » Commerce Addressbook
Version: » 7.x-2.x-dev

Please re-read my comment #6: the problem is precisely that this module doesn't take profile copying into account. : )

It needs to as of Commerce 1.4.

Cellar Door’s picture

I'm seeing this as well on a kickstart install. If you need patch testing on any solutions let me know.

Also in a semi related nature - what's a good way to bulk remove duplicates. Our test user has about 50-60 billing address profiles now :)

salbertz’s picture

I had the same problems with duplicate shipping addresses when "My shipping adress is the same as my billing address" is selected, while the addressbook selection remains on "-choose-" (and thus creates a new address). But the latter fact simply arises from the fact, that, after a regular purchase process, so far no default address has been selected - and thus the address selector will display "-choose-" on the next visit also and the same happens again and again...

As soon as you visit "My Account => Address Book" and specify a (shipping) address to be "set as default" this behavior seems to disappear. On the next visit the default address is selected automatically and there seem to be no more duplicates.

The behavior is not 100% perfect though, since the coexistence of the "use billing address as shipping address" and "select shipping address" is still logically inconsistent: if you enter a new billing address, it will be correctly copied as a new shipping address (if that option is selected) but the shipping adress selector still displays the old default shipping address - at least until the next submit. (The perfect way would be a realtime change via ajax.)

But at least to me a simple fix of the most annoying consequences of this copy/select-compatibility conflict would seem to simply set the first (billing and shipping address) being entered for a customer as default (there seem to be issues about that, e.g. http://drupal.org/node/1799958).

vasike’s picture

Status: Active » Needs review
FileSize
894 bytes

here is patch that change the addressbook selection order in checkout profile form and hides it if the copy option it's present and checked.

rszrama’s picture

Hmm, it looks like you took the opposite approach to what I recommended in #6, but I don't see why we can't have it both ways. With your patch as is, would it still create a duplicate profile if they chose an Addressbook address first and then clicked the profile copy checkbox?

vasike’s picture

duplicates still there.
but duplicates are made in Drupal commerce if the same data is entered twice.
some maybe we can have some validation in Drupal commerce about profiles duplicates. what do you think?

rszrama’s picture

I'm not talking about the expected duplicates but rather the unexpected ones that this issues is trying to mitigate. There shouldn't be any issue with profile duplication in Commerce core itself; we just needed to make sure that the Addressbook module and profile copying don't both act on the same time for an order's customer profile reference field.

jsacksick’s picture

Status: Needs review » Fixed

I commited a fix in dev that puts the default value to 0 for the profile copy checkbox when there's a default address selected.
When checking the checkbox, it flushed the address selected from the order, the $form_state['input'] and $form_state['values']

brephraim’s picture

Patch?

jsacksick’s picture

Update to dev and test it, it's probably quicker :)

jsacksick’s picture

Status: Fixed » Needs work

We still have duplication issues, the last commits try to fix ux issues but this still needs work...

rszrama’s picture

Let's get a link in to the original commit so we can talk about it: http://drupalcode.org/project/commerce_addressbook.git/commitdiff/3099a82

From here, it looks like you're simply disabling the checkbox by changing the default value. My recommendation in #6 was actually to disable that checkbox entirely - i.e. unset($form['...']['...']), not just set the #default_value = 0. Along with that, note that profile copying actually works based on a variable in the $order->data array that you'll have to unset as well, e.g. unset($order->data['profile_copy'][$pane_id]['status').

jsacksick’s picture

What we can probably do is :
1) When there's not yet any customer profile for a type, keep the checkbox, and then disable it once you already have one customer profile of this type, what do you think ?

bojanz’s picture

Quoting #1876282: billing address book strange entries are created when copy from shipping information option is selected (marked as duplicate) for some more info:

I set up the billing information in the checkout settings :
Home » Administration » Store » Configuration » Checkout settings
to
Enable profile copying on this checkout pane
and copy from Shipping information.

There are a couple of strange behaviours:

-First If option "Hide the country when only one is available" is selected, and the shipping information fields are empty and the option to copy the fields on the billing information is selected, if the user submits the empty form an error message is produced saying that the shipping fields are required but an entry in the billing information address book is created with just the default country field.

-Second, every time the user "ticks" and "unticks" the "My billing information is the same as my shipping information." and a default address is copied over from the shipping information pane, a new entry in the billing address book is created. I end up with many entries in the billing information address book, all with the same repeated address.

bojanz’s picture

I've thought about this problem a bit.

We have two solutions:

1) Quick & clean: Don't show the "profile copy" checkbox for panes that have an addressbook with entries in it. So if there are existing shipping addresses, you don't get the "Same as billing" checkbox because you can already use an address from the addressbook.

2) More complicated: if the "profile copy" checkbox is checked, and the source profile is selected through the addressbook dropdown, do an EFQ comparing (only) the addressfield, and if a matching profile of that type exists, select it. So if a shipping profile with the same address as a billing profile exists, it will be selected. Otherwise, the fields are filled normally and a new profile is created.

manuelgutierrezlopez’s picture

My vote for the two options in #28 would go for option 2. For a usability of the site my feeling is that it is less confusing if we have the profile copy option checked and (hidden usign the pacth in #7) the selected address dropdown is not shown.

brephraim’s picture

Option two is preferable, agreed.

checker’s picture

I would prefer option one because of "quick & clean". Later it is still possible to do a complicate version like option two and I guess this version needs more testings and could have bad side effects etc. Option one could be really fast rtbc.

emptyvoid’s picture

Hello,

I am getting duplicate billing addresses for a customer's account every time the profile is saved. It is my understanding that the system should "disable" previous revisions of the profile when an update is saved.

For a better picture (if you have installed this module for integration with Drupal Commerce) the following modules are used together to manage addresses.

Address Field (this module)
http://drupal.org/project/addressfield

This of course the Drupal Commerce Modules:
Drupal Commerce
http://drupal.org/project/commerce

It is important to note that commerce has several sub-modules most notably the customer module. The module has business logic to managing addresses (address field is a dependency). Address instances are appended to the customer profile entity in methods located in the commerce_customer.module file.

I'm leveraging a community module that extends the basic integration between addressfield and commerce entitled:
Commerce Address Book
http://drupal.org/project/commerce_addressbook

This module extends the basic integration and provides more business rules that don't exist in the standard commerce_customer module. Most notably it adds a management interface for the customer with the ability to create, edit, and delete addresses outside of the order process.

I believe the issue that causes duplicates is located here:

commerce_addressbook/includes/commerce_addressbook.user.inc
Line 30:

/**
 * Page callback for editing a customer profile.
 */
function commerce_addressbook_profile_options_edit($account, $customer_profile) {
  // Add the breadcrumb for the form's location.
  commerce_addressbook_set_breadcrumb($account, $customer_profile->type);

  // If the profile is referenced by an order, make sure it gets duplicated.
  $profile = clone($customer_profile);
  if (!commerce_customer_profile_can_delete($customer_profile)) {
    $profile->previous_id = $profile->profile_id;
    unset($profile->profile_id);
    unset($profile->revision_id);
    $profile->is_new = TRUE;
  }
  module_load_include('inc', 'commerce_customer', 'includes/commerce_customer_profile.forms');
  return drupal_get_form('commerce_addressbook_customer_profile_form', $profile);
}

This method calls "commerce_customer_profile_can_delete($profile) which states:
commerce/modules/commerce_customer.module

Online 628:
// Return FALSE if the given profile does not have an ID; it need not be
// deleted, which is functionally equivalent to cannot be deleted as far as
// code depending on this function is concerned.

And a little later in the method
Online: 635
// If any module implementing hook_commerce_customer_profile_can_delete()
// returns FALSE the customer profile cannot be deleted. Return TRUE if none
// return FALSE.

So a quick search of my source code reveals the following places where modules influence if a profile can be deleted:

commerce/modules/commerce_order.module
Online: 458
function commerce_order_commerce_customer_profile_can_delete($profile)

The function comments say it all:


function commerce_order_commerce_customer_profile_can_delete($profile) {
  // Look for any non-cart order with a reference field targeting the profile.
  foreach (commerce_info_fields('commerce_customer_profile_reference') as $field_name => $field) {
    // Use EntityFieldQuery to look for orders referencing this customer profile
    // and do not allow the delete to occur if one exists.
    $query = new EntityFieldQuery();

    $query
      ->entityCondition('entity_type', 'commerce_order', '=')
      ->fieldCondition($field_name, 'profile_id', $profile->profile_id, '=')
      ->count();

    // Add a condition on the order status if there are cart order statuses.
    $statuses = array_keys(commerce_order_statuses(array('cart' => TRUE)));

    if (!empty($statuses)) {
      $query->propertyCondition('status', $statuses, 'NOT IN');
    }

    // If the profile includes an order context property, we know this was added
    // by the Order module as an order ID to skip in the deletion query.
    if (!empty($profile->entity_context['entity_id']) && $profile->entity_context['entity_type'] == 'commerce_order') {
      $query->propertyCondition('order_id', $profile->entity_context['entity_id'], '!=');
    }

    if ($query->execute() > 0) {
      return FALSE;
    }
  }

  return TRUE;
}

So, if an order in the past used the address then modifying the address for a current or future order will result in the system duplicating the address with the changed values. Old addresses are not deleted, period.

What I'm assuming is not working is that the "expired" addresses should be hidden from the user and only the "new" address revision should be visible.

Retracing back to ye old commerce_addressbook module the submission method is suppose to accomplish this.

commerce_addressbook/includes/commerce_addressbook.user.inc
Online: 45

/**
 * Submit handler for commerce_addressbook_customer_profile_form.
 */
function commerce_addressbook_customer_profile_form_submit($form, &$form_state) {
  $profile = $form_state['customer_profile'];

  // The profile has been edited and duplicated.
  // Disable the previous one to prevent it from showing up in listings.
  if (!empty($profile->previous_id)) {
    $old_profile = commerce_customer_profile_load($profile->previous_id);
    $old_profile->status = 0;
    commerce_customer_profile_save($old_profile);

    // If the old profile was the default, then we need to set the new one
    // as the default.
    $default_profile_id = commerce_addressbook_get_default_profile_id($profile->uid, $profile->type);
    if ($old_profile->profile_id == $default_profile_id) {
      commerce_addressbook_set_default_profile($profile);
    }
  }

  // Set the profile as default if it has just been added, and the user has no
  // other active profiles of this type.
  if (!empty($profile->_is_new)) {
    $query = new EntityFieldQuery;
    $query->entityCondition('entity_type', 'commerce_customer_profile')
          ->entityCondition('bundle', $profile->type)
          ->propertyCondition('uid', $profile->uid)
          ->propertyCondition('status', 1)
          ->count();
    $result = $query->execute();

    if ($result < 2) {
      commerce_addressbook_set_default_profile($profile);
    }
  }

  $form_state['redirect'] = 'user/' . $profile->uid . '/addressbook/' . $profile->type;
}

Online 52 of the method the current "target" profile has the status set to 0. Meaning it is disabled and should not be visible any longer. However it still appears both in the management indexes and during order checkout.

:(

Cross Posted at:

http://drupal.org/node/1940156

http://drupal.org/node/1185996

rszrama’s picture

@emptyvoid It sounds to me like you're just describing the normal customer profile duplication process, the solution of Commerce core to maintaining the integrity of historical order data. The system is not able to use revisions (because we can't "branch" within a single customer profile) and does not do anything with previously used addresses. You can rig something up to disable previously used addresses via Rules if you want, or you might look into http://drupal.org/project/commerce_single_address from bojanz.

generalredneck’s picture

That's actually emptyvoid. Nadavoid isn't in this thread I believe.

rszrama’s picture

You're right, but I ended up responding to him elsewhere, too, and didn't bother coming back here... but since I'm here now. [edit]

acidpotato’s picture

Both of them are good alternatives in my book.

jsacksick’s picture

Here's a patch that implements the 1) solution proposed in the #28 comment, the 2) solution is longer to implement and I'll try to implement it later.

jsacksick’s picture

jsacksick’s picture

jsacksick’s picture

There was a last remaining bug, The latest patch only hide the profile copy checkbox if there is an address selected in the select list.
I'm marking this issue as fixed, we'll open a new issue to implement the 2) solution later.

Status: Fixed » Closed (fixed)

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

acidpotato’s picture

Status: Closed (fixed) » Needs review

Using patch in #40 the checkbox does get hidden. But if user selects "Choose" (unselecting all options) from the Addressbook select list, the checkbox shows up again and is checked by default. If this checkbox is unselected and selected again, a duplicate profile is created.

jsacksick’s picture

Status: Needs review » Closed (works as designed)

I kept the profile copy checkbox when you don't select an existing address allowing the users to copy the billing information if the address entered in the billing hasn't been stored yet.
So the profile duplication is the expected behavior there.