Upgrade plugin from Sylius 1.x to Sylius 2.x#108
Open
loevgaard wants to merge 25 commits into
Open
Conversation
Follows the Setono v1→v2 playbook (https://github.com/orgs/Setono/discussions/1) and aligns the layout with `setono/sylius-plugin-skeleton@2.2.x`. Key changes: - composer.json: PHP `>=8.2`, Symfony `^6.4 || ^7.4`, Sylius `^2.0`, Doctrine ORM `^3.0`. Drops FOSRestBundle, JMS Serializer, behat-transliterator, Psalm, phpspec, Buzz and the carrier-bundle dev deps; adds `setono/sylius-plugin: ^2.0`, `setono/doctrine-orm-trait`, `api-platform/core ^4`, `lexik-jwt ^3.1`, `dama/doctrine-test-bundle`. All five third-party carrier bundles (Budbee, CoolRunner, DAO, GLS, PostNord) move to `suggest`. - Bundle class overrides `getPath()` and `getConfigFilesPath()` so Doctrine-mapping discovery works against the flattened layout. - File layout: `src/Resources/{config,translations,views,public}` moves to repo-root `config/`, `translations/`, `templates/`, `public/`. - DI: XML services converted to PHP DSL under `config/services/**`, service IDs renamed to FQCN (with interface aliases) where appropriate; the registry, cache alias and provider tag IDs are preserved. The extension implements `PrependExtensionInterface` and inlines the messenger bus, `sylius_twig_hooks` configuration and the former `app/config.yaml` / `app/fixtures.yaml` imports. - AJAX endpoints rewritten to plain Symfony controllers returning `JsonResponse` via `Symfony\Component\Serializer`. JMS YAML metadata replaced by `#[Groups]` / `#[SerializedName]` attributes on `PickupPoint`. - `LoadPickupPointsHandler` switches from `EntityManagerInterface` to `ManagerRegistry` + `Setono\Doctrine\ORMTrait`, takes a class-string parameter, and uses `#[AsMessageHandler]`. - Trait `@ORM\Column` PHPDoc converted to PHP 8 attributes for ORM 3 compatibility. `PickupPoint` latitude/longitude mapping switched from `decimal` to `float` to match the PHP property types under ORM 3 type checks. - `Behat\Transliterator\Transliterator` swapped for `Symfony\Component\String\Slugger\AsciiSlugger`. - Plugin's `_javascripts.html.twig` and pickup-point shipment label auto-wired through `sylius_twig_hooks` (`sylius_admin.base#javascripts`, `sylius_shop.base#javascripts`, `sylius_admin.order.show.content.sections.shipments.item`). - Tooling rebuilt around `setono/sylius-plugin`: new `phpstan.neon` (level 6), `ecs.php`, `rector.php` (UP_TO_PHP_82, applied), `composer-dependency-analyser.php`, `phpunit.xml.dist` with unit/functional suites and the `DAMADoctrineTestBundle` extension, and a `.github/workflows/build.yaml` using the `setono/sylius-plugin/*@v2` composite actions across PHP 8.2/8.3/8.4 × Symfony 6.4/7.4 × lowest/highest. - Tests: phpspec dropped (6 specs ported to PHPUnit + Prophecy under `tests/Unit/`); Behat dropped; `tests/Application/` rebuilt against the 2.2.x skeleton with the plugin's resource overrides preserved, `Sylius\TwigHooks\SyliusTwigHooksBundle` and `DAMADoctrineTestBundle` registered, and `HEADER_X_FORWARDED_ALL` replaced with the Symfony 7 combination. Verification: composer validate, ECS, PHPStan, PHPUnit (15 tests), composer-dependency-analyser, Rector (applied 16 modernizations), `debug:container` (all FQCN services resolve), `debug:router`, `doctrine:schema:validate`, `lint:container`, `lint:yaml`, `lint:twig`, and an end-to-end smoke test booting the test app to confirm the plugin's Twig hooks fire on the shop layout and the AJAX endpoint returns JSON with CSRF enforcement intact. See UPGRADE-2.0.md for a full migration guide.
- `tests/Application/webpack.config.js`: replace the v1-era Encore script with the skeleton's `SyliusAdmin.getWebpackConfig()` / `SyliusShop.getWebpackConfig()` helpers so `yarn build` resolves the Sylius asset entry-points correctly. - `tests/Application/config/bundles.php`: filter the bundle list through `class_exists()`. The `static-code-analysis` matrix removes `sylius/sylius` before booting the kernel, which drops transitively-installed bundles (e.g. `Symfony\Bundle\MonologBundle\MonologBundle`); without the filter PHPStan crashes when its Symfony plugin boots the kernel. - `phpstan.neon`: drop the `symfony.consoleApplicationLoader`. We do not need container-resolved analysis, and skipping the kernel boot avoids the same missing-bundle class-load failures in the `static-code-analysis` matrix. - `tests/Unit/Provider/LocalProviderTest.php`: build the pickup-point `FactoryInterface` via Prophecy instead of instantiating `Sylius\Component\Resource\Factory\Factory` directly. The Sylius resource-bundle legacy namespace is exposed through `class_alias()`, which PHPStan cannot follow once the kernel is no longer booted.
… run The CI's functional-tests action runs `vendor/bin/phpunit --testsuite=functional` which fails with "Test directory ... tests/Functional not found" when the directory does not exist. Add an empty placeholder mirroring the plugin-skeleton layout.
The BC check tries to install 1.x's composer.json to compare API surface, but 1.x pins `api-platform/core ^2.7.16` which composer now blocks because of security advisories (PKSA-gs8r-6kz6-pp56, PKSA-gnn4-pxdg-q76m, PKSA-dsd6-6541-26zs). For a major version bump every BC break is intentional, so make the job informational rather than blocking until 2.0 ships.
Every API change in this PR is intentional — running Roave's BC check against 1.x serves no purpose for a major version bump. (As a bonus, the tool currently cannot even install 1.x because composer blocks the 1.x `api-platform/core ^2.7` constraint on security advisories.)
Drop the opt-in PSR-cache decorator and the local-snapshot fallback decorator along with the infrastructure that backed the snapshot: the `setono-sylius-pickup-point:load-pickup-points` console command, the messenger command/handler, the plugin-owned `PickupPoint` Doctrine resource and indices listener, and the `TimeoutException`. Each provider now talks to its carrier API directly. Tighten PHPStan to `level: max` and refresh `.gitattributes`, `UPGRADE.md`, and `README.md` to match the reduced surface.
Now that the LoadPickupPointsHandler infrastructure is gone, nothing in the plugin calls `ProviderInterface::findAllPickupPoints()`. Drop the method from the interface and every shipped provider implementation. Also drop `PickupPointInterface extends ResourceInterface` and the now-meaningless `PickupPoint::$id` / `getId()` — the model is a DTO populated from a carrier API response, not a Doctrine entity, so the `sylius/resource-bundle` shadow dep flagged by `composer-dependency-analyser` disappears. Remove `psr/http-client` from `require` — no production code in src/ touches PSR-18 directly (each carrier provider talks to its own bundle's client). PHPStan: silence the cross-Symfony-version `cast.useless` report on the `(array) $view->vars` cast in `PickupPointChoiceType::buildView()`. The cast is necessary because `FormView::$vars` is typed `array` on Symfony 7+ but left untyped (and so resolved as `mixed`) by phpstan-symfony's FormView stub and by Symfony 6.4. Drop the brittle `@param array<string, mixed>` PHPDoc on `ShippingMethodExampleFactory::create()` — it triggered a contravariance violation against the looser parent signature on the lowest-deps matrix.
`Model\PickupPoint` and `Model\PickupPointInterface` are gone; the replacement is `DTO\PickupPoint` — a final class with public properties, populated by each provider from the carrier API response. The two derived accessors (`getCodeValue()`, `getFullAddress()`) stay as methods so the serialization groups (`Detailed` / `Autocomplete`) remain stable for the AJAX endpoints. Providers now assign properties directly instead of going through setters; the interface and consumers (controller, data transformer) were updated to the new type.
Drop the `Model\PickupPointCode` value object. Its three pieces (provider, id, country) now live as public properties on `DTO\PickupPoint`, alongside the existing data fields. The wire-format string `provider---id---country` that drives the AJAX endpoint and the hidden form field is now produced by `PickupPoint::getCodeValue()` directly from those properties. `ProviderInterface::findPickupPoint()` now takes the id and country as separate string arguments instead of a `PickupPointCode` instance, and `PickupPointToIdentifierTransformer` parses the wire-format string inline (the only place that needs to).
Both were pulled in by the now-removed `PickupPointCode` value object (`Countries::exists()` and `mb_strtoupper()` respectively). Nothing in `src/` references them anymore, and the dependency-analysis CI matrix flagged them as unused.
…the DTO
Per request, `DTO\PickupPoint` is now a pure data bag with public scalar
properties and nothing else — no `getFullAddress()`, no `getCodeValue()`,
no `#[Groups]` / `#[SerializedName]` attributes. The form data transformer
now builds the `provider---id---country` wire-format string inline from
the DTO properties.
Heads-up: the AJAX endpoints (`PickupPointByIdAction`,
`PickupPointsSearchByCartAddressAction`) still call `$serializer->serialize(
$pickupPoint, 'json', ['groups' => ['Detailed']])`, and the shop JS reads
`value.code` and `value.full_address` off the response. Removing the
attributes means the JSON-with-groups output is now `{}`, so those
endpoints need a follow-up — either drop the `groups` filter and let
public-property serialization emit every field, or rebuild the response
payload in the controller.
The DTO no longer carries `#[Groups]` / `#[SerializedName]` attributes, so
serializing with `['groups' => ['Detailed']]` (or `['Autocomplete']`)
produces `{}`. Drop the groups arg from both AJAX actions
(`PickupPointByIdAction`, `PickupPointsSearchByCartAddressAction`) so the
serializer emits every public property on the DTO — `provider`, `id`,
`name`, `address`, `zipCode`, `city`, `country`, `latitude`, `longitude`.
The shop JS previously read `value.code` and `value.full_address` off the
response. Compose both client-side from the new individual fields so the
existing Twig template (`{code}` / `{full_address}` placeholders) keeps
working untouched.
- DTO becomes a pure data bag: `provider`, `id`, `name`, `address`, `zipCode`, `city`, `country`, `latitude`, `longitude`. Both `latitude` and `longitude` are now `?string` (carrier APIs return them as floats or strings; the DTO is the boundary, so coerce on the way in). - DTO implements `\JsonSerializable` (round-tripping via Doctrine JSON columns) and exposes a `fromArray(array): self` factory that defensively coerces each field through a private `stringOrNull()` helper. Providers now assign properties directly instead of going through setters. - `ProviderInterface::findPickupPoints()` returns `list<PickupPoint>` instead of `iterable<PickupPoint>`. The five non-generator providers already produced arrays; `DAOProvider`'s internal Generator was materialized into a list and its `findPickupPoint()` lookup reads `[0] ?? null` rather than iterating.
`PickupPointAwareInterface` (and `PickupPointAwareTrait`) gain a new `pickup_point` JSON column / `?PickupPoint $pickupPoint` accessor. The trait's property is typed `null|PickupPoint|array` so the setter can store a DTO directly (Doctrine serializes via `JsonSerializable`) while the getter handles both the in-memory DTO and the array form returned by Doctrine on hydration — re-inflating arrays via `PickupPoint::fromArray()`. The legacy `pickup_point_id` string column and its `hasPickupPointId()` / `setPickupPointId()` / `getPickupPointId()` methods stay in place behind `@deprecated since 2.0` notes so existing consumers keep working; nothing in the plugin reads the new column yet. UPGRADE.md gains a section documenting the deprecation table and an example Doctrine migration that backfills the JSON column by splitting the legacy `provider---id---country` string with `SUBSTRING_INDEX` (MySQL/MariaDB; PostgreSQL `split_part` shown as a commented variant).
Sylius 2.x admin/shop templates only render form fields whose Twig hook
is registered. The plugin's form extensions add `pickupPointProvider`
to the admin shipping-method form and `pickupPointId` to the shop
checkout shipment form, but until now those fields were in the form
yet never rendered (causing the admin field to be invisible and the
shop checkout to hit a 422 on submit because the JS-driven pickup-point
widget never reached the DOM).
`Extension::prepend()` now:
- Registers `@SetonoSyliusPickupPointPlugin/Form/theme.html.twig` via
`twig.form_themes` so the `setono_sylius_pickup_point_choice_row`
block applies wherever `PickupPointChoiceType` is rendered.
- Adds hooks `sylius_admin.shipping_method.{create,update}.content.form.configuration#pickup_point_provider`
pointing at a new
`Admin/ShippingMethod/Form/Configuration/pickupPointProvider.html.twig`.
- Adds hook `sylius_shop.checkout.select_shipping.content.form.shipments.shipment#pickup_point`
pointing at a new `Shop/Checkout/SelectShipping/Shipment/pickupPoint.html.twig`,
which renders `form.pickupPointId` only when the field is present on
the shipment form.
- `PickupPointsSearchByCartAddressAction` no longer validates an
`_csrf_token` query param. It now reads the new `provider` query
param via `Request::query->get()` and throws a `BadRequestHttpException`
for empty input. The Generator special-case (`iterator_to_array()`)
is gone — the provider contract guarantees a list now.
- `ShippingMethodChoiceTypeExtension` stops depending on
`CartContextInterface` and `CsrfTokenManagerInterface`; the
`choice_attr` callback only emits `data-pickup-point-provider`. The
controller service def loses `security.csrf.token_manager`, the form
extension service def loses the cart-context + csrf args, and the
unit test was rewritten around the slimmer constructor.
- Shop JS drops the `{_csrf_token}` substitution and renames the
`{providerCode}` placeholder to `{provider}`. The `_javascripts.html.twig`
template updates the search-URL query string to match.
CLAUDE.md gains a "Working agreements" section codifying two rules the user already directed in chat: - Don't run `git commit` (or `git push`) without explicit instruction. - After any change that affects the rendered admin/shop UI, drive the test app in a browser via the Playwright MCP tools and verify the flow before reporting the task done.
`PickupPoint` already implements `JsonSerializable`, so `JsonResponse`'s internal `json_encode()` call produces the same flat output the Symfony serializer used to emit — no need to inject the serializer just to serialize the array of DTOs ourselves. The action now returns `new JsonResponse($provider->findPickupPoints($order))` directly. The controller service definitions also gained `->public()` and dropped the `controller.service_arguments` tag, and the search action now reads the cart from `sylius.context.cart` (not the composite) — matching the broader Sylius 2.x conventions.
Modeled after `Symfony\Component\Console\Attribute\AsCommand`, the new
class-level attribute `Setono\SyliusPickupPointPlugin\Attribute\AsProvider`
declares the two pieces of metadata every pickup-point provider needs:
- `code` — machine identifier (registry key + wire-format `provider`
segment), e.g. `"faker"`, `"gls"`.
- `name` — label shown in the admin shipping-method form, typically a
translation key, e.g. `"setono_sylius_pickup_point.provider.faker"`.
The abstract `Provider` reads the attribute via reflection (with a per-
class static cache) to implement `getCode()` and `getName()`, so concrete
providers can drop those methods entirely. `FakerProvider`, `DAOProvider`,
`GlsProvider` and `PostNordProvider` are annotated with `#[AsProvider]`
and lost their hand-written getters.
DI side:
- Each shipped provider's service file now uses bare
`->tag('setono_sylius_pickup_point.provider')` (no inline `code` /
`name` attributes).
- `RegisterProvidersPass` resolves the `(code, name)` pair from the tag
attributes if present, otherwise reflects on the service's class for
`AsProvider` — and throws when neither source supplies them.
- The extension calls `registerAttributeForAutoconfiguration(AsProvider::class, …)`
so any consumer-side provider with `autoconfigure: true` is auto-tagged
the same way.
Drop the `Budbee` and `CoolRunner` carrier providers, their service definitions, their configuration-tree nodes, the `suggest` entries for `setono/budbee-bundle` and `setono/coolrunner-bundle`, and the phpstan / composer-dependency-analyser path exclusions that targeted those files. `UPGRADE.md` gains a "Removed providers" section pointing consumers at the remaining DAO / GLS / PostNord / Faker providers (or their own `ProviderInterface` implementation) and listing the YAML config keys that need to go. The DI extension test (`SetonoSyliusPickupPointExtensionTest`) drops both keys from its minimal configuration so the extension keeps loading against the updated config tree.
The `name` field on `#[AsProvider]` is now the human-readable carrier
brand as it appears in the admin shipping-method dropdown — `"Faker"`,
`"DAO"`, `"GLS"`, `"PostNord"` — rather than a translation key.
Updated the four shipped providers' attributes accordingly and dropped
the matching docblock guidance on `AsProvider::$name`.
The previously translated `setono_sylius_pickup_point.provider.*` keys
are removed from `translations/messages.{en,da}.yml`; they were the
former `name` values and are no longer referenced.
Rename every directory under `templates/` to snake_case to match the Sylius / Symfony convention (`templates/admin/...`, `templates/shop/...`, `templates/form/...`). File basenames stay camelCase so existing Twig namespaces and includes don't shift more than they need to. Updated references: - `SetonoSyliusPickupPointExtension::prepend()` — every `@SetonoSyliusPickupPointPlugin/...` template path in the `twig.form_themes` entry and the four `sylius_twig_hooks.hooks` entries now points at the lowercase directories. - `UPGRADE.md` — the README-imported `Shop/Label/Shipment/pickupPoint.html.twig` example reference was lowercased to match.
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
1 participant
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.
Summary
Brings the plugin onto Sylius 2.x following the Setono v1→v2 playbook (https://github.com/orgs/Setono/discussions/1) and aligns the layout with
setono/sylius-plugin-skeleton@2.2.x.>=8.2, Symfony^6.4 || ^7.4, Sylius^2.0, Doctrine ORM^3.0. Drops FOSRestBundle, JMS Serializer, behat-transliterator, Psalm, phpspec, Buzz, and the carrier-bundle dev deps; addssetono/sylius-plugin: ^2.0,setono/doctrine-orm-trait,api-platform/core ^4,lexik-jwt ^3.1,dama/doctrine-test-bundle. All five carrier bundles (Budbee, CoolRunner, DAO, GLS, PostNord) move tosuggest.src/Resources/{config,translations,views,public}flattened to repo-rootconfig/,translations/,templates/,public/. The bundle class overridesgetPath()andgetConfigFilesPath().config/services/**; service IDs renamed to FQCN where appropriate. The extension implementsPrependExtensionInterfaceand inlines the messenger bus,sylius_twig_hooksconfiguration, and the formerapp/config.yaml/app/fixtures.yamlimports.JsonResponseviaSymfony\Component\Serializer. JMS YAML replaced by#[Groups]/#[SerializedName]attributes onPickupPoint.LoadPickupPointsHandlernow usesManagerRegistry+Setono\Doctrine\ORMTrait+ a class-string parameter and is marked#[AsMessageHandler]. Trait@ORM\ColumnPHPDoc converted to PHP 8 attributes;PickupPointlat/long mapping switched fromdecimaltofloatto match the PHP property types._javascripts.html.twigand the pickup-point shipment label are auto-wired throughsylius_twig_hooks(sylius_admin.base#javascripts,sylius_shop.base#javascripts,sylius_admin.order.show.content.sections.shipments.item).setono/sylius-plugin— newphpstan.neon(level 6),ecs.php,rector.php(UP_TO_PHP_82, applied),composer-dependency-analyser.php,phpunit.xml.distwith unit + functional suites and theDAMADoctrineTestBundleextension, and a.github/workflows/build.yamlusing thesetono/sylius-plugin/*@v2composite actions across PHP 8.2/8.3/8.4 × Symfony 6.4/7.4 × lowest/highest.tests/Unit/); Behat dropped;tests/Application/rebuilt against the 2.2.x skeleton with the plugin's resource overrides preserved,Sylius\TwigHooks\SyliusTwigHooksBundleandDAMADoctrineTestBundleregistered, andHEADER_X_FORWARDED_ALLreplaced with the Symfony 7 combination.Migration guide: see
UPGRADE-2.0.md.Test plan
composer validate --strict+composer normalize --dry-runcomposer check-style(ECS, Sylius Labs ruleset)composer analyse(PHPStan level 6)composer phpunit— 15 unit tests passvendor/bin/composer-dependency-analyservendor/bin/rector process(applied 16 PHP 8.2 modernizations)bin/console cache:clear/lint:container/lint:yaml/lint:twig/doctrine:schema:validate/debug:router | grep pickup_javascripts.html.twighook fires onsylius_shop.base#javascripts; the/ajax/pickup-points/searchendpoint returns JSON and rejects an invalid CSRF token with 403; fixtures load and thefakerpickup-point shipping method materializes.fakershipping method → pickup-point dropdown renders → place order → admin order-show pickup-point label) — pending a fully built asset pipeline in the test app.