Followup to #3022850: [Addressbook, part 1] Rework the ownership model for customer profiles.

A customer profile inline form needs to support copying values from another inline form.
This should work even if the profiles use different form modes.
It should also be generic in terms of direction (billing same as shipping, shipping same as billing).

The CustomerProfile inline form plugin needs the following settings:
- instance_id (e.g "billing").
- reuse_source (e.g. "shipping". The instance ID of the source form)
- reuse_label ("My billing information is the same as my shipping information")

CustomerProfile::buildForm() puts its #parents into form state under the instance ID.
If reuse_source is defined, we look for its parents in form state, get its inline form, take the profile, steal the values.

Comments

bojanz created an issue. See original summary.

megachriz’s picture

megachriz
Primary language Dutch
commented
Related issues: +#2852207: Billing same as shipping

There is a similar issue in the Commerce Shipping project: #2852207: Billing same as shipping.

bojanz’s picture

bojanz commented
Title: Expand commerce_profile_select with the ability to copy values from another element ("Billing same as shipping") » [Addressbook part 2] Allow reusing profile values from another inline form ("Billing same as shipping")
Issue summary: View changes

Retitling. This is now a followup to #3022850: [Addressbook, part 1] Rework the ownership model for customer profiles. Patch incoming this week.

bgronek’s picture

bgronek commented

@bojanz Thought I'd check in to see how this one is coming along. Hate to be a pest and thank you for all of your work on this!!

bojanz’s picture

bojanz commented
Title: [Addressbook part 2] Allow reusing profile values from another inline form ("Billing same as shipping") » [Addressbook part 3] Allow reusing profile values from another inline form ("Billing same as shipping")

Retitling. We're finishing and committing #3053165: [Addressbook part 2] Complete the UI by allowing choice between multiple addressbook profiles first, which ended up taking significantly more time than planned. We might need to tag 2.14 before finishing this issue. Will post current code progress soon.

I've separated some of the logic into #3060433: Provide a way to collect all order profiles (billing, shipping, etc), but the rest of the patch that was written for ended up not being fully functional.

rgpublic’s picture

rgpublic
Primary language German
Location Düsseldorf 🇩🇪 🇪🇺
commented

Thanks a lot for all the work on this. One quick question: If the feature is finally realized, how exactly will the upgrade path for existing installations be? After all, the existing profiles must be copied/duplicated for each order they are used in and designated with the UID=0, right? Will there be an updatedb script that does this? And I wonder: Will the Admin account still be usable for shopping? Or will the Admin's address book be flooded with the UID=0 profiles?

ericchew’s picture

ericchew commented

@rgpublic Here is the update hook that is on -dev for setting profile uid=0
https://git.drupalcode.org/project/commerce/blob/8.x-2.x/modules/order/c...

uid=0 is for anonymous, admin account is uid=1...so I don't see that being an issue.

rgpublic’s picture

rgpublic
Primary language German
Location Düsseldorf 🇩🇪 🇪🇺
commented

Ah, understood. Very interesting & helpful that link. Thanks @ericchew! I learned two new things:

1) The update will not copy profiles but rather move them to the invisible state. Afterwards users won't find addresses in their address book anymore. I will need to discuss this with our clients in advance whether they want to just accept this or whether we need to write a custom script to "rescue" those addresses.

2) The update will not "unshare" shared profiles i.e if two or more orders currently point to the same profile they will continue to do so after the update.

bojanz’s picture

bojanz commented

@rgpublic
The current update hook was coded for performance, so that it doesn't crash if a site has a million orders.
We assume that no profiles are shared between orders, since Commerce didn't have reuse capabilities before. However, you are correct that any site using the old patches could have this problem.

I would be happy to commit a Drupal Console command that has additional logic:
- Do a query to get order IDs with shared profile IDs. Deduplicate those profiles.
- Load each customer's most recent placed order, add that profile to the address book.
Of course, we could load all orders and deduplicate the profiles, then add each unique address to the address book, but that would be significantly slower.

Opened #3066454: Add a Drupal Console command with an extended address book upgrade path to track this.

rgpublic’s picture

rgpublic
Primary language German
Location Düsseldorf 🇩🇪 🇪🇺
commented

@bojanz: Thanks for the clarification. Understood. Certainly would be helpful to have such a command, but for me it's basically okay like it is planned now. This is easy to fix even with some temporary script. But it is VERY good to know all this in advance. I can plan much better this way. Not so funny to do an update and suddenly all address book entries are gone. Sometimes a solution can instead of technical also be... "political" ;-) I wonder whether the "updatedb" script should at least print some warning or sth. ?

bojanz’s picture

bojanz commented

Yeah, we'd point the user to the Drupal Console command, or if we don't manage to complete it, to the change record at least (detailing what needs to be done, and at least some sample code).

bojanz’s picture

bojanz commented
Title: [Addressbook part 3] Allow reusing profile values from another inline form ("Billing same as shipping") » Allow reusing profile values from another inline form ("Billing same as shipping")

Removing the address book prefix because we weren't able to implement this feature as a part of the 2.14 address book effort. It remains a target for 2.15.

https://www.drupal.org/project/commerce/issues/2844920?page=1#comment-13... is an update regarding the other fixed issues.

jwwj’s picture

jwwj commented

Would be happy to see a patch for this that we can test during the next few weeks :) This is a major usability issue imho in the checkout flow atm. and even a one way option for now would help a lot (e.g. just allowing copy from shipping to billing).

bojanz’s picture

bojanz commented
Status: Active » Closed (outdated)

We now have a mostly-final patch up in the commerce_shipping issue queue: #2852207: Billing same as shipping. This was done on the shipping side because we realized that we can't support both directions with the same level of difficulty. They each require their own tests and have their own quirks.

A number of Commerce issues were fixed, in time for the 2.17 release, to make this patch smaller:
- #3113882: The inline form alter hooks must not be fired from a #process
- #2917102: Use separate form modes for each customer profile
- #3114480: Make TaxNumberDefaultWidget::getSelectedCountry() more robust
- #3115087: Clear dependent form input when a payment_method is selected at checkout
- #3115592: Improve field copying between payment method and order billing profiles

So, I'm closing this issue. Thank you everyone for your patience, this hasn't been easy.