Problem

In the current situation payment methods are responsible for influencing the checkout process. First of all, this is an unnecessary dependency as payment methods should be responsible for managing order transactions only. The checkout API should respond to changes in an order's transactions itself by moving the checkout to another page, for instance.
Two very real issues that this dependency is responsible for:

  1. Because payment methods do not get any context information, they will have to move the checkout page even when they are executed from the terminal. No checkout process exists at this point, so moving the page is pointless.
  2. If a payment method receives multiple feedback multiple times, it cannot move the checkout page multiple times, because commerce_payment_redirect_pane_next_page() and commerce_payment_redirect_pane_previous_page() only let you do that if the checkout is still in the payment phase.
    Say that in situation A a payment has been put on hold and the status won't change for a day. In this case the checkout may need to move forward in expectation of the payment to be completed. You can't go back and start a new payment, or you might end up with both payments being succesful.
    In situation B the payment is put on hold as well, but set to failed a split second after that.
    When one payment status is received, modules can only act on that status and change the transaction and/or checkout based on that one single status. They won't know if another status is to follow shortly after it, but if it does, they must also be able to respond to that second status, which is not possible right now as the functions to move the checkout page only let you move the page while you're still in the payment phase (which you can get out of by a single call to either of them).

Proposed solution

Make Commerce Payment responsible for moving the checkout process based on the order's transactions instead.

Comments

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Title: Do require that payment methods influence the checkout process » Do not require that payment methods influence the checkout process

.

rszrama’s picture

rszrama CreditAttribution: rszrama commented
Category: bug » task

We can't trust that a payment notification will be posted to us before the customer returns from a redirected payment service, nor do we know based on the transaction type (for example, an authorization only) how to move the order through checkout.

Right now the only place Commerce Payment requires a payment method to influence the checkout process is when an IPN is received and it isn't guaranteed that the customer will be returning from the remote payment service, such as with PayPal Payments Standard. This payment method needs to signal that the order should proceed to checkout completion in the event the customer never returns, and it isn't clear exactly what Commerce Payment should be looking for here.

I'm happy to tease this out further if you think there's some way to communicate an expectation and a delivery based on that expectation w/ respect to payment transactions, but I don't see the existing behavior as buggy - more a concession to these types of "return optional" redirected payment services. Others needn't worry about this - for example, PayPal Express Checkout will always result in the customer coming back to your site even though payment happens on PayPal, so I don't have to use the redirect functions.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

We can't trust that a payment notification will be posted to us before the customer returns from a redirected payment service, nor do we know based on the transaction type (for example, an authorization only) how to move the order through checkout.

Yes we can. Payment does that. The moment a payment is executed, it has an initial status. The same could be done for order transactions.

This payment method needs to signal that the order should proceed to checkout completion in the event the customer never returns, and it isn't clear exactly what Commerce Payment should be looking for here.

I think the solution lies in moving the logic to move the checkout from the payment method plugins to Commerce Payment. The moment the payment checkout pane is requested, it checks if the pane should be displayed. If not, move it based on the latest known payment status and possibly Rules configuration. This way, the decision to move the checkout or not is postponed until the last possible moment, maximizing the chance of making the right move (pun intended).

rszrama’s picture

rszrama CreditAttribution: rszrama commented

Pun appreciated. : )

I'll give it some thought. I typically prefer explicit behaviors, but if we can simplify it for payment gateway integrators while maintaining the current flexibility, I'm happy to give it a shot.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

Another POV is that the payment and checkout APIs work together, but that plugins of one API should not have to deal with (plugins of) the other. IMHO the coupling between all the subsystems is just too tight. This makes it harder to program for, which becomes very apparent when unit testing (which should at least be done for the D8 branch). If the payment API acts as a layer between payment method plugins and orders and checkouts, APIs will only have to communicate with the ones closest to them. Less dependencies, less complexity, more flexibility, and easier testing and QA.

We should definitely look into this for 8.x-2.x, if only because the whole dependency issue will make unit testing a horror story. For 7.x-1.x this behavior may be added as a configuration setting for payment method plugins, with the default being the 'old' behavior.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Issue summary: View changes

Added proposed solution.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

This problem keeps sticking its head up when going back in the browser history from a remote payment page, for instance. See #2118575: After going back into the browser history from a remote payment page, the order is finished without payment. This can be simplified to when a payment's first status after it has become pending, is its final status OR when it is not the final status (only one of these can be catered for).

The different problems are all discussed in #1982544: Call commerce_payment_redirect_pane_next/previous_page() in hook_payment_status_change().

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Active » Closed (won't fix)

We fixed this in D8 (gateways don't touch the order at all). It's most likely too late for D7 (especially considering how thin the D7 test coverage is).

I am closing this issue, to be reopened if we get an unexpected sponsorship or wave of enthusiasm for navigating the BC-breaking-waters-of-doom.