Skip to content

Commit 71b525a

Browse files
authored
Merge pull request #4072 from craftcms/nathaniel/pt-2823-5x-catalog-pricing-doesnt-update-for-purchasables-that-meet
[5.7] Consolidate processing of catalog pricing jobs
2 parents 89d401e + cb3b0bc commit 71b525a

9 files changed

Lines changed: 833 additions & 33 deletions

File tree

CHANGELOG-WIP.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
### Extensibility
2424

25+
- Added `craft\commerce\base\ShippingMethod::clearMatchingShippingRuleCache()`.
2526
- Added `craft\commerce\controllers\CartController::actionCartChallenge()`.
2627
- Added `craft\commerce\controllers\CartController::actionCartSent()`.
2728
- Added `craft\commerce\controllers\CartController::actionEmailChallenge()`.
@@ -33,6 +34,7 @@
3334
- Added `craft\commerce\controllers\OrdersController::actionRemoveCustomerDataModal()`.
3435
- Added `craft\commerce\controllers\SubscriptionsController::actionDeleteSubscriptions()`.
3536
- Added `craft\commerce\controllers\SubscriptionsController::actionDeleteSubscriptionsModal()`.
37+
- Added `craft\commerce\db\Table::CATALOG_PRICING_QUEUE`.
3638
- Added `craft\commerce\elements\Order::getCustomerDeleted()`.
3739
- Added `craft\commerce\elements\Order::hasPurchasables()`.
3840
- Added `craft\commerce\elements\Order::setCustomerDeleted()`.
@@ -42,10 +44,17 @@
4244
- Added `craft\commerce\elements\deletionblockers\OrderCustomersDeletionBlocker`.
4345
- Added `craft\commerce\elements\deletionblockers\SubscriptionCustomersDeletionBlocker`.
4446
- Added `craft\commerce\enums\ContainsPurchasablesMatch`.
47+
- Added `craft\commerce\elements\db\OrderQuery::$containsPurchasables`.
48+
- Added `craft\commerce\elements\db\OrderQuery::containsPurchasables()`.
49+
- Added `craft\commerce\elements\Order::hasPurchasables()`.
50+
- Added `craft\commerce\enums\ContainsPurchasablesMatch`.
4551
- Added `craft\commerce\events\PaymentCurrencyRateEvent`, allowing plugins to override a payment currency's exchange rate at the point of use.
46-
- Added `craft\commerce\base\ShippingMethod::clearMatchingShippingRuleCache()`.
52+
- Added `craft\commerce\records\CatalogPricingQueue`.
4753
- Added `craft\commerce\services\Carts::getLoadCartUrl()`.
4854
- Added `craft\commerce\services\Carts::peekCart()`.
55+
- Added `craft\commerce\services\CatalogPricing::reserveCatalogPricingQueueRow()`.
56+
- Added `craft\commerce\services\CatalogPricing::releaseCatalogPricingQueueRowById()`.
57+
- Added `craft\commerce\services\CatalogPricing::deleteCatalogPricingQueueRowById()`.
4958
- Added `craft\commerce\services\Orders::reassignOrders()`.
5059
- Added `craft\commerce\services\Orders::removeCustomerData()`.
5160
- Added `craft\commerce\services\PaymentCurrencies::EVENT_DEFINE_PAYMENT_CURRENCY_RATE`.

src/controllers/DownloadsController.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ private function renderEmailChallenge(
5050
bool $inline,
5151
array $errors = [],
5252
?string $email = null,
53-
): Response
54-
{
53+
): Response {
5554
$params = [
5655
'order' => $order,
5756
'orderNumber' => $orderNumber,

src/db/Table.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,7 @@ abstract class Table
7474
public const INVENTORYLOCATIONS = '{{%commerce_inventorylocations}}';
7575
public const INVENTORYLOCATIONS_STORES = '{{%commerce_inventorylocations_stores}}';
7676
public const INVENTORYTRANSACTIONS = '{{%commerce_inventorytransactions}}';
77+
78+
/** @since 5.7.0 */
79+
public const CATALOG_PRICING_QUEUE = '{{%commerce_catalogpricing_queue}}';
7780
}

src/migrations/Install.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use craft\commerce\models\SiteStore;
2020
use craft\commerce\models\Store;
2121
use craft\commerce\Plugin;
22+
use craft\commerce\records\CatalogPricingQueue;
2223
use craft\commerce\records\CatalogPricingRule;
2324
use craft\commerce\records\InventoryLocation;
2425
use craft\commerce\records\TaxCategory;
@@ -132,6 +133,18 @@ public function createTables(): void
132133
'uid' => $this->uid(),
133134
]);
134135

