See #13 / #15 for more detail.

Original report:

PayPal for payment is used in combination with payment commerce for a large drupal commerce webshop.

When the cancel link is used on the paypal website, the order will get the status finished and the client is redirected to the order completed confirmation page.

The client should be redirected to the review page of the current order.

The order has the status payment when the client is forwarded to PayPal.
When returning, the order gets the status finished.

I have done a lot of searching true the code but unfortunately didn't found the issue.

The finish method is called and after that a lot of things happen.
It looks like there is something going wrong in the commerce payment module (I have an almost similar issue with iDeal).

Files: 
CommentFileSizeAuthor
#17 payment_commerce_2118575_17.patch1.03 KBXano
FAILED: [[SimpleTest]]: [MySQL] 67 pass(es), 3 fail(s), and 2 exception(s).
[ View ]

Comments

Xano’s picture

Status:Active» Postponed (maintainer needs more info)

Are you using Payment for Drupal Commerce 7.x-1.4?

mvdve’s picture

Status:Postponed (maintainer needs more info)» Active

Indeed, the payment for commerce 7.x-1.4 is used.

It looks like there is an extra check needed on the status of the payment item.
Totally off topic but an other example is with the iDeal module:
When the back button of the browser is used, when the customer is in the iDeal environment and they get back at the checkout page.
The order status is updated to finished after a refresh of the browser.
This shouldn't be possible as the payment status is still "pending".

Xano’s picture

Title:When the PayPal transaction is cancelled, the order gets finished» After going back into the browser history from a remote payment page, the order is finished without payment
Project:PayPal for Payment» Payment for Drupal Commerce
Version:7.x-1.0» 7.x-1.x-dev

Can you please provide steps to reproduce the problem on a completely new and clean installation of Drupal with the latest dev versions of core and the required modules?

This does sound like #2118675: Do not require that payment methods influence the checkout process, though, which is a fundamental problem in Drupal Commerce.

mvdve’s picture

It will be a lot of work to get a fully fresh commerce system up and running but I will try to make time for this.

mvdve’s picture

Just to be sure, where are talking about two scenarios/issues:

payment iDeal V3:
- When the payment cancel link in the ideal environment is used, the client is redirected back to the review order (with payment select) page. This works as it should be.
- When a previous link from the history is selected (back button) the client lands on the review order page, but ajax keeps loading endlessly. When refreshing the page twice (sometimes ones) the order is spontaneously finished.

payment PayPal:
- When the payment cancel link in the PayPal environment is selected, the client is redirected to the shop (always), and the order is instantly finished.
- The history link issue is the same as with iDeal.

I will do more research tomorrow.

Xano’s picture

The only AJAX when making iDEAL payments is from Commerce.

Note that this is probably about completed checkouts and no torders that are fully paid for. Your site config should accommodate for that.

mvdve’s picture

Your correct on the Ajax issue. That's a commerce thing and has nothing to do with this one.

Back on this issue: I did a lot of testing, with my current installation and a installation with all de dev modules.
Unfortunately the issue still exists on the installation with dev modules.

