-
-
Notifications
You must be signed in to change notification settings - Fork 12
PaymentWebhookController bypasses state machine by calling setState() directly #337
Description
Description
The PaymentWebhookController updates the payment state by calling $payment->setState() directly instead of using Sylius's state machine. This bypasses all state machine guards, transitions validation, and — most importantly — the after
callbacks that Sylius relies on to keep orders consistent.
Root cause
In PaymentWebhookController::__invoke():
$payment = $order->getLastPayment();
$status = $this->getStatus($molliePayment);
if ($payment->getState() !== $status && PaymentInterface::STATE_UNKNOWN !== $status) {
$payment->setState($status); // ← direct state mutation
$this->paymentRepository->add($payment);
} The getStatus() method maps Mollie statuses to Sylius payment states (STATE_PROCESSING, STATE_COMPLETED, etc.) and writes them directly to the entity. This has several consequences:
What breaks
- State machine callbacks are never triggered:
- sylius_process_order (on fail/cancel) — responsible for auto-creating a new payment when one fails. Without it, the order gets stuck with no actionable payment. (maybe linked with #329 ?)
- sylius_resolve_state (on complete/process/authorize) — responsible for resolving the order-level payment state (awaiting_payment → paid). Without it, the order payment state is never updated. - Invalid transitions are possible:
- The state machine defines allowed transitions (e.g. complete can only happen from new, processing, or authorized). setState() allows any state change, including completed → failed → completed, which should never happen. - STATE_UNKNOWN can be written to the database:
- While STATE_UNKNOWN is filtered out, the default case in getStatus() returns PaymentInterface::STATE_UNKNOWN which is not a valid state in the Sylius payment state machine. If the condition check were slightly different, this could
corrupt data. - No null check on getLastPayment():
- $order->getLastPayment() can return null, but the result is used directly without a null check, which would cause a fatal error.
Expected behavior
The controller should use Sylius's StateMachineInterface to apply transitions (not states), and let the state machine validate the transition and execute callbacks:
$transition = $this->mapMollieStatusToTransition($molliePayment->status);
if (null !== $transition && $this->stateMachine->can($payment, PaymentTransitions::GRAPH, $transition)) {
$this->stateMachine->apply($payment, PaymentTransitions::GRAPH, $transition);
$this->entityManager->flush();
}With a mapping to transitions instead of states:
private function mapMollieStatusToTransition(string $status): ?string
{
return match ($status) {
PaymentStatus::STATUS_PENDING, PaymentStatus::STATUS_OPEN => PaymentTransitions::TRANSITION_PROCESS,
PaymentStatus::STATUS_AUTHORIZED => PaymentTransitions::TRANSITION_AUTHORIZE,
PaymentStatus::STATUS_PAID => PaymentTransitions::TRANSITION_COMPLETE,
PaymentStatus::STATUS_CANCELED => PaymentTransitions::TRANSITION_CANCEL,
PaymentStatus::STATUS_EXPIRED, PaymentStatus::STATUS_FAILED => PaymentTransitions::TRANSITION_FAIL,
default => null, // unknown status → do nothing
};
} This way:
- Transitions are validated before being applied
- All after callbacks are executed (order payment state resolution, auto-creation of new payment on fail, etc.)
- Invalid state changes are naturally rejected
- Unknown statuses are safely ignored (null → no transition applied)
Workaround
We overrode PaymentWebhookController in our application to use StateMachineInterface with transitions instead of direct setState() calls.
### Environment
- Sylius: 2.0
- Mollie Plugin: latest