136+
$this->archiveTableIfExists(Table::CATALOG_PRICING_QUEUE);
137+
$this->createTable(Table::CATALOG_PRICING_QUEUE, [
138+
'id' => $this->primaryKey(),
139+
'storeId' => $this->integer(),
140+
'type' => $this->enum('type', [CatalogPricingQueue::TYPE_PURCHASABLE, CatalogPricingQueue::TYPE_RULE])->notNull(),
141+
'ids' => $this->mediumText(),
142+
'reserved' => $this->boolean()->notNull()->defaultValue(false),
143+
'dateCreated' => $this->dateTime()->notNull(),
144+
'dateUpdated' => $this->dateTime()->notNull(),
145+
'uid' => $this->uid(),
146+
]);
147+
135148
$this->archiveTableIfExists(Table::CUSTOMERS);
136149
$this->createTable(Table::CUSTOMERS, [
137150
'id' => $this->primaryKey(), // Not used in v4 but is the old customerId
@@ -1091,6 +1104,8 @@ public function createIndexes(): void
10911104
$this->createIndex(null, Table::CATALOG_PRICING, ['purchasableId', 'storeId', 'isPromotionalPrice', 'price', 'catalogPricingRuleId', 'dateFrom', 'dateTo'], false);
10921105
$this->createIndex(null, Table::CATALOG_PRICING, ['purchasableId', 'storeId', 'isPromotionalPrice', 'price'], false);
10931106
$this->createIndex(null, Table::CATALOG_PRICING, ['purchasableId', 'storeId'], false);
1107+
$this->createIndex(null, Table::CATALOG_PRICING_QUEUE, 'reserved', false);
1108+
$this->createIndex(null, Table::CATALOG_PRICING_QUEUE, ['storeId', 'type', 'reserved'], false);
10941109
$this->createIndex(null, Table::CATALOG_PRICING_RULES, 'storeId', false);
10951110
$this->createIndex(null, Table::CATALOG_PRICING_RULES_USERS, 'catalogPricingRuleId', false);
10961111
$this->createIndex(null, Table::CATALOG_PRICING_RULES_USERS, 'userId', false);
@@ -1215,6 +1230,7 @@ public function addForeignKeys(): void
12151230
$this->addForeignKey(null, Table::CATALOG_PRICING, ['purchasableId'], Table::PURCHASABLES, ['id'], 'CASCADE', 'CASCADE');
12161231
$this->addForeignKey(null, Table::CATALOG_PRICING, ['storeId'], Table::STORES, ['id'], 'CASCADE');
12171232
$this->addForeignKey(null, Table::CATALOG_PRICING, ['userId'], CraftTable::USERS, ['id'], 'CASCADE');
1233+
$this->addForeignKey(null, Table::CATALOG_PRICING_QUEUE, ['storeId'], Table::STORES, ['id'], 'CASCADE', 'CASCADE');
12181234
$this->addForeignKey(null, Table::CATALOG_PRICING_RULES, ['storeId'], Table::STORES, ['id'], 'CASCADE', 'CASCADE');
12191235
$this->addForeignKey(null, Table::CATALOG_PRICING_RULES_USERS, ['catalogPricingRuleId'], Table::CATALOG_PRICING_RULES, ['id'], 'CASCADE', 'CASCADE');
12201236
$this->addForeignKey(null, Table::CATALOG_PRICING_RULES_USERS, ['userId'], CraftTable::USERS, ['id'], 'CASCADE', 'CASCADE');
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace craft\commerce\migrations;
4+
5+
use craft\commerce\db\Table;
6+
use craft\commerce\records\CatalogPricingQueue;
7+
use craft\db\Migration;
8+
9+
/**
10+
* m260407_000000_add_catalog_pricing_queue_table migration.
11+
*/
12+
class m260407_000000_add_catalog_pricing_queue_table extends Migration
13+
{
14+
/**
15+
* @inheritdoc
16+
*/
17+
public function safeUp(): bool
18+
{
19+
if (!$this->db->tableExists(Table::CATALOG_PRICING_QUEUE)) {
20+
$this->createTable(Table::CATALOG_PRICING_QUEUE, [
21+
'id' => $this->primaryKey(),
22+
'storeId' => $this->integer(),
23+
'type' => $this->enum('type', [CatalogPricingQueue::TYPE_PURCHASABLE, CatalogPricingQueue::TYPE_RULE])->notNull(),
24+
'ids' => $this->mediumText(),
25+
'reserved' => $this->boolean()->notNull()->defaultValue(false),
26+
'dateCreated' => $this->dateTime()->notNull(),
27+
'dateUpdated' => $this->dateTime()->notNull(),
28+
'uid' => $this->uid(),
29+
]);
30+
}
31+
32+
$this->createIndexIfMissing(Table::CATALOG_PRICING_QUEUE, 'reserved', false);
33+
$this->createIndexIfMissing(Table::CATALOG_PRICING_QUEUE, ['storeId', 'type', 'reserved'], false);
34+
$this->addForeignKey(null, Table::CATALOG_PRICING_QUEUE, ['storeId'], Table::STORES, ['id'], 'CASCADE', 'CASCADE');
35+
36+
return true;
37+
}
38+
39+
/**
40+
* @inheritdoc
41+
*/
42+
public function safeDown(): bool
43+
{
44+
echo "m260407_000000_add_catalog_pricing_queue_table cannot be reverted.\n";
45+
return false;
46+
}
47+
}

src/queue/jobs/CatalogPricing.php

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace craft\commerce\queue\jobs;
99

1010
use craft\commerce\Plugin;
11+
use craft\commerce\records\CatalogPricingQueue as CatalogPricingQueueRecord;
1112
use craft\queue\BaseJob;
1213

1314
class CatalogPricing extends BaseJob
@@ -29,14 +30,57 @@ class CatalogPricing extends BaseJob
2930

3031
public function execute($queue): void
3132
{
33+
$catalogPricingService = Plugin::getInstance()->getCatalogPricing();
34+
$isConsolidatedJob = $this->storeId === null && $this->purchasableIds === null && $this->catalogPricingRuleIds === null;
3235
$catalogPricingRules = null;
33-
if (!empty($this->catalogPricingRuleIds) && $this->storeId) {
34-
$catalogPricingRules = Plugin::getInstance()->getCatalogPricingRules()->getAllCatalogPricingRules($this->storeId)->whereIn('id', $this->catalogPricingRuleIds)->all();
36+
$reservedRowId = null;
37+
38+
// @TODO: remove these properties and behaviour at next breaking change
39+
$storeId = $this->storeId;
40+
$purchasableIds = $this->purchasableIds;
41+
$catalogPricingRuleIds = $this->catalogPricingRuleIds;
42+
43+
if ($isConsolidatedJob) {
44+
// New method of processing catalog pricing via queue table: reserve a row and process based on its type and IDs
45+
$reservedRecord = $catalogPricingService->reserveCatalogPricingQueueRow();
46+
47+
if (!$reservedRecord) {
48+
return;
49+
}
50+
51+
$reservedRowId = $reservedRecord->id;
52+
$storeId = $reservedRecord->storeId;
53+
54+
if ($reservedRecord->type === CatalogPricingQueueRecord::TYPE_PURCHASABLE) {
55+
// Specific purchasable IDs: regenerate against all applicable rules
56+
$purchasableIds = $reservedRecord->getIds();
57+
} elseif ($reservedRecord->type === CatalogPricingQueueRecord::TYPE_RULE) {
58+
$catalogPricingRuleIds = $reservedRecord->getIds();
59+
} else {
60+
throw new \UnexpectedValueException("Unrecognized catalog pricing queue row type: {$reservedRecord->type}");
61+
}
3562
}
3663

37-
Plugin::getInstance()->getCatalogPricing()->generateCatalogPrices($this->purchasableIds, $catalogPricingRules, queue: $queue);
64+
if (!empty($catalogPricingRuleIds)) {
65+
$catalogPricingRules = Plugin::getInstance()->getCatalogPricingRules()
66+
->getAllCatalogPricingRules($storeId)
67+
->whereIn('id', $catalogPricingRuleIds)
68+
->all();
69+
}
70+
71+
try {
72+
$catalogPricingService->generateCatalogPrices($purchasableIds, $catalogPricingRules, queue: $queue);
3873

39-
Plugin::getInstance()->getCatalogPricing()->clearCatalogPricingJob($this);
74+
if ($reservedRowId) {
75+
$catalogPricingService->deleteCatalogPricingQueueRowById($reservedRowId);
76+
}
77+
} catch (\Throwable $e) {
78+
if ($reservedRowId) {
79+
$catalogPricingService->releaseCatalogPricingQueueRowById($reservedRowId);
80+
}
81+
82+
throw $e;
83+
}
4084
}
4185

4286
protected function defaultDescription(): ?string
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
/**
3+
* @link https://craftcms.com/
4+
* @copyright Copyright (c) Pixel & Tonic, Inc.
5+
* @license https://craftcms.github.io/license/
6+
*/
7+
8+
namespace craft\commerce\records;
9+
10+
use craft\commerce\db\Table;
11+
use craft\db\ActiveRecord;
12+
use craft\helpers\Json;
13+
use yii\db\ActiveQueryInterface;
14+
15+
/**
16+
* Catalog Pricing Queue record.
17+
*
18+
* @property int $id
19+
* @property int|null $storeId
20+
* @property string $type
21+
* @property array|null $ids
22+
* @property bool $reserved
23+
* @property \DateTime $dateCreated
24+
* @property \DateTime $dateUpdated
25+
* @property string $uid
26+
* @author Pixel & Tonic, Inc. <support@pixelandtonic.com>
27+
* @since 5.7.0
28+
*/
29+
class CatalogPricingQueue extends ActiveRecord
30+
{
31+
/**
32+
* Row type for purchasable-ID-based catalog pricing work.
33+
*/
34+
public const TYPE_PURCHASABLE = 'purchasable';
35+
36+
/**
37+
* Row type for rule-ID-based (or full-regeneration) catalog pricing work.
38+
*/
39+
public const TYPE_RULE = 'rule';
40+
41+
/**
42+
* @inheritdoc
43+
*/
44+
public static function tableName(): string
45+
{
46+
return Table::CATALOG_PRICING_QUEUE;
47+
}
48+
49+
/**
50+
* Returns the decoded IDs array from the JSON column value.
51+
*
52+
* @return array|null
53+
*/
54+
public function getIds(): ?array
55+
{
56+
$raw = $this->getAttribute('ids');
57+
58+
if ($raw === null || $raw === '') {
59+
return null;
60+
}
61+
62+
$decoded = Json::decodeIfJson($raw);
63+
64+
return is_array($decoded) ? $decoded : null;
65+
}
66+
67+
/**
68+
* Encodes the IDs array to JSON and stores it in the column.
69+
*
70+
* @param array|null $ids
71+
*/
72+
public function setIds(?array $ids): void
73+
{
74+
$this->setAttribute('ids', $ids !== null ? Json::encode($ids) : null);
75+
}
76+
77+
/**
78+
* @return ActiveQueryInterface
79+
*/
80+
public function getStore(): ActiveQueryInterface
81+
{
82+
return $this->hasOne(Store::class, ['id' => 'storeId']);
83+
}
84+
}

0 commit comments

Comments
 (0)