Checkout "Set as your default card" does not set default for NEW cards

Noticing this issue as well.

I assume it's because it's trying to save the value on a card which doesn't exist yet.

Additionally if you manually called commerce_cardonfile_save($card) and set the default flag on that card, I doubt it would unset the others as defaults (and it should).

You're going to need something like a presave or postsave hook to deal with something like this. Either way I'm noticing that it's not working.

@recrit when I was developing the Amex Payment Gateway with your sandbox I set the default card in the Amex module and didn't realise it could have been set another way.

It would be nice if CoF handled this but it's seems slightly convoluted without the static (which as you said recrit feels awkward ). Seeing as this is setting the "instance default" should we consider it fair game to require gateways to set this and document it somewhere?

I was wondering if there are any more thoughts on this regarding CoF handling this or gateways handling this? I'm going to be working on creating a patch for this for a site I'm currently working on.

I believe, if users sets default card in CoF, it now sets it in CoF. It's up to you, to tell your credit card processor, what the customers new default card should be though.

Double check and confirm defaults are getting set though. I perhaps patched this myself, but I assume it was done in dev and this issue was not updated.

I reviewed the patch, and it is working as designed.

This patch is very helpful. Thank you!

Thank you, everything is working well with the patch.

I tested the patch and it does set instance_default as described for new cards that are the first of their payment instance. This fixes the problem of having no default cards to charge for recurring entities as mentioned above.

However, if:

1. multiple cards on file are allowed
2. the user creates a new card of the same instance during checkout
3. user checks the box to make the new card their default

the new card is NOT set as default.

In this case commerce_cardonfile_set_default_card() still never gets called (which would set the instance_default on the new card and clear it on the old default card) because when the commerce_cardonfile_commerce_checkout_form_submit() callback runs,
$form_state['values']['commerce_payment']['payment_details']['cardonfile'] = 'new'

In the payment method's submit callback (CALLBACK_commerce_payment_method_submit_form_submit()), none of the values are passed to the callback by reference with the exception of $order, so I'm not sure how you'd get the $card_data->card_id back to $form_state from there anyway for the cardonfile submit handler to use it.

Can confirm #10.

Invoking hook_entity_presave() with code modified from #6 sets the first card of the payment instance as default.

/**
 * Implements hook_entity_insert()
 */
function [module_name]_entity_presave($entity, $type) {
  // If there is no other card on file for this payment instance, save it as the instance default.
  if ($type == 'commerce_cardonfile') {
    if (empty($entity->card_id) && empty($entity->instance_default) && $entity->payment_method == 'commerce_stripe') {
      if (!empty($entity->uid) && !empty($entity->instance_id)) {
        if (!commerce_cardonfile_load_multiple_by_uid($entity->uid, $entity->instance_id)) {
          $entity->instance_default = 1;
        }
      }
    }
  }
}

The module properly handles setting existing cards to default during checkout.

The outstanding issue is setting a *new* card as default when an existing card is default.

The module properly handles setting existing cards to default during checkout.

The outstanding issue is setting a *new* card as default when an existing card is default.

I'm not seeing that as-is. First card added to the account is *not* set as default, even though that is what is indicated in checkout (as default; in fact, cannot be changed by user). See comment #10, etc.

Re: #12, yes that's right, out of the box the first card added is not set as default. One of the code snippets in this thread is required to resolve that.

The outstanding issue raised in #10 and #11 is not being able to set a new card as default when one or more cards of the same payment instance are already on file.

Hi, I'm running into this same issue with first card not being set as default. I tried applying patch to stable beta and dev versions and same issue still exists. Any thoughts?

This is a blocker for me on this module as commerce recurring requires a card to be set as default in order to know which card to process in a recurring transaction... I'm currently stuck back at beta1 due to this problem and am afraid to update due to comments such as #14 as I do have it working just fine right now. Of course I would like some of the added functionality since beta1...

Ok, here's a new patch that should apply. I made one fairly significant change though. Now any new card added to a user's account will be set as their default card by default when the option on the admin page to "Set newly added cards as the user's default card." is checked. With the patch in #6 it was ensuring that the new card was only set as default if the user didn't have any existing cards.

Why I made that change.
#11 and 12 talk about cards being set to default during checkout. From my experience and looking over the code, the only way I could find that a card was set as default is in the user's card management OR when an already existing card is selected during checkout. Whenever a new card is added it is NOT set as the default card as far as I can tell from looking over the code. This was causing problems for us on our site as people would have recurring payments going, then their subscription would expire and they would go back in to subscribe with a different card, but recurring payments would still try to process using the default card which was still set as the old card. So I made this change in the patch. There may be a better way to accomplish this, but as far as I can tell this is the best way for now.

I appreciate your input and review.

Anybody willing to review/commit this? It's a pretty simple patch, but it makes a HUGE difference. Especially for those of us using commerce_recurring.

I'm using this on 2 production sites now, without it this module is broken for our use case but with it everything works great.

Re: #16 I'm having the same issues and need to get this resolved. Should I apply both patch #6 and #16 or will #16 do the trick?

Thanks!

#16 should be all that you need to apply to solve this specific issue. If that's not the case please let us know and mark this as "needs work". If it does work for you please mark RTBC.

I appreciate it. I just applied the patch and won't know for a few weeks since that's when the next recurring payment is due. But it seems like the same issues as this thread were keeping charges from happening. Will update you as soon as I know. Thanks!

In #16 the line

$card_data->instance_default = 1;

should actually be

$card->instance_default = 1;

because there's no other instance of $card_data in that function or anywhere else in the file.

I can confirm the patch in #16 works after making the change to it in #21.

In addition, the patch resolves #2577815: Recurring Charges Failed - Hard Decline because recurring billing doesn't work if there's no default card.

Yep, tested #16 against 7.x-2.0-beta5 with the suggested change in #21. Thanks @deggersten and @LordPachelbel for patches/suggestions and, actually, all the contributors on this thread for their constructive/sane/helpful comments.

An updated patch, containing the suggested var name change, attached.

The current (unpatched) behaviour is very non-intuitive and positively unhelpful for anyone with recurring orders that are supposed to be re-charged using CoF data. (Is/was there any reason why the first card should not be set as the default, by default?)

I too arrived here after discovering during testing that a client's recurring orders will fail (CBL not CR, but I guess the trigger is irrelevant), despite CoF data being captured during the initial order.

Looks good to me! Would love to see this committed.

Setting to RTBC. Hopefully this can get committed soon.

This didn't make it into 7.x-2.0-beta6 but the patch from #23 can still be applied. (I had to apply updates to the site this morning so had a go!)

this no longer applies.

Rerolled patch for most recent dev.

Looks like there was something wrong with my last patch. This one should apply as expected to dev.