For Offsite payments, when creating the Payment entity, PaymentProcess should not blindly consider that the paid amount is the current order balance.

Created on 7 January 2025, 4 months ago

Hello,

I've noticed that the 'payment' step (PaymentProcess::buildPaneForm) is used in both this use cases:
1/ Onsite payment: the remote payment has not been created nor processed yet so it is created, processed and finally stored as a Commerce Payment entity.
2/ Offsite payment: the remote payment has already been processed and we now come from PaymentCheckoutController::returnPage so that we can create the corresponding Commerce Payment entity.

In both these use cases, PaymentProcess::buildPaneForm calls PaymentProcess::createPayment() which creates the Payment entity with the order balance as amount.

protected function createPayment(PaymentGatewayInterface $payment_gateway) {
    $payment_storage = $this->entityTypeManager->getStorage('commerce_payment');
    /** @var \Drupal\commerce_payment\Entity\PaymentInterface $payment */
    $payment = $payment_storage->create([
      'state' => 'new',
      'amount' => $this->order->getBalance(),
      'payment_gateway' => $payment_gateway->id(),
      'order_id' => $this->order->id(),
    ]);
    return $payment;
  }

Although it makes sense to retrieve the order balance when the remote payment has not been created yet, it is actually dangerous to blindly consider that the amount that has just been paid offsite is the current order balance. As a matter of fact, we've faced situations where the paid amount was different from the current order balance.

Maybe there should be some compulsory function in OffsitePaymentGatewayInterface such as "getRemotelyPaidAmount" that should allow for PaymentProcess to make sure the order balance equals the paid amount.

Until such a function exists, contrib modules providing OffsitePaymentGateways should be advised to make this verification in their own createPayment() function that is called a few lines afterwards:
$payment_gateway_plugin->createPayment($payment, $this->configuration['capture']);

📌 Task
Status

Active

Version

2.40

Component

Payment

Created by

🇫🇷France nicolas bouteille

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @nicolas bouteille
  • 🇮🇱Israel jsacksick

    I'm not really sure what you're suggesting but technically, in both cases, even for an offsite payment gateway, this code should be invoked prior to redirecting to the gateway meaning there no payment should have occurred yet?

    Also, it is the job of the payment gateway to register the payment with the right amount from onReturn(), so not really sure there is a bug to fix from Core here.

  • 🇫🇷France nicolas bouteille

    I see... well Commerce Stripe has implemented Payment Element which requires that the pane where the Stripe PaymentElement JS object is shown to the customer at the Review step. So maybe indeed they should not have done so.

  • 🇮🇱Israel jsacksick

    I know Tom (TomTech) is currently making changes to Commerce Stripe which are probably related to this... You probably should double check the work that is happening there.

  • 🇫🇷France nicolas bouteille

    I've opened an issue on Commerce Stripe so that they can explain why they needed to display the PaymentElement at Review step instead of Payment step. 🐛 Displaying PaymentElement at Review step might not be the way to go Active

Production build 0.71.5 2024