After going back into the browser history from a remote payment page, the order is finished without payment

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

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.

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.

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.

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

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.

(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.

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

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.

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.

@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.

This is blocked on #2353383: Branch test failures.

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

#2350709: Provide Rules actions to move the checkout panes has been fixed now.

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).

I've tried a lot with rules, but it seems rules alone can't fix the strange 'back' behavior.

Due to the way the commerce payment checkout page is setup, going back isn't allowed because
"buttons" is set to FALSE. (Why?)

function commerce_payment_commerce_checkout_page_info() {
  $checkout_pages = array();

  $checkout_pages['payment'] = array(
    'title' => t('Payment'),
    'help' => t('Use the button below to proceed to the payment server.'),
    'status_cart' => FALSE,
    'locked' => TRUE,
    'buttons' => FALSE,
    'weight' => 20,
  );

  return $checkout_pages;
}

You can change this behavior as follows:

/**
 * Enable going back from the external payment providers page.
 *
 * Implements hook_commerce_checkout_page_info_alter().
 *
 * @param $checkout_pages
 */
function your_module_commerce_checkout_page_info_alter(&$checkout_pages) {
  if (isset($checkout_pages['payment'])) {
    $checkout_pages['payment']['buttons'] = true;
  }
}

In combination with the patch in #28 this seems to fix the problem.
Note that in that patch I don't redirect to the previous page on failure. But rather set the order status back to the checkout page if the payment is not new or successful. This give a consistent behavior in our case, but should maybe be a setting because we can't be sure about everybody's flow. Using commerce_payment_redirect_pane_previous_page() however not always results in what you might expect.

(Patch against 1.7, but could work against 2.x (line number mismatch).)

This one makes sure that Basic Payment Methods still work (payments which don't need any additional processing).

And this one prevents calling commerce_checkout_complete() twice.

After reading all of this, I think I have a fairly clear understanding of what is going wrong in my commerce site, but to come up with a reasonable fix I have one very specific question:

In what circumstance / use case _exactly_ does it make sense for payment_commerce_payment_status_change to trigger a commerce_payment_redirect_pane_next_page, thereby presumably transitioning the order to the next state if the "status change" of the payment was from status X to status X (i.e. no actual status change at all)?

Seing as the Payment class (payment.classes.inc in payment module) while still triggering the payment_status_change hook, makes sure to not track duplicate status changes

// Prevent duplicate status items.
if ($this->getStatus()->status != $status_item->status) {
  $this->statuses[] = $status_item;
}

I don't see why payment_commerce couldn't/shouldn't do the same thing (that's what my attached patch does).

Unfortunatelly neither patch #30 nor patch #29 solved this issue for us.
Here's a patch that solved the problem for us. Hopefully for you as well.

@valentin schmid: from my (admittedly limited) understanding of the module, the logic in your code seems kinda ... strange. Do you know what combination of previous/current payment status is causing you trouble? From your code I'd have to guess that your payment is transitioning _from_ a failed state into something else. If that is true, then I would like to know what mechanism triggers that payment status change and why.

I also face the same issue in my installation.
Thanks

Hello. My scenario is this: After clicking the cancel button on PayPal, I get correctly sent back to the review page. If I click the continue button again though (to go PayPal again), all order statuses are set to pending which finishes the order before I have finished the payment.

Patch #31 works great, it takes me back to the previous stage (review) and no orders are finished no matter how often I cancel the payment process. Awesome, thanks!

Patch #29 also fixes the finished order issue, but takes me back to the checkout page (I'd prefer going to the last stage/review though).

Patch #30 doesn't change anything for me.

My modules:
payment 7.x-1.17
paypal_payment 7.x-1.4
payment_commerce 7.x-1.7

Changing priority to critical

Encountered the same issue (using payment + payment_commerce + mollie).

#31 seems to fix the issue

31 did not solve the issue, testing #29, will report back