Skip to content

6.x - #4124

Draft
lukeholder wants to merge 280 commits into
5.xfrom
6.x
Draft

6.x#4124
lukeholder wants to merge 280 commits into
5.xfrom
6.x

Conversation

@lukeholder

@lukeholder lukeholder commented Sep 23, 2025

Copy link
Copy Markdown
Member

@lukeholder
lukeholder requested a review from a team as a code owner September 23, 2025 07:42
@lukeholder
lukeholder marked this pull request as draft September 23, 2025 07:42
lukeholder added 27 commits July 1, 2026 17:12
Replace private function impl() helpers in all migrated src-yii2 services
with inline app(\CraftCms\Commerce\Services\X::class) calls at each method
site, matching how craftcms/yii2-adapter/legacy/services/ does it.

Affected: Currencies, PaymentCurrencies, ShippingCategories, ShippingZones,
TaxCategories, TaxZones.
…cingRules, CatalogPricing, Discounts, Sales)

- New `CraftCms\Commerce\Services\{Coupons,CatalogPricingRules,CatalogPricing,Discounts,Sales}` singletons in `src/Services/`
- Legacy `craft\commerce\services\*` wrappers in `src-yii2/services/` replaced with thin delegation to `app()` pattern
- `CraftCms\Commerce\Promotion\Models\Discount` updated to import `CraftCms\Commerce\Services\Coupons`
- Cross-cutting: `DB::table()` for reads, Yii2 ActiveRecord kept for writes, `event()` for Laravel events, `DB::beginTransaction/commit/rollBack`, `Cache::get/forever`, `Log::info()`, `DB::raw()` for expressions
…ariantQuery

craft\elements\db\NestedElementQueryTrait is now aliased to
CraftCms\Cms\Element\Queries\Concerns\QueriesNestedElements, which
declares two abstract methods that must be implemented by any class
using the trait. VariantQuery already handles elements_owners joins
manually in beforePrepare(), so these implementations just satisfy the
PHP abstract contract.
…re contract

ProductQuery and SubscriptionQuery both overrode statusCondition() returning
mixed/array, but CraftCms\Cms\Element\Queries\ElementQuery now declares the
return type as Closure. Updated both to return Closure using Laravel Builder
lambdas instead of Yii2 array conditions.
…sModel() contract

createSettingsModel() in the legacy Plugin declares ?craft\base\Model return type.
CraftCms\Commerce\Settings was extending CraftCms\Cms\Component\Component which is
not in the craft\base\Model hierarchy, causing a TypeError at runtime.

Switch Settings to extend craft\base\Model and convert validation from the new
getRules()/Laravel style to defineRules()/Yii2 style.
Migrate TaxRates, Taxes, and Vat from craft\commerce\services to
CraftCms\Commerce\Services. Legacy classes become thin Yii2 Component
wrappers delegating via app().

Bonus leaf-dependency migrations needed by Taxes:
- craft\commerce\engines\Tax -> CraftCms\Commerce\Tax\Engines\Tax
- craft\commerce\taxidvalidators\EuVatIdValidator -> CraftCms\Commerce\Tax\Models\EuVatIdValidator
  (Craft::createGuzzleClient() -> Http facade, Craft::error() -> Log::error())
- TaxEngineEvent::$engine now type-hints the new TaxEngineInterface

Resolved TODOs in TaxRate/TaxCategory/TaxAddressZone models to call the
new services directly. Dropped Vat::getVatValidator(), deprecated since
5.3.0.
… not the class_alias name

PHP's runtime type enforcement does not treat a class_alias()-derived name
as interchangeable with its target for parameter/return/property type
declarations, even though get_class()/instanceof/new all work correctly
through the alias. A legacy stub method declared `function getFoo(): ?Foo`
(where Foo is `use craft\commerce\models\Foo;`, an alias) throws a
TypeError when it returns an instance produced by the new service, which
returns CraftCms\Commerce\X\Models\Foo directly. Confirmed with a minimal
reproduction; reproduces identically for parameter types.

