Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions UPGRADE-3.4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# UPGRADE FROM 3.3 TO 3.4
Comment thread
TheMilek marked this conversation as resolved.

1. A Mollie payment reported as `open` now leaves the Sylius payment in `new` instead of moving it
to `processing`. In the Payum status flow, `processing` is now reached only from Mollie's
`pending`.

Anything that treats `processing` as "the customer started paying" changes meaning: admin grid
filters, reporting, exports and custom state-machine callbacks.

2. `ConvertMolliePaymentAction` now copies everything describing the Mollie session already tracked
on the payment into its result: `payment_mollie_id`, `order_mollie_id`, `webhookUrl`, `backurl`
and `metadata.refund_token`. Payum replaces the payment details with this result on every capture
while the payment sits in `new`, so a decorating action that drops these keys loses the payment
link, the abandoned payment link emails and Mollie side refunds for that payment.

`ConvertMollieSubscriptionPaymentAction` carries the same four top level keys over.

3. `CaptureAction` takes a resolver deciding what to do with a Mollie session already tracked on the
payment: leave it to the status flow, hand it back to the customer, or replace it. It also takes
the logger, and records the two Mollie failures it used to swallow: a tracked session it cannot
read, and a superseded session it cannot cancel.

```diff
public function __construct(
private OrderRepositoryInterface $orderRepository,
private MollieApiClientKeyResolverInterface $apiClientKeyResolver,
private PaymentRepositoryInterface $paymentRepository,
+ private ExistingMollieSessionResolverInterface $existingSessionResolver,
+ private MollieLoggerActionInterface $loggerAction,
) {
```

4. `MolliePaymentsMethodResolver` takes the surcharge amount calculator.

```diff
public function __construct(
private readonly MollieGatewayConfigRepository $mollieGatewayRepository,
private readonly MollieCountriesRestrictionResolverInterface $countriesRestrictionResolver,
private readonly ProductVoucherTypeCheckerInterface $productVoucherTypeChecker,
private readonly PaymentCheckoutOrderResolverInterface $paymentCheckoutOrderResolver,
private readonly MollieBasedPaymentMethodQueryInterface $mollieBasedPaymentMethodQuery,
private readonly MollieAllowedMethodsResolverInterface $allowedMethodsResolver,
private readonly MollieLoggerActionInterface $loggerAction,
private readonly MollieFactoryNameResolverInterface $mollieFactoryNameResolver,
private readonly DivisorProviderInterface $divisorProvider,
+ private readonly PaymentSurchargeAmountCalculatorInterface $surchargeAmountCalculator,
+ private readonly PaymentSurchargeAdjustmentsProviderInterface $surchargeAdjustmentsProvider,
) {
```

5. Once an order has been placed, only methods whose surcharge matches the one already charged are
offered when changing the payment method. Shops that configure different surcharges per method
will see a shorter list there than before. Nothing changes during checkout.

When a surcharge cannot be compared, only the method the order already carries is offered and the
reason is logged, so the total stays correct and the method list never fails because of it. See
point 6.

6. The payment fee calculators in `Sylius\MolliePlugin\Calculator\PaymentFee` also implement
`PaymentSurchargeAmountCalculatorInterface`, which reports a surcharge instead of applying it
to an order. `PaymentSurchargeCalculatorInterface` is unchanged and was deliberately left alone
rather than gaining the new method, because a class implementing it without `calculateAmount()`
would stop loading altogether.

So a custom calculator implementing only `PaymentSurchargeCalculatorInterface` keeps applying its
surcharge exactly as before and needs no change to keep working. What it cannot do is report an
amount, so the plugin cannot compare its surcharge against the one already on an order. After
checkout completion such an order is then offered only the method it already carries, which is
the one that produced its surcharge, and the reason is logged.

To take part in the comparison, implement `PaymentSurchargeAmountCalculatorInterface` as well and
have `calculate()` delegate to `calculateAmount()`, which is what the bundled calculators do, so
the applied and the reported value cannot drift apart.

Calculated amounts are unchanged. `FixedAmountAndPercentageCalculator` no longer adds and then
removes intermediate adjustments to arrive at its total, so an order carrying unrelated
`fixed_fee` or `percentage` adjustments is no longer affected by it.

Its second and third constructor arguments are now typed
`PaymentSurchargeAmountCalculatorInterface` instead of `PaymentSurchargeCalculatorInterface`.
Passing a calculator that implements only the latter no longer type checks.

```diff
public function __construct(
private readonly AdjustmentFactoryInterface $adjustmentFactory,
- private readonly PaymentSurchargeCalculatorInterface $percentageCalculator,
- private readonly PaymentSurchargeCalculatorInterface $fixedAmountCalculator,
+ private readonly PaymentSurchargeAmountCalculatorInterface $percentageCalculator,
+ private readonly PaymentSurchargeAmountCalculatorInterface $fixedAmountCalculator,
private readonly DivisorProviderInterface $divisorProvider,
) {
```

7. The log entry written when a paid Mollie payment is not the one being tracked is now recorded at
error level rather than as a notice, and its wording changed.

8. That same log entry is no longer written for Order API payment webhooks, where Mollie calls the
notify token with the `tr_` id of the payment inside the order.

9. `PaymentSurchargeAdjustmentsProviderInterface` is the single source of truth for the adjustment
types a payment surcharge can produce. `PaymentFeeAdjustmentClearer` reads them from it rather
than naming three types itself.

A new parameter has been introduced, `sylius_mollie.payment_surcharge_adjustments`, holding the
three built in types. Redefine it to have your own surcharge adjustments cleared and compared
along with them.

```diff
+public function __construct(
+ private readonly PaymentSurchargeAdjustmentsProviderInterface $surchargeAdjustmentsProvider,
+) {
+}
+
public function clear(OrderInterface $order): void
```

`PaymentFeeCalculateAction::PAYMENTS_FEE_METHOD` still holds the same three types and still
works, but the provider is what the plugin now reads.
68 changes: 68 additions & 0 deletions adr/2026_08_11_ignore_unexpected_paid_mollie_payments.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Ignore unexpected paid Mollie payments for superseded checkout sessions

* Status: Accepted
* Date: 2026-08-11

## Context and Problem Statement

