Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
curtismorte opened this issue Aug 17, 2022 · 8 comments
Assignees
Labels
bug commerce4 Issues related to Commerce v4

Comments

@curtismorte
Copy link

curtismorte commented Aug 17, 2022

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:

if ($this->makePrimaryShippingAddress && $currentUserIsCustomer && $this->sourceShippingAddressId) {
Plugin::getInstance()->getCustomers()->savePrimaryShippingAddressId($this->getCustomer(), $this->sourceShippingAddressId);
}

if ($this->makePrimaryBillingAddress && $currentUserIsCustomer && $this->sourceBillingAddressId) {
Plugin::getInstance()->getCustomers()->savePrimaryBillingAddressId($this->getCustomer(), $this->sourceBillingAddressId);
}

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 curtismorte added commerce4 Issues related to Commerce v4 bug labels Aug 17, 2022
@samueldraper
Copy link

@curtismorte is this a similar issue to #2926?

@curtismorte
Copy link
Author

@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
Copy link
Author

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

@lukeholder lukeholder self-assigned this Aug 29, 2022
@JoeriE
Copy link

JoeriE commented Jan 30, 2023

Any updates on this?

@judus
Copy link

judus commented Feb 8, 2023

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

@darinlarimore
Copy link

darinlarimore commented Feb 11, 2023

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);
                }
            }
        );

@curtismorte
Copy link
Author

@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.

@nfourtythree
Copy link
Contributor

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 (#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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug commerce4 Issues related to Commerce v4
Projects
None yet
Development

No branches or pull requests

7 participants