1.x rewrite (Sylius 1.13/1.14, no API layer) - #269
Open
loevgaard wants to merge 56 commits into
Open
Conversation
Phase 1 (skeleton): - New 1.x branch; drop API layer, GiftCardConfiguration family, Behat, psalm, knp-snappy, safe-writer, jms/lexik - Tooling per SyliusPluginSkeleton 1.14.x: PHPStan (max), Rector, Infection, shipmonk dependency analysis, playwright MCP; Symfony ^6.4, Sylius ~1.13/~1.14 - Bundle extension auto-configures host app via prepend() - Redemption mode config (adjustment|payment) selects service file Phase 2 (domain model): - Reshaped GiftCard (deliveryType enum, design FK, optimistic version, transaction ledger, isUsable/isPending); explicit initialAmount - New GiftCardDesign (translatable, front/back images, channels) + lazy seeding hooks - New GiftCardTransaction append-only balance ledger - Grouped unambiguous code generator + normalizer; SQL balance aggregation - All quality gates green; schema validates; container boots on Sylius 1.14
- GiftCardDesignProvider with lazy Classic seeding (bundled default image, concurrency-guarded) - Admin grid (prepended sylius_grid) with thumbnail field, admin form template, menu entry - Design fixture + example factory (front/back image upload) - Clean 1.x translations - Verified: fixtures load against DB, Classic design + image persist, 20 cards created - All quality gates green
- GiftCardInformation DTO + form (amount, message, design picker); designs unified across virtual/physical - AddToCartTypeExtension + CartGiftCardHandler: pending disabled GiftCard per OrderItemUnit at add-to-cart (units exist at POST_SUBMIT via quantity data mapper) - ValidGiftCardAmount constraint + channel-aware amount limits provider - PendingGiftCardCleanupListener (onFlush) removes pending cards for deleted units - Product-page section (sylius_ui prepend) with live HTML preview (vanilla JS/CSS) - gift_card_product fixture: product + delivery option + virtual/physical variants (verified: correct per-variant shipping_required) - Quality gates green; fixtures load clean
- OrderGiftCardOperator: reconcile (checkout complete), enable (pay), disable (cancel) - Winzou callbacks prepended (verified registered) - reconcile snapshots final amount, creates cards for quantity-bumped units, associates customer - send-on-pay deferred to phase 10 (email), scaffold to phase 11 Also: yarn install + build for the test app (shop assets)
Shared core: - EligibleTotalCalculator (excludes gift-card line items) - GiftCardCoverageCalculator + GiftCardCoverage VO (stacking, caps, skips unusable/mismatched) - GiftCardBalanceOperator: sole balance mutator, append-only ledger, idempotency, InsufficientGiftCardBalanceException, manual adjust() - GiftCardApplicator rewritten with guards, delegates to aliased redemption method - GiftCardRedemptionMethodInterface + abstract RedemptionMethod base - GiftCardIsApplicable compound constraint Adjustment mode: - GiftCardAdjustmentProcessor (negative order_gift_card adjustments from coverage) - AdjustmentRedemptionMethod (commit/rollback via balance operator, idempotency keys) - One pair of winzou callbacks (create->commit, cancel->rollback) for both modes Container lints, callbacks registered, quality gates green
- Apply action (POST + GiftCardIsApplicable validation), remove action (POST + CSRF) - Twig redemption extension/runtime (apply form, coverage, remaining total) - Cart apply box + totals partials via sylius_ui prepend - Playwright-verified end-to-end (adjustment mode): product gift card form + live preview render; add-to-cart creates the pending disabled card with correct amount/deliveryType/design/message; cart apply box attaches a card to the order - Test app: add StateMachineAbstractionBundle, remove stale 0.12.x overrides, fixture channel fallback, encore strict_mode off
- DompdfGiftCardPdfGenerator + pluggable interface; two-page pdf.html.twig - GiftCardEmailManager (Sylius mailer, in-memory PDF attachments via tempfile) - OrderGiftCardOperator.send() + pay->send winzou callback - SendGiftCardEmailSubscriber for admin-created cards - Admin PDF download + design preview-PDF actions/routes - Browser-verified: admin PDF download produces a valid 22KB PDF
…pages render styled
resolve-url-loader 3.x fails with dart-sass ('PostCSS received undefined');
disabling it lets Encore emit the Sylius CSS. Also gitignore the local dev router.php.
- GiftCardPaymentChecker + lazy GiftCardPaymentMethodProvider (offline gateway) - PaymentRedemptionMethod: commit creates completed gift-card payments + redeems balance; rollback refunds + restores (same winzou callbacks as adjustment mode) - GiftCardAwareOrderPaymentProcessor decorates checkout + after_checkout to size the gateway payment to (total - coverage) - Payment-step-skip checker + methods/default resolver decorators hide the gift-card method from checkout - Verified: container boots + lints in payment mode, decorators active
- Gift card admin grid (prepend): code/customer/amount/deliveryType/enabled/createdAt, filters, create/update/download-pdf/delete actions, hides pending cards - Balance dashboard action + template (SQL findBalance aggregation by currency) - Admin gift card create enabled (channel field on new cards) - Design form example-PDF preview button; menu items for designs + balance - Browser-verified: gift card list, balance dashboard render styled
- Gift card grid actions (adjust-balance, download PDF), balance dashboard (SQL) - AdjustGiftCardBalanceAction + form (delta + reason -> ledger via balance operator) - CreateGiftCardProductAction scaffold (verified: creates disabled gift card product with virtual + physical variants, redirects to edit) - Admin gift card create (channel field), design preview-PDF button, menus, translations - Note: local admin form submits blocked by node-sass/arm64 broken admin JS (test-app infra); covered by functional tests
- Unit suite (28): model, code normalizer, eligible-total + coverage calculators, balance operator (redeem/restore/adjust/idempotency/insufficient), configuration - Functional suite (3): balance operator + ledger + findBalance against a real DB - composer phpunit -> OK (31 tests, 53 assertions); PHPStan max / ECS / Rector clean - README rewritten for 1.x; UPGRADE-1.0.md clean-break guide Completes the 1.x rewrite (all 12 phases).
Follows Setono/SyliusPluginSkeleton@80cc9db: - package.json: use @sylius-ui/frontend (Dart Sass) instead of node-sass, which failed to compile against Node 22's V8 API on arm64; pin jquery via resolutions so jquery.dirtyforms loads (fixes 'jQuery.dirtyForms is not a function' console error that broke admin form submits) - .nvmrc: pin Node 20 for the asset build - webpack.config.js: build the vendor shop/admin entries directly (drops the redundant local assets/ re-export files) - Document the Node 20 requirement in CLAUDE.md - Add missing admin CRUD heading translations (edit/create gift card + design) Verified: yarn install + build succeed, admin renders fully styled, console is clean, and the adjust-balance form now submits end-to-end (balance 30000->35000 with a manual ledger row).
- EligibleTotalCalculator: add back applied order_gift_card adjustments so the per-card coverage shown in the cart stays stable once a card is applied. Previously, in adjustment mode, an applied card that fully covered the order drove the total to 0 and its own displayed coverage collapsed to $0.00 (the actual adjustment/ledger were always correct). +unit test. - Remove tests/Application stale 0.12.x Cart/summary.html.twig override that referenced a non-existent setono-sylius-gift-card-add-gift-card-to-order.js (404 + 'jQuery.addGiftCardToOrder is not a function' on the cart page) Both found via Playwright verification of the adjustment-mode redemption flow.
OrderItemTrait::equals() returned false for a gift card item even when compared to itself, so Sylius' OrderItemController::resolveAddedOrderItem() (getItems()->filter(equals)->first()) found nothing and ->first() returned false, raising a TypeError -> 500 on every gift card add-to-cart. Add an identity short-circuit so an item still equals itself while distinct gift card lines remain unmergeable. +regression test. Found via Playwright verification of the gift card purchase flow.
- Design name was required in every locale: a NotBlank form constraint on the translation was applied to all rendered locales. Move it to the GiftCardDesignTranslation entity validation so only the default locale (kept by ResourceTranslationsType) is required. +validators messages. - Design grid 'image' field 500'd (Can't read property 'image'): the twig field passed resource.image; add path: '.' so the template receives the design. - Add missing translations: new_gift_card_design heading, no_image label; drop stale 0.12.x gift_card_configuration/search validator keys. All found via Playwright verification of the design create/edit flow.
Sylius' PaymentMethodFactory::createWithGateway() only sets the gateway config factoryName, leaving gatewayName null. gateway_name is a NOT NULL column, so placing an order in payment mode (which lazily creates the offline gift card payment method) failed with a 500 integrity-constraint violation. Set gatewayName to the payment method code. Found via Playwright verification of payment-mode checkout.
…cale
- Fix GiftCardEmailManager::sendGiftCard(): it did not pass localeCode, so the
Sylius email layout threw 'Variable localeCode does not exist' (breaks admin
resend / single-card send). sendGiftCardsFromOrder was unaffected. Found by
the new functional test.
- Give the PDF attachment a clean customer-facing filename (gift-card-<code>.pdf)
via a per-send temp directory instead of the raw tempnam name.
- Functional tests (KernelTestCase, in-memory MessageEvent capture, CI-friendly):
* GiftCardEmailManagerTest: emails the customer with a valid PDF attachment +
clean filename; skips when there is no customer.
* GiftCardPaymentMethodProviderTest: lazily creates a persistable offline
payment method with gatewayName set (guards the payment-mode 500 regression).
* Extract GiftCardFunctionalTestCase base (schema + channel helpers).
- Verified real delivery + PDF attachment through the docker mailcatcher SMTP.
- docker-compose: drop obsolete 'version' key.
Full suite: 38 tests, 71 assertions.
…gn form Addresses reported issues: - expiresAt is now pinned to 23:59:59 (factory + admin date picker with a model transformer); the admin field is a date picker, not a datetime one. +unit test. - Add a gift card show page with a details panel and the transaction ledger (explicit show route + template + grid Show action). - PDF rendered 3 pages; two full-height cards + html/body height:100% (no explicit page break) now yields exactly 2 pages. +functional test. - Gift card grid defaults to 100 per page (limits [100, 200, 500, 1000]). - Adjust-balance and balance-dashboard pages now use the proper admin layout (header macro + breadcrumbs + form theme). - Design translations use the default Sylius translationForm accordion (locale tabs). - Design image upload: guidelines (dimensions/format, accept=image/*) + a preview thumbnail of already-uploaded images via a custom form widget. Full suite: 41 tests, 76 assertions. All browser-verified.
- Add UniqueDesignImageTypes constraint on GiftCardDesign so a design can have at most one image per type (front/back). Previously two 'Front' images could be added. +constraint validator with unit tests; browser-verified the error. - Restructure the design edit form into two columns (Details | Design images).
Mimics a clean minimalist design: - Front (dark, framed): brand mark + channel name, 'GIFT CARD' tag, 'The gift of choice' eyebrow, large 'Gift Card' heading, VALUE + amount, NO. + code. - Back (cream): dark stripe band, 'How to redeem' instructions, a dashed 'Redemption code' panel, a barcode strip, and a terms + brand/hostname footer (terms note the expiry date when set). - Designs with an uploaded front image still render it full-bleed with a scrim showing the amount + code; back falls back to the cream layout. - New pdf.* translations. Still exactly 2 pages (covered by the functional test).
…tial) - Extract the gift card front into a shared Twig partial (_card.html.twig) + CSS (_cardStyle.html.twig), used by BOTH the PDF and the on-site preview — one source of truth, no html2canvas. - Product page: render the shared card as a prominent, full-width live preview at the top of the gift card section; it updates amount (currency-formatted via Intl), message, and the selected design image live, scaled to fit via a transform. Rewrote product-gift-card.js/.css accordingly. - PDF template consumes the same partial for its front; back unchanged. Still 2 pages (functional test green).
The live preview only handled designs with a front image, so selecting an image-less design left it unchanged. Render both the image and framed layouts in preview mode and toggle .ssgc-card--has-image from JS based on whether the selected design has an image — matching what the PDF produces for that design. Also show the customer message in the image scrim.
templates/bundles/SyliusShopBundle/Taxon/_horizontalMenu.html.twig dropped the 'ui large stackable menu' wrapper (and used taxon.children instead of enabledChildren), so the top navigation rendered as unstyled left-aligned links. Deleting the override restores the vendor default (centered, styled menu).
…nfig - All plugin view folders and files are now snake_case (admin/gift_card/..., shop/gift_card/_card_style.html.twig, email/gift_cards_from_order.html.twig, admin/gift_card/grid/field/delivery_type.html.twig, etc.). Updated every reference (grid/ui/mailer prepend config, routes, pdf service arg, the two balance controllers, and the include/form_theme paths inside templates). - Removed dead 0.12.x leftovers that the extension never loads: grids.yaml + grids/, sylius_ui.yaml (root), routes_no_locale.yaml, state_machine/, and the templates only they referenced (Grid/Action/*, item_units_order, Order/coveredByGiftCards, giftCardBalance, GiftCard/create). Verified: container + twig lint, PHPStan, 43 tests, and browser rendering of the product preview, admin grid, and PDF.
…ntation - Every plugin-defined service id is now its class FQCN, and each interface is aliased directly to the concrete FQCN (autowiring-idiomatic). Updated all argument references, route _controller values, the winzou state-machine callbacks, and test/container references accordingly. - Fixed YAML escaping: the state-machine 'do' service refs use single quotes so the FQCN backslashes aren't treated as escape sequences. - Intentional exceptions kept with dotted ids: the two Sylius service overrides (sylius.factory.add_to_cart_command, sylius.form.type.add_to_cart), the Sylius ImagesUploadListener instance, and the two GiftCardAwareOrderPaymentProcessor decorators (same class -> can't both be the FQCN id). Convenience aliases kept: setono_sylius_gift_card.redemption_method (public, for the state machine) and setono_sylius_gift_card.pdf.generator, both pointing at the concrete FQCN. Verified: container lint, PHPStan, 43 tests, and an end-to-end redemption checkout (winzou -> redemption_method -> operator all resolve; balance debited with a ledger row).
Previously the plugin replaced two Sylius services outright: - sylius.factory.add_to_cart_command was redefined with our factory class - sylius.form.type.add_to_cart was redefined with our command as data_class Both are now non-destructive: - AddToCartCommandFactory decorates sylius.factory.add_to_cart_command and delegates to the inner factory, then wraps the result in our command with the gift card information (also gives it a proper FQCN id). - The form's data_class is set from AddToCartTypeExtension::configureOptions (the extension already extends AddToCartType), so no service override is needed. Verified: container lint, PHPStan, ECS, 43 tests, and browser add-to-cart of both a gift card (pending card created) and a regular product.
loevgaard
commented
Aug 5, 2026
| use Symfony\Component\Validator\Constraints\NotEqualTo; | ||
| use Webmozart\Assert\Assert; | ||
|
|
||
| final class AdjustGiftCardBalanceType extends AbstractType |
Member
Author
There was a problem hiding this comment.
Should this have a data class? Maybe add this to CLAUDE.md that types preferably should have a data class?
Member
Author
There was a problem hiding this comment.
Done in ffd5d25 — it binds to AdjustGiftCardBalanceCommand now, following AddGiftCardToOrderCommand, and the constraints moved onto that class as attributes.
Worth noting what it was costing: the action had to carry /** @var array{amount: int, reason: string} $data */ to make anything of $form->getData(), which is exactly the annotation a data class removes.
Also added the preference to CLAUDE.md as you suggested: form types should bind to a data_class rather than produce an array, with constraints on that class, so the rules travel with the data instead of with the one form that happens to produce it.
Verified in the browser rather than just by the type checker: submitting zero/blank still gets rejected by the moved constraints, and a valid +12.34 adjustment lands as a manual ledger row with its reason, leaving the card reconciling at 7500 + 1234 = 8734.
loevgaard
commented
Aug 5, 2026
Comment on lines
+15
to
+18
| // NotBlank lives on the GiftCardDesignTranslation entity (validation/GiftCardDesignTranslation.xml) rather than | ||
| // here: a form-level constraint would be applied to every rendered locale, forcing the name to be filled in all | ||
| // languages. On the entity it is only validated for the translations kept by ResourceTranslationsType (the | ||
| // default locale), so only the default locale name is required. |
Remove the setono_sylius_gift_card.pdf.generator alias. The interface alias already exists, so the mailer and the two controllers now reference GiftCardPdfGeneratorInterface directly. Map the transaction idempotency key with unique="true" instead of a table level unique-constraint. The generated schema keeps its unique index; only the index name changes, from an explicit one to Doctrine's generated UNIQ_* name. Clear gift card adjustments in GiftCardAdjustmentProcessor itself rather than relying on Sylius' order adjustments clearer. The sylius.order_processing.adjustment_clearing_types parameter was only introduced in Sylius 1.14.2 and never backported to 1.13, so on every supported version below that the compiler pass silently did nothing and the negative adjustments accumulated on each processing run. Clearing happens before the early return so adjustments do not survive removing the last gift card either, which the core clearer would otherwise have handled. The compiler pass is now redundant and is removed; the core clearer runs at priority 60 and this processor at 5, so the self-clearing cannot race it. Drop the hasExtension() guard when prepending. Every extension prepended here (winzou_state_machine, liip_imagine, sylius_mailer, sylius_ui, sylius_grid) comes from a bundle that sylius/core-bundle requires, so the guard could never be false and only served to hide a misconfigured application.
The plugin only prepended winzou callbacks, so on an application configured with sylius_core.state_machine.default_adapter: symfony_workflow none of them fired and gift cards were silently never reconciled, enabled, emailed or disabled. That option exists in both Sylius 1.13 and 1.14, and both ship the framework.workflows definitions, so the gap applies to every supported version. Add the six Symfony Workflow listeners matching the prepended winzou callbacks one to one, registered on workflow.<graph>.completed.<transition> exactly as Sylius registers its own. Priorities preserve the ordering the winzou callbacks had, where declaration order decides: enable before send, rollback before disable. Registering both sets is safe because Sylius applies a transition through a single configured adapter, so only that adapter emits events and the work is never done twice. symfony/workflow is now referenced from src, so it is declared explicitly rather than relied on through sylius/core-bundle.
The plugin took three of the admin menu's slots. Gift cards keeps its entry; designs and the outstanding balance report become actions on the gift cards index instead. Both destinations would otherwise be dead ends now that they are not in the menu, so the designs grid gets an action leading back to gift cards and the balance report gets gift cards in its breadcrumbs. The two new actions use short labels, because with the full ones the button row overflowed its container and clipped the last button even at 1680px wide. The designs action also had to stop using the palette icon: that name does not exist in Semantic UI, so it silently rendered no icon at all, in the old menu entry too. It uses paint brush now. Also add the functional test for the Symfony Workflow side that the previous commit left out. It builds an order carrying a gift card and applies the pay transition through Symfony Workflow itself, which is what Sylius does when an application selects the symfony_workflow adapter, then asserts the card ends up enabled. Verified it fails when the listener is unwired.
Drop the hasParameter() guard from ValidateAddToCartCommandClassPass. The plugin always declares the parameter, and an application that unset it would fail the container build anyway on the two services interpolating it, so the early return could not be reached. Cover GiftCardAwareDefaultPaymentMethodResolver, which had no tests at all: the decorated resolver returning a normal method, returning the gift card method with a fallback available, and the two cases that legitimately throw. Verified they fail when the fallback block is removed. Move the preview gift card construction from GiftCardFactory::createExample() into PreviewGiftCardDesignPdfAction, its only caller. The factory interface no longer carries demo data, and the preview message becomes translatable instead of a hardcoded English string. Resync the Danish and French catalogs, which had drifted badly: 68 of the 98 message keys were missing and 45 were left over from features the rewrite dropped (gift card configurations, channel configurations, date periods, resend email, gift card search), plus the same in flashes and validators. All three domains now match en exactly, with placeholders verified intact.
The plugin leaves column names implicit in its mappings, so with Doctrine's default strategy its columns came out camelCase — initialAmount, currencyCode, deliveryType, customMessage and expiresAt on the gift card table, plus giftCard on sylius_product. They were the only camelCase columns in the whole schema. Which naming strategy applies is the application's call, not the plugin's, and Sylius names its own columns explicitly so it is unaffected either way. The test application now picks the strategy a Sylius application normally would, so it exercises the plugin the way a real one does. Nothing in the plugin depends on the resulting names: it ships no migrations and its repositories query by property name through DQL.
The test application overrides Sylius' product details tab to render the gift card checkbox, and that override dropped the remote_url and load_edit_url options Sylius passes to the options autocomplete. The form theme reads remote_url unconditionally, so every non-simple product's admin edit page failed with a 500 — including, but not limited to, gift card products. This was not caught because UI was only ever verified by hand on the pages a change touched. CLAUDE.md now requires UI to be covered by Playwright tests instead, names the pages that coverage has to include, and calls out that overrides of Sylius templates drift and should be diffed against the original. Also correct two stale claims in CLAUDE.md: prepended configuration is built as PHP arrays rather than loaded from src/Resources/config/prepend/, which no longer exists, and state machine callbacks are now registered for both winzou and Symfony Workflow.
CLAUDE.md asked for UI to be covered by Playwright tests, but nothing enforced it. tests/Playwright now runs 17 specs against a served tests/Application and CI runs them, so the rule has something behind it. Covered: the admin gift cards index, show and edit, designs index and edit, the balance report, the gift card and design preview PDFs, and product edit for simple, configurable and gift card products, plus the shop gift card product page, its locales and adding a gift card to the cart. Two specs exist for regressions this branch already hit, and both were confirmed to fail when the fix is reverted: product edit dropping the autocomplete's remote_url, and the plugin taking more than one admin menu entry. Specs discover their subjects through the admin grids and the locale switcher rather than hardcoding ids, codes or locales, so the suite survives a reseed. The admin project reuses one signed in session; the shop project runs anonymously. The PHP tooling is pointed away from tests/Playwright, since ECS otherwise walks into its node_modules. Checkout in both redemption modes is still uncovered and is noted as the remaining gap in CLAUDE.md.
The adjust balance form rendered unstyled: Semantic UI scopes its field styling under .ui.form, and form_start() was called without that class, so the markup the form theme produced had nothing to style it. The edit form also exposed the amount, which let an admin move a gift card's balance directly. That bypasses the balance operator, so no transaction was written and the ledger silently stopped explaining the balance. The amount is now only available while issuing a card, where it also seeds the initial amount; afterwards the balance belongs to the adjust balance action. The field is removed in a listener rather than never added, because the minor units transformer attaches to it at build time. Add the ui.new_gift_card key, which Sylius' create template asks for as <app>.ui.new_<resource>. It was missing, so the gift card create page had always shown the raw key as its header. Cover all of this in the Playwright suite: the balance being settable only while issuing, and the adjust balance form carrying the class its styling depends on.
The end to end job failed with every admin spec landing on the login form. The trace shows those requests carried no cookie at all, so the stored session was never applied, while the setup that produces it reported success — fourteen failures none of which pointed at the cause. Resolve the storage state to an absolute path, so the setup project and the admin project cannot disagree about which file they mean, and have the setup verify its own output: it now asserts cookies were captured, that the file was written, and that a fresh context built from it actually reaches the admin without being redirected to the login form. If this recurs, the run fails at setup with a message that says so and skips the admin specs, instead of failing all of them for reasons that look unrelated. The root cause in CI is not proven — the job already ran from tests/Playwright, so the path was not obviously ambiguous there.
The ledger only knew about redemptions, restorations and manual adjustments, so a freshly issued gift card showed an empty transactions list while plainly holding money, and the rows never summed to the balance they were supposed to account for. Add GiftCardTransactionInterface::TYPE_ISSUE and a balance operator issue() method that records the balance a card was issued with. It deliberately does not move the balance — the card already holds it, the ledger is only catching up. Recording is idempotent per card, keyed off the unique gift card code, so the nullable unique index makes issuance impossible to record twice no matter how often callers ask. Cards bought in the shop are issued when the order is paid, because a pending card's amount is re-snapshotted during reconciliation and that is the first moment the balance is final. Cards created from the admin hold their balance immediately, so a listener on the resource post_create event records them there. Fixtures issue through the operator as well, so the seeded data does not reproduce the very problem this fixes. The ledger now reconciles: across the whole seeded data set every card's balance equals the sum of its transactions, which a functional test also asserts.
The form produced a bare array, which the action then had to describe with an array shape annotation to make anything of. It now binds to AdjustGiftCardBalanceCommand, following AddGiftCardToOrderCommand, and the constraints move onto that class as attributes so the rules travel with the data rather than with the one form that happens to produce it. Record the preference in CLAUDE.md, and drop the comment on GiftCardDesignTranslationType.
Checkout redemption was the last uncovered part of the UI, and the part where the plugin's two modes actually diverge. The mode decides which service file the extension loads, so it is fixed when the container is compiled — an environment variable cannot switch it, as the loader then looks for services/redemption/%env(...)%.xml. Covering both modes therefore means building a second container, which config/redemption_payment.yaml exists for; CI now runs the shop specs a second time against it. The specs assert what genuinely differs rather than asserting the same thing twice. In adjustment mode the gift card is a negative adjustment and reduces the order total. In payment mode it becomes a payment, so the order still costs what it did and a "Remaining to pay" row appears instead — asserting the total dropping there was wrong, and writing the first version that way is how the difference got pinned down. Also seed a gift card with a known code, so the shop specs have something to redeem without reading it out of the admin.
Researching how other platforms handle this found none that make the mechanism configurable. Shopify and BigCommerce treat a gift card as a payment method; Magento treats it as a discount and has a long-standing feature request to stop, because order management and accounting systems expect a payment instrument, and a gift card reducing the subtotal collides with promotions. The payment reading is also the one that matches the accounting: selling a gift card takes money for a liability, and redeeming it settles that liability rather than reducing what the order is worth. So the choice goes rather than the capability. A redeemed gift card is now always a Payment against the order, and the redemption.mode setting, the adjustment redemption method, the adjustment order processor, the order_gift_card adjustment type and the payment-mode-only Twig flag are all removed. The eligible total no longer has to add gift card adjustments back before computing coverage — nothing subtracts them any more, so the order total is already the pre-redemption total. GiftCardRedemptionMethodInterface stays: it is a reasonable seam for an application that wants to substitute its own, it is just no longer a switch the plugin flips. CI drops the second Playwright pass, and the specs assert the remaining-to-pay figure instead of branching on the mode.
This was referenced Aug 17, 2026
loevgaard
commented
Aug 17, 2026
| final class AdjustGiftCardBalanceCommand | ||
| { | ||
| /** Minor units, positive to increase the balance and negative to decrease it */ | ||
| #[Assert\NotNull(groups: ['setono_sylius_gift_card'])] |
Member
Author
There was a problem hiding this comment.
Shouldn't this be done i XML according to conventions?
loevgaard
commented
Aug 17, 2026
| public function __invoke(Request $request): Response | ||
| { | ||
| /** @var ProductInterface $product */ | ||
| $product = $this->productFactory->create([ |
Member
Author
There was a problem hiding this comment.
Fixture related code should not be used to produce live things imo
loevgaard
commented
Aug 17, 2026
|
|
||
| public function __invoke(): Response | ||
| { | ||
| $balances = $this->giftCardRepository->findBalance(new \DateTimeImmutable()); |
Member
Author
There was a problem hiding this comment.
Maybe the default for findBalance should be 'now' so that we don't have to input the datetime if we just want 'now'
loevgaard
commented
Aug 17, 2026
| * behaves the same whichever state machine adapter the application is configured with. Only the adapter | ||
| * actually applying the transition emits its events, so the two can never both run | ||
| */ | ||
| final class CommitRedemptionListener |
Member
Author
There was a problem hiding this comment.
Make this an event subscriber instead inside src/EventSubscriber
…eftovers Everything reacting to a Symfony event is now an event subscriber, so the event name and the priority live in getSubscribedEvents() next to the code that cares about them instead of in a service tag: the six Symfony Workflow handlers, the gift card issuance recorder and the admin menu builder. Renamed and moved to match, since a class called *Listener implementing EventSubscriberInterface reads as a mistake. Two stay listeners because they cannot be anything else. The design image upload is Sylius' own ImagesUploadListener, a vendor class we cannot add the interface to, and PendingGiftCardCleanupListener is on Doctrine's onFlush, which is not a Symfony event at all. Verified through debug:event-dispatcher that all eight land on the same events with the same priorities as before, including where two of them share an event and the order between them matters. Also remove the psalm leftovers: the @psalm-suppress in Configuration, the @psalm-return on OrderInterface::getGiftCards() — replaced by the plain @return Collection<array-key, GiftCardInterface> the rest of the models use — and the export-ignore for a psalm.xml that no longer exists.
The skeleton uses setono/sylius-plugin-pack, and it is the better fit: it keeps php >=8.1 and PHPUnit 9, and it dropped psalm in favour of PHPStan, which is what this plugin has actually been analysed with all along. code-quality-pack still pulls psalm on the 2.x line we were pinned to, and its 3.x line requires php >=8.2, so it could not be upgraded in place. sylius/sylius is no longer required directly either — the pack provides it, along with phpstan and its extensions, rector, infection, prophecy, phpunit and the dependency analyser. The pack pins sylius/sylius to ~1.14.19, so the CI jobs that forced ~1.13.0 can no longer resolve and the sylius matrix dimension is removed. Sylius 1.13 is therefore no longer exercised anywhere, though nothing in the plugin's own constraints forbids it yet. The upgrade brings PHPStan 1 -> 2, Rector 1 -> 2 and adds phpstan-strict-rules, which surfaced 46 errors. All of them are fixed rather than suppressed: generics declared on the form types and repository traits, @var tags that narrowed a native type replaced by real narrowing, mixed values asserted at the point of use, and the fixture factories' create() widened back to the signature the interface declares. The one exception is the traits the plugin ships for host applications to apply, which PHPStan can only ever see as unused; that is ignored by identifier and path, with the reason written down.
Adds the mutation testing job the skeleton has and this repository was missing, even though infection.json.dist has been committed all along and the plugin pack provides infection. It is scoped to the unit suite: infection runs the test suite itself, and the functional one needs a booted kernel and a database that the job deliberately does not provision. Verified locally — 1234 mutants, covered code MSI 100%, and minMsi 0 in the config means it reports rather than gates. Also picks up actions/checkout@v5, `rector process --dry-run` and the verbose flag on doctrine:schema:validate that makes it print the missing SQL. Not adopted: the skeleton's `lowest` dependency dimension on the analysis, unit and integration jobs. On lowest, lexik/jwt-authentication-bundle and sylius-labs/polyfill-symfony-security resolve to versions whose signatures are incompatible with Symfony 6.4, which is why the skeleton pins them in require-dev. Both only reach us transitively through sylius/sylius, and pinning them would mean re-adding dependencies this rewrite deliberately removed with the API layer, purely to satisfy resolution. The dependency analysis job already installs the production dependencies at their lowest versions, which is what actually guards the constraints in `require`. The skeleton's symfony matrix is also skipped: it carries the single value ~6.4.0, and composer.json already constrains Symfony to ^6.4.
Adds the lowest dimension the skeleton has on static analysis, unit and functional tests, so both ends of the declared constraints are exercised rather than only the newest releases that happen to satisfy them. Three dev dependencies are pinned to make that resolve, exactly as the skeleton does. All three arrive transitively through sylius/sylius and are unused here, but at their lowest versions they are incompatible with Symfony 6.4: sylius-labs/polyfill-symfony-security declares getSalt() incompatibly with LegacyPasswordAuthenticatedUserInterface, lexik/jwt-authentication-bundle does the same for AuthenticatorInterface::authenticate(), and api-platform/core wires a service that does not exist. With those resolved, lowest surfaced two real problems. SchemaTool wants a list and getAllMetadata() returns a plain array, which is now wrapped. And Twig runtime extensions are declared as [RuntimeClass::class, 'method'], a form Twig 3 types explicitly but the Twig 2 we still support only documents as callable|null — ignored by path with the reason recorded, since the code is correct on both. Mutation testing stays on highest alone, as in the skeleton.
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
This is the ground-up 1.x rewrite of the plugin, targeting Sylius 1.13/1.14, PHP ≥ 8.1, Symfony ^6.4. It replaces the
0.12.xarchitecture (base branch of this PR).The rewrite simplifies aggressively and hardens what remains — see
REWRITE.mdfor the full architecture, decisions and progress log, andUPGRADE-1.0.mdfor the migration inventory.Highlights
variant->isShippingRequired(), not a special product type.OrderItemUnitat add-to-cart, reconciled to final amounts at checkout completion, enabled + emailed (PDF attachment) on payment, disabled on cancel.Paymentagainst the order via a lazily-created offline gift card payment method; the order total stays intact, the remainder goes through the normal gateway, and the payment step is skipped at full coverage. See the note below.GiftCardTransactionledger (audit + idempotency) which accounts for the whole balance, opening balance included —sum(transactions) == amount. Balance mutations go through a single balance operator; nothing below controllers flushes.sylius_core.state_machine.default_adapteris set to.Why redemption is a payment
0.12.x reduced the order total with a negative
order_gift_cardadjustment. 1.x does not: a redeemed gift card is a payment against the order.This started life as a configurable
redemption.modewith both mechanisms, and was cut back to one after checking what other platforms actually do. None of them make it configurable, and the split is instructive: Shopify and BigCommerce treat a gift card as a payment method, while Magento treats it as a discount and has a long-standing feature request to stop — because order management and accounting systems expect a payment instrument, and a gift card reducing the subtotal collides with promotions.It also matches the accounting: selling a gift card takes money for a deferred revenue liability, and redeeming it settles that liability rather than reducing what the order is worth.
This is the sharpest upgrade edge in the PR: orders no longer carry
order_gift_cardadjustments at all, so any host-app template, report or ERP export reading them silently returns nothing.UPGRADE-1.0.mdcalls it out.Testing
The Playwright suite covers the admin (gift cards index/show/edit, designs, balance report, gift card and design preview PDFs, product edit for simple/configurable/gift card products) and the shop (gift card product page, locales, add-to-cart, and redemption in the cart). Specs discover their subjects through the admin grids and the locale switcher rather than hardcoding ids, so they survive a reseed.
Notes for reviewers
1.x rewrite phase N …) followed by polish, review follow-ups and bug fixes.prepend(); host apps only register the bundle, import routes and apply the entity traits.REWRITE.md: [Feature request]: Recipient name and email fields on a gift card #270, [Feature request]: Per-channel default validity periods #271, [Feature request]: Preset amount chips on the gift card product page #272, [Feature request]: Edit a gift card line in the cart #273, [Feature request]: Public gift card balance page #274, [Feature request]: CR80 (credit card sized) print template for gift card PDFs #275, [Feature request]: Expiry reminder emails #276.