Repository navigation
6.x - #4124
Draft
lukeholder wants to merge 435 commits into
Draft
6.x#4124lukeholder wants to merge 435 commits into
lukeholder wants to merge 435 commits into
Conversation
lukeholder
marked this pull request as draft
September 23, 2025 07:42
lukeholder
added a commit
that referenced
this pull request
Aug 20, 2026
PR #4124 (head 6.x, base 5.x) stays open for the whole migration, so every push already triggers a pull_request run. The push: branches: [6.x] trigger was firing a second, redundant full CI run for the same commit.
… live autoload root
Two arch rules, both scoped to src/ and verified to actually catch violations
(Pest's toUse()/toBeUsedIn() resolve targets as classes/namespaces via its
ObjectsRepository, so plain function names outside its small hardcoded
core-language-construct list, and Class::method static-call strings, are
silently never matched — confirmed empirically before relying on either):
- No debug functions (die/dd/dump/env) in src/.
- src/ must not reference legacy Craft core classes, excluding craft\commerce
(allowed during the migration per this repo's CLAUDE.md).
Getting any arch() rule to run at all required moving
src-yii2/test/{fixtures/elements/ProductFixture.php,mockclasses/Purchasable.php}
to tests-yii2/, matching where every other pre-Pest reference/porting fixture
already lives. Pest's arch plugin enumerates every PSR-4 namespace declared in
composer.json to build its analysis graph, not just the expect() target, so
these two forgotten files sitting inside the live craft\commerce\ (src-yii2/)
autoload root — referencing craft\base\ElementInterface, which isn't even
aliased anymore — fataled the whole test run before either rule could
evaluate. Updated their two internal namespace declarations and the three
call sites that imported them (craft\commerce\test\* -> craftcommercetests\*)
to match their new location.
…rce\Gql\...) Same class of bug as the earlier src-yii2/Base and src-yii2/Enums fix, this time in the new src/ tree: namespace CraftCms\Commerce\Gql\Handlers (etc.) declared throughout, but the directory was src/gql/handlers (lowercase). Unlike the earlier StoreTrait case, this didn't fatal — Plugin::boot() registers CraftCms\Commerce\Gql\Handlers\HasProduct as a GQL argument handler via is_a($handler, ArgumentHandlerInterface::class, true), and is_a() with $allow_string swallows a failed autoload and just returns false. That surfaced as "Argument handler [...] must implement [ArgumentHandlerInterface]" on Tests/Feature CI, which was misleading: the class autoloads fine on macOS (case-insensitive host filesystem mounted into the ddev container) and so genuinely does implement the interface — it just couldn't be found by name on GitHub's case-sensitive Linux runners. Renamed src/gql -> src/Gql, handlers -> Handlers, types -> Types, types/input -> Types/Input, types/input/criteria -> Types/Input/Criteria. Re-scanned both src/ and src-yii2/ in full for any other namespace/directory casing mismatches; none remain.
…ueries, arguments)
Completes the GraphQL migration checklist: Arguments/Elements/{Product,Variant},
Interfaces/Elements/{Product,Variant}, Types/Elements/{Product,Variant},
Types/Generators/{ProductType,VariantType}, Types/Input/{IntFalse,Product,Variant},
Types/SaleType, Resolvers/Elements/{Product,Variant}, and Queries/{Product,Variant},
all under CraftCms\Commerce\Gql\. helpers/Gql.php was already fully ported in an
earlier commit; its legacy src-yii2 counterpart is now a thin deprecated subclass
matching the established pattern.
The "wire up schema-registration" checklist item turned out to already be mostly
done — Plugin.php already registered the GqlArguments handlers and the
GqlSchemaComponentsResolving/GqlEagerLoadableFieldsResolving listeners. The only
missing piece was populating the new $gqlTypes/$gqlQueries properties the base
Plugin class's HasGql concern reads automatically, replacing the legacy
Event::on(Gql::EVENT_REGISTER_GQL_TYPES/QUERIES) wiring entirely.
Also fixes 3 stray legacy craft\gql\* imports in already-migrated files
(Catalog/Variants.php, Gql/Types/Input/Criteria/{Product,Variant}Relation.php)
that the new Arch tests would otherwise have flagged, and points those two
Criteria classes at the new Arguments classes instead of the legacy ones.
Verified beyond phpstan/check-cs: manually executed a GraphQL query through
products -> variants -> sales against a seeded schema, and prebuilt/validated
the full schema (introspection path), both via a throwaway test since this
repo has no GQL feature-test harness yet.
…on on SQLite
Two compounding bugs, both in date/timezone handling around the "today" stat:
1. LocalTimestamp's SQLite branch returned the raw (UTC) column unconverted,
on the assumption "SQLite is only used by the test suite, which always
runs in UTC" — false: tests/TestCase.php pins the app's timezone to
America/Los_Angeles for determinism. Day/month grouping (TotalOrders and
every other stat using getChartQueryOptionsByInterval) would bucket orders
under their UTC calendar date instead of the app's configured one,
splitting a single day's data across two chart entries whenever UTC and
LA disagreed on the current date (i.e. for most of each 24-hour period).
Fixed by resolving the configured timezone's current UTC offset in PHP
(DateTimeZone::getOffset(), which accounts for DST) and applying it via
SQLite's datetime(column, '+/-N minutes') modifier, since SQLite has no
named-timezone SQL functions to call directly.
2. Separately, TotalOrdersTest's "today" dataset computed its expected
start/end dates via new DateTime('now') inside the ->with() array. Pest
resolves dataset closures before beforeEach()/app boot, i.e. before
TestCase::setUp() pins the timezone — so the dataset's "now" could be
read under a different default timezone than the one Stat itself later
uses when it independently recomputes "today" inside TotalOrders's own
constructor. Moved the date computation into the test body, after the
fixture (and app boot) has already run, so both sides agree by construction.
Verified with 5 repeated runs of the previously-100%-reproducible failure,
the full Stats suite, and the full test suite (109/109 passing, first fully
clean run this session).
Ports the five remaining Yii2 console controllers to CraftCms\Commerce\Console\Commands\*,
following the Illuminate\Console\Command + CraftCommand pattern already established by
the resave commands. GatewaysController's two real actions (list, webhook-url) split into
separate command classes, matching how cms-6 splits its own multi-action controllers
(e.g. project-config:get/set/apply). Legacy `commerce/*` CLI routes are preserved as
command aliases via $aliases, so existing scripts/muscle-memory keep working.
Destructive/interactive flows were translated to their Laravel-idiomatic equivalents where
that's a strict improvement with no behavior change for the required inputs: ResetData now
uses ConfirmableTrait (a --force bypass plus a components->task()-driven, single
DB::transaction()-wrapped delete, rather than the ad-hoc yes/no string prompt + manual
begin/commit/rollBack); ExampleTemplates and TransferCustomerData use Laravel's own
ask()/confirm() and only prompt for options not already supplied on the command line.
console/Controller.php (an empty pass-through base with nothing else extending it once the
above landed) is deleted outright, not stubbed — console controllers are CLI entry points
invoked by route string, not classes anything else in the codebase instantiates or
type-hints against, so there's no back-compat surface to preserve the way there is for
services/models.
Verified against a real dev install (not just phpstan/tests), which surfaced two real,
unrelated pre-existing bugs the migrated code paths hadn't exercised before:
- Gateway::set{Billing,Shipping}AddressCondition() didn't accept null, despite handling it
in the method body (setOrderCondition already did) - throws the moment any real gateway
config has a null condition, i.e. immediately for `commerce:gateways:list`.
- CatalogPricing::setQueueProgress() called method_exists(null, ...), which throws a
TypeError under PHP 8's stricter argument types - hits every no-queue call, i.e.
immediately for `commerce:pricing-catalog:generate`.
Both fixed. commerce:reset-data was verified by code review (DB::transaction wrapping,
correct table/column names) rather than executed live, since the harness's destructive-
action guard correctly declined to run a bulk-delete command even against a dev database
confirmed to have zero rows in every affected table.
Follow-up to 0a53428: a bad pathspec in that commit's `git add` (src-yii2/console, already handled by a separate git rm) silently aborted staging every other file passed in the same invocation, so Plugin.php's $commands registration for the 5 new console commands, the Gateway.php/CatalogPricing.php bugfixes those commands surfaced, and the CHANGELOG-WIP.md entry never actually made it into that commit — only the new command classes and the src-yii2 deletions did. The commands existed but were never wired up. Re-verified with phpstan and the full test suite now that Plugin.php's registration is actually included.
…ectConfigData,Purchasable}.php
All 8 already had complete src/Helpers/ counterparts from earlier migration work,
so this is mostly the usual legacy-stub conversion + consumer rewiring — but
verifying each one caught two real, latent bugs:
- src/Helpers/{Cp,Currency,Purchasable}.php still imported the legacy core
craft\helpers\Cp instead of CraftCms\Cms\Cp\FormFields (fieldHtml/moneyInputHtml/
textHtml) or the CraftCms\Cms template()/TemplateMode helpers (renderTemplate)
or FormFields::lightswitchFromConfig()->toHtml() (lightswitchHtml, itself
deprecated in cms-6's own yii2-adapter). This is a real gap in the "src/ should
not reference legacy Craft core classes" Arch rule added earlier this session:
Pest-arch's dependency-layer resolution excludes vendor-directory namespaces
entirely, so any craft\* class living under vendor/craftcms/cms (as opposed to
this repo's own src-yii2/) is invisible to it and can slip through undetected.
That's a separate, wider-reaching finding worth its own follow-up pass.
- CraftCms\Commerce\Helpers\Localization no longer `extends \craft\helpers\
Localization` (it only needs normalizePercentage(), and normalizeNumber() is
called via CraftCms\Cms\Support\Facades\I18N internally), but
PaymentsController.php called the commerce subclass's *inherited*
normalizeNumber() directly - undefined once the extends was dropped. Pointed
it at the I18N facade instead, matching Localization's own internal usage.
Rewired the 15 already-migrated src/ consumers that still imported the legacy
craft\commerce\helpers\* classes to use CraftCms\Commerce\Helpers\* directly.
Verified beyond phpstan/tests: called the FormFields-backed methods
(Cp::taxZoneFieldHtml, Currency::moneyInputHtml, Purchasable::skuInputHtml,
Purchasable::availableForPurchaseInputHtml) against a real dev install via
tinker, since none of the existing test suite exercises CP-rendering helpers.
…idgetTrait,StoreTrait,TaxEngineInterface,TaxIdValidatorInterface,ZoneInterface} All 9 already had complete src/ counterparts from earlier migration work (most already carried @deprecated docblocks pointing at them, and the CHANGELOG-WIP.md entries for their replacements already existed - this was purely the stub-conversion + verification pass). StatTrait is deliberately left as a real, untouched legacy trait, matching the GatewayTrait precedent from the Gateway migration: its properties were merged directly onto the new Stat class rather than ported to a dedicated trait, so there's no clean class_alias target, and it costs nothing to leave as free-standing legacy code for any third-party code still using it directly. Model.php is a genuine dead end: an empty pass-through subclass of craft\base\Model with zero consumers anywhere in the codebase. Pointed its stub at CraftCms\Cms\Component\Component directly (the documented modern base for what used to extend craft\base\Model - see docs/6.x/extend/models.md and every already-migrated Commerce model) rather than a nonexistent "new commerce Model" class. Comparing old vs new caught one real fidelity gap: legacy Stat implements both StatInterface and HasStoreInterface, but the new class only implemented StatInterface even though StoreTrait already satisfies HasStoreInterface's contract - just a dropped `implements` clause. Added it back. Rewired the 2 already-migrated src/ consumers (Catalog/Elements/Product.php, Store/Models/SiteStore.php) still importing the legacy craft\commerce\base\StoreTrait.
…ias TypeError race in service delegation layer
Wires the product/variant/transfer element-edit screens to the generic
EditElementController in routes/cp.php, moves Variables::getDonation() to
Plugin::getDonation(), and deletes LegacyRoutingModule.php (dead code - no
controllers remain in craft\commerce\controllers for it to resolve).
Also fixes a TypeError that any craft\commerce\services\* delegation method
returning/accepting a craft\commerce\{models,elements}\* class alias could
throw on its first call in a process (class_alias's own autoload racing the
return-type check). All 22 affected wrappers now type-hint against the
CraftCms\Commerce\* class directly, and Transfer::canView()/canSave()/
canDelete() now check the permission that's actually registered
(commerce-manageInventoryTransfers, not commerce-manageTransfers).
…ce/settings/producttypes
ProductTypesController::productTypeIndex()/editProductType() never passed
readOnly to their templates even though index.twig/_edit.twig both reference
it, throwing a Twig RuntimeError. productTypeIndex() also wrapped
index.twig's own `{% extends "commerce/_layouts/settings" %}` in a
CpScreenResponse, double-rendering the CP chrome - switched it to
pageTemplate() like every other index-listing controller (OrderStatuses,
Pdfs, etc.), since editProductType()'s _edit.twig is content-only and
CpScreenResponse is correct there.
…to native Laravel ShouldQueue jobs The 3 legacy Yii2-style jobs (already deleted in the previous commit) are replaced by src/Email/Jobs/SendEmailJob.php, src/Catalog/Jobs/ ResaveProductVariantsJob.php, and src/CatalogPricing/Jobs/CatalogPricingJob.php, each extending CraftCms\Cms\Queue\Job. All 3 call sites now use Job::dispatch() instead of craft\helpers\Queue::push(), so Commerce no longer routes any job through the yii2-adapter's LegacyJobWrapper shim. CatalogPricingJob widens Job::setProgress() to public since CatalogPricing::generateCatalogPrices() reports progress on its $queue argument via duck typing.
…lidator to the new Ruleset system Commerce 6.0 only ships Install.php for fresh installs, so the 6 loose point-release migrations drafted in database/migrations/ are unnecessary: 4 are schema changes already present in Install.php's table definitions (catalog pricing queue table, orders.customerDeleted, the subscriptions userId FK cascade, order notices.noticeType), and 2 are one-time data repairs (product type permission renaming, allVariants->variants changedattributes cleanup) for bugs that don't exist in a fresh 6.0 schema. CouponsValidator.php (which nothing referenced anymore) is replaced by a closure rule on Coupon::getRules()['code'], checking case-insensitive uniqueness against every other coupon row. It validates per-coupon inside Coupons::saveDiscountCoupons()'s save loop rather than batch-checking the whole Discount.coupons array like the legacy validator did, since each coupon is validated then immediately persisted before the next one in the loop - which naturally catches same-batch duplicates too, without needing the legacy version's own-discountId exclusion workaround.
# Conflicts: # src/Http/Controllers/Settings/ProductTypesController.php
…ugin convention HasTranslations checks dirname(getBasePath()).'/lang' before falling back to src/translations, so plugin translations belong at the package root. Stripped the old copyright header from each locale file to match core's app.php style.
…, changelog Previous commit's git add -A failed silently on a stale pathspec before it could stage the header strip and the crowdin.yml/.codecov.yml/changelog updates, so they landed in the working tree but not the commit.
Replaces the StoreBehavior-dependent getGlobals() lookup with
Sites::getCurrentSite()->getStore(), the Site::macro('getStore', ...)
registered in Plugin::registerBehaviorMacros(). CP asset bundles and Twig
templates stay in src-yii2/ for now (separate, larger migration task).
Confirmed via cms-6's yii2-adapter (Yii2ServiceProvider::registerCraftVariableCompatibility()) that the legacy CraftVariable EVENT_INIT bridge only forwards getComponents(), not attached behaviors, so this class never reached the live craft Twig global under Craft 6. craft.commerce/ orders/products/variants are already fully served by the NewCraftVariable macros in src/Plugin.php. Verified live via TemplateManager::renderString() before and after removal.
The alias was left over from when this file needed to disambiguate from the legacy craft\web\twig\variables\CraftVariable; that legacy import (and the CraftVariableBehavior it backed) is gone now, so there's no collision. Also fixed a stale docblock describing the pre-deletion legacy mechanism.
…umers, add opt-in serialization fields Site/User/Address no longer support attachBehavior(), so StoreBehavior, StoreLocationBehavior, ValidateOrganizationTaxIdBehavior, CurrencyAttributeBehavior, CustomerBehavior, and CustomerAddressBehavior have been dead/orphaned code since earlier migration stages stripped their EVENT_DEFINE_BEHAVIORS wiring. All six are deleted; their functionality was already independently reimplemented via macros and event listeners registered in src/Plugin.php. Auditing CurrencyAttributeBehavior's *AsCurrency consumers surfaced two live regressions (Transaction and CatalogPricing were both missing getters their own callers already used, throwing UnknownPropertyException on the order-edit and Settings > Product Pricing CP screens) and a third-party BC gap (ShippingMethodOption/ OrderAdjustment were public API via the legacy behavior's magic __call, independent of Commerce's own in-repo callers) — all four now have explicit get*AsCurrency() getters matching the pattern already established for Order/LineItem/Purchasable/Product. Closed the serialization half of the defineFields()/defineRules() design gap (no cms-6 core hook exists for adding keys to User::toArray()/Address::toArray()) with four new opt-in, read-only custom field types using dbType(): null — real custom fields flow through toArray()/GraphQL automatically once attached to a field layout via the CP, verified live end-to-end. Also deleted the two legacy Codeception tests asserting attachBehavior()/getBehavior() semantics no longer possible on the new element classes, and cleaned up dangling IDE-hint-only references to the deleted classes.
Also fixes src/Catalog/Products.php, which was referencing the legacy craft\commerce\Plugin class instead of the new CraftCms\Commerce\Plugin.
[6.x] Render product sources in the secondary nav
# Conflicts: # src/Plugin.php
[6.x] Put nav into group
[6.x] Fix customize sources modal
…e controllers to Ui
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://linear.app/craftcms/issue/COM-613/laravel-port