search

thephpleague / omnipay-common

Core components for the Omnipay PHP payment processing library
MIT License
329 stars 242 forks source link

[3.0] Roadmap (2017) #146

Closed barryvdh closed 6 years ago

barryvdh commented 7 years ago

Main goals

omnipay/omnipay

omnipay/common

Tests

Gateways

delatbabel commented 7 years ago

Can you give me a to-do list and I will try tackling the changes with one of the gateways that I'm familiar with (either Stripe or PIN or Fat Zebra I guess).

barryvdh commented 7 years ago

See the changes in https://github.com/thephpleague/omnipay-mollie/pull/34 and comments in https://github.com/thephpleague/omnipay-common/pull/137

delatbabel commented 7 years ago

OK but what I was hoping for was a step-by-step guide that we can hand off to gateway plugin developers so that they can 3-ify their code. Rather than having to say to them "have a look at some code changes that someone else made and hopefully when you do something similar it will work".

Can we wiki that up somewhere?

barryvdh commented 7 years ago

@delatbabel Added https://github.com/thephpleague/omnipay-common/blob/master/UPGRADE.md

barryvdh commented 7 years ago

Soo, did any get a chance to test the alpha? I used it on a gateway, so as far as I'm concerned, I'm gonna release a beta.

dpfaffenbauer commented 7 years ago

@barryvdh I tested the alpha and migrated worldpay to it (https://github.com/thephpleague/omnipay-worldpay/pull/29, still waiting to get merged). I am also already migrated Payum OmnipayBridge (https://github.com/Payum/Payum/issues/675), but still waiting for this to get merged (https://github.com/thephpleague/omnipay-dummy/pull/6).

But: everything is working fine on my local dev-environment.

When are you going to release a beta?

Good job 👍

barryvdh commented 7 years ago

@dpfaffenbauer Did you encounter any issues or missed something from how to upgrade?

dpfaffenbauer commented 7 years ago

In fact, I found your upgrade guide when I was already finished :D

It was kinda clear for me on how to change worldpay to make it work. If you take a look at my PR, it's actually very easy to migrate: https://github.com/thephpleague/omnipay-worldpay/pull/29/commits/87a2d03c32b713b4bacf6cb32093565f5f00f34c

barryvdh commented 7 years ago

Yeah I tried to keep it minimal :)

I want to release it soonish, only thing I'm not certain about is the HttpClient stuff. Want to get this right and there is a PSR coming for Http Clients (currently in draft), so kind of inclined to wait for that to be accepted. See https://groups.google.com/forum/#!topic/php-fig/iFZF6T9L2zA

judgej commented 7 years ago

I've done a conversion for Sage Pay in this branch: https://github.com/thephpleague/omnipay-sagepay/tree/30alpha1

A few tests I've run to the test API instance seem to work fine. I started this ages ago, and this thread has given me the kick needed to finish it off. I'm not super clear on the dependencies. I assume those will need to be updated as the core 3.0 goes to beta and then production?

barryvdh commented 7 years ago

You should be able to set the dependencies to 3.0 already, but lower your minimum stability to test it (to dev or alpha etc)

judgej commented 7 years ago

Thanks @barryvdh I really need to read up on the way composer does its matching of version numbers, which seems to include some implied regex stuff that I've never got my head around.

judgej commented 6 years ago

@barryvdh Any thoughts on the Price of an Item in the basket ItemBag for v3? At the moment it contains (I assume!) the line total, and is a float, presumably in major currency units and fractions thereof. In order to pass the price to another library that requires a money/money object, I'm having to load up a money parser to parse it, and even then it needs to be cast to a string for that to work. An example of that is here: https://github.com/academe/Omnipay-AuthorizeNetApi/blob/master/src/Message/AuthorizeRequest.php#L155

It would be nicer if the Item is already able to supply a money/money value for the price, sop the drivers don't need to make so many assumptions. It would also help in any calculations needed. For example, the Authorize.Net JSON API asks for the unit price, so in theory the line price needs to bne divided buy the Quantity before passing to the gateway.

I guess there are lots of additional properties and features the Item could do with supporting (tax, gross, net, item cost, line cost, discount, etc.) but just getting money/money into it for current fields would be a start. Is this something that should be in 3.0?

barryvdh commented 6 years ago

FYI, I want to update it to PSR-18 when accepted, and then release soon: https://groups.google.com/forum/#!topic/php-fig/7PFR-KAAcMk

judgej commented 6 years ago

Should I be able to try out a 3.x adapter using composer to install? I've not managed to get it to work yet, and cannot work out why.

For example, I can clone https://github.com/academe/Omnipay-GiroCheckout master, which is 3.x, install all its dependencies and run some tests (both unit tests in the package, and some external tests to the gateway). That all works fine.