Fixed by importing the new FQCN directly (under the old short name) in
every affected stub: TaxRates, Taxes (getEngine() needed two imports,
since the class's own `implements TaxEngineInterface` must stay on the
legacy interface while getEngine()'s return points at the new one),
Coupons, CatalogPricingRules, Discounts, Sales. Stage 6a/6c stubs already
used the new-FQCN pattern and needed no changes.

Also fixes craft\commerce\collections\UpdateInventoryLevelCollection::make(),
whose signature was incompatible with the current
Illuminate\Support\Collection::make($items = [], ...$args) (missing the
variadic ...$args), causing a compile-time fatal on any use.

Found while verifying Stage 6e (not yet committed) but affects already-
shipped Stage 6b/6d code, hence the standalone commit. Not fixed (flagged
in CHANGELOG-WIP.md for follow-up): DiscountEvent::$discount has the same
root-cause bug as a typed property, and InventoryManualMovement's
to/fromLocationAfterQuantity() declare `: int` but DB::value() can return
a numeric string under strict_types.
Migrate Inventory and InventoryLocations from craft\commerce\services to
CraftCms\Commerce\Services. Legacy classes become thin Yii2 Component
wrappers delegating via app().

Transfers (the service) is deferred -- it's tied to the Transfer element
and Craft's legacy Field Layout/project-config system (ConfigEvent,
craft\models\FieldLayout, TransferManagementField), none of which are
migrated yet. Same blocker class as ProductType in Stage 5.

All raw SQL (craft\db\Query, [[col]] quoting, yii\db\Expression) converted
to Laravel's query builder, including a subquery join (leftJoinSub) and
raw CASE WHEN pivots (selectRaw with bindings) in getInventoryLevelQuery().
Craft::$app->getDb()->beginTransaction()/commit()/rollBack() -> DB
equivalents; Craft::$app->getUser()->getIdentity()?->id ->
request()->craftUser()?->id; Db::prepareDateForDb(new \DateTime()) ->
now()->toDateTimeString().

getInventoryLevelQuery()'s $limit/$offset must only call
->limit()/->offset() when non-null -- Laravel's query builder (unlike
Yii2's Query) emits a literal OFFSET 0 for a null offset, which MySQL
rejects without an accompanying LIMIT.

The two Inventory events still fire through the legacy
Plugin::getInstance()->getInventory() component so existing
Event::on(Inventory::class, ...) listeners keep working. The two
InventoryLocations element-authorization event handlers are still
registered against the legacy component instance in Plugin.php for the
same reason.

Also resolved TODOs in InventoryItemTrait, InventoryLocationTrait,
InventoryLevel, InventoryTransaction, InventoryFulfillmentLevel,
InventoryLocation, DeactivateInventoryLocation, TransferDetail, and Store
-- they now call the new services directly instead of going through
Plugin::getInstance().

Verified live in ddev: location CRUD, inventory item creation, level
computation (the subquery+CASE WHEN pivot query), set/adjust inventory
levels, cross-location movement via location deactivation, all through
both the new service and the legacy delegation path.
…wayInterface

GatewayInterface::processWebHook() already declares Illuminate\Http\Response
as its return type, but Dummy, Manual, and MissingGateway still declared
the legacy craft\web\Response, violating the interface.

Also update ShippingMethod::get() to call the new
CraftCms\Commerce\Services\ShippingMethods service directly instead of
going through Plugin::getInstance().
src/Helpers/Gql extended the deprecated craft\helpers\Gql wrapper instead
of CraftCms\Cms\Gql\GqlHelper directly, and imported the wrong GqlSchema
(CraftCms\Cms\Gql\Models\GqlSchema, an Eloquent persistence model) instead
of CraftCms\Cms\Gql\Data\GqlSchema, the runtime schema-scope object that
GqlHelper::isSchemaAwareOf()/extractAllowedEntitiesFromSchema() actually
expect -- a real latent type mismatch, not just a style issue.

The four gql/handlers classes (HasProduct, HasVariant, RelatedProducts,
RelatedVariants) extended the deprecated craft\gql\base\ArgumentHandler/
RelationArgumentHandler instead of CraftCms\Cms\Gql\Handlers\*  directly.
Confirmed the new base classes provide the same $argumentName/
$argumentManager/handleArgument()/getIds() surface these subclasses rely
on, so the swap is behavior-preserving.
Ports 5.x changes (323 commits since the 6.x branch point) onto the
Laravel migration in progress. See CHANGELOG-WIP.md for a summary of
what was ported into already-migrated src/ services vs left as
src-yii2/ legacy code.
…rt collision in CatalogPricing

