Fields altered onto the checkout form outside of panes don't prevent form submission when they fail validation

Can you print a dump of the $order variable right before that if statement? We'll need to figure out why you don't have a value set for payment method - I can't think of a reason in the core checkout form for that to happen.

Yep, you're right about it missing the payment_method key, so I'd just focus on figuring out why some users are progressing past the payment page without a payment method specified. It should be impossible, but one thing you can do to check is toggle on the payment pane's setting to ensure orders can't progress past the pane without selecting a payment method.

Maybe look to see if the $order->data is being overwritten prior to save such that the pane does set the payment method key but then somehow it's removed in a follow-up save.

I wouldn't expect the payment method key to be set until the Payment checkout pane on page 3 has submitted.

Works for me. If there's some problem with the core payment pane, I'd just as soon have this issue dangling around on the chance we could nail it down and prevent it in the future. Glad it's working in the meantime. : )

Aye aye, thanks for keeping it up to date. : )

Hi

I'm also experiencing this issue using Sagepay server (iframe) integration - thanks for posting up your findings. Its a very intermittent problem that i've only managed to reproduce 'by accident'. However our client has had reports of it on production.

Its a particularly dangerous bug because payment is skipped altogether.. meaning customers have been putting orders through for free! Perhaps in the code snippet in commerce_payment.checkout_pane.inc there should be more failsafe / checking to ensure this cant happen? Maybe a master variable for example flagging whether payment can be bypassed (which is useful for dev of course). Or even better, just check to see if order object has been malformed for whatever reason?

@RichieRampage - could you give any more info as to what was causing it in your case? We have made quite a few modifications to the checkout form like you were describing, which could be causing it

Many thanks

I just had an issue where Anonymous checkout was working flawlessly. However, an Authenticated user would skip the payment action all together. After 3-4 hours of trial and error, rules investigation, and face palming...

I have found that the checkout panes are to blame in my case. I have 'Billing Info' and 'Payment' (with 'require a payment..' checked) in the PAYMENT pane. Payment is completely skipped in this case. If I move those two items into REVIEW ORDER pane my problem is fixed.

Can we shed any light on the reason for this?

I have been trying to resolve this when related to commerce_stripe since I am not the only one with this issue, maybe it is related to something else. Here is my commerce_stripe issue for more information https://www.drupal.org/node/2530918

#14 suggests that the payment be in review, which is what I have done, but I am still experiencing this issue, and it only seems to happen for certain users. I require payment, but yet the payment still proceeds, just without processing, because no payment_method is attached to these orders. I don't know why this is the case, but since I have checked the box on Payment "Require a payment method at all times, preventing checkout if none is available." It should not go through. Anyone else able to figure this one out?

Hi,

I managed to fix this issue in the end, but it was stemming from a (rather obscure) conflict between Commerce and a custom module. Its difficult to understand the root cause myself, but maybe it will help @rszrama

Briefly, in my custom module was:

1) A definition of a custom checkout pane
2) Within the pane was a form with single checkbox (for accepting T&C's)
3) A validation function for the form, attached using
'#element_validate' => array('my_validation_function')

Now.. if the user was to alter the 'country' selector on the addressfield when checking out (causing ajax form actions to fire etc), then on submission the payment pages would skip and the order would go through for free..!

When i removed the '#element_validate' property and implemented via the standard FORM_ID_validate() convention, the problem went away.

So in conclusion... it seems like if some kind of validation error / misconfiguraton happens in a checkout pane, then it can cause odd behaviour such as payment pages skipping. This was clearly a convoluted scenario which took me a long time to reproduce and trace through (there were no errors in watchdog etc), but maybe it sheds some light for other people having the same problem

I think, in either case, Commerce should have more robust failsafe mechanisms to prevent this from happening

I've put in a temporary hacked fix until I can diagnose this problem. Please be aware that this may only mask the problem. It stops people from checking out if there is no payment method selected. I have not submitted it in patch format, because it's not a proper fix. Just a stop gap

Original (line 304 of commerce/modules/payment/includes/commerce_payment.checkout_pane.inc):

// If the payment method doesn't exist or does not require a redirect...
  if (!$payment_method || !$payment_method['offsite']) {
    if (!$payment_method) {
      $log = t('Customer skipped the Payment page because no payment was required.');
    }
    else {
      $log = t('Customer skipped the Payment page because payment was already submitted.');
    }

    // Advance the customer to the next step of the checkout process.
    commerce_payment_redirect_pane_next_page($order, $log);
    drupal_goto(commerce_checkout_order_uri($order));
  }

New (line 304 of commerce/modules/payment/includes/commerce_payment.checkout_pane.inc):

