backend contributions fail (see #208)

[jmcclelland]

If I click the Membership tab of an existing contact, then click "Submit credit card membership" I get a white screen.

My web logs report:

2017/06/12 11:46:43 [error] 5667#5667: *632 FastCGI sent in stderr: "PHP message: PHP Fatal error: Cannot access protected property CRM_Member_Form_Membership::$_paymentProcessors in /var/www/powerbase/sites/all/extensions/stripe/CRM/Core/Form/Stripe.php on line 13" while reading response header from upstream, client: 192.168.69.149, server: xxxxx.loc.cx, request: "GET /civicrm/contact/view/membership?reset=1&action=add&cid=3&context=membership&mode=live&snippet=json HTTP/1.1", upstream: "fastcgi://127.0.0.1:11044", host: "xxxxx.loc.cx", referrer: "https://xxxxx.loc.cx/civicrm/contact/view?reset=1&cid=3"

And sure enough, in CRM/Contribute/Form/AbstractEditPayment.php, we have:

protected $_paymentProcessors = array();

See also #210.

[h-c-c]

Thanks, but I'm not in favor of #219 as it is. I think our strategy should be to use treat frontend differently from backend and use the _processors array (which is public) to fish out the active payrocessor ids.

219 returns the first Stripe id if it is active or not.

Also, I want to make sure we're not breaking the social contract, in that we advertise that we support multiple Stripe payprocessor instances within CiviCRM. I'm pretty sure always returning the first one could cause an improper transaction.

[jmcclelland]

Thanks for the feedback. I see your point - I've made a new commit to ensure that the payment processors returned only include active ones and only include test or live depending on the context. However, I'm not sure how to pick the right one if there are more than one stripe payment processors on the page.

Also, I'm not sure what you mean by treating the frontend differently from the backend?

[jmcclelland]

Well, now I'm just flailing. Open to suggestions.

[axaak]

Thanks for looking at this, we have this bug too. Although we only have one Stripe payment processor instance I can see the presence of more than one means you have no way of choosing which (active) one to use. To my mind only the admin knows this. If thats true the admin must decide and there are 3 ways of doing this: 1) Add a check box to the payment processor screen to mark a particular instance as the one to use for back-end membership card submissions, or 2) On clicking "Submit credit card membership" link, present a pop-up to the user with available active card processors (may be others, not just Stripe) and they choose which one on this occasion. 3) If an active card processor is marked as the default payment processor (not guaranteed) then use that. Option (2) would be more flexible but may well be more work. Hope that helps with the flailing ;-) Happy to discuss this more if its helpful, apologies if I've misunderstood the issue and I'm way off.

Regards, axaak

[jmcclelland]

I continue to flail... handling each error one at a time as I get them...

Right now I'm dealing with a user that has two Stripe payment processors. The publicly facing contribution page works great - picking the correct payment processor.

However, the backend (aka "offline") form returned an error about an unknown token.

I read your response @axaak - however, in this situation, the user is presented with a form asking them which payment processor to choose, but the stripe code is ignoring their choice.

The problem is that the javascript code sets the publishable key when the form is first loaded.

If you are making a backend contribution, and you change the payment processor, CiviCRM will reload the CRM_Financial_Form_Payment part of the form (but not the whole form). In the process, it will add a new hidden element with #id stripe-pub-key set to the new, proper value.

However, there are two problems:

• Now we have two elements with the same id (since the original element is part of the parent form, but not part of the form snippet being swapped in so it can't tell that there is another stripe-pub-key).
• Even if it could tell, the javascript won't call the Stripe code to reset that value anyway.

So, I've modified the code to try to work around this problem. Unfortunately, I have no idea what impact these changes may have on other parts of CiviCRM.

[axaak]

Hi @jmcclelland, thanks for your work on this. Does your mod update the existing element id with the new pub key? Or does it do something else? I've been hit before by others making changes to Civi that have unintended consequences for our use cases so you're right to be cautious. That said updating the dom with the new key for backend contribs where there is >1 card processor seems OK in principle to me. Obviously needs testing tho.

Regards, axaak

[h-c-c]

Hey, have you tested #208 in terms of dealing with #218 in the short term?

I'm sort of cross-posting this comment since we're talking about how to address multiple Stripe payprocessors in the backend.

I think to accommodate multiple Stripe ppids in the backend what we will be doing eventually is encoding an array of all active ppids and keys with php. Then the javascript can pick out the right ppid/key when it is chosen.

I'm not sure if @agh1 had considered the consequences of multiple stripe account support in terms of the backend transactions.

[jmcclelland]

I wasn't following #208 - thanks for the heads up. I haven't tested it.

@axaak - unfortunately, my change doesn't simply update the existing html element with the new value - because the change is triggered in the PHP code and I can't figure out how to insert javascript via PHP to make that change.

Instead, it inserts a new element (stripe_pub_key_updated). Then I added javascript code to check for this new element prior to contacting Stripe. If it exists and it is different then stripe_pub_key, then we reset the pub key prior to contacting Stripe.

[laryn]

I just ran into this, too, on a site that has only one payment processor (Stripe). The front end donations seem to work, but the backend admin button fails with "Stripe.js was not passed".

For the moment I reverted to a -dev version I had been using previously, before 4.7.2 was released and it works again.