I am using the function commerce_cardonfile_set_default_card to set a card as the default.

It sets the value properly but does not unset the default flag on any existing default cards - therefore you can end up with multiple default cards.

Comments

dwkitchen’s picture

Assigned: Unassigned » dwkitchen
Status: Active » Needs work
dwkitchen’s picture

Assigned: dwkitchen » Unassigned
Status: Needs work » Needs review

The set default function now unsets the other cards of the same payment method instance this is best designed to work on sites where there is only one payment method instance that is enabled for card on file.

j0rd’s picture

Status: Needs review » Needs work

I've tested this, and it "works" for default workflow.

Problem is I don't think the logic is in the right place, as currently commerce_cardonfile_set_default_card() needs to get called explicitly at each entry point cardonfile is used.

It would perhaps be wise to put this logic in something like hook_entity_presave which would remove "being default" for all other cards in this set, then upon finally saving this entity, it gets set as the new default.

This ensures that no matter who / where / what calls commerce_cardonfile_save($card_data), that there's always only be a single default card.

So something like

list($current_default_card, $not_default_card) = reset(commerce_cardonfile_load_multiple_by_uid($my_uid));

$not_default_card->instance_default = 1;

commerce_cardonfile_save($not_default_card);

// We now have two default cards for this user.

j0rd’s picture

There's also a bug in this code, which causes all "cardonfiles" for a "payment instance_id" to get reset to 0, instead of all other cards for a user to get reset to 0. Which is not the desired outcome.


    // From Amex
    $card_data->payment_method = $payment_method['method_id'];
    $card_data->instance_id = $payment_method['instance_id'];

    // From Authnet
    $card_data->payment_method = $payment_method['method_id'];
    $card_data->instance_id = $payment_method['instance_id'];

    // From commerce_cardonfile_set_default_card()
    $query = new EntityFieldQuery();                                                                                         
    $query->entityCondition('entity_type', 'commerce_cardonfile');                                                           
    $query->propertyCondition('instance_id', $card->instance_id); 
    // This needs to get added
    //$query->propertyCondition('uid', $card->uid);
    $result = = $query->execute();
dwkitchen’s picture

Just committed the extra condition on the query by user.

The Default card functionality needs a bit more thinking. It doesn't take in to account the possibility of multiple payment methods on one site very well. Maybe they should be weighted in order rather than just having a default?

This would be good for dunning management in subscriptions so a second card can be charged if the first doesn't work or has expired. This would also need a charge for subscriptions option on the card on file entity.

j0rd’s picture

I'm going to open up a can of worms here and say, cardonfile assumes 1 default card per user, which is a pretty large assumption. It should probably be 1 default card per processor per entity. By default this entity can be a user, but it could also be a node or something else.

---

These should probably be other tickets, but cardonfile has lots of other issues to get to before handling anything luxury features like dunning are implemented. Not to say they shouldn't be thought about though.

I'm not personally using commerce_cardonfile and commerce_recurring so I don't need dunning as the processor will handle this for me.

Some pain points I've been having:

1. Allow the user to edit their card should it become not longer billable, and continue to allow their subscriptions to continue. Currently by default, the user can only change expiry date. User should be able to edit all details related to their credit card and have this updated. I think making this edit form a standard "edit edit form" and including fields into this form is most likely best. Not sure if you can recycle Drupal's core entity edit form configuration UI for this, but if so, this is ideal. Contrib could then hook into the submit and update the token. This is better than having each processor contrib create their own unique forms imho, as this will lack consistency.

2. Figure out some kind of method, which doesn't require the user to select "save this card on file" during checkout for subscription sales. Currently if a user is purchasing a subscription, and does not check this option, their card token will not be stored and thus will never be able to be rebilled. Again this can be handled via contrib, but you'll have many competing solutions again. It would be best if this functionality was in core, and then contrib modules could turn it on or off as needed. Personally I'm just going to be hiding these fields, as they're required to be enabled for someone who's purchasing a subscription and instead I will be providing them with a warning that their card tokens will be saved and later rebilled. I also believe some countries have laws regarding what text needs to be displayed for recurring subscriptions.

3. There are issues with billing address attached to cardonfiles. Currently I believe in most commerce workflows the customer is asked to input their billing address, and then they are forwarded to the "input your credit card" options. So if they input a different billing address, than is previously associated with a cardonfile, we have an issue of data in-consistency. For processors which link a card token to a billing address + credit card data, this becomes an issue. IMHO this is more of an issue with commerce lacking a complete ajax'd one page checkout and because of this, these types of issues come up. One solution would be to provide another checkout pane, which allows the user to input a billing address or choose a previously existing card on file. If they choose a previously used cardonfile associated with a billing address and press next, they are presented with no credit card options and simply the "review" order form. Of course this is all difficult, as people can have different checkout workflows.

dwkitchen’s picture

j0rd, thanks for your feedback.

There is a mismatch in card on file as it allows one default card per payment method instance per user but when it comes to charge assumes there will be only one default (usually by only having one payment method instance).

1. Editing card details is all dependant on the Payment Gateway, some only allow change of expiry date, some allow the change of the card number. I think this needs to be configurable by the payment gateway rather than making assumptions in the Card on File module. I also want to add support for hosted payment gateways like in the checkout process. This would then allow iframes etc for updating card details.

2. At the moment there is a settings option for card on file to require cards to be saved, and then the tick box goes and it is forced. However this is blanket and not so good if your store has mixed sales. This is something that would need to be some option that card on file needs to know if there is some thing on the order that requires the card on file to be saved.

3. This is something that needs to be looked at and the flow thought about. At the moment there is not relation between the billing address and the card on file, as you say with most checkouts the card on file billing address has to be the billing address for order.

bojanz’s picture

Issue summary: View changes
Status: Needs work » Fixed

I've addressed #3 with http://drupalcode.org/project/commerce_cardonfile.git/commitdiff/1a0692b...

I will reply in #1985544: Handle the card's billing address about the address point.

I agree that the instance_default is awkward. We probably want to replace that with a global (meaning for all payment methods) "default" flag.
In most cases it doesn't matter because 1 card per user is the preferred behavior in most use cases (and we need to enforce it better).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.