// If the payment method doesn't exist or does not require a redirect...
  if (!$payment_method || !$payment_method['offsite']) {
    if (!$payment_method) {
      $log = t('Customer skipped the Payment page because no payment was required.');
      $loglink = l("contact us", 'contact');
      $msg = "We could not process the checkout, please try again. If the problem persists please $loglink.";
      drupal_set_message($msg);
      drupal_goto("checkout");
    }
    else {
      $log = t('Customer skipped the Payment page because payment was already submitted.');
    }

    // Advance the customer to the next step of the checkout process.
    commerce_payment_redirect_pane_next_page($order, $log);
    drupal_goto(commerce_checkout_order_uri($order));
  }

I've had 2 sites report a similar issue recently. The log entry says "Customer skipped the Payment page because no payment was required." which means that the site was unable to load any payment methods for the order. Both sites use Auth.net and don't have any conditions on the payment rule.

Just like @rich.3po, both of these sites have a clause to accept before checking out. However in my case, I'm just adding a required checkbox field to the existing checkout pane. There are no additional checkout panes or validation/submit callbacks.

I'm unable to reproduce the issue, but I'll report back as soon as a I find the culprit.

The updated title points to the culprit: our error trapping code that allows us to display form errors inline with their checkout panes does not properly accommodate fields that exist outside of checkout panes. We need to ensure any leftover errors from our special handler are still displayed and prevent form submission.

Nice detective work Ryan :D

I also have this problem. Any updates on this?
I find this error very critical as it can result in items being shipped before payment unless we do a manual control every time.

Thank you.

Its good to see the root cause of this issue has been identified - nice one Ryan

Agree it should be higher priority so ramping this up...

I won't change it back down, but I actually wouldn't consider it a major issue since the fix is so easy: put your custom form element inside an existing checkout pane or create a new one. : )

I think it's major because it was a major pain in my ass :D ... Using standard FAPI techniques that should work for all of Drupal expose a big problem.

I'm still a little unclear (as i suspect others are too..?) as to what exactly triggers this bug.

When you say the issue is triggered by "fields that exist outside of checkout panes", could you give an example of this and how it should be done instead?

In my case, i was in fact defining my own checkout pane, with fields within it defined in BASE_checkout_form() (ie the standard method AFAIK), but i still got the issue until i removed the '#element_validate' key...

Hey @rich.3po, I think you should be using the hook to define your checkout pane validator instead of using #element_validate. The pane validator has to return TRUE or FALSE (unlike standard form validators)

See: http://api.drupalcommerce.org/api/Drupal%20Commerce/sites!all!modules!co...

Hopefully the fix for this issue will address your instance as well. Basically if any form element on the page fails validation it will prevent the form from being submitted and advancing to the next order status.

Have you seen the module Commerce Commonwealth.
They describe the exact same issue as here and have just released a module update.
I cannot say it is the same as I dont use the Commerce Commonwealth module, but it looks to be the same problem.
Read more here https://www.drupal.org/node/2542380

Nah, that's unrelated.

I believe something like this is the fix? The problem is we only loop over errors by pane_id, so anything outside a pane_id gets displayed, but doesn't trigger the rebuild to keep us on the same page. I just added a basic check in front to set form validation = false if there are ANY form errors, then all the pane specific stuff can continue below as normal.

We have this issue now and then with PayPal - I even created a Rule that tells people to email us if they make it to the "Checkout Complete" page without paying when they should have - so this is definitely in our wheelhouse. I'll give the patch a try. Thanks, @smccabe!

I can't say with 100% certainty but at this point we haven't had anyone skipping checkout since applying this patch. If that changes I will update the status again, but I think this is a good candidate for RTBC.

We applied it on our client's site and it fixed the issue.

However, it brought up a separate issue with Commerce Discount. :-(

#2618488: Commerce Discount line items cause errors in Commerce

Relating that issue as we'll need to track down what validation is failing.

@merauluka do you know what errors or a stack trace?

Attaching a minor reroll w/ comments on why it was important to check for global error messages. I'm going to hold off on committing until we're sure we can resolve the issue in Discount.

https://www.drupal.org/node/2661530

Fairly confident if we can resolve this commerce entity_unchanged load from calling refresh we will likely be in a better world;)

Discussed briefly with Ryan at midcamp. Will find andy and others to try and pickup the discussion.

Ok less confident on my last point. Will test tomorrow at sprint

I tried to reproduce the discount problem with the latest patch and was unable to, so I don't see currently that this patch should be held up. I'll wait for someone to provide more details as maybe I missed something in reproducing the discount validation issue.

I could not reproduce the related issue, and I'm pretty sure it's outdated now given the work we did in Commerce Discount 7.x-1.0-beta1 to fix issues with discount deletion / recreation and compatibility checking. As such, committing this one at long last. Sorry for the delay, everyone.