[4.x]: Can not set a shipping address on the order that is not owned by the order.

[MoritzLost]

What happened?

Description

We recently updated from Commerce 4.6.2 to 4.7.1. After this update, some visitors started to get internal server errors (not quite sure if it's related to the update, but it's likely):

Can not set a shipping address on the order that is not owned by the order.

Stack trace:

#0 /…/vendor/craftcms/commerce/src/elements/Order.php(2044): craft\\commerce\\elements\\Order->setShippingAddress()
#1 /…/vendor/craftcms/cms/src/services/Elements.php(3450): craft\\commerce\\elements\\Order->afterSave()
#2 /…/vendor/craftcms/cms/src/services/Elements.php(1109): craft\\services\\Elements->_saveElementInternal()
#3 /…/vendor/craftcms/commerce/src/controllers/CartController.php(525): craft\\services\\Elements->saveElement()
#4 /…/vendor/craftcms/commerce/src/controllers/CartController.php(327): craft\\commerce\\controllers\\CartController->_returnCart()
#5 [internal function]: craft\\commerce\\controllers\\CartController->actionUpdateCart()
#6 /…/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array()
#7 /…/vendor/yiisoft/yii2/base/Controller.php(178): yii\\base\\InlineAction->runWithParams()
#8 /…/vendor/yiisoft/yii2/base/Module.php(552): yii\\base\\Controller->runAction()
#9 /…/vendor/craftcms/cms/src/web/Application.php(341): yii\\base\\Module->runAction()
#10 /…/modules/Shop/controllers/ShopController.php(74): craft\\web\\Application->runAction()
#11 [internal function]: modules\\Shop\\controllers\\ShopController->actionRegisterForEvent()
#12 /…/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array()
#13 /…/vendor/yiisoft/yii2/base/Controller.php(178): yii\\base\\InlineAction->runWithParams()
#14 /…/vendor/yiisoft/yii2/base/Module.php(552): yii\\base\\Controller->runAction()
#15 /…/vendor/craftcms/cms/src/web/Application.php(341): yii\\base\\Module->runAction()
#16 /…/vendor/craftcms/cms/src/web/Application.php(640): craft\\web\\Application->runAction()
#17 /…/vendor/craftcms/cms/src/web/Application.php(303): craft\\web\\Application->_processActionRequest()
#18 /…/vendor/yiisoft/yii2/base/Application.php(384): craft\\web\\Application->handleRequest()
#19 /…/web/index.php(10): yii\\base\\Application->run()
#20 {main}

We use Commerce for an event registration system. In the registration form, we call a custom action on our ShopController that runs some preliminary checks to make sure there are no other line items in the order, the request includes a valid purchasableId that is an available event, etc. Then we let Commerce handle the request by running commerce/cart/update-cart:

Craft::$app->runAction('commerce/cart/update-cart');

This worked perfectly before and has never caused any problems. After the update, some visitors started getting internal server errors with the above stack trace in the logs.

The problem comes from Order::setShippingAddress (as seen in the stack trace). This line throws the error:

        // Ensure that address can only belong to this order
        if ($address->ownerId != $this->id) {
            throw new InvalidArgumentException('Can not set a shipping address on the order that is not owned by the order.');
        }

I managed to reproduce the issue, in this case the $address->ownerId is null, causing this problem. I also found where this happens. The user selects one of their addresses in the form as billingAddressId, the shipping address is set automatically through a hidden input with shippingAddressSameAsBilling. In CartController::_setAddresses() (which is called in CartController::actionUpdateCart()), the address with that ID is fetched and then duplicated with the owner set to the cart:

// If a user's address ID has been submitted duplicate the address to the order
if ($userBillingAddress) {
    $this->_cart->sourceBillingAddressId = $billingAddressId;

    /** @var Address $cartBillingAddress */
    $cartBillingAddress = Craft::$app->getElements()->duplicateElement($userBillingAddress, [
        'owner' => $this->_cart,
    ]);
    $this->_cart->setBillingAddress($cartBillingAddress);

    if ($shippingIsBilling) {
        $this->_cart->sourceShippingAddressId = $userBillingAddress->id;
        $this->_cart->setShippingAddress($cartBillingAddress);
    }
}

I put in Craft::dd($cartBillingAddress) to check the duplicated address. The $cartBillingAddress->owner is correctly set to the current cart ($this->_cart), but the $cartBillingAddress->ownerId remains empty! That's what's causing the error down the line, when the shipping address is copied over from the billing address.

I don't know what exactly causes this to happen. It only happens for some users.

Steps to reproduce

See above.

Craft CMS version

4.13.5

Craft Commerce version

4.7.1

PHP version

8.3

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

[linear]

PT-2374 [4.x]: Can not set a shipping address on the order that is not owned by the order.

[nfourtythree]

Hi @MoritzLost

Thanks for reporting, this sounds like a strange one.

The setOwner() method on the Address element should be setting the owner ID.

With your ability to replicate the issue are you able to check if the cart has an ID at the point the code is called?

Thanks!

[MoritzLost]

@nfourtythree Looks like it doesn't have an ID!

craft\commerce\elements\Order {#755 ▼
  -_events: array:1 [▶]
  -_eventWildcards: []
  -_behaviors: array:2 [▶]
  -_errors: null
  -_validators: null
  -_scenario: "default"
  #revisionCreatorId: null
  #revisionNotes: null
  -_canonicalId: null
  -_canonical: null
  -_canonicalAnySite: null
  -_canonicalUid: null
  -_outdatedAttributes: null
  -_modifiedAttributes: null
  -_outdatedFields: null
  -_modifiedFields: null
  -_initialized: true
  -_fieldsByHandle: []
  -_fieldParamNamePrefix: "fields"
  -_normalizedFieldValues: null
  -_allDirty: false
  -_dirtyAttributes: []
  -_savedTitle: null
  -_dirtyFields: []
  -_parentId: null
  -_parent: null
  -_hasNewParent: null
  -_prevSibling: null
  -_nextSibling: null
  -_eagerLoadedElements: []
  -_eagerLoadedElementCounts: []
  -_currentRevision: null
  -_enabledForSite: true
  -_uiLabel: null
  -_uiLabelPath: []
  -_isFresh: null
  +id: null
  +tempId: null
  +draftId: null
  +revisionId: null
  +isProvisionalDraft: false
  +uid: null
  +siteSettingsId: null
  +fieldLayoutId: null
  +structureId: null
  +contentId: null
  +enabled: true
  +archived: false
  +siteId: 1
  +title: null
  +slug: null
  +uri: null
  +dateCreated: null
  +dateUpdated: null
  +dateLastMerged: null
  +dateDeleted: null
  +root: null
  +lft: null
  +rgt: null
  +level: null
  +searchScore: null
  +trashed: false
  +awaitingFieldValues: false
  +propagating: false
  +validatingRelatedElement: false
  +propagateAll: false
  +newSiteIds: []
  +isNewForSite: false
  +resaving: false
  +duplicateOf: null
  +firstSave: false
  +mergingCanonicalChanges: false
  +updatingFromDerivative: false
  +previewing: false
  +hardDelete: false
  +number: "80fcecba1dfca694ac1a9f61ac3a12af"
  +reference: null
  +couponCode: null
  +isCompleted: false
  +dateOrdered: null
  +datePaid: null
  +dateAuthorized: null
  +currency: "EUR"
  +gatewayId: null
  +lastIp: "172.18.0.1"
  +message: null
  +returnUrl: null
  +cancelUrl: null
  +orderStatusId: null
  +orderLanguage: "de"
  +orderSiteId: 1
  +origin: "web"
  +orderCompletedEmail: null
  +billingAddressId: null
  +shippingAddressId: null
  +makePrimaryShippingAddress: false
  +makePrimaryBillingAddress: false
  +shippingSameAsBilling: false
  +billingSameAsShipping: false
  +estimatedBillingAddressId: null
  +estimatedShippingAddressId: null
  +sourceBillingAddressId: 23139
  +sourceShippingAddressId: null
  +estimatedBillingSameAsShipping: false
  +shippingMethodHandle: ""
  +shippingMethodName: null
  -_customerId: 14085
  +registerUserOnOrderComplete: false
  +saveBillingAddressOnOrderComplete: false
  +saveShippingAddressOnOrderComplete: false
  +paymentSourceId: null
  +storedTotalPrice: null
  +storedTotal: null
  +storedTotalPaid: null
  +storedItemTotal: null
  +storedItemSubtotal: null
  +storedTotalShippingCost: null
  +storedTotalDiscount: null
  +storedTotalTax: null
  +storedTotalTaxIncluded: null
  +storedTotalQty: null
  -_recalculationMode: "all"
  -_shippingAddress: null
  -_billingAddress: null
  -_estimatedShippingAddress: null
  -_estimatedBillingAddress: null
  -_lineItems: array:1 [▶]
  -_orderAdjustments: null
  -_paymentCurrency: "EUR"
  -_transactions: null
  -_customer: craft\elements\User {#675 ▶}
  -_paymentAmount: null
  +suppressEmails: false
  -_notices: []
}

It's possible that the cart is created during that very request. From the user's perspective, they're going to the event registration page and then submitting the registration form. The registration form then updates the cart with the event ID (purchasableId, events are Commerce products) as described above. So the cart might not have been saved yet at this point?

We also run Craft::$app->runAction('commerce/cart/complete') to complete the purchase in the same request. Though the error already occurs during the update-cart action, so I don't think that is related to this issue.

[nfourtythree]

Hi @MoritzLost

Thank you for the updated information. Usually the cart has been saved when it is created so it should have an ID by this point.

As a bit of a test, when you send the payload could you also send a key of forceSave which is set to true and see if the error still occurs?

Thanks!

[MoritzLost]

@nfourtythree Just tried it, looks like that works! When I set forceSave, the registration is completed successfully.

Any ideas why that is necessary after the last update?

I checked a bit deeper. When the error occurs, Carts::getCart() creates a new cart, which does not have an ID yet. Then it only saves the cart if anything (addresses, IP address, etc) has changed. Since it's a new cart, it hasn't, so the cart won't be saved. Maybe I'm missing something, but where would you expect the cart to be saved (if forceSave is not set)?

Actually, I would expect that the cart is saved here, since autoSetAddresses is called:

$autoSetAddresses = $this->_cart->autoSetAddresses();

I'm setting a billingAddressId (always set to the ID of an existing address in the user's account) and also shippingAddressSameAsBilling. So I would expect this to cause the cart to be saved here. But I checked and $autoSetAddresses is false here. Not sure what's going on there, maybe I'm reading the code wrong.