PHP use-statement conflict detection is case-insensitive, so importing
both craft\helpers\Db and Illuminate\Support\Facades\DB in the same file
is a fatal error ("Cannot use ... as DB because the name is already in
use"), regardless of casing. Aliased the Craft helper import to CraftDb
since the Laravel facade has far more call sites in this file.
It describes order notices (customer/admin), not inventory — belongs
alongside CraftCms\Commerce\Order\Models\OrderNotice rather than under
CraftCms\Commerce\Inventory\Enums.
Laravel's Migrator derives a migration's class name from its filename
assuming the YYYY_MM_DD_HHMMSS_description.php convention. Commerce's
Yii2-style mYYMMDD_HHMMSS_description.php files don't fit, so the
derivation produces a bogus name, the "already loaded" guard misses,
and the Migrator does a second bare require on a file already
require_once'd - a straight "Cannot redeclare class" fatal on every
`craft:migrate/all` run.

Fix:
- Rename the 6 migrations that haven't been applied anywhere yet to
  Laravel's naming convention, and rewrite them as genuine Laravel
  migrations (return new class extends CraftCms\Cms\Database\Migration,
  using Schema/DB facades) rather than Yii2-style named classes -
  needed because Laravel's own MigrationStarted/MigrationEnded events
  hard-type-hint Illuminate\Database\Migrations\Migration, which a
  Yii2 Migration object can never satisfy via duck-typing alone.
- Delete the other 134 migrations: Install.php already represents the
  full current schema (verified table-by-table) and they're already
  applied on every real install, so per the plugin migration docs'
  own guidance ("bring Install up to date, then cull the rest") they
  no longer need to exist as files. Laravel's migration repository
  keeps their historical rows regardless.
- Add getConnection()/withinTransaction compat members to the
  yii2-adapter's craft\db\Migration (cms-6 repo) so any remaining
  Yii2-style Migration objects that don't hit the strict-event-typing
  path (e.g. Install.php, looked up by hardcoded filename rather than
  the event-firing incremental-migration path) work with the new
  Migrator too.

Verified: `ddev artisan craft:migrate/all` applies all 6 cleanly, a
second run reports nothing pending, and the resulting schema/FK
changes were spot-checked directly against the database.
Turns out the naming-convention fix alone wasn't enough: Laravel's
Migrator fires MigrationStarted/MigrationEnded events that hard-type-hint
Illuminate\Database\Migrations\Migration, which a Yii2 \yii\db\Migration
object (even with the getConnection()/withinTransaction compat added to
craft\db\Migration) can never satisfy - PHP type-hints require actual
instanceof, not duck-typing, and a class can't extend both migration
base classes at once.

Rewrote all 6 as `return new class extends CraftCms\Cms\Database\Migration`
using Schema/DB facades instead of craft\db\Migration's Yii2-style
helpers. The one case needing an existing-FK lookup-and-drop (subscriptions
FK swap) uses craft\helpers\Db::dropForeignKeyIfExists() directly, which
is a static helper independent of which Migration base class is in use.

Verified: `ddev artisan craft:migrate/all` applies all 6 cleanly, second
run reports nothing pending, and the schema/FK changes were confirmed
directly against the database.
craft\commerce\Plugin now extends CraftCms\Commerce\Plugin (new,
extends CraftCms\Cms\Plugin\Plugin - a Laravel ServiceProvider) instead
of the legacy craft\base\Plugin (a yii\base\Module). These two plugin
systems aren't bridged, so this drops Commerce out of the Yii2
Module/component-locator system entirely.

The only real gap was the component locator backing all 46
`Plugin::getInstance()->getFoo()` service getters (838 call sites
across src-yii2/, confirmed every single one resolves through a named
getter with zero raw/dynamic locator access) - ported as
src/Plugin/Concerns/HasServices.php, a lazy-instantiate-and-cache
trait mirroring the old getter API exactly, now inherited by
craft\commerce\Plugin instead of using-and-superseding
src-yii2/plugin/Services.php (deleted).

Everything else (46 Event::on() registrations, projectConfig
listeners, GQL, widgets, permissions, etc. in the ~19 `_register*()`
methods) turned out not to depend on Module-ness at all - they're
static Event::on() calls and $this->getFoo() getter calls, both
unaffected by the base class swap. So init() just became boot()
(dropping the now-meaningless parent::init() call) with its body
otherwise untouched.

One real behavioral fix was needed: the new base's getCpNavItem()
returns a NavItem object instead of an array, and NavItem::$subnav is
typed array|false (not assoc-array-keyed-by-handle), so the existing
$ret['subnav']['orders'] = [...] pattern would have fataled on the
false default. Convert to array via NavItem::toArray() first and
normalize subnav to [] before the existing per-permission nested
assignments, which needed no other changes.

Also: is()/$edition/editions() didn't need porting at all - already
provided by the new base's HasEditions concern with identical
semantics. Only Plugin::getInstance()->id (a single call site in
Products.php) needed fixing, to ->handle, since the new base has no
id property.

composer.json needed no changes - plugin discovery already keys off
extra.handle with no extra.class or extra.laravel.providers entry, and
the discovered class name/namespace (craft\commerce\Plugin) didn't
change, only what it extends.

Verified: plugin loads and instantiates as craft\commerce\Plugin, all
service getters resolve and memoize correctly, a live HTTP request to
/admin/login returns 200 (exercising the real web bootstrap path,
where boot() must have run without throwing), and the NavItem/subnav
conversion was verified in isolation against the exact shape
NavItem::toArray() actually produces. Full manual click-through of the
CP (nav items, widgets, permission-gated pages) still needed since an
authenticated browser session wasn't available in this environment -
see next steps.
…s resolve again

Switching craft\commerce\Plugin off yii\base\Module (Stage 8) broke
every legacy-dispatched Commerce CP/site route with a 404, even though
the URL rules themselves still matched correctly. Traced with targeted
diagnostics (confirmed and removed): boot() runs every request, the
Event::on(UrlManager::class, EVENT_REGISTER_CP_URL_RULES, ...) handler
fires and adds the rules, and UrlManager::parseRequest() correctly
resolves 'commerce/orders' to the route string 'commerce/orders/order-index'.

The break is one level deeper: yii\base\Module::createController()
looks up Craft::$app->getModule('commerce') to find the controller
namespace for any route starting with 'commerce/'. That only ever
returned an instance of craft\commerce\Plugin because it was itself a
Module (via the old craft\base\Plugin ancestry) - once it stopped
being one, getModule('commerce') returns null and controller
resolution fails, independent of the URL rule matching working fine.

None of the 48 controllers or craft\web\Controller reference $this->module,
so a full standalone shim is safe: LegacyRoutingModule extends
yii\base\Module, sets controllerNamespace to craft\commerce\controllers,
and gets registered via Craft::$app->setModule('commerce', ...) in
boot(). This is routing-only - it doesn't touch settings/permissions/
services, all of which already work via the new Plugin class.

Verified against a real authenticated CP session (via
craft:users:impersonate): /admin/commerce/orders, /commerce/products,
/commerce/settings/general all return 200 with correct page content;
/commerce/store-management redirects to the default store as expected;
CP nav still renders with working links to all of these.
Migration fixes for Craft 6's Laravel migrate runner, plus Stage 8
(Plugin.php + ServiceProvider) and the legacy routing module fix
needed to keep Commerce's CP working under the new Plugin lifecycle.
…ayments)

Migrates the four payment-related services to src/Services/, in
leaf-to-root order, with legacy src-yii2/services/ stubs delegating via
app(X::class) per the established pattern.

Fixes a latent bug found while wiring events: TransactionEvent,
PaymentSourceEvent, ProcessPaymentEvent, and RefundTransactionEvent have
no constructor, so constructing them Yii2-style silently discarded the
data instead of throwing. Also widens RefundTransactionEvent::$amount to
nullable to match its only real caller.

Removes Gateways::getGatewayOverrides() and Transactions::deleteTransaction(),
both long-deprecated with zero remaining call sites.
Migrates three of the four Stage 6g services to src/Services/, with
legacy src-yii2/services/ stubs delegating via app(X::class).

ProductTypes is deferred to Stage 7: saveProductType() directly persists
the same dual FieldLayoutBehavior (Product + Variant) data that already
deferred the ProductType model in Stage 5l.
Purchasables::$purchasableById was never cleared when a purchasable was
saved with new attribute values, only on deletePurchasableById() - a
purchasable resolved (and cached) earlier in a request would keep
returning the stale pre-save instance for the rest of the request.
Same gap exists in the real 5.x branch (commerce-5/src/services/Purchasables.php),
so this isn't a migration-introduced bug, just never fixed upstream either.

Extracted the cache-forgetting logic from deletePurchasableById() into
forgetCachedPurchasable(), and call it from Purchasable::afterSave() too.
Explanatory prose on afterValidate(), the LineItemRules class
docblock, and a test comment - all removed in favor of self-evident
code and method/test names. Kept the @extends Ruleset<LineItem>
annotation (a real phpstan generic-type hint) and the qty-vs-fulfilled
@todo (a genuine tracked task).
…4255)

