Guard catalog pricing rule lookup during migrations#4309
Guard catalog pricing rule lookup during migrations#4309boboldehampsink wants to merge 6 commits into
Conversation
|
Added a second guard for the same migration-order path. After the catalog pricing rules table guard, the upgrade continued until product relation serialization hit |
|
Added another migration-order guard. The next failure was from |
|
Added a migration data-repair fix for The migration assumes every |
There was a problem hiding this comment.
Pull request overview
This PR hardens Commerce’s catalog pricing/store-aware query paths so Craft/Commerce upgrades can run Craft migrations without triggering SQL against Commerce tables/columns that may not exist yet.
Changes:
- Guard
CatalogPricingRules::hasCatalogPricingRules()so missingcommerce_catalogpricingrulesis treated as “no rules yet”. - Add schema-existence guards in
PurchasableQuery/ProductQueryto avoid joining store/inventory/catalog-pricing tables during partial schema states. - Ensure the donation multi-store migration creates a corresponding
commerce_purchasablesrow when missing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/services/CatalogPricingRules.php | Avoids querying catalog pricing rules when the table hasn’t been created yet. |
| src/migrations/m240219_194855_donation_multi_store.php | Ensures donations have a purchasable row before creating per-store purchasable-store rows. |
| src/elements/db/PurchasableQuery.php | Adds guards/fallback selects to prevent store/inventory/catalog pricing joins when schema isn’t ready. |
| src/elements/db/ProductQuery.php | Adds guards/fallback selects to prevent store/catalog pricing joins when schema isn’t ready. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ($this->_hasCatalogPricingRules === null) { | ||
| $this->_hasCatalogPricingRules = $this->_createCatalogPricingRuleQuery()->exists(); | ||
| $this->_hasCatalogPricingRules = Craft::$app->getDb()->tableExists(Table::CATALOG_PRICING_RULES) && | ||
| $this->_createCatalogPricingRuleQuery()->exists(); | ||
| } |
There was a problem hiding this comment.
Updated in f382275. hasCatalogPricingRules() now returns false early while commerce_catalogpricingrules is missing, without writing to the _hasCatalogPricingRules cache. Once the table exists later in the same migration/request, the method will query and memoize the real result.
| if ($this->db->columnExists(Table::PURCHASABLES, 'taxCategoryId')) { | ||
| $purchasable['taxCategoryId'] = (new Query()) | ||
| ->select('id') | ||
| ->from(Table::TAXCATEGORIES) | ||
| ->orderBy(['id' => SORT_ASC]) | ||
| ->scalar(); | ||
| } | ||
|
|
||
| $this->insert(Table::PURCHASABLES, $purchasable); |
There was a problem hiding this comment.
Updated in f382275. The donation purchasable backfill now includes dateCreated, dateUpdated, and a generated uid. It also checks for an available tax category before setting taxCategoryId, and throws a clear migration exception if one cannot be found rather than attempting an invalid insert.
| new Expression('NULL as [[availableForPurchase]]'), | ||
| new Expression('NULL as [[basePrice]]'), | ||
| new Expression('NULL as [[basePromotionalPrice]]'), | ||
| new Expression('NULL as [[freeShipping]]'), | ||
| new Expression('NULL as [[maxQty]]'), |
There was a problem hiding this comment.
Updated in f382275. The missing-store-table fallback now selects boolean-safe defaults for availableForPurchase and freeShipping (1 and 0) instead of NULL, so hydration won’t assign null to non-nullable bool properties.
| new Expression('NULL as [[minQty]]'), | ||
| new Expression('NULL as [[inventoryTracked]]'), | ||
| new Expression('NULL as [[allowOutOfStockPurchases]]'), | ||
| new Expression('NULL as [[promotable]]'), | ||
| new Expression('NULL as [[shippingCategoryId]]'), |
There was a problem hiding this comment.
Updated in f382275. The fallback selects 0 for inventoryTracked, allowOutOfStockPurchases, and promotable instead of NULL, matching the non-nullable bool properties on Purchasable.
Summary
falsefromCatalogPricingRules::hasCatalogPricingRules()when the catalog pricing rules table has not been created yet.commerce_catalogpricingruleswhile Craft/Commerce migrations are still being applied.Context
During a Craft 5 / Commerce 5 upgrade, a Craft CMS migration can save single entries and serialize product relation fields before Commerce has applied
m221026_105212_add_catalog_pricing_table. That path callsProductQuery::beforePrepare(), which callshasCatalogPricingRules(), and currently attemptsexists()againstcommerce_catalogpricingruleseven when the table does not exist yet.Treating the missing table as “no catalog pricing rules yet” lets migrations continue until Commerce creates the table normally.