Skip to content

Upgrade for v3 #40

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 25, 2018
Merged

Upgrade for v3 #40

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74a8ed2
Upgrade for v3
ptuchik May 24, 2018
013056d
Updated composer json
ptuchik May 24, 2018
34b1289
Throw Omnipay NotFound in case resource not found on gateway
ptuchik May 25, 2018
31c68f5
Reverted back to Symphony's NotFoundHttpException until the release o…
ptuchik May 25, 2018
d1e1aed
Reverted back the exception part
ptuchik May 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
"psr-4": { "Omnipay\\Braintree\\" : "src/" }
},
"require": {
"omnipay/common": "~2.0",
"omnipay/common": "^3",
"braintree/braintree_php": "^2.39|^3.0"
},
"require-dev": {
"omnipay/tests": "~2.0"
"omnipay/tests": "^3"
},
"extra": {
"branch-alias": {
Expand Down
2 changes: 1 addition & 1 deletion src/Gateway.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use Omnipay\Common\AbstractGateway;
use Braintree_Gateway;
use Braintree_Configuration;
use Guzzle\Http\ClientInterface;
use Omnipay\Common\Http\ClientInterface;
use Symfony\Component\HttpFoundation\Request as HttpRequest;
/**
* Braintree Gateway
Expand Down
2 changes: 0 additions & 2 deletions src/MerchantBusiness.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Omnipay\Braintree;

use DateTime;
use DateTimeZone;
use Omnipay\Common\Helper;
use Symfony\Component\HttpFoundation\ParameterBag;

Expand Down
2 changes: 0 additions & 2 deletions src/MerchantFunding.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Omnipay\Braintree;

use DateTime;
use DateTimeZone;
use Omnipay\Common\Helper;
use Symfony\Component\HttpFoundation\ParameterBag;

Expand Down
2 changes: 1 addition & 1 deletion src/Message/AbstractRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Omnipay\Braintree\Message;

use Braintree_Gateway;
use Guzzle\Http\ClientInterface;
use Omnipay\Common\Http\ClientInterface;
use Omnipay\Common\Exception\InvalidRequestException;
use Symfony\Component\HttpFoundation\Request as HttpRequest;
use Omnipay\Common\Message\AbstractRequest as BaseAbstractRequest;
Expand Down
2 changes: 0 additions & 2 deletions src/Message/CreateCustomerRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<?php
namespace Omnipay\Braintree\Message;

use Omnipay\Common\Message\ResponseInterface;

/**
* Authorize Request
*
Expand Down
17 changes: 13 additions & 4 deletions src/Message/FindCustomerRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
<?php

namespace Omnipay\Braintree\Message;

use Braintree\Exception\NotFound;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

/**
* Find Customer Request
*
* @method CustomerResponse send()
*/
class FindCustomerRequest extends AbstractRequest
Expand All @@ -16,12 +19,18 @@ public function getData()
/**
* Send the request with specified data
*
* @param mixed $data The data to send
* @return CustomerResponse
* @param mixed $data
*
* @return \Omnipay\Braintree\Message\CustomerResponse|\Omnipay\Common\Message\ResponseInterface
* @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
*/
public function sendData($data)
{
$response = $this->braintree->customer()->find($this->getCustomerId());
try {
$response = $this->braintree->customer()->find($this->getCustomerId());
} catch (NotFound $exception) {
throw new NotFoundHttpException($exception->getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't throw this error, but rather Omnipay\Common\Exception\InvalidResponseException. This error could cause the application to render a 404 page instead of an error page (for example Laravel converts the HttpExceptions to specific errors)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your comment. I agree with you, but the main reason I'm throwing not found exception is for those (like me), who wants to recreate payment profile if the existing one is invalid or not found, without showing the error. So I'm throwing the general not found here, catching it in my Billable trait and recreating the profile. And I used generic Not Found instead of Braintree's one to have the logic working with other gateways also.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's better then to create your own exception that extends the omnipay invalidresponse exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, the problem is that if I create may own in this package, I can't catch it generalized, because other payment gateway packages will not have that exception. That's why I used Symphony's one

Copy link
Member

Choose a reason for hiding this comment

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

If you extend the InvalidResponse one, all gateway can catch that. You can just add specific logic for that exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created, here is the PR for Omnipay Common thephpleague/omnipay-common#187 and I've updated this one as well

Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the try/catch for now and leave it as before, so we can merge this? We can update it once we merge the PR and a new tag is released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, just 2 questions before I remove:

  1. When you think the tag will be released?
  2. May I leave the previous version with Symphony's NotFound until the release of Common package? To be able to handle it without hardcoding "Braintree" namespace in my application

Copy link
Member

Choose a reason for hiding this comment

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

  1. probably with a week or so, but not sure.
  2. No but you could use a gateway specific exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, done

}

return $this->response = new CustomerResponse($this, $response);
}
Expand Down
14 changes: 12 additions & 2 deletions src/Message/UpdatePaymentMethodRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

/**
* Update PaymentMethod Request
*
* @method Response send()
*/
class UpdatePaymentMethodRequest extends AbstractRequest
Expand All @@ -28,11 +27,12 @@ public function getData()
* Send the request with specified data
*
* @param mixed $data The data to send
*
* @return ResponseInterface
*/
public function sendData($data)
{
$response = $this->braintree->paymentMethod()->update($data['token'], $data['options']);
$response = $this->braintree->paymentMethod()->update($data['token'], $data);
Copy link
Member

Choose a reason for hiding this comment

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

Is this change related to the v3 upgrade?

Copy link
Contributor Author

@ptuchik ptuchik May 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

No, this was a bug in previous version. I fixed there on my fork when using with v2.x and it is still actual.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested with $data['options'] ?? [] instead of data, it is not working because of Braintree\Util's validation. So the only way it will work is the one I've commited


return $this->createResponse($response);
}
Expand All @@ -47,6 +47,16 @@ public function setPaymentMethodToken($value)
return $this->setParameter('token', $value);
}

/**
* @param $value
*
* @return \Omnipay\Common\Message\AbstractRequest
*/
public function setMakeDefault($value)
{
return $this->setOptions(['makeDefault' => (bool) $value]);
}

/**
* @param array $options
*
Expand Down