Order::fields() re-serializes datetime attributes into ['date' => ...,
'time' => ...] arrays for the control panel's Vue components (already
present in 6.x, same @todo-flagged legacy shape as 5.x). Since
renderObjectTemplate()/renderSandboxedObjectTemplate() source template
variables from fields() before falling back to raw attributes, any
object template referencing a datetime attribute (order reference
format, a PDF file name format) received that array instead of a
DateTime, throwing "Array to string conversion".

Added Order::getObjectTemplateVariables(), passed as extra $variables
at every call site that renders an object template against an order
(order reference generation, DownloadsController, PaymentsController,
DownloadOrderPdfAction, Emails) - renderObjectTemplate() won't
overwrite variables that are already set. Ported from 5.x commit
e516aa1, including its test.
order.getpaymentAmount() (wrong case, no formatting) -> order.getPaymentAmount()|number.
Fixes #4109.
…limits

DiscountsController::clearDiscountUses() had no permission check at all,
unlike every sibling action in the controller (updateStatus, save, delete).
Any user with commerce-manageOrders could delete any customer's payment
source. Now requires editUsers plus (commerce-editOrders or
commerce-deleteOrders) to delete a payment source belonging to another
customer, matching the final state of 5.x's iterative fix
(1b99f75/b40ac34eb/aff5eb41a).
Payment currencies' CP routes (index/edit) and action routes (save/delete)
sat in the outer commerce-manageStoreSettings group with no inner
permission gate, unlike every sibling settings area (shipping, tax,
promotions), which each nest their routes in their own can:commerce-manageX
middleware group.
Cp::requestedSite() returns null when the current user has no editable
sites, and ->getStore() on null fatals. All 11 Commerce dashboard widgets
now fall back to the primary store's ID in that case, matching 5.x's fix.
Fixes #4347.
…orders/carts

setFieldValuesFromRequest('fields') ran before shippingMethodHandle (and
other order/cart attributes) were updated from the request, so a field
layout visibility condition based on those attributes was evaluated
against stale state and the field got silently skipped. Moved the call
to after attributes are updated in both CartController::updateCart() and
OrdersController::save(). Fixes #4198.
5.x's src/ tree no longer maps directly onto 6.x's structure (split
into src/ under the CraftCms\Commerce\ namespace and src-yii2/ under
craft\commerce\), so a normal content merge isn't meaningful here.

Every real behavioral change since the last sync (5.7.3) was manually
ported to its src/ equivalent in the preceding commits: the Order
object-template datetime bug (#4255), a payment-amount JS formatting
typo (#4109), a missing commerce-editDiscounts check on clearing
discount usage limits, a payment source deletion ownership check,
missing commerce-managePaymentCurrencies route gating, dashboard
widgets fataling for users with no editable sites (#4347), missing
cross-store write authorization across ~10 settings controllers, and
custom fields with visibility conditions not saving on orders/carts
(#4198). Two fixes were N/A (subscriptions/plans were removed entirely
in 6.0) and three were already fixed differently or predated this sync
window (the createObject RCE, dompdf 3, a search-index purge fix).

This merge just records that origin/5.x up to 9269350 has been
incorporated, so it becomes the new merge-base for the next sync.
@linear-code

linear-code Bot commented Sep 15, 2026

Copy link
Copy Markdown

COM-613

…COM-632)

Adds convertCurrency() to CraftCms\Commerce\Payment\PaymentCurrencies
(the migrated Laravel service never had it — only convert()/convertAmount()
were carried over, which don't cover converting a float amount between two
arbitrary non-primary ISO currencies with optional rounding). The legacy
src-yii2 wrapper now delegates to it instead of reimplementing the logic,
and OrdersController::paymentAmountData()/Order::isPaymentAmountPartial()
call the migrated service directly instead of going through
Plugin::getInstance()->getPaymentCurrencies().
…yParams (COM-614)

cms-6's Laravel condition system dropped the legacy Yii2 mechanism that let a
condition instance restrict its own selectable rules via a $queryParams
allow-list, replacing it with rules' static isSelectableForCondition() (which
can't express this per-instance case). HasOrdersConditionRule used this to
hide the redundant Customer rule from its nested order condition builder,
since that condition is already scoped to a single customer externally.

Adds OrderCondition::$queryParams plus an isConditionRuleSelectable()
override that excludes rules whose getExclusiveQueryParams() overlaps it,
reusing the (previously unused) getExclusiveQueryParams() methods already
present on several Order\Conditions\* rules. HasOrdersConditionRule now sets
queryParams = ['customerId'] again instead of leaving the TODO.
Ports tests-yii2/unit/{models,adjusters}/{Sale,TaxRate,Store,LineItem,
Discount,Tax}Test.php and unit/services/GatewaysTest.php to Pest under
tests/Unit and tests/Feature, deleting the originals now that they're
fully replaced. Adjuster/Gateways mocking is rebuilt with Mockery
partial mocks in place of Codeception's stubbing.

Also fixes two real bugs surfaced while exercising this code for the
first time: LineItem::extraFields() never got ported (silently
dropped the 'purchasable' key from toArray()), and Tax adjuster's
per-purchasable-taxable branch was missing a (float) cast on a Teller
result, throwing a TypeError for any order taxed that way.
…OM-654)

Ports the remaining coverage from tests-yii2/unit/elements/order/
conditions/{OrderCondition,CouponCodeConditionRule,CustomerConditionRule}
Test.php (matchElement/modifyQuery/registry-completeness), which the
existing Pest ports for these classes hadn't covered yet — those only
tested newer functionality (queryParams scoping, case-insensitive
matching) added during the port itself.

Fixes two real bugs surfaced by actually exercising modifyQuery()
end-to-end for the first time:

- CustomerConditionRule's IN branch set the legacy
  $elementQuery->customerId() scope property, which is only consumed
  by OrderQuery's own beforeQuery callback — registered earlier (in
  the constructor) than the condition system's deferred rule-
  application callback ever runs, so the property was always set too
  late and the filter silently never applied. A "Customer" order
  condition rule matched every order regardless of the selected
  customer. Fixed to write to the query builder directly, matching
  the already-correct NOT_IN branch and CouponCodeConditionRule.
- The NOT_IN branch's coalesce(customerId, -1) null-handling trick
  loses column affinity in SQLite (this project's test driver),
  silently never matching; verified the original was correct on
  MySQL, but replaced it with a portable whereNull()/orWhere() form
  regardless.
… fix ordering bugs (COM-654, COM-662)

Extract OrderQuery/PurchasableQuery/ProductQuery scope filters into
Queries*Attributes traits with public static apply*() helpers, mirroring
core's QueriesAssetLocation/QueriesFields convention, and switch every
condition rule that was calling a scope setter on $elementQuery (always too
late, since the owning query's own beforeQuery callback registers first) to
call the relevant static directly against $query instead. Completes the
audit of condition rules flagged in the prior two commits: fixes 22 Order
rules via QueriesOrderAttributes, SkuConditionRule via
QueriesPurchasableAttributes, ProductTypeConditionRule's inline fix
converted to QueriesProductType, and PurchasableConditionRule/
VariantConditionRule/CatalogPricingRuleCustomerConditionRule/
CatalogPricingRulePurchasableCategoryConditionRule/
VariantProductConditionRule via core's inherited applyId()/applyRelatedTo()
or a bespoke ownerId fix.
…x bug)

Several lines assigned $request->input(...) straight to non-nullable/strictly-
typed ProductType properties with no fallback or (bool) cast, ported verbatim
from 5.x's getBodyParam() calls. 5.x's Yii2 model never declared strict
property types so this was silently harmless; the new Data class does declare
them, so the exact same request shape (e.g. saving with a title-format field
hidden/unsubmitted) throws a TypeError instead.

Also adds VariantQuerySmokeTest, closing a coverage gap for VariantQuery's
own scopes (typeId, productStatus, editable) that had no dedicated tests
before or after last session's Concerns/ trait extraction.
getConditionRules() returned a plain array in real 5.x, so empty() on it
correctly detected "no rules configured". The ported condition system wraps
it in a ConditionGroupInterface object instead, which empty() can never
treat as falsy — every affected check silently always evaluated true.

Real-world impact: Discount::hasOrderCondition() and friends always
returned true once a condition object existed, regardless of whether it had
any rules; CatalogPricingRule::getPurchasableIds() always ran its narrowing
queries even with no condition configured; CatalogPricing::generateCatalogPrices()
incorrectly skipped rules with no customer condition for most customers;
Product's config-export never found a ProductTypeConditionRule to narrow by.

Fixed by calling ->getRules() to reach the actual rule array. Found via
composer run phpstan (empty.expr); confirmed as a porting regression, not
a 5.x behavior, via git show origin/5.x.
…OM-661)

Interim step ahead of the Form API/new-UI condition builder: replace the
commerceConditionBuilderHtml Twig filter with controller-rendered
ConditionBuilderRenderer output passed into templates as plain variables.
Also fixes shippingzones/taxzones address-condition fields, which were
silently broken via a condition.builderHtml property that no longer exists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant