Original report:

When Commerce Addressbook module is enabled profile copying checkbox (Enable profile copying on this checkout pane, helping customers avoid having to enter the same address twice) disappear from shipping address in checkout.

Note:

This seems to have been intentional until now. Around #1794064-28: Update after Commerce 1.4 to work alongside customer profile copying there was a lot of discussion and different tactics to solve this issue, and people went for the 'quick and easy' solution before tackling the more complicated one. That more complicated one (and then some) is attempted in #5 below.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jasom’s picture

Issue summary: View changes

d

jasom’s picture

Issue summary: View changes

df

roderik’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
7.66 KB

This seems intentional; the code explicitly hides the checkbox, when the order already has a profile assigned.

Anyway, I think it's confusing and don't really agree with the current concept. If you want to enter a new address as a billing and shipping address, now there is no way except to type it all in twice.

See the below patch. It keeps the checkbox visible above the select element, but takes care of the 'illegal input' case where the box is checked and an existing address is selected, by:

  • unchecking the checkbox, if an address has just been selected (and the form is doing an AJAX reload)
  • emptying the address selector otherwise. (Note we're not emptying out the address fields, but they will get ignored on submit, if the box is checked.)

Also: excuse me for adding patch hunk #1 that has nothing to do with the patch and doesn't do anything in practice... Just seen it in rszrama's Commerce Checkout Login module, and figured it was a tiny cleanup. You need to drush cc registry after applying, though.

roderik’s picture

FileSize
5.27 KB

Patch redone a little; no real changes.

roderik’s picture

Title: When CA is enabled profile copying checkbox dissapper » Fix interaction between address books & Copy Profile checkbox
Category: Bug report » Feature request
Issue summary: View changes
FileSize
30.74 KB

After some clicking, the UX still didn't feel right to me. And then I disappeared down a rabbit hole.

When I saw the light again, I saw that this issue was actually discussed around a year ago in & around #1794064-28: Update after Commerce 1.4 to work alongside customer profile copying. I've done essentially Option 2++.

This patch can only be applied on top of #2070643-5: Default address not always used in checkout pane

Behavior with this patch:

Visiting the checkout form (with address pane and 'source' address pane)

If source and target pane have a selected address (whether 'default' or already attached to the order): the value of the 'copy' checkbox will be determined by whether the profiles match.

Changing the 'copy' checkbox

When turned off: nothing (except the address fields will appear as usual)

When turned on: the destination address list will change. it will be either set to a profile matching the current source address, or emptied.

Changing the 'source' address list

If the 'copy' checkbox is on, the destination address will change too. it will be either set to a matching address, or emptied.

If the 'copy' checkbox is off, then the 'copy' checkbox is turned on if the new address-profile matches the destination address-profile.

Changing the 'destination' address list

The 'copy' checkbox is turned on/off depending whether the new address-profile matches the source address-profile.

There are some smaller changes in here too (some variable renames etc); I'll consider splitting things out after I hear feedback.

roderik’s picture

Issue summary: View changes
FileSize
39.91 KB
21.32 KB

And then there was another patch. Please look at #3 first, and then consider this one.

The patches regularly compare source & target profiles. The difference is that patch #3 compares profiles as they are stored in the database, and this patch compares profiles taking into account form input that has changed (and is being submitted during an AJAX request from the address selectors, the 'copy' checkbox, the 'country' selectors in the addressfields, or potentially other elements on the page).

I'm divided on its usefulness, and think that there may be more discussion about the way this is coded, than only #3. Which is why looking at #3 first is probably better.

But I decided to finish and test it anyway (because I was dealing with too many issues at the same time, and needed clarity). I might end up needing this in the near future, because some bright mind decided that the 'country' selector of my billing pane (which usually copies from the shipping pane) should be moved outside the billing pane, or at least should be influenced from outside. Which creates potential issues - I may end up needing to be able to reset the 'copy' checkbox based on submitted form input. Which is what this patch enables.

roderik’s picture

Oh yeah, the patch in #4 also requires #2169863: Fix duplicate saving of customer profiles - if that isn't applied, the comparison results between a submitted and a database-loaded profile are unpredictable so the behavior will be wonky.

roderik’s picture

Fixes to my code; no extra comments. (I assume noone's looked at the above yet, anyway.)

Please see comments #3 and #4, to go with the patches I'm uploading now (and forget the patches uploaded with those comments).

mibfire’s picture

Where will commerce_addressbook.form.inc be loaded? Cos i get the following error: PHP Fatal error: Call to undefined function _commerce_addressbook_check_copy_profile() in commerce_addressbook.module]

I patched the module with addressbook-ensure-default-address-selected-in-checkout-2070643-5.patch and 21067250-4a-do-not-test.patch. Did i miss sg out? thx

roderik’s picture

@mibfire, sorry for not reacting quicker. But I don't know how your problem can be caused. There is only one call to _commerce_addressbook_check_copy_profile() (in line 457 if you apply those 2 patches). And commerce_addressbook.form.inc is loaded with the module_load_include() command on line 456.

In the meantime there were some more changes. Again,

- no details on the changes since #4, since I assume noone needs that;

- uploaded as 3b and 4b, because: please look at comments 3 / 4 / 5 for details.

a.ross’s picture

I'm trying the patch 3b, but it doesn't apply, as the function commerce_addressbook_commerce_checkout_pane_info_alter has been removed. I'll see if I can re-roll the patch on the latest dev version.

a.ross’s picture

Seems the module changed pretty significantly. What version of the module was this patch intended for?

roderik’s picture

7.x-2.0-rc7 with #2070643-5: Default address not always used in checkout pane applied. That is, patch #5, not the one that fietserwin proposed and that was committed to the module 4 days ago.

I think I have an issue with the fix that was committed eventually, but I should seriously sit down and work out why / if I'm not mistaken. Of course it's my fault for not replying in that issue, but since the module saw no maintainer activity anyway...

... I'm really behind on maintaining contrib patches. If you need this done, ping me privately - maybe we can exchange some work - I'm pretty sure if I don't hear anything, this will take 2nd/3rd place behind other stuff.

Nodz’s picture

I've just tried the patch and it's not compatible with 7.x-2.0-rc8 because of the 2070643-6.patch that was committed in #2070643-14: Default address not always used in checkout pane.

mglaman’s picture

Status: Needs review » Needs work

Neither variations apply, moving to Needs Work.