In Slack:

> basically, we always rely on the admin to do the manual update
> your order is in validation, you have a payment, you mark it as complete

We should document on these methods that it is not their responsibility to do anything to the order such as cancel it or set it to complete.

Store admins should manually update the order when they see the payment has come in.

CommentFileSizeAuthor
#6 2930789-5.patch2.4 KBbojanz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

casey’s picture

casey CreditAttribution: casey at SWIS commented

Note that if this is the case (that payment gateways should not update the order status), that #2934647: The order can stay unplaced after an offsite payment.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Assigned: Unassigned » bojanz

#2934647: The order can stay unplaced after an offsite payment has landed, let's add those docs now.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Title: add docs to SupportsNotificationsInterface::onNotify() and OffsitePaymentGatewayInterface::onReturn() to explain orders don't need their state changing » Expand code documentation for off-site payment gateways

Now that #3009962: Invalid step ID "" passed to redirectToStep() in PaymentCheckoutController->returnPage is in, we also need to document that onReturn() will be skipped if onNotify() resulted in a placed order.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Active » Needs review

Here's a patch.

And here's the entire OffsitePaymentGatewayInterface docblock for context:

 * Defines the base interface for off-site payment gateways.
 *
 * Off-site payment flow:
 * 1) Customer hits the "payment" checkout step.
 * 2) The PaymentProcess checkout pane shows the "offsite-payment" plugin form.
 * 3) The plugin form performs a redirect or shows an iFrame.
 * 4) The customer provides their payment details to the payment provider.
 * 5) The payment provider redirects the customer back to the return url.
 * 6) A payment is created in either onReturn() or onNotify().
 *
 * If the payment provider supports asynchronous notifications (IPNs), then
 * creating the payment in onNotify() is preferred, since it is guaranteed to
 * be called even if the customer does not return to the site.
 *
 * Note that onReturn() will be skipped if onNotify() was called before the
 * customer returned to the site, completing the payment process and
 * placing the order.
 *
 * If the customer declines to provide their payment details, and cancels
 * the payment at the payment provider, they will be redirected back to the
 * cancel url.
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
FileSize
2930789-5.patch2.4 KB

Now with 100% more patch!

zaporylie’s picture

zaporylie
he/him
Primary language Polish
CreditAttribution: zaporylie at Linkfactory A/S commented 
+++ b/modules/payment/src/Plugin/Commerce/PaymentGateway/OffsitePaymentGatewayInterface.php
@@ -40,6 +44,11 @@ interface OffsitePaymentGatewayInterface extends PaymentGatewayInterface, Suppor
+   * the parent order should not be touched. The order state is updated

The parent order should not be touched whatsoever or it should not be placed/transition should not be fired?

I'm wondering if it is strictly forbidden for payments to modify the order, even if only to set some metadata on the order itself.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented

I'll change "should not be touched" to "does not need to be touched", since it's not forbidden.

  • bojanz committed 7d8ab84 on 8.x-2.x 
    Issue #2930789 by bojanz, joachim, zaporylie: Expand code documentation...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Fixed

Fixed #8 and committed. Thanks, everyone.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.