commerce icon indicating copy to clipboard operation
commerce copied to clipboard

[4.x]: Calling Update Cart w/ makePrimaryShippingAddress or makePrimaryBillingAddress Should Create/Update Customer Addresses

Open curtismorte opened this issue 2 years ago • 3 comments

What happened?

When submitting a form to update addresses for a cart, specifying makePrimaryShippingAddress and makePrimaryBillingAddress doesn't create an Address on the User's account when the Order's shipping and billing addresses aren't set from address sources already defined on the User's profile (commerce default shipping and billing addresses).

Description

When submitting the following form:

<form method="post">
    {{ csrfInput() }}
    {{ hiddenInput('action', 'commerce/cart/update-cart') }}
    {{ redirectInput('checkout/review') }}

    {# Shipping Address Fields #}
    ...
    <input type="checkbox" name="makePrimaryShippingAddress" value="1">

    {# Billing Address Fields #}
    ...
    <input type="checkbox" name="makePrimaryBillingAddress" value="1">

    <button type="submit">Continue</button>
</form>

I would expect that if the user had selected the checkboxes for "makePrimaryBillingAddress" and "makePrimaryShippingAddress", that the Addresses now associated with the cart would also be saved as Addresses for the User that are marked as the User's primary shipping and billing addresses.

Currently, this is not the case because of these lines: https://github.com/craftcms/commerce/blob/05e677446c75f00e65cbc1c25515818435c41a94/src/elements/Order.php#L2031-L2033 https://github.com/craftcms/commerce/blob/05e677446c75f00e65cbc1c25515818435c41a94/src/elements/Order.php#L2052-L2054

The lines above are only making the Addresses the primary addresses if the Cart's Shipping and Billing Addresses were originally set from a User's default billing and shipping addresses.

From the documentation on Addresses from Commerce, it states:

Every order address designates the order as its ownerId. If a customer uses one of their saved addresses for an order’s shipping or billing, the address will be cloned to that order with references to the original address element stored in order.sourceBillingAddressId and order.sourceShippingAddressId.

Since a new Cart will set the User's default Shipping and Billing addresses automatically (commerce clones addresses from Users to Orders, changing the ownerId for the newly cloned Address models, and then specifies the order source attributes on the Order to preserve the original source of the Addresses), it would also seem that this should be true in reverse. This means that an Address owned by an Order should be able to be cloned as an Address now owned by the User when the cart (an incomplete order) is updated with the values makePrimaryShippingAddress and makePrimaryBillingAddress specified.

Primary Shipping and Billing Addresses are a function of commerce, but since they are technically fields on an Address owned by a User, this is why I expect the User's primary Shipping and Billing Addresses to be updated when specifying makePrimaryShippingAddress and makePrimaryBillingAddress

Steps to reproduce

  1. Login to a front end user account
  2. Verify the user doesn't have any default billing or shipping addresses
  3. Create a new cart for the user (to remove any old information)
  4. Submit a form using the action commerce/cart/update-cart where you specify shipping and billing address details (shippingAddress[], billingAddress[]) including values being set for makePrimaryShippingAddress and makePrimaryBillingAddress.

Expected behavior

Four Addresses should be created:

  1. Shipping owned by Order
  2. Shipping owned by User
  3. Billing owned by Order
  4. Billing owned by User

The address source fields for the cart should be set to the Address IDs owned by the User

Actual behavior

Two Addresses are created:

  1. Shipping owned by Order
  2. Billing owned by Order

The address source fields for the cart aren't being set.

Craft CMS version

4.2.1.1

Craft Commerce version

4.1.0

PHP version

8.1

Operating system and version

No response

Database type and version

MySQL

Image driver and version

No response

Installed plugins and versions

curtismorte avatar Aug 17 '22 20:08 curtismorte

@curtismorte is this a similar issue to #2926?

samueldraper avatar Aug 22 '22 10:08 samueldraper

@samueldraper directly, no. Indirectly, maybe.

I view this as a bug because the fields below are used during the update cart action:

  1. makePrimaryShippingAddress
  2. makePrimaryBillingAddress

The problem here is that Addresses are now separated by owner. So there is an Address model in the database, and either a user or order is the owner. This is essentially what they were telling you in the issue you mentioned. However, the usage of the fields above relates the two types of ownership together because the primary addresses are owned by the user, and not the order.

curtismorte avatar Aug 22 '22 15:08 curtismorte

Any update here, or can we have a discussion / conversation about this being the desired functionality or not @lukeholder ?

curtismorte avatar Aug 26 '22 15:08 curtismorte

Any updates on this?

JoeriE avatar Jan 30 '23 13:01 JoeriE

Can you communicate anything please? We urgently need this fixed.

judus avatar Feb 08 '23 14:02 judus

I've had the same issue, I used an event to check for those fields and save the addresses like this:

https://craftcms.com/docs/commerce/4.x/extend/events.html#modifycartinfo

In this case I just wanted to allow a user to set their address if they don't have one already stored.

Event::on(
            BaseFrontEndController::class,
            BaseFrontEndController::EVENT_MODIFY_CART_INFO,
            function(ModifyCartInfoEvent $cart) {
                $cart  = $cart->cart;
                $customer = Craft::$app->getUser()->getIdentity();

                if(!$customer->getAddresses()) {

                    if ($cart->makePrimaryShippingAddress) {
                        $shippingAddress = $cart->getShippingAddress();
                        $shippingAddress->ownerId = $customer->id;
                        $shippingAddress->title = Craft::t('commerce', 'Shipping Address');
                        Craft::$app->getElements()->saveElement($shippingAddress, false);
                        $customer->primaryShippingAddressId = $shippingAddress->id;
                    }
                    if ($cart->makePrimaryBillingAddress) {
                        $billingAddress = $cart->getBillingAddress();
                        $billingAddress->ownerId = $customer->id;
                        $billingAddress->title = Craft::t('commerce', 'Billing Address');
                        Craft::$app->getElements()->saveElement($billingAddress, false);
                        $customer->primaryBillingAddressId = $billingAddress->id;
                    }

                    Craft::$app->elements->saveElement($customer, false);
                }
            }
        );

darinlarimore avatar Feb 11 '23 06:02 darinlarimore

@darinlarimore we ended up doing something similar (but it's been a while so I don't recall exactly). For others out there, having the ability to write your own controller actions, hook into events, etc. with custom modules how we were able to get past this issue temporarily.

I would have hoped that this might be solved by now, which is pretty surprising given how good the Craft team and community is.

curtismorte avatar Feb 15 '23 16:02 curtismorte

Hi All

Thank you very much for the feedback, it is really appreciated.

There are certain areas we can improve on in terms of documentation and functionality.

Looking at the makePrimaryBillingAddress and makePrimaryShippingAddress properties these are designed to be used in conjunction with a credentialed logged-in user that is currently going through the checkout process. It allows the customer to set an address to be a primary based on the address they selected from their address book.

Because these properties are only in memory (for the current request) they do not persist on the order through the checkout process. Meaning that we cannot deal with them, say, during/after they have been to the gateway to make payment.

As the functionality stands it is currently working for its intended purpose. That doesn't mean that all hope is lost.

There is a feature request (https://github.com/craftcms/commerce/discussions/2843) to add properties to the order to allow the saving of addresses at the end of the checkout process. This is something we will be looking into for a future release to make this core functionality.

We will also review the documentation to make sure we are making it clear what the intended purposes are for the current properties.

Thanks!

nfourtythree avatar Jul 05 '23 08:07 nfourtythree