I have a use case in which we are actually fulfilling orders first (due to volatile inventory levels, accepting payment on the spot can result in a ton of voids/refunds) and THEN processing all the payments once we've settled everything up, etc.

I see that there is a hook_menu entry for /user/%user/stored-payment-methods, but this will actually show an access denied if a person doesn't have a card on file and there is no way to add one there.

Basically I'd like to have the option of customers to get their card into the system and then use it for recurring shipments. But because order quantities vary even day by day, using the typical recurring payments style simply doesn't work here.

I have no problem attempting a patch, but I first want to get a feel if this is something that would be accepted if I provided it.

CommentFileSizeAuthor
#1 1812740-commerce_cardonfile-1.patch560 bytesrickmanelius

Comments

rickmanelius’s picture

rickmanelius commented
StatusFileSize
new 1812740-commerce_cardonfile-1.patch560 bytes

At the very minimum, I think we should eliminate the access denial for users without cards because this module already has output ready for the situation in which they have no saved cards on file as shown here: 

// Add an empty message in case the tab got rendered without stored cards.
  if (empty($rows)) {
    $rows[] = array(array('data' => t('You do not have any stored payment methods.'), 'colspan' => 2));
  }

Therefore I'd like to start by removing that access control check with the following patch.

rickmanelius’s picture

rickmanelius commented

I've never had such a hard time trying to figure out how to load the standard drupal commerce pane via the form API. The closet thing I can find is trying to load commerce_cardonfile_update, but that requires that the card already exists.

I'd like to be able to either place a new "add card" form in either the user/%user/stored-payment-methods or (as a new tab) user/%user/stored-payment-methods/add.

Any pointers would be appreciated!

joshmiller’s picture

joshmiller
he/him
Primary language English
Location Indianapolis, Indiana, USA
commented
Status: Active » Postponed

rickmanelius...

Fantastic idea! This is definitely something that should be added in the future. I'm simply postponing until we can get some traction. The question you should be asking is, "Do you have to make a payment to store a card?" The answer is that it's very API-specific. So, that would be the job of Authorize.net to implement.

Josh

recrit’s picture

recrit commented

@joshmiller, @rick
We've been working through some improvements to the card on file framework @see http://drupal.org/sandbox/recrit/1810130. One of the improvements we're developing is the add a new card. As Josh stated, it is very api specific so its up to the gateway module to implement , ie only register customer profile or create an order and capture an authorization for $4.

If you want to test, post issues at http://drupal.org/sandbox/recrit/1810130... I'll warn that it will be changing fairly rapidly

rickmanelius’s picture

rickmanelius commented

@recrit
I think an API would be terrific... allowing a payment gateway submodule to add this specific behavior on a per gateway basis (if the gateway allows for it).

I have a client with a use case that requires this for a project, so I'll be happy to assist in the testing, feedback, etc.

rickmanelius’s picture

rickmanelius commented

Hi @recrit,

I took a quick peek at your sandbox that you posted in #4. First, I'm impressed by how ambitious you are in adding a lot of functionality to this module. But my only concern is using your forked version because our client will require this to remain fairly unchanged after it's installed and we won't be able to keep up with a rapidly evolving module!

My question to you then is... are you planning on trying to merge this functionality into the current module, OR are you planning forking this to a separate module altogether?

Given that the current module has less than 300 active installations, has 11 total commits, and doesn't have a stable release yet... I'd vote for taking what you've done and merge it in. But, of course, I'm not the maintainer of this module.

Please feel free to ping me in IRC as I'd love to hear your thoughts on this, timelines, etc.

rickmanelius’s picture

rickmanelius commented

Also @recrit, would you prefer that I post any followup questions at the sandbox project?

And, in your opinion, how hard would it be to parse out the code required for just this particular feature? Or has the code base changed so much that there really is no want to just port it over from the sandbox?

recrit’s picture

recrit commented

@rickmanelius:
For follow ups, let's use the sandbox issue until its merged with this project.

For the "create a new card" functionality, the code base has changed significantly but it would be possible with the existing code base. You'd have to reproduce the menu paths ('user/%user/stored-payment-methods/add/...') and the create page/form (commerce_cardonfile_create_page) that takes the user input and invokes the 'create callback' for the payment method.

Merging into this module: I'll need to review with Berdir what approach to take - merge 1.x, create a 2.x, etc. The CRUD API for the card data is the same as the 1.x branch so it would not disrupt any payment module that integrates with card on file. Also, the development on the fork is winding down since we've completed the improvements needed for our implementation.

rickmanelius’s picture

rickmanelius commented

@recrit

Thanks for the info. I'll test out the sandbox then based on the fact that you've stated the changes/developments are winding down.

And personally, based on the quantity of changes you're suggesting, it might be better to consider it a 2.X branch. However, given that there is no stable release for the 1.x branch and the number of reported installations is still small, I could argue the case that keeping 1.x shouldn't be an issue.

I'd be interested to know of any follows after your discussion with Berdir to know if your current plan changes in any way!

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

@rickmanelius: My response was more or less the same and that I want to check with Ryan, the original author of this project before making any major changes.. I'm just a co-maintainer :)

rickmanelius’s picture

rickmanelius commented

@Bedir: understood, but that does mean you have commit access :)

But on a serious note, I now know I can proceed down this path knowing it won't lead me (and my client) to dead end. Thanks, and I'll start testing this probably within 1-2 days. Thanks!

rickmanelius’s picture

rickmanelius commented
Status: Postponed » Active

FYI (to gain visibility here where this started). I've submitted the patch for the commerc_authorize.net module here #1850100: Add "create callback" functionality for Commerce Authorize.net using the sandbox version of this module here http://drupal.org/sandbox/recrit/1810130.

Setting back to active... because this requires both the merging of the sandboxed version (when ready) and then reviewing and accepting the patch in the other thread.

rickmanelius’s picture

rickmanelius commented

A friendly ping on this one. I've managed to get the sandboxed version working for the commerce authorize.net module using the patch in #12. I know this is spread out over several issues, so it might take some coordination to get traction... but I can't wait until the sandbox'd version by @recrit gets merged into the official card on file branch!

rickmanelius’s picture

rickmanelius commented

Hi Everyone,

Thanks to all of @recrit's hard work in the sandbox module (http://drupal.org/sandbox/recrit/1810130), this and many other features are ready to pull back into the main module (http://drupal.org/node/1821366#comment-6938902).

At this point, I'd like to ping @Berdir again to see if Ryan would give you his blessing with respect to bringing these changes in.

-Rick

dwkitchen’s picture

dwkitchen commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Active » Fixed

This has now been included in the 2.x branch

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.