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.7] Performance improvements to shipping rule order condition formula matching #3653

Open
wants to merge 12 commits into
base: 4.7
Choose a base branch
from
2 changes: 2 additions & 0 deletions CHANGELOG-WIP.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
- It’s now possible to specifically make discounts require a coupon code. ([#3132](https://github.com/craftcms/commerce/issues/3132))
- Country code defaults to the store’s country when creating a new address on the Order Edit page. ([#3306](https://github.com/craftcms/commerce/issues/3306))
- Product conditions can now have a “Variant Search” rule. ([#3689](https://github.com/craftcms/commerce/issues/3689))
- Improved the performance of adding items to the cart.
- Improved the performance of shipping rule matching when an order condition formula is used. ([3653](https://github.com/craftcms/commerce/pull/3653))

### Administration

Expand Down
14 changes: 10 additions & 4 deletions src/controllers/CartController.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public function actionUpdateCart(): ?Response

// Get the cart from the request or from the session.
// When we are about to update the cart, we consider it a real cart at this point, and want to actually create it in the DB.
$this->_cart = $this->_getCart(true);
$this->_cart = $this->_getCart();

// Can clear line items when updating the cart
$clearLineItems = $this->request->getParam('clearLineItems');
Expand Down Expand Up @@ -580,7 +580,7 @@ private function _getCart(bool $forceSave = false): Order
// Get the cart from the order number
$cart = Order::find()->number($orderNumber)->isCompleted(false)->one();

if (!$cart) {
if ($cart === null) {
throw new NotFoundHttpException('Cart not found');
}

Expand All @@ -590,7 +590,13 @@ private function _getCart(bool $forceSave = false): Order
$requestForceSave = (bool)$this->request->getBodyParam('forceSave');
$doForceSave = ($requestForceSave || $forceSave);

return Plugin::getInstance()->getCarts()->getCart($doForceSave);
$this->_cart = Plugin::getInstance()->getCarts()->getCart($doForceSave);

if (!$this->_cart->id) {
Craft::$app->getElements()->saveElement($this->_cart, false);
}

return $this->_cart;
}

/**
Expand Down Expand Up @@ -685,7 +691,7 @@ private function _setAddresses(): void
$this->_cart->sourceBillingAddressId = $billingAddressId;

/** @var Address $cartBillingAddress */
$cartBillingAddress = Craft::$app->getElements()->duplicateElement($userBillingAddress, [
$cartBillingAddress = Craft::$app->getElements()->duplicateElement($userBillingAddress, [
'owner' => $this->_cart,
]);
$this->_cart->setBillingAddress($cartBillingAddress);
Expand Down
8 changes: 4 additions & 4 deletions src/models/ShippingRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ function($attribute) {
if (!$order) {
$order = new Order();
}
$orderAsArray = Plugin::getInstance()->getShippingMethods()->getSerializedOrderForMatchingRules($order);
$orderConditionParams = [
'order' => $order->toArray([], ['lineItems.snapshot', 'shippingAddress', 'billingAddress']),
'order' => $orderAsArray,
];
if (!Plugin::getInstance()->getFormulas()->validateConditionSyntax($this->{$attribute}, $orderConditionParams)) {
$this->addError($attribute, Craft::t('commerce', 'Invalid order condition syntax.'));
Expand Down Expand Up @@ -273,10 +274,9 @@ public function matchOrder(Order $order): bool
$lineItems = $order->getLineItems();

if ($this->orderConditionFormula) {
$fieldsAsArray = $order->getSerializedFieldValues();
$orderAsArray = $order->toArray([], ['lineItems.snapshot', 'shippingAddress', 'billingAddress']);
$orderAsArray = Plugin::getInstance()->getShippingMethods()->getSerializedOrderForMatchingRules($order);
$orderConditionParams = [
'order' => array_merge($orderAsArray, $fieldsAsArray),
'order' => $orderAsArray,
];
if (!Plugin::getInstance()->getFormulas()->evaluateCondition($this->orderConditionFormula, $orderConditionParams, 'Evaluate Shipping Rule Order Condition Formula')) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/services/Carts.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public function init()
/**
* Get the current cart for this session.
*
* @param bool $forceSave Force the cart to save when requesting it.
* @param bool $forceSave Force the cart.
* @throws ElementNotFoundException
* @throws Exception
* @throws Throwable
Expand Down
27 changes: 27 additions & 0 deletions src/services/ShippingMethods.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ class ShippingMethods extends Component
*/
private ?array $_allShippingMethods = null;

/**
* @var array
*/
private array $_serializedOrdersByNumber = [];

/**
* Returns the Commerce managed shipping methods stored in the database.
*
Expand Down Expand Up @@ -141,9 +146,31 @@ public function getMatchingShippingMethods(Order $order): array
$shippingMethods[$method->getHandle()] = $method; // Keep the key being the handle of the method for front-end use.
}

// Clear the memoized data so next time we watch to match rules, we get fresh data.
$this->_serializedOrdersByNumber = [];

return $shippingMethods;
}

/**
* Creates an order as an array for matching rules.
* We do this centrally here so that we can clear the memoized data centrally.
*
* @param Order $order
* @return array
*/
public function getSerializedOrderForMatchingRules(Order $order): array
{
if (isset($this->_serializedOrdersByNumber[$order->number])) {
return $this->_serializedOrdersByNumber[$order->number];
}

$fieldsAsArray = $order->getSerializedFieldValues();
$orderAsArray = $order->toArray([], ['lineItems.snapshot', 'shippingAddress', 'billingAddress']);
$this->_serializedOrdersByNumber[$order->number] = array_merge($orderAsArray, $fieldsAsArray);
return $this->_serializedOrdersByNumber[$order->number];
}

/**
* Get a matching shipping rule for Order and shipping method.
*
Expand Down
Loading