Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated code for Omnipay 3 #6

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

emergingdzns
Copy link

I modified the composer file and readme to reference my repo for my own testing and development (global find/replace). Feel free to ignore those changes. The only changes in the composer.json you need to keep are the require and require dev omnipay version numbers.

In the readme you have the instruction to require version 1.0. I changed it to 2.0.

Otherwise very little needed to be changed. Omnipay 3 no longer uses Guzzle so the 404 error check is unnecessary in the AbstractRequest sendData method.

Also the send data is now $this->httpClient->request('POST',$this->getEndpoint(), [], http_build_query($data)); instead of the old ->post() method due to the change in client library.

In the phpunit xml file the Mockery listener is removed per the Omnipay upgrade documentation.

To be honest, I have no idea how to run the unit tests nor how to edit the tests to adjust to the changes (if they even need to be changed).

@emergingdzns
Copy link
Author

I have no idea how to do the travis testing. I had a couple changes I had to make today for refunds.

@@ -1,5 +1,6 @@
<?php

use Log;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log doesn't appear to be used anywhere. Can it be omitted here?

@emergingdzns
Copy link
Author

yes sorry. i have since removed it. I forgot to do another PR. I made another couple changes. I'll do a PR again.

@emergingdzns
Copy link
Author

Ok so I think that does it. The latest change was to the refund request. The problem is that BP has so many different APIs I was mixing up the formatting. They want people to be using their bp10emu endpoint now instead of the bp20post which is what we are using now. However it's not scheduled for deprecation yet so it's ok to use for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants