Skip to content

Commit

Permalink
Merge pull request #3652 from craftcms/bugfix/improve-purchasable-que…
Browse files Browse the repository at this point in the history
…ry-performance

[5.1] Improve purchasable query performance
  • Loading branch information
nfourtythree authored Sep 4, 2024
2 parents 0184f6b + 24b533c commit 3c077b7
Show file tree
Hide file tree
Showing 11 changed files with 421 additions and 36 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG-WIP.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Store Management
- It’s now possible to manage transfers between inventory locations.
- Catalog pricing rules now support flexible matching based on product and variant conditions. ([#3544](https://github.com/craftcms/commerce/issues/3544))
- Variant conditions can now have an “SKU” rule.

### Administration
- It’s now possible to create custom line items. ([#2301](https://github.com/craftcms/commerce/discussions/2301), [#2233](https://github.com/craftcms/commerce/discussions/2233), [#2345](https://github.com/craftcms/commerce/discussions/2345))
Expand All @@ -23,6 +24,10 @@
- Added `craft\commerce\elements\conditions\transfers\TransferCondition`.
- Added `craft\commerce\elements\conditions\variants\CatalogPricingRuleVariantCondition`.
- Added `craft\commerce\elements\db\TransferQuery`.
- Added `craft\commerce\elements\Product::$defaultBasePrice`.
- Added `craft\commerce\elements\Product::$storeId`.
- Added `craft\commerce\elements\Product::getCurrencyAttributes()`.
- Added `craft\commerce\elements\Product::getStore()`.
- Added `craft\commerce\elements\Transfer`.
- Added `craft\commerce\enums\LineItemType`.
- Added `craft\commerce\enums\TransferStatusType`.
Expand Down Expand Up @@ -55,12 +60,14 @@
- Added `craft\commerce\records\ProductType::$propagationMethod`.
- Added `craft\commerce\records\ProductType::$variantTitleTranslationKeyFormat`.
- Added `craft\commerce\records\ProductType::$variantTitleTranslationMethod`.
- Added `craft\commerce\services\CatalogPricing::createCatalogPricesQuery()`
- Added `craft\commerce\services\InventoryLocations::getAllInventoryLocationsAsList`
- Added `craft\commerce\services\LineItems::create()`.
- Added `craft\commerce\services\LineItems::resolveCustomLineItem()`.
- Added `craft\commerce\services\Transfers`.
- Deprecated `craft\commerce\models\LineItem::populateFromPurchasable()`. `populate()` should be used instead.
- Deprecated `craft\commerce\models\LineItem::refreshFromPurchasable()`. `refresh()` should be used instead.
- Deprecated `craft\commerce\services\CatalogPricing::createCatalogPricingQuery()`. `createCatalogPricesQuery()` should be used instead.
- Deprecated `craft\commerce\services\LineItems::createLineItem()`. `create()` should be used instead.
- Removed `craft\commerce\fieldlayoutelements\UserCommerceField`.

Expand Down
14 changes: 14 additions & 0 deletions src/base/StoreTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace craft\commerce\base;

use craft\base\Element;
use craft\commerce\models\Store;
use craft\commerce\Plugin;
use yii\base\InvalidConfigException;
Expand All @@ -25,6 +26,19 @@ trait StoreTrait
*/
public function getStore(): Store
{
// If the store ID is not set check to see if the class has a `siteId` property and use that.
if ($this->storeId === null && !$this instanceof Element) {
throw new InvalidConfigException('Store ID is required');
}

if ($this->storeId === null && $this instanceof Element) {
$store = Plugin::getInstance()->getStores()->getStoreBySiteId($this->siteId);
if (!$store) {
throw new InvalidConfigException('Unable to locate store for site ID: ' . $this->siteId);
}
$this->storeId = $store->id;
}

if (!$store = Plugin::getInstance()->getStores()->getStoreById($this->storeId)) {
throw new InvalidConfigException('Invalid store ID: ' . $this->storeId);
}
Expand Down
31 changes: 29 additions & 2 deletions src/controllers/OrdersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use craft\commerce\behaviors\StoreBehavior;
use craft\commerce\collections\InventoryMovementCollection;
use craft\commerce\db\Table;
use craft\commerce\elements\db\PurchasableQuery;
use craft\commerce\elements\Order;
use craft\commerce\enums\InventoryTransactionType;
use craft\commerce\enums\LineItemType;
Expand Down Expand Up @@ -44,6 +45,7 @@
use craft\db\Query;
use craft\db\Table as CraftTable;
use craft\elements\Address;
use craft\elements\db\ElementQuery;
use craft\elements\User;
use craft\errors\ElementNotFoundException;
use craft\errors\InvalidElementException;
Expand Down Expand Up @@ -1817,12 +1819,37 @@ private function _getTransactionsWithLevelsTableArray(array $transactions, int $
private function _addLivePurchasableInfo(array $results, int $siteId, int|false|null $customerId = null): array
{
$purchasables = [];
$store = Plugin::getInstance()->getStores()->getStoreBySiteId($siteId);
$baseCurrency = $store->getCurrency();

$elementIdsByType = [];
foreach ($results as $r) {
if (!array_key_exists($r['type'], $elementIdsByType)) {
$elementIdsByType[$r['type']] = [];
}
$elementIdsByType[$r['type']][] = $r['id'];
}

$purchasablesById = [];
foreach ($elementIdsByType as $type => $ids) {
if (!class_exists($type)) {
continue;
}

/** @var ElementQuery $query */
$query = $type::find();

if ($query instanceof PurchasableQuery) {
$query->forCustomer($customerId);
}

$purchasablesById = [...$purchasablesById, ...$query->id($ids)->all()];
}

foreach ($results as $row) {
/** @var PurchasableInterface|null $purchasable */
$purchasable = Plugin::getInstance()->getPurchasables()->getPurchasableById($row['id'], $siteId, $customerId);
$purchasable = ArrayHelper::firstWhere($purchasablesById, 'id', $row['id']);
if ($purchasable) {
$baseCurrency = $purchasable->getStore()->getCurrency();
// @TODO revisit when updating currencies for stores
$row['price'] = $purchasable->getSalePrice();
$row['promotionalPrice'] = $purchasable->getPromotionalPrice();
Expand Down
39 changes: 28 additions & 11 deletions src/elements/Product.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
use craft\base\Element;
use craft\base\ElementInterface;
use craft\base\Field;
use craft\commerce\base\HasStoreInterface;
use craft\commerce\base\StoreTrait;
use craft\commerce\behaviors\CurrencyAttributeBehavior;
use craft\commerce\elements\actions\CreateDiscount;
use craft\commerce\elements\actions\CreateSale;
use craft\commerce\elements\conditions\products\ProductCondition;
Expand Down Expand Up @@ -65,16 +68,25 @@
* @property ProductType $type
* @property Variant[]|array $variants an array of the product's variants
* @property-read string $defaultPriceAsCurrency
* @property-read string $defaultBasePriceAsCurrency
* @property float|null $defaultPrice
* @author Pixel & Tonic, Inc. <support@pixelandtonic.com>
* @since 2.0
*/
class Product extends Element
class Product extends Element implements HasStoreInterface
{
use StoreTrait;

public const STATUS_LIVE = 'live';
public const STATUS_PENDING = 'pending';
public const STATUS_EXPIRED = 'expired';

/**
* @var float|null
* @since 5.1.0
*/
public ?float $defaultBasePrice = null;

/**
* @inheritdoc
*/
Expand Down Expand Up @@ -611,6 +623,15 @@ public static function prepElementQueryForTableAttribute(ElementQueryInterface $
*/
private ?NestedElementManager $_variantManager = null;

/**
* @inheritdoc
* @since 5.1.0
*/
public function currencyAttributes(): array
{
return ['defaultPrice', 'defaultBasePrice'];
}

/**
* @throws InvalidConfigException
*/
Expand All @@ -625,6 +646,11 @@ public function behaviors(): array
],
];

$behaviors['currencyAttributes'] = [
'class' => CurrencyAttributeBehavior::class,
'currencyAttributes' => $this->currencyAttributes(),
];

return $behaviors;
}

Expand All @@ -648,15 +674,6 @@ public function getDefaultPrice(): ?float
return $this->_defaultPrice ?? $this->getDefaultVariant()?->price;
}

/**
* @return string
* @throws InvalidConfigException
*/
public function getDefaultPriceAsCurrency(): string
{
return $this->getDefaultVariant()?->priceAsCurrency ?? '';
}

public function canCreateDrafts(User $user): bool
{
// Everyone with view permissions can create drafts
Expand Down Expand Up @@ -1558,7 +1575,7 @@ protected function attributeHtml(string $attribute): string
}
case 'defaultPrice':
{
return $this->defaultPriceAsCurrency;
return $this->defaultBasePriceAsCurrency;
}
case 'stock':
{
Expand Down
5 changes: 4 additions & 1 deletion src/elements/conditions/variants/VariantCondition.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace craft\commerce\elements\conditions\variants;

use craft\commerce\elements\conditions\purchasables\SkuConditionRule;
use craft\commerce\elements\Variant;
use craft\elements\conditions\ElementCondition;

Expand All @@ -18,6 +19,8 @@ class VariantCondition extends ElementCondition
*/
protected function selectableConditionRules(): array
{
return array_merge(parent::selectableConditionRules(), []);
return array_merge(parent::selectableConditionRules(), [
SkuConditionRule::class,
]);
}
}
13 changes: 8 additions & 5 deletions src/elements/db/ProductQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -711,14 +711,12 @@ protected function afterPrepare(): bool
// Store dependent related joins to the sub query need to be done after the `elements_sites` is joined in the base `ElementQuery` class.
$customerId = Craft::$app->getUser()->getIdentity()?->id;

$catalogPricingQuery = Plugin::getInstance()
$catalogPricesQuery = Plugin::getInstance()
->getCatalogPricing()
->createCatalogPricingQuery(userId: $customerId)
->createCatalogPricesQuery(userId: $customerId)
->addSelect(['cp.purchasableId', 'cp.storeId'])
->leftJoin(['purvariants' => Table::VARIANTS], '[[purvariants.id]] = [[cp.purchasableId]]')
->andWhere(['purvariants.isDefault' => true])
;
$catalogPricesQuery = (clone $catalogPricingQuery)->andWhere(['isPromotionalPrice' => false]);
->andWhere(['purvariants.isDefault' => true]);

$this->subQuery->leftJoin(['sitestores' => Table::SITESTORES], '[[elements_sites.siteId]] = [[sitestores.siteId]]');
$this->subQuery->leftJoin(['catalogprices' => $catalogPricesQuery], '[[catalogprices.purchasableId]] = [[commerce_products.defaultVariantId]] AND [[catalogprices.storeId]] = [[sitestores.storeId]]');
Expand Down Expand Up @@ -747,14 +745,19 @@ protected function beforePrepare(): bool
'commerce_products.postDate',
'commerce_products.expiryDate',
'subquery.price as defaultPrice',
'commerce_products.defaultPrice as defaultBasePrice',
'commerce_products.defaultVariantId',
'commerce_products.defaultSku',
'commerce_products.defaultWeight',
'commerce_products.defaultLength',
'commerce_products.defaultWidth',
'commerce_products.defaultHeight',
'sitestores.storeId',
]);

// Join in sites stores to get product's store for current request
$this->query->leftJoin(['sitestores' => Table::SITESTORES], '[[elements_sites.siteId]] = [[sitestores.siteId]]');

$this->subQuery->addSelect(['catalogprices.price']);

if (isset($this->postDate)) {
Expand Down
17 changes: 6 additions & 11 deletions src/elements/db/PurchasableQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -636,20 +636,15 @@ protected function afterPrepare(): bool
$customerId = null;
}

$catalogPricingQuery = Plugin::getInstance()
$catalogPricesQuery = Plugin::getInstance()
->getCatalogPricing()
->createCatalogPricingQuery(userId: $customerId)
->createCatalogPricesQuery(userId: $customerId)
->addSelect(['cp.purchasableId', 'cp.storeId']);
$catalogPricesQuery = (clone $catalogPricingQuery)->andWhere(['isPromotionalPrice' => false]);
$catalogPromotionalPricesQuery = (clone $catalogPricingQuery)->andWhere(['isPromotionalPrice' => true]);
$catalogSalePriceQuery = (clone $catalogPricingQuery);

$this->subQuery->leftJoin(['sitestores' => Table::SITESTORES], '[[elements_sites.siteId]] = [[sitestores.siteId]]');
$this->subQuery->leftJoin(['purchasables_stores' => Table::PURCHASABLES_STORES], '[[purchasables_stores.storeId]] = [[sitestores.storeId]] AND [[purchasables_stores.purchasableId]] = [[commerce_purchasables.id]]');

$this->subQuery->leftJoin(['catalogprices' => $catalogPricesQuery], '[[catalogprices.purchasableId]] = [[commerce_purchasables.id]] AND [[catalogprices.storeId]] = [[sitestores.storeId]]');
$this->subQuery->leftJoin(['catalogpromotionalprices' => $catalogPromotionalPricesQuery], '[[catalogpromotionalprices.purchasableId]] = [[commerce_purchasables.id]] AND [[catalogpromotionalprices.storeId]] = [[sitestores.storeId]]');
$this->subQuery->leftJoin(['catalogsaleprices' => $catalogSalePriceQuery], '[[catalogsaleprices.purchasableId]] = [[commerce_purchasables.id]] AND [[catalogsaleprices.storeId]] = [[sitestores.storeId]]');
$this->subQuery->leftJoin(['inventoryitems' => Table::INVENTORYITEMS], '[[inventoryitems.purchasableId]] = [[commerce_purchasables.id]]');

return parent::afterPrepare();
Expand Down Expand Up @@ -689,8 +684,8 @@ protected function beforePrepare(): bool

$this->subQuery->addSelect([
'catalogprices.price',
'catalogpromotionalprices.price as promotionalPrice',
'catalogsaleprices.price as salePrice',
'catalogprices.promotionalPrice',
'catalogprices.salePrice',
]);

if (isset($this->sku)) {
Expand Down Expand Up @@ -719,11 +714,11 @@ protected function beforePrepare(): bool
}

if (isset($this->promotionalPrice)) {
$this->subQuery->andWhere(Db::parseNumericParam('catalogpromotionalprices.price', $this->promotionalPrice));
$this->subQuery->andWhere(Db::parseNumericParam('catalogprices.promotionalPrice', $this->promotionalPrice));
}

if (isset($this->salePrice)) {
$this->subQuery->andWhere(Db::parseNumericParam('catalogsaleprices.price' , $this->salePrice));
$this->subQuery->andWhere(Db::parseNumericParam('catalogprices.salePrice' , $this->salePrice));
}

if (isset($this->shippingCategoryId)) {
Expand Down
2 changes: 2 additions & 0 deletions src/migrations/Install.php
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,8 @@ public function createIndexes(): void
$this->createIndex(null, Table::CATALOG_PRICING, 'purchasableId', false);
$this->createIndex(null, Table::CATALOG_PRICING, 'storeId', false);
$this->createIndex(null, Table::CATALOG_PRICING, 'userId', false);
$this->createIndex(null, Table::CATALOG_PRICING, ['purchasableId', 'storeId', 'isPromotionalPrice', 'price'], false);
$this->createIndex(null, Table::CATALOG_PRICING, ['purchasableId', 'storeId', 'isPromotionalPrice', 'price', 'catalogPricingRuleId', 'dateFrom', 'dateTo'], false);
$this->createIndex(null, Table::CATALOG_PRICING_RULES, 'storeId', false);
$this->createIndex(null, Table::CATALOG_PRICING_RULES_USERS, 'catalogPricingRuleId', false);
$this->createIndex(null, Table::CATALOG_PRICING_RULES_USERS, 'userId', false);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace craft\commerce\migrations;

use craft\db\Migration;

/**
* m240830_081410_add_extra_indexes_to_catalog_pricing migration.
*/
class m240830_081410_add_extra_indexes_to_catalog_pricing extends Migration
{
/**
* @inheritdoc
*/
public function safeUp(): bool
{
$this->createIndex(null, '{{%commerce_catalogpricing}}', ['purchasableId', 'storeId', 'isPromotionalPrice', 'price'], false);
$this->createIndex(null, '{{%commerce_catalogpricing}}', ['purchasableId', 'storeId', 'isPromotionalPrice', 'price', 'catalogPricingRuleId', 'dateFrom', 'dateTo'], false);

return true;
}

/**
* @inheritdoc
*/
public function safeDown(): bool
{
echo "m240830_081410_add_extra_indexes_to_catalog_pricing cannot be reverted.\n";
return false;
}
}
Loading

0 comments on commit 3c077b7

Please sign in to comment.