Home » Forums » Ubercart » Development

Credit Card Start Date & Issue Number patch

Submitted by solarian on Sun, 05/25/2008 - 10:52
solarian
Posts: 56
 Joined: 04/20/2008
Bug FinderGetting busy with the Ubercode.

I'm attaching a patch for start date & issue number on card payments, which is required for Maestro. This is based on today's CVS uc_credit.module.

The issue number is protected in the same way as the card number & the CVV. It should work in debug mode too.

$order->payment_details['cc_start_month']
$order->payment_details['cc_start_year']
$order->payment_details['cc_issue']

Obviously it needs proper testing, but I'm hoping this can eventually make it into core.

AttachmentSize
startdate.diff9.49 KB
permalink #1 Tue, 05/27/2008 - 07:12
solarian
Posts: 56
 Joined: 04/20/2008
Bug FinderGetting busy with the Ubercode.

No takers?

I suppose the alternative for payment gateways is a form_alter(), but that would seem to lead to duplication of effort.

permalink #2 Tue, 05/27/2008 - 11:22
Ryan

Posts: 4771
 Joined: 08/07/2007
AdministratorHead Code Monkey - I eat bugs.

I'll check it out... I already feel like there are too many options for CC setup as is, but we did just up the max length to 18 for card numbers to accommodate Maestro cards, so we may as well make sure everything is there that is needed to support them.

permalink #3 Wed, 05/28/2008 - 16:31
Ryan

Posts: 4771
 Joined: 08/07/2007
AdministratorHead Code Monkey - I eat bugs.

Can you check out my latest reply in this thread? There's just too much necessary code for me to feel comfortable committing it when I want to do a 1.0 release by the end of the week.

permalink #4 Wed, 05/28/2008 - 17:44
solarian
Posts: 56
 Joined: 04/20/2008
Bug FinderGetting busy with the Ubercode.

I quite understand, but my Protx Protocol Guidelines state that the Start Date is also required for some Amex cards too, so bear in mind that this is not solely a concern for Maestro (and Solo) cards and therefore a Maestro module would be inappropriate.

permalink #5 Wed, 05/28/2008 - 17:54
Ryan

Posts: 4771
 Joined: 08/07/2007
AdministratorHead Code Monkey - I eat bugs.

Perhaps make it a CC Enhancement module? I have one of these for checkout already, and I have every intention of rolling them into core... we just can't get everything into 1.0 or it'll never come.

I didn't know that about AmEx, but none of the core payment gateways take start date fields anyways. But still, I'd love for it to be supported in core.

permalink #6 Wed, 05/28/2008 - 18:20
solarian
Posts: 56
 Joined: 04/20/2008
Bug FinderGetting busy with the Ubercode.

Ah, versioning is a pain! By all means, I agree, get 1.0 out the door and then it'll be a lot easier to think about these things.

I'm rather too busy at the moment to do a CC enhancement module myself, but it's not an unworkable idea. I just think it would duplicate effort, and would also increase the complication and difficulty of configuring Ubercart as a whole. Wouldn't it be easier to stick the patch into CVS after releasing 1.0 and just rely on testers? If it gets into CVS then I'll release my Protx module and we'll get a few guinea pigs... I mean beta-testers Sticking out tongue

Just a thought... looking ahead to 2.0... I would advocate a rethink of how cards are handled. At the moment, it seems necessary to do a regex to determine what card people are trying to pay with (because the select list is free text), and converting that to whatever code the merchant service requires; but if we're looking long-term, would it not be better to have each card as a kind of object (residing in a file, perhaps, like countries) that could have start date, validation type, etc., as required. Then, depending on which cards are enabled, suitable fields would appear as required. I am not convinced this abstraction layer is actually necessary, so it really is just a thought.

P.S. I think I may have spotted a bug in uc_payment_process() on line 907... it seems to turn all UIDs to 0. I don't know if this is intentional or not.

permalink #7 Thu, 05/29/2008 - 10:14
Ryan

Posts: 4771
 Joined: 08/07/2007
AdministratorHead Code Monkey - I eat bugs.

Weird... I think you're right about that UID thing. I went ahead and took away the ! as it doesn't make sense to totally ignore what gets returned. Sticking out tongue I think I've actually wondered before why I was missing UIDs in my payment tracking table.

Also, I think you're moving in the right direction regarding card profiles. I'm not totally happy with the way things are now either. I'm not sure that it'll be necessary to store them in external files since they're not going to be changing fast, but even just defining them in a single function the credit card module would work. I suppose it wouldn't hurt to make it a hook up front for the off chance that a new card type gets discovered somewhere that we need to accommodate. Sticking out tongue

permalink #8 Fri, 06/06/2008 - 18:37
solarian
Posts: 56
 Joined: 04/20/2008
Bug FinderGetting busy with the Ubercode.

Well, I'm attaching the latest diff against the current bazaar version of uc_credit.module, which seems to be the same as the v1.0 release. I hope this can make it into core, since it would seem rather bizarre to require an extra module to cope with Maestro and Amex, the current core implementation being otherwise broken.

AttachmentSize
startdate.diff9.46 KB
permalink #9 Mon, 06/09/2008 - 17:55
psynaptic

Posts: 305
 Joined: 08/28/2007
Early adopter... addicted to alphas.Not KulvikTheminator

+1 to get this into core.

--

Freestyle Systems
Contributions API

permalink #10 Sun, 06/15/2008 - 08:07
solarian
Posts: 56
 Joined: 04/20/2008
Bug FinderGetting busy with the Ubercode.

I have now added this to the d.o issue tracker in line with efforts to move more serious stuff away from these forums & the local issue tracker. I'd appreciate any feedback on it -- I haven't used it myself on a working site yet, though it seems to work fine in tests.