However, if I try a composer require academe/omnipay-girocheckout in any form, in a clean empty directory, I just cannot get it to install at all. Should I not be able to do that? Even a composer require omnipay/common does not work, and the errors make no sense to me. The 2.5 branch works fine composer require omnipay/common:2.5. Same with any 2.x adapters - they simply install. But anything 3.x dos not.

Is there a simple composer command that will install ANY 3.x Omnipay adapter, just so I can see it working without thinking that I can be going mad?

judgej commented 6 years ago 
composer require "omnipay/common:v3.0-alpha.1"
...
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Installation request for omnipay/common v3.0-alpha.1 -> satisfiable by omnipay/common[v3.0-alpha.1].
    - omnipay/common v3.0-alpha.1 requires php-http/client-implementation ^1 -> no matching package found.

What is this php-http/client-implementation? Does a HTTP client need to be installed before Omnipay 3.x? Installing the dependencies from a clone of the adapter works, but installing the dev-master through composer without cloning first does not. So far as I can see, the dependency tree should be equivalent.

-bash-4.1$ composer require php-http/client-implementation

  [InvalidArgumentException]
  Could not find package php-http/client-implementation.

  Did you mean this?
      php-http/client-implementation

Has a dependency perhaps simply gone from packagist?

judgej commented 6 years ago

Okay, it looks like the HTTP client does need to be installed first. This works:

composer require php-http/guzzle6-adapter
composer require "academe/omnipay-girocheckout:~3.0"

Without installing the guzzle6 adapter (or an equivalent) first, the girocheckout fails to install. Some magical group of necessary packages masquerade as a "virtual" package called php-http/client-implementation. Being a virtual package, it is not a real package. You need to install an equivalent package.

Sorry if you already know this stuff. I didn't, so I'm sure others won't either. I think installing from the clone just happened to work because the clone was installing omnipay tests, and the tests install php-http/guzzle6-adapter.

judgej commented 6 years ago

Sorry for spamming - I'm going through a learning curve too loudly. Some points and questions though, after sleeping on it:

On the whole though, it's looking good - great work :-)

barryvdh commented 6 years ago

Yeah that part is a bit tricky and not sure what the best tradeoff between flexibility and ease-of-use is. The dev version is required for testing indeed, because guzzle adds some useful features there.

omnipay/omnipay DOES add a default client (see https://github.com/thephpleague/omnipay/blob/master/composer.json), just not omnipay/common. So libraries etc can just require common, but end-users will get a default client installed (Guzzle), but no gateway should depend on Guzzle ever.

I want to use the PSR version instead of Httplug for the interface: https://github.com/php-http/fig-standards/blob/master/proposed/http-client/http-client.md but that's not accepted still..

judgej commented 6 years ago

omnipay/omnipay DOES add a default client

Ah, that's what I read, and probably what I should have installed when testing standalone locally. Makes sense now, thanks. I'll think about what I can add to the docs to explain it.

digilist commented 6 years ago

Is there any plan to release 3.0 until a specific date? I'd like to use omnipay in a current Symfony version, which is currently unfortunately not possible because of Guzzle.

barryvdh commented 6 years ago

I'd like to wait for PSR-18 (HttpClient), but that isn't moving along so fast as I hoped.. https://groups.google.com/forum/#!topic/php-fig/7PFR-KAAcMk

barryvdh commented 6 years ago

I've bumped PHP to 7.1 because that is the last supported version (see http://php.net/supported-versions.php ) I've tagged https://github.com/thephpleague/omnipay-common/releases/tag/v3.0-alpha.2 Gateways testing should probably use a fixed version of omnipay-common.

barryvdh commented 6 years ago

FYI, I've updated the HttpClient, so any implementations using the dev versions should take note: https://github.com/thephpleague/omnipay-common/commit/46daacf063af044ba86a1ab312bed9dd212a6fe8

Best to require a fixed alpha tag.

barryvdh commented 6 years ago

So the first alpha release of the PSR HttpClient is released, so I've included that: https://github.com/php-fig/http-client/releases

As far as I'm concerned, the omnipay-common API shouldn't have to change, we only need to change the underlying HttpClient when it's updated. But the exceptions and usage is the same (unless the spec drastically changes).

So if there are no objections, we can tag a beta release and start testing some upgrades.

delatbabel commented 6 years ago

Still unwell, still only checking into github periodically but l;et me know if there's anything you need me to do especially on the PayPal or other gateways side.

barryvdh commented 6 years ago

So quite a few gateways have been upgraded, according to https://github.com/thephpleague/omnipay#payment-gateways

I've removed the dependency on the PSR http-client package, but mimic it's behavior. I think we can just use that. Did any of you encounter any problems? If not, I think we're good to go for stable.

barryvdh commented 6 years ago

It's tagged. Not all gateways yet, but upgrade guide is here: https://omnipay.thephpleague.com/changelog/