The where and how did got much clearer. The error occurs when:
1 - A order is created -> goto checkout pane
2 - The customer fills in there billing and shipping information -> goto shipping pane
3 - The customer selects a shipping service -> goto preview pane
4 - The customer selects the payment method -> forwarded to the external payment service (iDeal or PayPal. the same error occurs on both).
5 - When the review pane is loaded (http://www.sitename.com/checkout/[order-number]/review) by hitting the back button of the browser, or selecting the link from the history, the customer gets forwarded again tot the payment service.

The error
6 - When repeating step 5, the customer is not redirected to the payment service again, but to the checkout-complete page.

I did a cross-check with the commerce_paypal module. This module does not have this behaviour. It keeps forwarding the customer to the payment service.

mvdve’s picture

Additional information the post #7:

On step 5, the customer is forwarded to the payment service. At the same time, the status of the order is set to processing and the checkout is completed (e-mail is send to the customer by the checkout complete event in rules).

1 - A order is created -> goto checkout pane. Order status: shopping cart
2 - The customer fills in there billing and shipping information -> goto shipping pane. Order status: Checkout: Shipping
3 - The customer selects a shipping service -> goto preview pane. Order status: Checkout: Review
4 - The customer selects the payment method -> forwarded to the external payment service (iDeal or PayPal. the same error occurs on both). Order status: Checkout: Payment
5 - When the review pane is loaded (http://www.sitename.com/checkout/[order-number]/review) by hitting the back button of the browser, or selecting the link from the history, the customer gets forwarded again tot the payment service.
Order status: Processing (no payment so the order is not set to complete).

6 - When repeating step 5, the customer is not redirected to the payment service again, but to the checkout-complete page.

mvdve’s picture

Another update on this issue after some code searching:

When the customer is forwarded to the PayPal environment, the status of the payment is set to pending instead of new.
Now, when the review pane is loaded again, the payment_commerce_payment_status_change() function sets the order status to the next pane (line 98 of payment_commerce.module).

This is why the checkout gets completed but the customer is still in the external payment environment.

Xano’s picture

That's not what happens. If you look closely, line 98 explicitly prevents moving the checkout pane if the old status was PAYMENT_STATUS_NEW and the new status is PAYMENT_STATUS_PENDING.

mvdve’s picture

I'm sorry. I was not clear in my explanation.

When the customer is forwarded to the external payment service for the first time, the payment status is directly set to pending.
The status new is skipped.

This is a very big assumption but should is be like this?
- Customer is forwarded to external service -> payment status = new
- Customer completes payment -> payment status is pending or completed when fulfilled.

Xano’s picture

See Payment::__construct(). New means new. Pending means pending, and not completed.

mvdve’s picture

As discussed on IRC:

When the customer is forwarded to the external payment service for the first time, the payment status is set to pending.
When the review page is now reloaded the if statement on line 98 gets "pending" as previous and current status.

Now the payment_commerce_redirect_pane() gets executed and the order is set to complete.

Not sure how to catch this one.

For example, when a customers chooses to pay with a delayed payment (with PayPal) the previous and current statuses are also "pending" when completing the payment. This would result in an error when the customer is redirected back to the shop.

Xano’s picture

This looks like it's caused by Commerce's limited ability to deal with checkout status changes. #2118675: Do not require that payment methods influence the checkout process tries to address this. #1982544: Call commerce_payment_redirect_pane_next/previous_page() in hook_payment_status_change() is when we decided to let Payment for Drupal Commerce do what it does now.

roderik’s picture

Issue summary:View changes

(Note: I'm on purpose not setting 'needs review', because this is not a general patch. This is only meant
* as a detailed summary of what I encountered in the code flow
* to give people who encounter this issue and are OK with coding / applying patches, a hint, where to look.)

Thanks for this discussion; it gives some perspective on why this bug is still here. I traced the code last august to 'when/where this happens', but still had no idea 'how to fix it'.

'when/where it happens':

(mvde/#13 also said this, but in more detail):
When a user presses the browser "back" button, it goes back to checkout/N/PREVIOUS_PANE. commerce_checkout_router() says "Oh no you don't! Your cart is locked!" and drupal_goto()'s back to checkout/N/payment. What happens now, depends on my browser(!):

* e.g. on my Chrome/MacOS, the browser remembers that checkout/N/payment actually redirected to [exernal-payment-page] last time, so it sends you there directly. So nothing bad happens (except if you press your browser "back" button enough to get out of the checkout pages... you're back on the website, with an empty cart. Does not have anything to do with this issue.)

* e.g. on my Safari/MacOS and on someone else's Chrome/Windows, a HTTP request goes out to checkout/N/payment. This will redirect back to [exernal-payment-page], but before doing that, it changes the order status**! (Meaning that the next time the user will press back/visit a checkout page, they'll be redirected to the "order completed" page.)

** hook_payment_status_change() is called when the payment status changes from PAYMENT_STATUS_PENDING to PAYMENT_STATUS_PENDING. Even though there is no status change, payment_commerce_redirect_pane() is still called, which changes the order status to 'completed' and calls rules_invoke_all('commerce_checkout_complete').

'how to fix it':

I don't know a general solution because, as I get from this discussion + talking IRL to Xano,
- a non-change from PAYMENT_STATUS_PENDING to PAYMENT_STATUS_PENDING should still invoke hook_payment_status_change().
- Payment cannot know when (not) to advance and this should be handled by Commerce. (I don't get 100% of the reasoning but I take this as a given :) )

@#12:

For example, when a customers chooses to pay with a delayed payment (with PayPal) the previous and current statuses are also "pending" when completing the payment. This would result in an error when the customer is redirected back to the shop.

Just a remark: as an outsider, this sounds to me like there should be different statuses for this, since 'set to "pending" initially by Payment module' and 'confirmed by PayPal as being recorded in their system, but still "pending"' are two slightly different things. Am I wrong?

'how to hack it':

OK, so we can't make a general fix, but your website needs a fix now because this is a major issue:

I 'fixed' it by preventing a 'change from pending to pending' from calling payment_commerce_redirect_pane(), as in the following patch. The issue with this is: it hardcodes the machine name for the payment method (in my case 'ideal') so it's not general code.

roderik’s picture

StatusFileSize
new1.12 KB

After all that typing I forgot to attach the patch...

Xano’s picture

Status:Active» Needs review
StatusFileSize
new1.03 KB
FAILED: [[SimpleTest]]: [MySQL] 67 pass(es), 3 fail(s), and 2 exception(s).
[ View ]

Roderik and discussed this problem in more detail at DrupalCon Amsterdam and we believe the problem here may be caused by the fact that Payment for Drupal Commerce should only move the checkout when the payment has been successfully completed. The attached patch shows this.

Status:Needs review» Needs work

The last submitted patch, 17: payment_commerce_2118575_17.patch, failed testing.

mvdve’s picture

Correct me if i am wrong but what happens when the consumer cancels the payment in the external environment? The returned status will be cancelled, but the order payment status is set to payment. In this case a checkout redirect to the previous state is needed.

Xano’s picture

I talked to @bojanz (one of the Drupal Commerce maintainers) this morning and he confirmed that the way payment methods have to manipulate the checkout process is completely messed up.

In short: there may not always be one single answer to this particular problem.

What I'm thinking about doing is allowing Payment for Drupal Commerce users to manipulate the checkout process using Rules, so they can choose the configuration that best fits their particular needs.

Xano’s picture

Xano’s picture

@roderik and I discussed the Rules solution and concluded that it's probably best to create one rule per payment method, because different methods need different behavior. When using different rules, bridge modules can also add or alter the behavior for a specific payment method.

Xano’s picture

This is blocked on #2353383: Branch test failures.

Xano’s picture

Version:7.x-1.x-dev» 7.x-2.x-dev

Moving this to the newly created 7.x-2.x branch, so we do not have to worry about backwards compatibility breaks.

Xano’s picture

lmeurs’s picture

We are also experiencing order statuses getting set to "Completed" after a user leaves the payment provider's site using the browser's back button. I tried the following rule which seems to work, but has two problems:

  1. The rule is also being fired on successful payments. This causes a completed order's status to be set to "Checkout: overview" again.
  2. Our payment provider OmniKassa already registered the transaction's ID, so on the next attempt the transaction is being refused.
{ "rules_prevent_back_button_changing_order_status" : {
    "LABEL" : "Prevent back button on checkout from changing order status to completed",
    "PLUGIN" : "reaction rule",
    "OWNER" : "rules",
    "REQUIRES" : [ "rules", "commerce_payment", "entity" ],
    "ON" : { "commerce_payment_transaction_update" : [] },
    "IF" : [
      { "data_is" : {
          "data" : [ "commerce-payment-transaction-unchanged:status" ],
          "value" : "pending"
        }
      },
      { "data_is" : {
          "data" : [ "commerce-payment-transaction:status" ],
          "value" : "pending"
        }
      }
    ],
    "DO" : [
      { "commerce_payment_redirect_pane_previous_page" : { "commerce_order" : [ "commerce-payment-transaction:order" ] } }
    ]
  }
}

Can someone tell me the correct way to prevent the order's status from changing?
Can this solely be done by Rules or is the patch from #17 still required (which seems to be to no avail in this case)?

The great thing about this method is that the order is not being completed and products remain in the cart so the user can easily continue the checkout process.

We are using Commerce 7.x-1.11, Payment 7.x-1.x-dev, Payment Commerce 7.x-1.7 and OmniKassa (the only payment provider for this site).