Feature/upgrade v2 part 2#275
Conversation
…with-mollie Fix compatibility when mollie plugin is here
PPSYL-177 - handle canceled by oney status
…device, do not check against
Feature/PPSYL-173 handle refunds properly
PPSYL-176 - Refact payment methods / improve ApplePay integration
There was a problem hiding this comment.
Pull request overview
This PR continues the Sylius v2 upgrade for the PayPlug plugin, focusing on checkout payment selection UX, Apple Pay flow modernization, and refund/notification handling aligned with Sylius PaymentBundle.
Changes:
- Refactors checkout payment selection templates/controllers (Stimulus) and updates payment method logos/assets.
- Reworks Apple Pay integration (template + JS + provider/controller flow) and introduces “not available” messaging.
- Updates refund/notify processing to better support Sylius v2 (new notify provider, repository nullable return, handler tweaks), plus documentation/CI adjustments.
Reviewed changes
Copilot reviewed 56 out of 61 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| translations/messages.it.yml | Adds Apple Pay “not available” translation. |
| translations/messages.fr.yml | Adds Apple Pay “not available” translation. |
| translations/messages.en.yml | Adds Apple Pay “not available” translation. |
| templates/shop/select_payment/choice.html.twig | Wraps per-gateway hook output into a Stimulus-controlled container. |
| templates/shop/select_payment/_payplug.html.twig | Hooks integrated payment controller and passes paymentInputId downstream. |
| templates/shop/select_payment/_oney.html.twig | Moves Oney logo handling to a Stimulus “payment-logo” controller and simplifies choice container. |
| templates/shop/select_payment/_apple_pay.html.twig | Replaces inline Apple Pay settings script with Stimulus-driven settings + new Apple Pay SDK URL + inline submit flag. |
| templates/shop/integrated/index.html.twig | Exposes integrated-payment container as a Stimulus target and adds inline-submit flag. |
| templates/form/sylius_checkout_select_payment_row.html.twig | Adds Stimulus actions to saved-card radios to show/hide integrated fields + enable next-step. |
| src/Resolver/PaymentStateResolver.php | Switches GatewayConfigInterface import to Sylius Payment component. |
| src/Resolver/OneyPaymentMethodsResolverDecorator.php | Switches GatewayConfigInterface import to Sylius Payment component. |
| src/Resolver/ApplePayPaymentMethodsResolverDecorator.php | Removes device readiness filtering and relies on supported-method provider. |
| src/Repository/PaymentRepositoryInterface.php | Makes findOneByPayPlugPaymentId() nullable. |
| src/Repository/PaymentRepository.php | Uses getOneOrNullResult() for PayPlug payment lookup. |
| src/Provider/SupportedRefundPaymentMethodsProviderDecorator.php | Switches GatewayConfigInterface import to Sylius Payment component. |
| src/Provider/SupportedMethodsProvider.php | Switches GatewayConfigInterface import to Sylius Payment component. |
| src/Provider/Payment/ApplePayPaymentProvider.php | Refactors Apple Pay create/patch/cancel to use Sylius notify route + adds logging + removes internal flushes. |
| src/Provider/OneySupportedPaymentChoiceProvider.php | Switches GatewayConfigInterface import to Sylius Payment component. |
| src/Provider/ApplePayOrderProvider.php | Removes request-stack based order id provider (no longer used). |
| src/Provider/AbstractSupportedRefundPaymentMethodsProvider.php | Switches GatewayConfigInterface import to Sylius Payment component. |
| src/PaymentProcessing/RefundPaymentProcessor.php | Uses typed PayPlug client + asserts payment_id string before refund calls. |
| src/PaymentProcessing/RefundPaymentHandler.php | Merges item + shipment unit refunds into a single units array. |
| src/PaymentProcessing/AbortPaymentProcessor.php | Uses PayPlugApiClientFactory per payment method and skips abort when missing payment_id/method. |
| src/OrderPay/Provider/NotifyRefundPaymentProvider.php | Adds notify provider for refund webhooks to resolve Sylius payment from payload payment_id. |
| src/OrderPay/Provider/NotifyPaymentProvider.php | Tightens supports() to object === payment and improves payload safety. |
| src/MessageHandler/RefundPaymentGeneratedHandler.php | Adds Symfony Messenger handler attribute and updates GatewayConfigInterface import. |
| src/Gateway/Validator/Constraints/PayplugPermissionValidator.php | Skips permission validation for disabled payment methods. |
| src/Gateway/Validator/Constraints/IsOneyEnabledValidator.php | Skips validation for disabled methods + handles GatewayConfigurationException. |
| src/Gateway/Validator/Constraints/IsCanSavePaymentMethodValidator.php | Skips validation for disabled methods + handles GatewayConfigurationException. |
| src/Form/Extension/PaymentTypeExtension.php | Switches GatewayConfigInterface import to Sylius Payment component. |
| src/Exception/GatewayConfigurationException.php | Introduces a dedicated exception for gateway config problems. |
| src/Creator/RefundUnitsCommandCreatorDecorator.php | Simplifies converter usage, adds supported-method list, and improves request parsing. |
| src/Creator/PayPlugPaymentDataCreator.php | Resets details ArrayObject to avoid carrying stale payment details into new requests. |
| src/Controller/OrderController.php | Updates Apple Pay routes to {id}, adds redirect helper, adds more logging, changes failure handling. |
| src/Controller/OneClickAction.php | Marks controller deprecated and validates gateway name before token creation. |
| src/Controller/IpnAction.php | Marks deprecated and handles null payment lookup safely. |
| src/Command/Handler/StatusPaymentRequestHandler.php | Adds logger, handles missing payment_id, avoids updating already-completed payments, adds canceled_by_oney mapping. |
| src/Command/Handler/NotifyPaymentRequestHandler.php | Switches to treating webhook payload via API client, supports refund resources, avoids duplicate updates for completed payments. |
| src/Checker/ApplePayCheckerInterface.php | Removes Apple Pay device readiness checker interface. |
| src/Checker/ApplePayChecker.php | Removes Apple Pay device readiness checker implementation. |
| src/ApiClient/PayPlugApiClientFactory.php | Throws GatewayConfigurationException for missing/invalid credentials rather than LogicException. |
| src/Action/StatusAction.php | Marks Payum action deprecated. |
| src/Action/NotifyAction.php | Marks Payum action deprecated. |
| src/Action/ConvertPaymentAction.php | Marks Payum action deprecated. |
| src/Action/CaptureAction.php | Marks Payum action deprecated. |
| src/Action/Admin/RefundUnitsAction.php | Removes admin refund-units controller (Sylius refund plugin flow change). |
| src/Action/Admin/CompleteRefundPaymentAction.php | Removes admin “complete refund payment” controller (Sylius refund plugin flow change). |
| src/Action/Admin/Auth/UnifiedAuthenticationController.php | Clears cached JWT config after OAuth callback to prevent stale credentials. |
| ruleset/phpstan-baseline.neon | Removes baseline entry for deleted ApplePayOrderProvider. |
| public/assets/bancontact/logo.svg | Replaces Bancontact SVG asset. |
| public/assets/apple-pay/logo.svg | Replaces Apple Pay SVG asset. |
| public/assets/american-express/logo.svg | Replaces American Express SVG asset. |
| config/services.yaml | Enables autowire on custom OrderController service definition. |
| composer.json | Loosens PHPUnit constraint to ^9.6. |
| assets/shop/dist/payment/integrated.css | Replaces minified CSS with expanded version (packaging/output change). |
| assets/shop/dist/payment/index.css | Adjusts checkout payment CSS and broadens selector scopes. |
| assets/shop/controllers/integrated-payment_controller.js | Adds container target + show/hide hooks + scopes DOM queries for integrated form. |
| assets/shop/controllers/checkout-select-payment_controller.js | Reworks payment method choice visibility and next-step button enable/disable behavior. |
| assets/shop/controllers/apple-pay_controller.js | Rewrites Apple Pay flow without jQuery, adds capability checks and disables method when unsupported. |
| README.md | Updates badges, compatibility matrix, Sylius 2.0 assets install instructions, and installation steps. |
| .github/workflows/sylius.yaml | Simplifies CI job by removing local cert/webserver/headless browser steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $paymentResource = $this->applePayClient->createPayment($paymentData); | ||
| $this->logger->notice('[Payplug] ApplePay payment resource', ['payment' => (array) $paymentResource]); |
There was a problem hiding this comment.
Logging the full PayPlug payment resource at notice level risks leaking sensitive/payment data into logs. Please remove or sanitize, and prefer debug logging guarded behind configuration.
| throw new \InvalidArgumentException('Missing token in request'); | ||
| } | ||
|
|
||
| $this->logger->notice('[Payplug] ApplePay payment token', ['token' => $token]); |
There was a problem hiding this comment.
The Apple Pay token is sensitive and should never be logged. Please remove this log entirely or replace it with a non-sensitive correlation identifier.
| 'success' => false, | ||
| 'data' => $dataResponse, | ||
| ]; | ||
|
|
There was a problem hiding this comment.
This error response path returns without flushing. Since ApplePayPaymentProvider::patch() applies payment state transitions but no longer flushes internally, those FAILED/updated details changes may not be persisted unless the controller flushes before returning.
| $this->manager->flush(); |
| return \str_contains($paymentMethod->getGatewayConfig()?->getFactoryName() ?? '', 'payplug') && | ||
| $request->getPayload()->has('id') && | ||
| $request->getPayload()->has('metadata') && | ||
| $request->getPayload()->has('object') && | ||
| $request->getPayload()->get('object') === 'refund'; |
There was a problem hiding this comment.
supports() checks for payload key 'id', but getPayment() actually requires 'payment_id'. This can make supports() return true and then getPayment() throw. Consider checking for 'payment_id' here (and aligning the getPayment() exception message).
| $details = $lastPayment->getDetails(); | ||
| $details['status'] = PaymentInterface::STATE_COMPLETED; | ||
| $details['created_at'] = $response->created_at; |
There was a problem hiding this comment.
details['status'] is used elsewhere as a PayPlug API status (created/captured/authorized/failed…), but here it’s set to a Sylius payment state (completed). This inconsistency can break downstream state resolution (e.g. StatusPaymentRequestHandler). Use a PayPlugApiClientInterface status value here (e.g. captured) or store the Sylius state in a separate details key.
| ->where('o.details LIKE :payplugPaymentId') | ||
| ->setParameter('payplugPaymentId', '%' . $payplugPaymentId . '%') | ||
| ->getQuery() | ||
| ->setMaxResults(1) | ||
| ->getSingleResult() | ||
| ->getOneOrNullResult() |
There was a problem hiding this comment.
With a LIKE filter on details, switching to getOneOrNullResult() can silently return an arbitrary match if multiple rows match the pattern. Previously getSingleResult() would surface the data issue. Consider tightening the query (exact JSON key match if possible) or adding ordering/handling for duplicates.
| data-validate-merchant-route="{{ path('payplug_shop_checkout_apple_prepare', {'id': order.id}) }}" | ||
| data-payment-authorized-route="{{ path('payplug_shop_checkout_apple_confirm', {'id': order.id}) }}" | ||
| data-session-cancel-route="{{ path('payplug_shop_checkout_apple_cancel', {'id': order.id}) }}" |
There was a problem hiding this comment.
This template guards amount when order is null, but the route parameters still unconditionally use order.id. If order can be null here (as the amount guard suggests), this will throw a Twig error. Either assert order is always present (and remove the null-guard) or guard these route attributes when order is null.
| const isPayplugSelected = document.querySelector(`[id*="checkout_select_payment_payments"][value="${payplug_integrated_payment_params.payment_method_code}"]`)?.checked; | ||
|
|
||
| if (isOtherCardChecked && isPayplugSelected) { | ||
| this.openFields(); |
There was a problem hiding this comment.
In the saved-cards branch, the change handler only calls openFields() when Payplug + “other card” are selected, but never calls closeFields() when the selection changes away. This can leave the integrated form opened/loaded unexpectedly. Add an else branch to call closeFields() when the condition is not met.
| this.openFields(); | |
| this.openFields(); | |
| } else { | |
| this.closeFields(); |
| $paymentData = $paymentDataObject->getArrayCopy(); | ||
| $paymentData['notification_url'] = $this->router->generate('payplug_sylius_ipn', [], UrlGenerator::ABSOLUTE_URL); | ||
| $paymentData['notification_url'] = $this->router->generate('sylius_payment_method_notify', ['code' => $payment->getMethod()?->getCode()], RouterInterface::ABSOLUTE_URL); | ||
| $this->logger->notice('[Payplug] ApplePay payment data', ['data' => $paymentData]); |
There was a problem hiding this comment.
This logs the full Apple Pay payment data payload at notice level. These payloads typically contain sensitive customer/payment metadata and should be avoided or heavily redacted (and usually logged only at debug with opt-in).
d82b031
into
payplug:feature/upgrade-v2
Hello !
Second batch of dev about sylius v2