Payment method instance should be available for CALLBACK_commerce_payment_method_submit_form()

This actually seems quite reasonable to me. Perhaps we can get some other eyes on it, but I think this might solve some of the weird edge cases we've seen where an order needing payment somehow skips it. (Could be wrong.)

+++ b/modules/payment/includes/commerce_payment.checkout_pane.inc
@@ -140,9 +140,7 @@ function commerce_payment_pane_checkout_form($form, &$form_state, $checkout_pane
-    $method_info = $order->payment_methods[$pane_form['payment_method']['#default_value']];
-    $payment_method = commerce_payment_method_load($method_info['method_id']);
-    $payment_method['settings'] = $method_info['settings'];
+    $payment_method = commerce_payment_method_instance_load($pane_form['payment_method']['#default_value']);

So basically instead of retrieving the payment method from the order's data attribute and rebuilding settings, you're loading it fresh. Correct?

This would need testing alongside other payment gateway contributed projects. I'm not sure if there's an easy way, but I'd want to check out these modules http://contrib.drupalcommerce.org/payment-modules and see how they are checking this data attribute.

For example, Commerce PayPal EC uses it to see if it's enabled for the order (and not affected.)

return in_array('paypal_ec|commerce_payment_paypal_ec', array_keys($order->payment_methods));

Would need rszrama to brain dump anything on this :)

Ah, yes. On second thought you might be right. There are other instances in the form validate and submit funcs where it checks order data, so I do think it might be more important to use that from the beginning.

@zaporylie : Do you have a use case you can describe that brought you to create this patch? As @mglaman pointed out, we'd need some test coverage to ensure this works. I might suggest (correct me if I'm wrong) creating two payment methods and then using a condition only make one of them available to the order. If for some reason both methods are still available then I think we'll know this patch needs work. (At least, this makes sense in my head.)

zaporylie, would a use case be: a shop in the United States which supports a lot of sales in Canada. You have two Authorize.net accounts, one for each country. If an order is for a Canadian customer you need to use that gateway.

The current implementation makes discerning the above difficult, correct? In knowing which gateway is currently being used?

Just used this to debug issue mentioned in #7. This lets me identify the instance ID so I can tell if the Authorize.net USD or CAD method was used.

Nice! Should we write a test for that?

Can we get a look at how the variable differs pre / post patch? Also, are there any points of alteration for one that wouldn't apply to the other that could cause unexpected behavior?

I'm not sure this change is required to address the OP's issue (for example, you can derive the currently selected payment method even on the initial page load just by looking at the radios element's options array / default value), so if anything is substantively different, it would probably be better to avoid the change.

I reviewed the 2 variables and the only difference with the new implementation is that the instance_id is provided, everything else is literally identical.

I think this does help OP's issue as they are referring to 2 payment gateways that are the same and may otherwise have the same options.

Committed, thanks!