0

I'm trying to implement the Commerce Ingenico payment gateway for a Drupal 8 Commerce site. As this module is still in unfinished for Drupal 8, I'm fixing it up as I go. Most of the implementation is located in Composer packages, so it's mostly plumbing.

I'm hitting a bit of a wall now though. I'm using the off-site payment gateway, and I've configured the IPN (server to server notification). The values for the payment in the database look good*, but the Cart never turns into an Order. Somehow the trigger for signaling a completed payment to the order is missing.

The results is that after returning from the offsite payment (and the onNotify has already been triggered, so the payment is marked as completed), I end up back at the start of the payment flow; the order summary.

How are these parts supposed to fit together?

Here's the code to the onNotify method in the PaymentGateway:

public function onNotify(Request $request) {
    parent::onNotify($request);

    // Common response processing for both redirect back and async notification.
    $payment = $this->processFeedback($request);

    // Let's also update payment state here - it's safer doing it from received
    // asynchronous notification rather than from the redirect back from the
    // off-site redirect.
    $state = $request->query->get('STATUS') == PaymentResponse::STATUS_AUTHORISED ? 'authorization' : 'completed';
    $payment->setState($state);
    $payment->setAuthorizedTime(REQUEST_TIME);
    if ($request->query->get('STATUS') != PaymentResponse::STATUS_AUTHORISED) {
      // setCapturedTime has been relegated to the mists of history
      $payment->setCompletedTime(REQUEST_TIME);
      // $payment->setCapturedTime(REQUEST_TIME);
    }
    $payment->save();
  }

And this is the resultant row in the commerce_payment table:

 payment_id
13

type
payment_default

uuid
payment_default

payment_gateway
ogone_offsite

payment_gateway_mode
test

payment_method
NULL

order_id
29

remote_id
3039111240

remote_state
9

amount__number
26.400000

amount__currency_code
EUR

refunded_amount__number
0.000000

refunded_amount__currency_code
EUR

state
completed

authorized
1534516744

expires
0

completed
1534516744

test
NULL

captured
NULL
  • Of course I don't know what these values are supposed to look like. I'm actually a little concerned about payment_method being NULL in this row. Is that normal for an offsite payment gateway, or should that be set to something?
  • I haven't worked with commerce on drupal 8 but typically on drupal 7 there'd be a rule fired when the payment's made to check if it matched the order total and set the order status accordingly. Does that help at all? – Alfred Armstrong Aug 17 '18 at 15:09
  • Rules isn't even a dependency for the new version anymore. I think it's all in the code now, and if I'm really unlucky obfuscated through lots of DI and services. – Menno van den Heuvel Aug 17 '18 at 15:21
  • I was never a fan of the Rules integration but it did mean some of the most critical logic could be tweaked without coding. Seems like a step back if that's not accessible any more. – Alfred Armstrong Aug 17 '18 at 15:25
  • You might be looking for EventSubscribers/TransitionStates. – Kevin Aug 17 '18 at 20:52
  • Try doing it exactly how the OnReturn method does it in payment_example OffsiteRedirect.php which uses a create method. – No Sssweat Aug 17 '18 at 22:52
1
+50

A similar scenario has been discussed on #commerce Slack on Monday. In contrast to onReturn() no transition is triggered after onNotify().

@bojanz says:

it's a safeguard we'll need to add to Commerce, but won't have the resources to do so before october

once we have $order->balance and an order paid in full event, we can use it to auto-place the order

at least those two parts could land in september for 2.10


TL;DR

onReturn() must create a Payment, as suggested in a comment from @No Sssweat. In case PaymentGatewayException is thrown Commerce will redirect to the previous checkout step. onNotify() can register info about payment but, at the current state, it won't place the order for you.

  • I have to continue using onNotify, as onReturn has proven to be unreliable; customers tend to close the tab after paying, and before returning. This is the exact scenario onNotify was created to avoid. For the time being I manually trigger the transition after verifying payment success in onNotify. Would you say this is the best solution in the current state of things? – Menno van den Heuvel Sep 6 '18 at 7:07
  • I must say no - too much logic in onNotify(). I understand the reasons why onReturn() is unreliable, I did experience it myself for some payment gateways. Nevertheless, I would leave onNotify() without an extra bit of code that does order transition, so once Commerce adds "order paid in full" event you'll be covered, and onReturn() will still do its job. In the same time I'll probably implement some custom safeguard, such as hook_cron to determine orders fully paid that haven't been * placed * and run the transition. – zaporylie Sep 6 '18 at 17:33
  • FYI In the past I also had issues with Payment providers that pushed notification only once, not logging failure or anything, and they even admitted that their delivery rate is quite poor. In addition there was no way to secure webhook endpoint on Drupal side because they were using random(ish) IPs, couldn't deliver hmac, token or anything. I ended up pulling status every time I got any request on webhook. I'm saying that just to highlight that one cannot always rely on onNotify() either. – zaporylie Sep 6 '18 at 17:37
  • This particular provider I can rely on: it will push multiple notifications if an error occurs, and the whole thing is signed with a private key. I'll contact the original developers and see if they want my fixes. If so I'll hook into cron for your suggested method. – Menno van den Heuvel Sep 7 '18 at 7:20
  • I'm not sure why they haven't just used the transitions on the Payment object as well. Could the order then not just subscribe to payment transitions and reliably finish up orders? I'm sure there's a reason - there always is. – Menno van den Heuvel Sep 7 '18 at 7:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.