[nfourtythree]

Hi @MoritzLost

Glad using forceSave has at least go things working and your project can continue as expected.

We are constantly trying to make sure Commerce is as performant as possible. This includes making sure we aren't making unnecessary saves of elements. When retrieving the cart we try not to make a save if it is not required. This is in an attempt to avoid having many completely blank carts cluttering the system.

With all that said, your case does seem to be an interesting one. In our testing we haven't seen the issue that you raised, this could be just a case of getting the correct combination to be able to replicate it.

In regards autoSetAddresses() this is the feature that will try and add a user's primary addresses to the cart if they are logged in (using the autoSetNewCartAddresses setting). This doesn't get involved when it comes to specifically setting addresses on a cart using the update cart action.

You mentioned that you are calling your own controller, which of course is absolutely fine but adds in another layer. I would be interested to know if you took that layer out if you would see any difference in behaviour when it comes to the core functionality and the error appearing.

We will keep monitoring for anyone else reporting the issue and then we can take a further look into it. Will close the issue for now but if you do get a reliable way to replicate it with just Commerce core then we can reopen an investigate.

Thanks!

[MoritzLost]

@nfourtythree Thanks for the explanation!

I just tried reproducing the error without our controller. For that, I replaced our custom controller action with commerce/cart/update-cart. I'm still getting the same error I mentioned above. Our custom controller is not called at all in this case, so it looks like this is really an issue in Commerce, unrelated to our module.

Here's the entire request payload:

Most of those parameters are set as hidden inputs (forceSave is commented out to reproduce the error):

<form class="event-registration" method="post" action="{{ registration_url }}" accept-charset="UTF-8">
    {{ csrfInput() }}
    {{ actionInput('commerce/cart/update-cart') }}
    {{ hiddenInput('purchasableId', event.defaultVariant.id) }}
    {{ hiddenInput('clearLineItems', 1) }}
    {{ hiddenInput('qty', 1) }}
    {# {{ hiddenInput('forceSave', 1) }} #}
    {{ hiddenInput('shippingAddressSameAsBilling', '1') }}
    {{ redirectInput(registration_url) }}
    {{ successMessageInput('My custom success message') }}

The only other form field is a select field for the billingAddressId, which is always an existing address on the user account. The interface forces the user to create an address before they can submit this form.

The user doesn't have a cart at this point, so this request would need to create a new cart and save it, since we're setting shippingAddressSameAsBilling. Maybe the issue is caused by setting a billing address and shippingAddressSameAsBilling in the same request that the cart is initially created and populated with a purchasable? That might be uncommon in a normal e-commerce scenario where you first select some products and then choose billing, shipping etc.

I can fix the issue for now using forceSave. But maybe you can take another look, I think the above scenario should work by default.

Thanks!

[nfourtythree]

Hi @MoritzLost

Thank you for taking the time to provide this data and feedback.

I will reopen the issue and use this information to track down what is happening.

Thankfully you can at least have a working project for the moment and hopefully we can get this sorted so you can remove the forcing of the save in the future.

Thanks!

[MoritzLost]

@nfourtythree Thanks for the support! I've pushed the forceSave adjustment to production, and it seems to be working. I'm fine with keeping it in, since our controller completes the purchase in the same request where it's created, unless there are errors, so it shouldn't result in too many unnecessary cart saves. But yeah, good idea to figure out whether this is also a problem in other contexts.