Stripe breaks other payment processors

[universalhandle]

Environment

CMS: Drupal 7 CRM: CiviCRM 4.7.9 Other PP: PayPal Pro, set as default

Steps to Reproduce

I'm not sure how easy this is to reproduce, or which details are relevant. In this case, another consultant installed the Stripe extension, created a PP, and probably set it as the default. Later the client changed payment gateways, so a PayPal Pro PP was created and set as default.

When processing a payment through the backend (from the Contributions tab on the contact's summary screen), using the PayPal PP, I got this error:

Payment Error Response: Error: Invalid API Key provided: ************1234

Google landed me at a StackExchange post which suggested disabling Stripe to fix this. I did, and I was able to process transactions.

I did not find anything in my CiviCRM log that was obviously related to this, but I guess that's not too surprising since most of the magic is on the client side, right?

[drastik]

Hi @GinkgoFJG Using the latest 4.7-dev code for civicrm_stripe from GitHub here?

[drastik]

I have found it difficult to detect in JS which payproc is currently selected. When I have had success determining this, another CiviCRM update breaks the code.

I need information from someone more up-to-speed on current CivICRM changes, that may know if something was done to support this.

@eileenmcnaughton, ideas?

[eileenmcnaughton]

Try adding the js in the same way paypal does through the buildForm function https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment/PayPalImpl.php#L111

[drastik]

You mean through template?

[eileenmcnaughton]

I mean adding the template in from the buildForm function on the Payment_Stripe class.

[tttp]

Hi, have you figured out this one?

[drastik]

There is a large PR from @h-c-c that I am reviewing which addresses dozens of things. https://github.com/drastik/com.drastikbydesign.stripe/pull/172

[h-c-c]

Thanks @drastik. This #172 doesn't address the issue here, but I just took another run at it. https://github.com/h-c-c/com.drastikbydesign.stripe/commit/c2101111cdfb6b5823443c680ed41566b0a91872

If I understand what @eileenmcnaughton means, just moving the hunk of code dealing with buildform/including stripe.js etc. out of stripe.php to Stripe.php, where it's in the CRM_Core_Payment_Stripe class means that we're limiting when it is run. FYI, this works for me to process transactions, but I don't have a PP account currently....so I haven't tested fully.

[drastik]

Ah, now I see what she was pointing out. I was thinking she meant to override the billing tpl file(s).

The function name will drop the hook_ prefixes, and just become: public function buildForm(&$form) {

I'll update in a moment. Thanks.

[drastik]

Okay @h-c-c , your commit is in, and renamed function to buildForm in another commit. https://github.com/drastik/com.drastikbydesign.stripe/commit/5e0af5f382a1f1946d37d316043389e4bcdd6299#diff-8dcbf2bfe3fcb3899e049399543df444R273

[drastik]

This is all untested, I don't know how well any of this will work.

Also, we lost the bit that removes the fields after submission (preventing sensitive details hitting the local server). Will have to re-visit that.

[h-c-c]

Cool! If this does address the issue of breaking other payprocessors, maybe this could be refactored somewhat too? If the CRM_Core_Payment_Stripe class is being called, we probably don't have to work that hard to tell if Stripe is in play.

[drastik]

we probably don't have to work that hard to tell if Stripe is in play.

Well, I thought so, but even the PayPal one still checks if PP is the processor: https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment/PayPalImpl.php#L111

Maybe it is just checking which version of PayPal, I'm guessing they are sharing a class.

With some testing I would know a lot more.

Regarding the problem with other PayProcs: I'm not entirely sure. The theory is that this will do a better job of adding Stripe's JS when Stripe is the payproc. However, if you switch between Stripe, then choose another payproc on the form, Stripe's JS will still be present, and attached to the submission event of the form. It's like we need an unload type hook.
Need to find the event occurring when payprocs are switched on the form, and use that to de-register Stripe event handling on form submission.

[h-c-c]

Hey, heads up....I'm sorry I was wrong about this commit c2101111cdfb6b5823443c680ed41566b0a91872 processing transactions! I had a different version running in my apache config. Duh!

[h-c-c]

Ok, we're transacting again! f5d1473 The stripe specific functions aren't recognized inside the class. I moved them outside. I also moved validateform back into stripe.php.

The theory is that this will do a better job of adding Stripe's JS when Stripe is the payproc.

Feel like I said this better in a deleted comment...I could be mistaken, but I thought it was more like now we're doing a better job at not loading civicrm_stripe.js when Stripe is not the chosen PP on a given contribution page or event. SInce stripe.php is like a module, it's buildform function (as an example) would be evaluated for every Civi form. Now that it's in the class it's only run when Stripe is a chosen PP. I guess the supposition is that our logic determining if the form was for Stripe wasn't working the way we expected, although I don't see it. The issue wasn't that stripe was interfering with another PP while they were both enabled on a given contribution page, it's that stripe was interfering by just being installed. I imagine a contribution page with both Stripe and another PP enabled might still be a problem. Come to think of it, I don't know if multiple payprocs per contribution pages are really supported. The choice is a checkbox, but if I select Stripe and a Dummy PP there's no other choices presented to define what PP takes what contribution. Is it for high availability....round robin in case one is down?

[drastik]

If you choose 2 payprocs for a contribution form, the end user (donator) should be presented with the option to switch right on the donation form (radio buttons last I recall).

For the multiple-payproc issue, we need to know when the user clicks to switch payprocs (I've seen the JS, but not a clear indicator of which payproc was just activated/de-activated).

Similarly, when the form is loaded, we need to know if Stripe is the default, because I think it currently just assumes.

---

Situation for those unfamiliar (as I understand it):

• Form is loaded, Stripe is a payproc (note: not necessarily the payproc), Stripe JS is added & attaches event handler to the form submission (essentially, takes over).
• Whether another payproc is selected or not, Stripe's JS is still attached to the event of submitting the form, and will attempt to pass the transaction to Stripe.

[drastik]

Our JS integration file got a bit unwieldy after trying to accommodate Webform, discounts, and other plugins.

Regardless, here is a bit that made its way in, attempting to discover if Stripe is the PP. I don't think this is working for every situation: https://github.com/drastik/com.drastikbydesign.stripe/blob/4.7-dev/js/civicrm_stripe.js#L158

[drastik]

Where it is looking to compare #stripe-id, seems like a form element that was / should be added in buildForm() but is not. I think that is the essential comparison, because #stripe-token might always be there, but this is our chance to compare processorId's (and we aren't actually using it).

[tttp]

Hi, We have it working to have several PP, including stripe, but we had to modify a few things. I'm looping in @rthouvenin that can detail what was done

X+

On 16 January 2017 at 14:03, Joshua Walker [email protected] wrote:

Where it is looking to compare #stripe-id, seems like a form element that was / should be added in buildForm() but is not. I think that is the essential comparison, because #stripe-token might always be there, but this is our chance to compare processorId's (and we aren't actually using it).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/drastik/com.drastikbydesign.stripe/issues/155#issuecomment-272856924, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH1qkOVLDDIWnypdHNsOeDa1udrQCZJks5rS2qUgaJpZM4JyWm- .

[rthouvenin]

We indeed add <input type="hidden" id="stripe-id" value="1" /> to our form. If I remember correctly, this was to avoid the form script enter the stripe-related code when the PP is something else. I figured that out by reading the scripts.

[h-c-c]

Thanks for the hint, @rthouvenin!