When a customer abandons a Mollie checkout mid-payment and later retries, possibly with a different payment method, the plugin creates a new Mollie payment for the order (see issue [#329](https://github.com/Sylius/MolliePlugin/issues/329), PR [#335](https://github.com/Sylius/MolliePlugin/pull/335)). It also best-effort cancels the superseded one, but Mollie only supports programmatic cancellation for some payment methods, and which ones is theirs to decide. For the rest, the old Mollie payment can remain genuinely payable after the new one has already been created.

We asked Mollie support directly what to do about this. Their answer suggested choosing one of 3 options.

## What Mollie advised

Their guidance came in two parts.

**On which payment is authoritative** (June): keep a record of every payment created for a checkout session and expect payment only from the last one created, for example after the customer changes method. They cannot reliably link payments to a single session on their end, so that bookkeeping has to live with us. The three options below all concern what to do with an unexpected paid payment **given** that rule, rather than being alternatives to it.

**On the disposition** (2026-08-13): after we described logging the event and leaving the order uncredited, they confirmed that log-only is acceptable, said our reluctance to auto-refund was understandable, and framed the right choice as depending on what the surrounding ecosystem treats as normal integration behaviour.

Two things follow. Log-only is endorsed rather than merely tolerated, so Option 1 is not a compromise we settled for. And the condition attached to it is testable and worth revisiting: it holds as long as other payment integrations in the Sylius ecosystem behave comparably. If one of them starts surfacing this to the merchant, Option 2 comes back into scope.

**What the rule does not cover.** Expecting only the last payment does not prevent a double charge. If two payments from one session are both paid, the money has been collected twice regardless of which one we honour, because open payments cannot be cancelled for most methods. The rule decides only whether the *order* is credited, and the cost is the mirror image: a payment the customer genuinely made can be refused, leaving them charged with no order. This reading is ours and has not been put to Mollie in these terms.

## Decision Drivers

* **Correctness/safety**: the order must never end up completed twice, or completed off a payment that isn't the one the merchant actually expects.
* **Implementation scope**: this is a fix for an existing bug report (#329), not a platform for new merchant-facing features; scope creep risks delaying the actual fix.
* **Dependency footprint**: the plugin already treats refund functionality as optional (e.g. `StatusAction::isRefundingPossible()` checks whether `SyliusRefundPlugin` is even installed before attempting a refund).
* **Reversibility**: whichever option we pick now should not block adopting a stronger one later without re-architecting.
* **Precedent**: Mollie states other integrations already ship the "ignore" behavior successfully.

## Considered Options

### Option 1: Ignore the unexpected payment (log only)

* **Good**, because it's the simplest option and matches what Mollie says other integrations already do.
* **Good**, because it keeps Sylius's order state safe by construction: the webhook/notify handlers only ever act on the currently-tracked Mollie payment id, so a stale one becoming paid can never cause a double completion. It is a pure no-op plus a log line.
* **Good**, because it needs no new infrastructure (no admin UI, no dependency on `SyliusRefundPlugin`) and ships entirely within the existing webhook/notify code paths.
* **Good**, because it doesn't foreclose Option 2 or 3 later. The log line names both the payment that arrived and the one we expected, which is what either of them would act on.
* **Bad**, because the money mismatch (Mollie collected a payment that Sylius's order total doesn't reflect) isn't surfaced anywhere actionable, so a merchant who never checks logs or the Mollie dashboard won't notice it happened.
* **Bad**, because reconciliation, if it's ever needed, is manual (grep the log / cross-reference Mollie's dashboard by `metadata.order_id`).

### Option 2: Flag/create a task for the merchant to act on

* **Good**, because it gives the merchant visibility and a concrete next action (refund manually, contact the customer, etc.), with no automatic money movement.
* **Good**, because the merchant stays in control of what happens to the unexpected payment.
* **Bad**, because there is no existing mechanism in this plugin for merchant-facing tasks/notifications, and building one (admin dashboard entry, order note, email, or similar) is a feature in its own right, not a bug fix.
* **Bad**, because it still requires manual merchant effort every single time this happens; doesn't scale for a merchant with meaningful abandon-and-retry volume.
* **Bad**, because deciding *where* this should surface (admin grid? order timeline? notification center?) is a product decision that would stall this fix waiting on it.

### Option 3: Automatically refund the unexpected payment

* **Good**, because it's the only option that leaves no lingering money mismatch, since the customer gets their money back without merchant involvement.
* **Good**, because Mollie says they do something similar themselves for other integrations (e.g. Shopify, when final order confirmation can't be obtained).
* **Bad**, because it requires refund capability that isn't always available: this plugin's own refund flow is conditional on `SyliusRefundPlugin` being installed (`StatusAction::isRefundingPossible()`), so this option can't be unconditionally relied on.
* **Bad**, because it automates an irreversible money movement based on the same webhook/notify logic that would need to correctly distinguish "genuinely orphaned" from "the payment we actually expected". A false positive there (e.g. a future regression in that detection logic) would incorrectly refund a legitimate payment with no merchant review step in between.
* **Bad**, because refunds are not instantaneous either, and can themselves fail or be delayed, introducing a second asynchronous process to reason about, on top of the payment-creation race this PR already deals with.

## Decision Outcome

**Chosen option**: **Option 1: Ignore the unexpected payment (log only)**, because it's the option Mollie explicitly validates as sufficient, it requires no new merchant-facing surface or optional-plugin dependency, and it keeps Sylius's own order/payment state safe by construction rather than by process. It is also the least risky to ship as part of a bug-fix PR: it only ever prevents an action (adopting/completing a stale payment) rather than taking a new, irreversible one.

This does **not** eliminate the underlying risk Mollie themselves called out. A real, uncancellable payment can still be collected twice for the methods where cancellation is unavailable, and no option here fully solves that. Options 2 and 3 remain available as future, additive work on top of this one if reconciliation volume ever justifies the investment, and nothing in this decision blocks either.

### Where this is implemented

* `src/Payum/Action/NotifyAction.php` (`logOrphanPaidPayment()`), the Payum notify-token webhook path used by the standard checkout flow. Deliberately an **error-level** entry: `MollieLoggerAction::canSaveLog()` discards notices unless the gateway logs everything, which would leave the event with no trace under the more common settings. It is still lost entirely when logging is disabled outright.
* `src/Controller/Shop/PaymentWebhookController.php`, the webhook path used by QR code, Apple Pay and API-storefront flows. Its mismatch guard **predates this decision**, having shipped with the webhook validation work on `3.3`. It is listed because it enforces the same rule, not because this PR added it.
6 changes: 6 additions & 0 deletions config/services/calculator.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,16 @@
alias="sylius_mollie.calculator.payment_fee.composite"
/>

<service
id="Sylius\MolliePlugin\Calculator\PaymentFee\PaymentSurchargeAmountCalculatorInterface"
alias="sylius_mollie.calculator.payment_fee.composite"
/>

<service
id="sylius_mollie.calculator.clearer.payment_fee_adjustment"
class="Sylius\MolliePlugin\Calculator\Clearer\PaymentFeeAdjustmentClearer"
>
<argument type="service" id="sylius_mollie.provider.payment_surcharge_adjustments"/>
</service>
<service id="Sylius\MolliePlugin\Calculator\Clearer\PaymentFeeAdjustmentClearerInterface" alias="sylius_mollie.calculator.clearer.payment_fee_adjustment"/>
</services>
Expand Down
2 changes: 2 additions & 0 deletions config/services/payum/action.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
<argument type="service" id="sylius.repository.order" />
<argument type="service" id="sylius_mollie.resolver.mollie_api_client_key"/>
<argument type="service" id="sylius.repository.payment" />
<argument type="service" id="sylius_mollie.payum.resolver.existing_mollie_session"/>
<argument type="service" id="sylius_mollie.logger.mollie_logger_action"/>
<tag name="payum.action" factory="mollie" alias="payum.action.capture"/>
<tag name="payum.action" factory="mollie_subscription" alias="payum.action.capture_subscription"/>
</service>
Expand Down
3 changes: 3 additions & 0 deletions config/services/payum/factory.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,8 @@

<service id="sylius_mollie.payum.checker.mollie_gateway_factory" class="Sylius\MolliePlugin\Payum\Checker\MollieGatewayFactoryChecker" public="true" />
<service id="Sylius\MolliePlugin\Payum\Checker\MollieGatewayFactoryCheckerInterface" alias="sylius_mollie.payum.checker.mollie_gateway_factory" />

<service id="sylius_mollie.payum.resolver.existing_mollie_session" class="Sylius\MolliePlugin\Payum\Resolver\ExistingMollieSessionResolver" />
<service id="Sylius\MolliePlugin\Payum\Resolver\ExistingMollieSessionResolverInterface" alias="sylius_mollie.payum.resolver.existing_mollie_session" />
</services>
</container>
13 changes: 13 additions & 0 deletions config/services/provider.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,25 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"
>
<parameters>
<parameter key="sylius_mollie.payment_surcharge_adjustments" type="collection">
<parameter type="constant">Sylius\MolliePlugin\Model\AdjustmentInterface::FIXED_AMOUNT_ADJUSTMENT</parameter>
<parameter type="constant">Sylius\MolliePlugin\Model\AdjustmentInterface::PERCENTAGE_ADJUSTMENT</parameter>
<parameter type="constant">Sylius\MolliePlugin\Model\AdjustmentInterface::PERCENTAGE_AND_AMOUNT_ADJUSTMENT</parameter>
</parameter>
</parameters>

<services>
<defaults public="true" />

<service id="sylius_mollie.provider.divisor" class="Sylius\MolliePlugin\Provider\DivisorProvider"/>
<service id="Sylius\MolliePlugin\Provider\DivisorProviderInterface" alias="sylius_mollie.provider.divisor" />

<service id="sylius_mollie.provider.payment_surcharge_adjustments" class="Sylius\MolliePlugin\Provider\PaymentSurchargeAdjustmentsProvider">
<argument>%sylius_mollie.payment_surcharge_adjustments%</argument>
</service>
<service id="Sylius\MolliePlugin\Provider\PaymentSurchargeAdjustmentsProviderInterface" alias="sylius_mollie.provider.payment_surcharge_adjustments" />

<service id="sylius_mollie.provider.customer" class="Sylius\MolliePlugin\Provider\CustomerProvider">
<argument type="service" id="sylius.repository.customer" />
<argument type="service" id="sylius.factory.customer"/>
Expand Down
2 changes: 2 additions & 0 deletions config/services/resolver.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
<argument type="service" id="sylius_mollie.logger.mollie_logger_action"/>
<argument type="service" id="sylius_mollie.resolver.mollie_factory_name"/>
<argument type="service" id="sylius_mollie.provider.divisor"/>
<argument type="service" id="sylius_mollie.calculator.payment_fee.composite"/>
<argument type="service" id="sylius_mollie.provider.payment_surcharge_adjustments"/>
</service>
<service id="Sylius\MolliePlugin\Resolver\MolliePaymentsMethodResolverInterface" alias="sylius_mollie.resolver.payment_methods" />

Expand Down
Loading
Loading