eileenmcnaughton / nz.co.fuzion.omnipaymultiprocessor

Omnipay Multi Processor Payment Processor For CiviCRM
Other
13 stars 44 forks source link

Omnipay Mollie integration #6

Closed magnolia61 closed 6 years ago

magnolia61 commented 9 years ago

Hi Eileen, Feel free to redirect me to a different place to ask my question but I would like to get the Mollie payment processor working from within your extension. Can you outline what the proces would look like?

Would it be as simple as adding a section to processors.mgd and the composer.json? Obviously I have not clue on how to format and how to name the fields in the mgd file (username & pwd_label / url site&api default / etcetera.

I have a LIVE & Test API key for Mollie which is all set up and would love to get it working as Mollie would be great for as, charging no monthly fee and low transaction costs.

Would love to learn some more on how to get going :-) Thanks for your great initiative on this extension!

Richard van Oosterhout, The Netherlands

eileenmcnaughton commented 9 years ago

"Would it be as simple as adding a section to processors.mgd and the composer.json" In theory, yes...

I just added a readme and some notes around the processors.mgd file

When you are ready to do a PR then just add the composer.json changes & mgd changes - I will run the composer update & commit that but I think including changes make by composer.json in the PR could lead to messy PRs

magnolia61 commented 9 years ago

Thanks, I'll have a try at it. Can you hint me on where to find the readme? Still local with you?

magnolia61 commented 9 years ago

OK, Here's my PR so far. I'm curious where this will lead to and if there are any major roadblocks ahead :-) https://github.com/eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor/pull/7

eileenmcnaughton commented 9 years ago

I've merged that & also pushed the composer updates

eileenmcnaughton commented 9 years ago

NB - I haven't updated the wiki page for a while but you might want to add Mollie if you get it going - http://wiki.civicrm.org/confluence/display/CRMDOC/Payment+Processors

magnolia61 commented 9 years ago

Thanks for the composer updates. So far, I get the a white screen after the confirmation screen. The error log reads: Class '\Omnipay\mollie\Gateway' not found. It seems to origin from: nz.co.fuzion.omnipaymultiprocessor/vendor/omnipay/common/src/Omnipay/Common/GatewayFactory.php Is that a familiar kind of error message for you?

eileenmcnaughton commented 9 years ago

The lower case M looks wrong to me.I think the mgd file should have been 'name' => 'omnipay_Mollie',

magnolia61 commented 9 years ago

Hmm, bit strugling now. Now no fatal error immediate after proceeding from the confirmation screen, but a long wait and then a timeout. A bit of that was solved by setting the php memory limit higher (130MB was not enough), but now I get hundreds (!) of other for me non-understandable errors:

Warning: curl_multi_exec() has been disabled for security reasons in Guzzle\Http\Curl\CurlMulti->executeHandles() (line 238 of /home/ozk/domains/ourdomain.nl/public_html/crm/sites/all/modules/civicrm_extensions/nz.co.fuzion.omnipaymultiprocessor/vendor/guzzle/http/Guzzle/Http/Curl/CurlMulti.php).

The promising part is that I feel we're getting closer step by step :-)

eileenmcnaughton commented 9 years ago

Guzzle is the package that handles http communication - in this case curl - I think the error might be a server config issue - see

http://www.2by2host.com/articles/php-errors-faq/disabled_curl_multi_exec/

NB 130MB is not a high memory limit for Civi but it does seem very high for this interaction - however, the error rendering might be what is sucking it up

magnolia61 commented 9 years ago

I'm asking my host to enable it for further testing. Would you think this could be the last hurdle? #wishfull

eileenmcnaughton commented 9 years ago

Of course it will be the last hurdle ......

magnolia61 commented 9 years ago

Our host enabled the curl_multi_exec but as you see below there are more hurdles ahead. I appreciate your hopefull aproach though :-) Any idea on this error?

1 /home/ozk/domains/ourdomain.nl/public_html/crm/sites/all/modules/civicrm/CRM/Utils/Request.php(105): CRM_Core_Error::fatal("Could not find valid value for id")

2 /home/ozk/domains/ourdomain.nl/public_html/crm/sites/all/modules/civicrm/CRM/Event/StateMachine/Registration.php(53): CRM_Utils_Request::retrieve("id", "Positive", Object(CRM_Event_Controller_Registration), TRUE)

magnolia61 commented 9 years ago

When logged in as admin in the backend in another tab I see a notification with: Sorry, there was an error processing your payment. Please try again later. And another one with: Payment redirect failed Would this error (Could not find valid value for id") point to the contact_id?

eileenmcnaughton commented 9 years ago

do you want to email me your test credentials & I'll see what happens when I plug them in locally - pretty sure my email is in the .info file

eileenmcnaughton commented 9 years ago

issue turned out to be https://github.com/thephpleague/omnipay-mollie/issues/13

I also added code for it to send through a webhook address - not sure if that overwrites what you configured or not - I expect they will be the same value. Anyway - I didn't test past getting the Mollie page to load - so you should get that far at least (with the latest version)

magnolia61 commented 9 years ago

YES! I'm getting as far as the Mollie screen. After I submit a payment (no worries, I gave you the test_api, you can use it as well, np), i get redirected to what seems to be the proper url: civicrm/event/register?_qf_ThankYou_display=1&qfKey=38105591etcetera4fb2_9180

But the actual screen that shows is still the confirmation screen and I get no notification of a succesfull payment. Feel free to use my test_api code to reproduce this. I feel we're getting closer :-) Thanks a lot!

BTW: just tried it with my 'live' api-key and a real payment, and what happens is the same: redirect back to the confirmation screen and no notification of succes (screen of mail). Very nice to see Mollie in action though!

eileenmcnaughton commented 9 years ago

Hmm - I'm not using a publicly accessible site so it can't 'hit' me - is there anything in your civicrm_system_log table? (assuming 4.5)

magnolia61 commented 9 years ago

In the drupal recent error log I see the following: Location: https://www.ourdomain.nl/civicrm/payment/ipn?processor_id=33&mode=live Message: Recoverable fatal error: Argument 3 passed to CRM_Utils_SystemLogger::log() must be of the type array, string given, called in /home/ozk/domains/ourdomain.nl/public_html/crm/sites/all/modules/civicrm_extensions/nz.co.fuzion.omnipaymultiprocessor/CRM/Core/Payment/PaymentExtended.php on line 255 and defined in CRM_Utils_SystemLogger->log() (line 51 of /home/ozk/domains/ourdomain.nl/public_html/crm/sites/all/modules/civicrm/CRM/Utils/SystemLogger.php).

In the civcrm_system_log table I discovered something perculiar: {"q":"civicrm\/payment\/ipn","processor_id":"48","testByMollie":"1","id":"","IDS_request_uri":"\/civicrm\/payment\/ipn?processor_id=48&testByMollie=1","IDS_user_agent":"Mollie.nl HTTP client\/1.0"}

Yesterday in the Mollie control panel I changed the webhooks to https://www.ourdomein.nl/civicrm/payment/ipn?processor_id=33 but it seems this morning (NL timezone) still the old value of 48 was used, which probably explains why the contribution was not completed. I am going to re-test and contact Molie on how they cache our settings. #needle #haystack #progress!

eileenmcnaughton commented 9 years ago

NB - I just pushed a fix to address the recoverable fatal error - probably not the best one - casing to an array rather than figuring our why it wasn't an array in the first place

magnolia61 commented 9 years ago

Hi Eileen, I generated new api keys for my live & test and ensured they match the proper webhook link. The payment now still does not get completed, I still go back to the confirmation screen (with thank-you url). In the civicrm_system_log I see an alert and an error:

alert: message: payment_notification processor_id=33 context: {"q":"civicrm\/payment\/ipn","processor_id":"33","amp;mode":"live","id":"tr_U6BCiHeW6H","IDS_request_uri":"\/civicrm\/payment\/ipn?processor_id=33&mode=live","IDS_user_agent":"Mollie.nl HTTP client\/1.0"}

error: message: tr_U6BCiHeW6Hid is not a valid integer context: ["ipn_completion"]

eileenmcnaughton commented 9 years ago

Just thinking about this - the normal flow is that an invoice id (the contribution id) is passed to the payment processor and it would pass that back with the webhook in order for matching to take place. In this case the only id passed back 'tr_U6BCiHeW6H' - appears to be Mollie-generated so I'm wondering how CiviCRM would know how to match it. (I don't know much about Mollie of course)

eileenmcnaughton commented 9 years ago

This is what I think it happening https://github.com/thephpleague/omnipay-mollie/issues/15

magnolia61 commented 9 years ago

Thanks for your continued effort Eileen! True spirit of open-source coding. Hope the league guys can be of help. I the meantime I would be glad to do some more dubugging. Any more info that could be usefull?

magnolia61 commented 9 years ago

Hi Eileen. At cividay in the Netherlands I met some people from CiviCoop who would like to help out with getting Mollie to work. What is the current status? Do we need more info from Mollie itself or does generic the Mollie/omnipay integration need to change a bit?

eileenmcnaughton commented 9 years ago

Basically I think the change needs to happen in the omnipay-mollie plugin - OR in my fork of it - since my other patch hasn't been integrated

https://github.com/thephpleague/omnipay-mollie/issues/15

I'm imagining something like

+++ b/src/Message/PurchaseRequest.php @@ -29,6 +29,10 @@ class PurchaseRequest extends AbstractRequest $data['redirectUrl'] = $this->getReturnUrl(); $data['method'] = $this->getPaymentMethod(); $data['metadata'] = $this->getMetadata();

One the retrieve I am using $request->getTransactionReference - but I'm a bit confused looking as there is

public function getTransactionId()
{
    return $this->getParameter('transactionId');
}

public function getTransactionReference()
{
    return $this->getParameter('transactionReference');
}

One should be your reference & the other should be the Mollie one - but I'm not sure that they are being set consistently across all gateway plugins. (would need to dig further)

eileenmcnaughton commented 9 years ago

I've added the proposed Mollie change to this extension - I wouldn't expect it to fully work - but hopefully the url per your 9 Jan comment will have the contribution id in it

magnolia61 commented 9 years ago

Hi Eilien, thanks for revisiting the mollie-issue :-) Unfortunately I experience no change with the current code. BEFORE MOLLIE PART: civicrm/event/register?_qf_Confirm_display=true&qfKey=2133adbc7bb289eaa0300493d6536522_9064 AFTER MOLLIE PART: civicrm/event/register?_qf_ThankYou_display=1&qfKey=2133adbc7bb289eaa0300493d6536522_9064 BUT the confirmation screen is shown, instead of the thank you screen. In which commit did you put the Mollie updates?

eileenmcnaughton commented 9 years ago

check your server log - your browser might be there but hopefully your browser or an ipn has touched a url like civicrm/payment/ipn....

magnolia61 commented 9 years ago

YEP :-) I see "POST /civicrm/payment/ipn?processor_id=33 HTTP/1.1" 200 5230 "-" "Mollie.nl HTTP client/1.0" in my apache logs. Note: in reality the cid=2131 and id=2289 Good sign? Halfway there? Nowhere near? Can I do some testing/debugging?

eileenmcnaughton commented 9 years ago

Are you on 4.5 ? if so you can check the system_log table to see what was posted. I'm looking to see if the contribution_id was posted back...

magnolia61 commented 9 years ago

I get the same error as before. alert: message: payment_notification processor_id=33 context: {"q":"civicrm\/payment\/ipn","processor_id":"33","id":"tr_6JWAXMpQek","IDS_request_uri":"\/civicrm\/payment\/ipn?processor_id=33","IDS_user_agent":"Mollie.nl HTTP client\/1.0"}

error: message: tr_6JWAXMpQekid is not a valid integer context: ["ipn_completion"]

Were your Mollie updates included in https://github.com/eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor/commit/174c090c7398a8b9a263bfa2117cce4a78d2722f ?

eileenmcnaughton commented 9 years ago

yes - the key thing is that there is a metadata param that gets passed out to Mollie - I'm trying to add contribution id to it - I hoped that if I did it would use it in the IPN.

I haven't read the Mollie documentation but tr_6JWAXMpQek is a Mollie generated ID & I'm not sure how Mollie expects us to link that back to the civicrm id

magnolia61 commented 9 years ago

As an outsider I am a bit confused that a pp like Mollie can be included in the omnipay library but would not work 'out of the box', since the whole idea of the omnipay ecosystem is translation to uniform interfaces :-) I would also expect the upstream mollie integration to spit out something more than tr_6JWAXMpQek. Maybe we need to buy an enigma of contact relatives of Alan Turing to break that code... I can also try to contact Mollie directly on this matter tomorrow and see what they can say about this.

magnolia61 commented 9 years ago

Eileen. Just tested with the current master, with the same results for Mollie. Do you know if there are still upstream Mollie changes needed for this to work?

eileenmcnaughton commented 9 years ago

I think Mollie probably needs this to be able to pass the CiviCRM reference number

https://github.com/thephpleague/omnipay-mollie/issues/15

magnolia61 commented 9 years ago

We are getting somewhere! With the patched Mollie files I was finally able to witness the transaction marked as completed :-) completed The only critical thing left I believe is that Mollie still redirects to the contribution page. I am not quite sure how it can redirect over there and still be able to mark the contribution complete. redirect Feels you're getting closer to a working Mollie integration :-)

eileenmcnaughton commented 9 years ago

I'm a bit confused about that page - it's the 'right' url - ie. under the 'right' circumstances it will display a thank you - I'm just not quite sure what they are...

magnolia61 commented 9 years ago

I experience the same with the redirect if I use a regular contribution, or an event registration. In the latter case I get redirected to the event registration confirmation screen. #mystery

Although the wrong page was showed, using the proper url, the contribution got recorded and I was sent a confirmation email. So I would say this is 98% close to work... It's the last 2% that are always interesting...

eileenmcnaughton commented 9 years ago

see latest patch - if this fixes the only issue is that upstream mollie haven't accepted my patch yet https://github.com/thephpleague/omnipay-mollie/issues/15

You might want to go over there & feed back that it works

magnolia61 commented 9 years ago

Using the current master of this extension, combined with the 3 patched Mollie files pending to be merged upstream I get the following error: transact When I revert the commit of May 20th ( vendor/omnipay/common/src/Omnipay/Common/Message/AbstractResponse.php) the Mollie payment gets through, but still redirects to the confirmation page (even though your patched CRM/Core/Payment/OmnipayMultiProcessor.php is in there). (also: the payment is correctly recorded and a confirmation mail is sent)

It seems something triggers the fatal error after the last 2 commits. The civicrm_system_log shows: transact 2

Edit: I just found out the Mollie admin panel also reported the error: request methode: GET request timeout: 15 seconds url: https://www.mollie.com/payscreen/report/4uezAmtnU7?transaction_id=dc3639b53d8433b775354ddc82c61dae parameters:

magnolia61 commented 9 years ago

@catorghans Would you be able to have a glance at why the redirect is not functioning well? I am happy to cooperate debugging and testing, also you can have access to our site for analysis.

magnolia61 commented 9 years ago

Did some debugging again with a recent upstream (unmerged) PR https://github.com/thephpleague/omnipay-mollie/pull/22 for Mollie. The results are the same. The added CRM_Core_Session::storeSessionObjects(); ( #11 ) breaks with a fatal error (see screenshot above).(br> yet: mollie handling is ok, payment getś updated in the DB, confirmation mail gets sent. Which is all nice :-) But still the it redirects back to the confimation screen, probably because I cannot use PR #11 Civicrm logfile reads: Aug 04 13:16:45 [info] Contribution record updated successfully Aug 04 13:16:46 [info] Receipt sent Aug 04 13:16:46 [info] Success: Database updated I would love to test out some more scenario's or test patches. Any suggestions are welcome :)

catorghans commented 9 years ago

I had something similar with my omnikassa extension. Not sure if it is related, I really have not dived into omnipay or mollie. Omnikassa extension works with a form submit to the omnikassa system. I had to do the following to make sure civi kept the session alive in the page that created the forward form:

    require_once 'CRM/Core/Session.php';
    CRM_Core_Session::storeSessionObjects( );
    exit;
magnolia61 commented 8 years ago

Hello Eileen, Thanks to the suggested include by Hans I was able to get the Mollie implementation fuly working. For this I also had to revert the getReturnSuccessUrl change (https://github.com/eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor/commit/ddad971fa5a6a1c20541b9454b8ad037b0eccda0) and include some changes you also suggested upstream in the Mollie library. PR: https://github.com/eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor/pull/16/files Tested on civicrm 4.6.10 / Drupal 7.41 and Joomla 3.4.6

magnolia61 commented 8 years ago

You can see a test contribution page with Mollie in testmodus as payment processor here: https://www.onvergetelijk.nl/civicrm/contribute/transact?reset=1&id=2 Feel free to try with bogus information :-)

Alienpruts commented 8 years ago

I can confirm it works on my setup as well.

The solution offered by @catorghans is a far better one than the one I suggested at https://github.com/thephpleague/omnipay-mollie/issues/18, because it uses Core CiviCRM functionality (where I directly accessed the Session to store the data in), but in effect they are the same. :+1:

Alienpruts commented 8 years ago

Just noticed a small problem with your PR @magnolia61, especially in my use case then.

I'm using Mollie in Omnipay to try to register people to Events. When I use your code in PR https://github.com/eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor/pull/16/files my participants' statuses don't get set to "registered", they get set to "Pending from incomplete registration".

When I use my code as suggested in https://github.com/thephpleague/omnipay-mollie/issues/18 , the participants do get set to "Registered" (only with a succesfull payment, of course :) ).

Just thought I'd mention :)

magnolia61 commented 8 years ago

I use the code solely for event registrations and with me the participant get the status 'registered' after the (IPN) feedback from Mollie that the payment was successful. Is that configured correctly with you. E.g. does the event registration mail get sent?

Alienpruts commented 8 years ago

That is a good question : I was solely concentrating on getting the Mollie implementation to work that I did not stop to check that. I assume it does, since the participants get set to 'registered' status after succesfull payments from Mollie, but my test event has the option to send mails turned off. I should check that it actually does.

Apart from that, I believe the (small) issue is that I explicitly store the TransactionReference in Session, whereas the code containing

CRM_Core_Session::storeSessionObjects( ) ;

does not (or so I think).

I am not a Civi connaisseur per sé, I only had to make the Mollie payments work, so it is entirely possible this small issue arises from a different configuration of the event in regards to your setup.

Thoughts?

eileenmcnaughton commented 8 years ago

So, my expectation is that the transactionId should be passed to Mollie and included in the return data from Mollie (which is how other processors work) - via the functions getTransactionId & setTransactionId ie

https://github.com/eileenmcnaughton/omnipay-mollie/commit/90e48cc619e61b621f4bdf5561887f38c7c64d8b#diff-7438915c1a28f2250393e293288f46dfR33

and

https://github.com/eileenmcnaughton/omnipay-mollie/commit/22956c1a62a9662afa5f5d119723b413770ac525

It seems to me the session issue is only being raised because that part is NOT working