Upgrade plugin from Sylius 1.x to Sylius 2.x#108
Merged
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.
Restructure routing to match `setono/sylius-plugin-skeleton@2.2.x`: thin
top-level entrypoints delegate to per-section files under `config/routes/`.
- `config/routes.yaml` (new) — localized entrypoint, imports
`config/routes/shop.yaml` with `prefix: /{_locale}` and the skeleton's
`_locale` requirement regex.
- `config/routes_no_locale.yaml` (new) — non-localized entrypoint for
stores with localized URLs disabled (imports the shop file with no
prefix), carrying the skeleton's explanatory header.
- `config/routes/shop.yaml` — now holds the actual route definitions; the
`/ajax/pickup-points` segment that used to live on the import prefix is
baked into each `path` so the final URLs are unchanged.
- Removed the old `config/routes/shop_non_localized.yaml` entrypoint and
the nested `config/routes/shop/ajax/pickup-point.yaml` leaf.
No admin route file is created — the plugin registers no admin routes (the
admin shipping-method field renders via a Twig hook, not a route).
Route names and resulting URLs are unchanged
(`setono_sylius_pickup_point_shop_ajax_pickup_points_search_by_cart_address`
→ `/{_locale}/ajax/pickup-points/search`,
`setono_sylius_pickup_point_shop_ajax_pickup_point_by_id`
→ `/{_locale}/ajax/pickup-points/{pickupPointId}`), so the templates that
reference those names keep working.
The test app now imports `@SetonoSyliusPickupPointPlugin/config/routes.yaml`,
and the README/UPGRADE routing sections point at the new entrypoints.
The `getExtendedTypes()` assertion used a hardcoded class-name string, which Rector's StringClassNameToClassConstantRector flagged — failing the `coding-standards` CI job (it runs `rector process --dry-run` without continue-on-error). Import the class and assert against `ShippingMethodChoiceType::class` instead.
Add a Commands-section callout: the repo's vendor/ is locked to PHP >= 8.4 while the shell default is often 8.1 (failing Composer's platform check). Run the versioned Homebrew binary directly — e.g. "$(brew --prefix php@8.4)/bin/php" vendor/bin/... — rather than the 8.1/8.2/8.3/8.4 switcher aliases, which brew-link the system-wide default and can break other in-progress work on the machine.
loevgaard
commented
May 29, 2026
loevgaard
commented
May 29, 2026
ProviderInterface::findPickupPoints() now takes a new immutable Setono\SyliusPickupPointPlugin\DTO\Address value object instead of an OrderInterface, so providers can be used outside the checkout flow. Address has nullable street/postcode/city/countryCode and a fromOrder(OrderInterface) named constructor (returns an all-null instance when there is no shipping address). This also centralizes the getShippingAddress() extraction that was duplicated across all four providers. findPickupPoint(string $id, string $country) becomes findPickupPoint(string $id, array $metadata = []) (@param array<string,mixed>), an open SPI hook since the data needed to resolve a pickup point by id is carrier-specific. The transformer passes ['country' => $country]; PostNord reads $metadata['country'] (guarded by is_string); DAO/GLS/Faker ignore it. The controller now calls findPickupPoints(Address::fromOrder($order)).
Replace the two helpers (stringOrNull + scalarStringOrNull) with a single scalarOrNull used for every field. The int/float -> string coercion now also applies to text fields, which is a net positive: a numeric JSON zipCode (e.g. 9000) rehydrates as "9000" instead of being dropped to null. bool/array/null still map to null.
Symfony's AddConstraintValidatorsPass always registers a constraint validator in the factory locator under its class name, and the Foo/FooValidator naming convention means the default Constraint::validatedBy() (static::class.'Validator') resolves HasPickupPointSelectedValidator with no config. So drop both the `alias` tag attribute and the validatedBy() override. Update the constraint unit test to assert the FQCN-by-convention resolution.
`postalCode` is the term already used by the carrier code (PostNord's query param and the PickupPoint/visitingAddress field), so the value object now matches that instead of introducing a third spelling. Address::fromOrder() still maps Sylius's getPostcode() into it via the positional constructor arg.
All three form type extensions implement getExtendedTypes(), which Symfony's FormPass uses when the tag has no `extended_type` attribute. The attribute was a redundant second source of truth that actually overrides getExtendedTypes(), so it could silently drift; dropping it makes getExtendedTypes() authoritative. Also removes the now-unused Sylius form-type imports. Verified via debug:form that each extension still attaches to the same type.
Replace the inline 'provider---id---country' wire format with an opaque, URL/form-safe base64url-JSON token owned by a single PickupPointIdentifierEncoder service. PickupPointIdentifier is a JsonSerializable value object with a matching fromArray(), so the VO owns the wire shape and the encoder owns only transport. Keep country as a first-class PickupPoint property (set by providers) while adding an open metadata map for consumer extensibility; fromPickupPoint() folds country into the identifier's metadata, which is what ProviderInterface::findPickupPoint() consumes to re-resolve a point. Serialize the search results through the Serializer and add a PickupPointNormalizer (NormalizerAware, delegates the base shape and augments it with the encoded 'identifier'); the shop JS and form theme read that 'identifier' directly instead of re-deriving the format client-side.
Rename PickupPointByIdAction to PickupPointByIdentifierAction (route .../from-id -> .../from-identifier, query param -> 'identifier') since it resolves a point from the opaque identifier token, not a raw id. The action now decodes the token via PickupPointIdentifierEncoder and resolves through the ProviderRegistry directly, instead of borrowing the form DataTransformer. That leaves PickupPointToIdentifierTransformer unused (it was attached to no form), so it is removed.
loevgaard
commented
May 29, 2026
loevgaard
commented
May 29, 2026
The loose `api-platform/core: ^4.0.3` let it float to the monolithic 4.3.x, which forces `symfony/type-info ^7.4||^8.0` and reflects resource types during route building. That trips over gedmo/doctrine-extensions' redundant `@template T of Loggable|object` phpdoc under type-info's strict union check, so the API Platform router cannot build and every page 500s on a cold cache. Sylius 2.2 uses the split api-platform packages on the 4.2 line, which does not do that route-time reflection. Pinning core to ~4.2.1 keeps the whole suite there. Also broaden symfony/var-exporter to ^6.4 || ^7.4 to match the sibling dev dependencies.
The checkout field was a bare HiddenType with no transformer, so it dumped the raw identifier token into the deprecated `pickup_point_id` string column while the `pickup_point` JSON column went unused. Re-introduce PickupPointToIdentifierTransformer and actually attach it to the form type as a model transformer, so a submitted token resolves (decode + ProviderRegistry::findPickupPoint) to a PickupPoint that Doctrine stores as JSON in `pickup_point`. The field now binds to $pickupPoint, the validator checks hasPickupPoint(), and the shipment label renders the stored object directly. `pickup_point_id` stays deprecated/BC-only and is no longer written. Collapse the two form types into one accurately-named PickupPointType: drop the vestigial PickupPointIdChoiceType and the unused choice_name/choice_value/ multiple/placeholder options and buildView() (leftovers from the 1.x choice design), and rename PickupPointChoiceType -> PickupPointType (block prefix setono_sylius_pickup_point) since it is a HiddenType, not a ChoiceType. Also fix a latent PHPStan contravariance error in PickupPointNormalizer that a clean (uncached) run flags.
Update the AJAX-response description (opaque `identifier` token via the serializer/normalizer, not a client-side `provider---id---country` build), the shop route names/URLs (`from-cart` / `from-identifier`), the PickupPointByIdentifierAction service mapping, and the DTO description (open `metadata` map + fromArray()/jsonSerialize() helpers).
Rendering the shipping step no longer calls any pickup point provider: a slow or unavailable carrier API would otherwise stall the whole page. The page now renders with zero provider calls and a small vanilla-JS chooser (no Stimulus) fetches every pickup-capable method's points for the cart from a new GET /pickup-points endpoint once the page is on screen — with loading/empty/error states and instant toggling as the shipping method changes (no re-fetch on toggle). The endpoint wraps each provider in its own try/catch so one failing carrier cannot take the others down. The selected point travels as a full base64url-encoded token, so on submit a model transformer decodes it straight into the PickupPoint and persists it: no provider call on submit, and no faker non-determinism. Drop symfony/event-dispatcher, add symfony/http-foundation.
The chooser's loading/empty/error keys were added under
src/Resources/translations/, but the bundle's getPath() is the repo root, so
that catalog was never loaded and the strings rendered as raw translation keys.
Move them into the real translations/messages.{en,da}.yml catalogs alongside the
existing keys.
Both still described the previous Serializer/two-endpoint/identifier-token design, and CLAUDE.md's architecture section still documented 1.x components removed in 2.0 (provider decorators, the messenger-driven local snapshot, PickupPointCode, the plugin-owned resource). Describe the async client-side fetch, the single GET /pickup-points endpoint, the full-point token carried by PickupPointEncoder/PickupPointTransformer, the shop-only javascripts hook, and the current provider-registration and resource-extension model.
Replace the flat radio list with a compact summary that expands on demand. When a pickup method is selected the nearest point is pre-selected (providers return them ordered by distance from the address) and shown as a summary with a "Change" button; "Change" opens a scrollable list with a "Currently …/Keep this" header and a "Selected" badge on the chosen point. The widget is built client-side from Bootstrap 5.3 utilities plus a small CSS file (wired via the sylius_shop.base#stylesheets hook), uses the Sylius brand (teal) colour, and is inserted into the selected method's card. The hidden field template is now just a carrier for the form value and the localized strings.
Add messages.* and validators.* catalogs for sv, no, fi, de, fr, es, it, nl, pl, pt, cs, hu, ro and uk (joining the existing en/da), and record the convention in CLAUDE.md: new translation keys must be provided in every supported locale. The non-en/da strings are an initial pass and warrant a native-speaker review.
PickupPointsAction now resolves the methods supported for each of the cart's shipments (ShippingMethodsResolverInterface) rather than every channel-enabled method, so it only calls the providers behind methods the shopper can actually select — the same methods Sylius renders as the radios. Encoding a point that cannot be serialized (e.g. a carrier returning a non-UTF-8 string) now skips just that point instead of 500-ing the whole endpoint, and the encoder documents the @throws. Drop the now-unused doctrine/persistence dep.
Set required=false on the shipping method's pickupPointProvider field — a method need not be a pickup-point method. Also includes minor formatting/docblock cleanups in the pickup-point transformer and the shipping-method-selection requirement checker.
loevgaard
commented
Jun 8, 2026
The admin order-show pickup-point label rendered a bare `shipment` variable that the hook does not expose, 500-ing every order page that has a shipment; it now reads `hookable_metadata.context.shipment` (like the checkout template uses `.form`). That template also drove an admin page from under templates/shop/, so it moves to templates/admin/order/show/. The checkout JS/CSS now attach to `sylius_shop.checkout#javascripts` / `#stylesheets` instead of `sylius_shop.base#…`, so they load only on the checkout instead of on every shop page.
Render the admin order-show page (asserting the pickup-point label) and the shipping-method edit form (asserting the provider field) against the loaded fixtures — coverage unit tests cannot give, and which would have caught the order-page 500. Pulls in symfony/browser-kit, css-selector and dom-crawler.
Replace the screenshots with fresh Sylius 2 captures (the two-state checkout
chooser, the admin order with its pickup point, the shipping-method provider
field), and fix the intro wording ("<select>" -> chooser) and the Twig-hook
description. The stale checkout "complete step" screenshot/section is dropped —
there is no such pickup-point display in 2.x.
Explain the strtr/rtrim transform on the encode line in both pickup-point encoders.
…ution A provider that opens an external connection in its constructor — e.g. the GLS provider, whose SOAP client loads a remote WSDL — threw during service resolution when that carrier was down, 500-ing checkout (the registry, the shipping-method form and the /pickup-points endpoint all instantiate it) before any runtime guard could run. RegisterProvidersPass now marks every tagged provider lazy, so a provider is built only when actually called — inside PickupPointsAction's per-provider try/catch — and a down carrier yields an empty list for itself while the others keep working. Refs #59
Pull setono/gls-webservice-bundle (^1.4) into require-dev, register the bundle and enable the gls provider plus a gls shipping-method fixture so the live GLS shop finder is exercised end to end in tests/Application. Inject the GLS client by its FQCN (Setono\GLS\Webservice\Client\ClientInterface) rather than the setono_gls_webservice.client alias, which is deprecated in gls-webservice-bundle 1.4 and removed in 2.0.
The concrete providers are final, so the default lazy ghost — generated by subclassing the class on PHP < 8.4 — cannot be built and the container fails to compile with "Cannot generate lazy proxy: class … is final" (caught by the coding-standards CI job, which runs a lower PHP than the local 8.4). Tag the lazy provider definitions with `proxy` + the ProviderInterface so Symfony builds a virtual proxy that implements the interface instead of extending the final class. Every method called on the proxy (setCode/findPickupPoints/findPickupPoint) is declared on the interface.
Sylius' debug error handler registers an exception handler while handling the
request and never restores it, which PHPUnit 11 reports as a hard failure
("Test code or tested code did not remove its own exception handlers") that
failOnRisky="false" does not suppress. Pop it in tearDown().
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.