Closed benjam-es closed 2 years ago
Interested in integrating along side this for our implementation. We would rather commit to this repository & get it working here mind you.
Is there anything outstanding work wise on this PR? I'm happy to review it. Worth noting that I'll likely be tinkering with this locally while we develop.
More than happy to either bring https://github.com/thephpleague/omnipay-sagepay/pull/169 into this PR, or offer the same assistance for that PR.
Would be nice to get these pull requests moving as there is a lot of value in them. @barryvdh
@JoshStaff I just hacked together something to get my implementation working. It would probably need reviewing in detail to ensure it is written well enough, and inline with the rest of the package.
Was intended as a starting point to help the maintainer along, but I never really heard back since July.
@JoshStaff. Part of the problem is that I wanted to originally incorporate a way to work with Sagepay PI, so I hacked that together, then I needed protocol 4 to also work with my PI code, so this pulled request is already covering two things, which isn't ideal.
In a perfect world, we would need a maintainer to merge one of the pull requests for protocol 4, then a separate pull request for Opayo PI, and then finally compatibility with PHP 8.x
PHP 8 couple of tests broken
I'm not familiar with this gateway. What do you think @judgej ?
I needed to get SagePay Pi (now Opayo Pi) fixed on an existing project and ended up with this https://github.com/academe/opayo-pi I may be of some help. It's not Omnipay, but comes from an old hand-written PSR-7 package, before any OpenAPI descriptions were available. Starting from scratch today, I would use the OpenAPI Generator project to create the PSR-7 DTOs for all the messages, then wrap those up in an Omnipay package.
Now, the process for how this gateway works, is pushing the boundaries of Omnipay's normal processes, so it feels like putting a square peg into a round hole to create something that works, but is not an easy swap for other gateways. But maybe it's the round holes that need expanding and squaring a bit, so a kind of evolution?
I would personally not try to squeeze Opayo Pi into this Server/Direct/Form Omnipay package. There is really nothing that they share with Pi to make the effort worth while. I would start with a fresh package, and hopefuly there is something useful in the link above. A separate package would be so much simpler to manage IMO.
I'll help as much as I can, but really overloaded with project work at the moment (and very aware of the looming 3DS v2 technical debt deadlines). I'm happy testing/merging, but it's got to be in small steps so it's clear exactly what is being changed. I know it's too easy to end up putting lots of changes in one PR, then it's hard to pick it apart. But that's how it goes :-)
Not sure if that has helped at all?
I think what's missing is the documentation on how to use the Pi (aka "rest", though it's not really) API. There is lots of new requests and models in this PR. It needs at least a quickstart few paragraphs to try it out and see how it works.
So, if there are any details available on how to use the Pi API, I will give it a try.
Part of the problem is that I hacked together something to work for PI, and then had to add in the changes for protocol v4.
For everyone else, there may be a simpler solution for v4 (and 3DS V2), without using the Payment Integration (PI) integration.
@judgej If you wanted to use #163 as a base, and use mine to fill any gaps you might have, I'll happily help get the package to a point where users will be able to carry on using it when 3DS v2 kicks in fully.
That would allow you to hopefully address the upcoming cutoff point, without the PI side muddying the waters
@JoshStaff and I ended up forking this for our own purposes. Unfortunately it's probably not in a state where it can be merged upstream, but anyone is welcome to take a look at it and reuse anything that might be useful: https://github.com/openplayuk/omnipay-opayo
Changes include:
We've followed the omnipay interface as much as possible, and the parameters used for the requests simply map to what's in the Opayo docs https://developer-eu.elavon.com/docs/opayo/spec/api-reference-0. However, we haven't added specific documentation to this package as it has for the other implementations.
@judgej I can see the merit to breaking this out into its own omnipay package because, as you say, there's very little shared with the Server/Direct/Form implementations and in fact some of the bugs we found were due to relying on an overlap that didn't actually exist - even things like transaction status values are different.
Unfortunately, like you, we just don't have the capacity at the moment to maintain this and just had to get something working for our needs but, again, anyone else is welcome to run with it if they can make sense of it 😅
@judgej i've updated the original comment at the top from the pull request with some idea of what would be used for a PI transaction
@adam-openplay Any chance you can create a branch from your fork, and revert the name changes (just in a temporary throw away branch)... will make it easier to read the diff without so many name only changes
@adam-openplay Any chance you can create a branch from your fork, and revert the name changes (just in a temporary throw away branch)... will make it easier to read the diff without so many name only changes
@benjam-es this is a very quick and dirty revert with conflicts resolved so no guarantee it will actually work but hopefully a cleaner diff https://github.com/openplayuk/omnipay-opayo/tree/temp-revert-opayo-rename
All of the changes look good to me on first scan
If anyone else who doesn't use Sagepay/Opayo PI could test out Adam's version, then that might be a good reference point for thephpleague to pull from.
So is this a new feature? Let's stick to 4.x gateway support + PHP8 support first and tag that. Merged https://github.com/thephpleague/omnipay-sagepay/pull/173 for now.
I'll close my pull requests, as there are better options for protocol 4 which all seem to address it well, and hopefully others will test. Hopefully that will close most/all protocol 4 and PI issues/pull requests.
Worth revisiting the PI option, maybe with Adam's code further in a separate pull request as you say above.
@adam-openplay any chance you are able to merge in the changes from #173 into your version (including the temp branch)?
Would mean your package would track the up to date master here, and then I could potentially refer back to yours for an office "PI" pull request back here
This is a similar pull request dealing with #43 to replace pull request #164.
In addition, this is my working version for the newer 3d secure version.
Example request data (I have an initialiseData() method for this)
Initialise the sage rest gateway
Get a merchant session token for use within the frontend
On a purchase end point (the sage pi code would use merchant session token, and user details to get a card token)
On another end point for 'completion' to recieve the return from 3dsecure