Docs

[alexrussell]

This package is in serious need of documentation. The standard docs for Omnipay are good enough to get something started but then it's just roadblock after roadblock using trial-and-error to actually complete the job. It doesn't help that Omnipay's normal docs kinda assume a less awkward payment gateway. The way Sage Pay works (I've had to integrate it many times so I feel your pain here) means that you have to do a bunch of extra stuff (especially Sage Pay Direct which I'm using) that isn't covered in the Omnipay docs (as it's specific to Sage Pay) and there isn't any documentation for this package so it's not there either.

I am aware of a vague start to documentation up at @judgej's fork but at the moment it's less helpful for actually using the package and more for thinking about what the package is trying to do under the hood (which, in a way, the whole point of using Omnipay is to not have to care about this stuff).

Something with a few full-on code samples (Server and Direct methods, 3D-secure and no 3D-secure) to actually get people going would be super handy. For example, I've found that I have to pass the normal Omnipay firstName and lastName in the card data (so the Sage Pay cardholder field is populated), but also Sage Pay's own billingFirstnames and billingSurname (I'd have thought the package could do this for me, but maybe Omnipay's too abstracted to allow this kind of stuff, I don't know). Similarly, nothing is mentioned about how to handle the incoming 3D-secure return. I guessed at Omnipay's completePurchase method, but the docs for completePurchase specifically say that for a direct integration it's not used, which threw me off the scent.

[greydnls]

I'm working on documentation, and I'm always open for questions. Sorry you've been hitting so many hurdles with it.

Regarding the first name/last name thing, that might be something we can fix behind the scenes, actually. I'd be willing to accept a PR, elsewise I can take a look at implementing it soon.

[alexrussell]

I think all that's really required for the time being is a few code samples in the readme. Let the developer know, once they have the gateway with Omnipay::create('Sagepay_Direct') what they can (test mode, simulator mode) and should (set vendor name) do with it. I had to look at the output of getDefaultParameters to work out what I could set on the gateway instance (and even then I didn't believe it only had three options so I looked at the source, turns out it was correct and I just had weird ideas about my integration). Also, the code samples should explain what a dev should do about the potential responses they get (this is covered to an extent in the Omnipay docs, but doesn't fully explain the fact that a 3D secure redirect requires a completePurchase on return).

In the end, I had a look at the unit tests to work out what to actually do (e.g. returnUrl and transactionId). Maybe this is my extremely limited knowledge of Omnipay, however, but even the Omnipay docs weren't overly clear on this kind of stuff either, just suggesting that some gateways may have different requirements to others but I haven't found anything saying exactly what Sage Pay does require/expect.

Also I think the unit tests, while handy, really need to be integration tests. Mocking the request/response is great for testing that the code handles some well-crafted responses properly, but testing that it will actually talk to Sage Pay seems pretty sensible to me. The getValidCard method is handy, but you also need to account for invalid cards and the kinds of failures that could result from that (e.g. the billingFirstnames key being required like I found - if the tests were actually speaking to the simulator, this would be flagged up). That said, I'm aware that for the most part, tests should not really rely on communication with external sources. I just think that in this case (payment gateways), if there's a way to do an integration test, a few should be done to ensure that Sage Pay still works the way the code assumes. Also the tests don't seem to test a 3D secure return (i.e. completePurchase) which would have been some documentation when I was looking at the tests for how to work with the package.

[judgej]

If there is somewhere to put it, I'm more than happy to share code I've used in the callback controller, with explanations of what each bit does. For me, understanding the flow of data (which applies to most gateways) is pretty important when implementing. It might just be me, but I can stare at a piece of code in a driver all day and still not understand what it does. But once I have a clear picture in my head of what data is moved from where and to where, and what happens when it gets there, the code takes on a life of its own, because you can mentally hang that code on the parts of that process that it interacts with.

I think using the (or a) WIKI would help immensely with gathering these ideas together. It was not until I set up a WIKI in our office earlier this year for dumping all our procedures, links, lists, usful technical details etc. that it has really struck home just how much shit we need to carry around in our heads, with easy access, constantly.

I agree with what you say about the tests. Putting in the unit tests are great for confirming a change you, or the next person, makes has not broken some basic functionality. But real interactions with the remote gateway are another thing altogether. Integration tests - how do they even work in a project like this? I run my own by knocking up a local script tied to my test account, but that's not in the slightest bit formal, and I'm not sure how that can be automated. It would also test that tweaks SagePay have made at their end have not changed behaviour. Since the integration tests will need a callback endpoint to be available, that complicates things even further (again, locally, it can be knocked up informally in a few lines, but it needs to run on a public URL).

[judgej]

For the validation, there is a whole bunch that can (and should) be done at the merchant site before submitting to SagePay. Every field has a mandatory/optional condition, it has valid and invalid characters, it has min and max lengths, plus other conditions that may apply. If you validate all this on the merchant site end, you can present as many of these validation error messages as you like to the end user, so they can correct them.

If you leave the validation to the remote SagePay gateway, then it aborts on the FIRST error, so you only get to know about that one error. I've found a similar thing with Authorize.Net and the DirectPost method, so you need to do as much HTML5/JS validation as you can.

Now, if we caught these errors (e.g. a missing FirstName) in OmniPay, then we are no better off than passing it to SagePay to validate. If this is caught by exception, then we only get to know about this ONE error, need to re-present the entire form to the user with details of this. If those kinds of errors are going to be caught so late, then you might as well let SagePay catch them - you get a unique error code to translate for the user anyway.

Just a side-note here, with SagePay, I always present the user with the personal details that will be submitted, before they actually get submitted. Some implementations (e.g. osCommerce and WooCommerce plugins) don't do this, and if there is any invalid data, such as hyphen in a postcode, which I get a lot for some reason) then the user is stuck - they have no idea what they can do to allow their payment to go through, and it usually involves me updating their personal details in the database to remove invalid - for SagePay - characters from their account. It's a mess having to do that.).

The tl;dr version of that last bit: SagePay has very strict rules on what personal details can and must be submitted. You can very seldom, if ever, just take the data from the user's account and post it off with a payment request, and expect the personal details and address to simply be accepted without some kind of rejection message. It has to go through a form the user can correct, or you have to be very clever at cleaning up the details to make it valid for SagePay.

---

One last thing: this may all change in a few months. SagePay, I understand, are playing catch-up with many of the more modern payment gateways (and they fully know it). They are aiming to release a JavaScript direct-post version of their gateway, so it will be following in the footsteps of Stripe, Authorize.Net et al. I got this from some hints they dropped at a presentation. Whether they do it right -- UTF-8 PLEEEASE -- is something else we will have to see.

[andy1547]

Hi @judgej , I'd greatly appreciate it if you could provide me with a link to your sample Sagepay Server code. I've already integrated Sagepay Into my Laravel 5.1 project but at the cost of high coupling... regarding storing the unique transaction ID required for identification in for the notification URL, I'm currently storing this in the database which is ugly, any thoughts on persisting that outside of a relational database?

[alexrussell]

@andy1547 out of interest, why do you feel that storing the transaction ID in the database is ugly? That's what I do (in the "attempted transactions" log for a given order/payment) so I can know which order to retrieve on return to mark the order as paid (or part-paid, implementation-depending).

As for ideas to persist outside of a DB - why not use Laravel's 'Cache' component? You can store any HP object against a string key. Sounds ideal if you aren't storing a log of transactions and each transaction is throwaway and you only care about whether it ends up paid.

[delatbabel]

Excuse me for chiming in here.

In the generic world, storing the transaction ID and other txn data in the database is almost required, and I wouldn't trust it to Laravel cache. You're going to need to do that in the following cases:

• For any redirect gateway -- when you redirect a customer away from your code to the gateway's code, you have no real idea of how long it's going to take for them to come back. Have a look at the example code in the RestPurchaseRequest message in omnipay-paypal for an example and rationale.
• For any gateway that sends notify callbacks. You have to be able to match the transaction ID that you create with whatever the callback sends, and you can't always trust the gateway to send you back a useful or unique transaction reference. Most gateways support a per-transaction notify URL so that you can include the transaction ID in the URL to help you match.

Because these are fairly generic cases, I believe that it's good practice to store the transaction ID in the database in all cases -- just in case you have to switch to a different gateway that will require that.

In relation to callbacks, be aware of 3 things:

• Callbacks can arrive before the purchase() call completes. Don't rely on being able to store the transaction ID and be able to handle the callback after the end of the purchase() call, I can name at least 2 gateways that have a habit of sending callbacks before the end of the purchase() call.
• Callbacks can arrive even for transactions that fail -- some examples are MultiCards (which sends a very brief failure message in the response to the purchase() call, and then a much longer block of explanatory data in a callback some time later, and also AliPay which can send up to 5 callbacks for each failed transaction), so don't just store the transaction ID for successful transactions, do it for all of them.
• Callbacks can arrive some time after the purchase() call completes -- I have seen gateways send callbacks 2 months after the purchase() call. Storing the transaction IDs in a cache probably isn't going to work for that case.

[andy1547]

@alexrussell @delatbabel I complete agree with persisting your own transaction ID, I was talking more about the Sagepay transaction IDs and parameters which may/may not be applicable to any other gateways (excuse my limited gateway knowledge). @alexrussell When I was referring to 'ugly', I was referring to my database storing many Sagepay specific columns.

Sagepay recommends your store these values at a minimum:

VPSTxID - Sagepay's primary key/transaction registration reference. I assume this is applicable to most gateways? I've had to quote this value to Sagepay support several times when trying to debug failed transactions.

VendorTxCode - Your unique reference code, I've used my own transaction ID here. This is also used to with the security code to validate the Sagepay notification request has not been tampered with.

Security Key - used only to detect if the request has been tampered with, not needed with SSL

TxAuthNo - unique successfully completed transaction reference.

So if you decide to use your primary key as the VendorTxCode and not to use SSL you'd only need to store the TxAuthNo, VendorTxCode/primary key and the VPSTxID.

I've seen an approach to this by storing the transaction ID and Sagepay specific data as JSON in the database. Although there's the benefit of abstraction, doesn't this go against first normal form of relational databases?

[andy1547]

Additionally I'd add some validation to the notification URL code to validate the request was made from Sagepay's IP Range 195.170.169.*
This validation should be togglable to allow for testing.

[alexrussell]

@andy1547 yeah I store the SP-specific stuff as JSON in a stringy column in the DB (in fact omnipay-sagepay specifically encodes this as JSON for your convenience) so you only need one column. If you even change gateway, the same column can be used for the potentially-just-a-single-string ID. Though, of course, you now have two different types of data in one column, so maybe you need a second column for "gateway used" or something. Who knows? I don't even intend to change gateways on my projects! :)

[judgej]

@andy1547 Only the TransactionReference must be stored as a retrievable value for the Sage Pay gateway callback. It actually contains three items that are needed (those you have listed), and wrapped up into a single JSON string by OmniPay specifically so you don't need to store three Sage Pay specific fields.

Personally, I store everything that goes out, and everything that comes back, as JSON strings. Laravel wraps and unwraps that for me using eloquent, so my database is simple, but the eloquent model has all the Sage Pay fields without any effort. Other gateways use the same table. Sites I'm involved in generally use multiple gateways

Validating against SP IP address? Probably not worth it, but harmless. Checking the transaction is signed against the security key, now that is vital (OmniPay does that, as do most decent gateway plugins, but a select few don't, so be warned).

You must also store the final result in the database. What you absolutely cannot do, is to give the result to the user to carry back to your final purchase page through their browser. Instead, store the result in the database, then fetch it out again (using the transactionId in the session) after the use is redirected at the end.

Storage of this stuff: don't use a cache. A cache is temporary and will disappear at some point. You will need a permanent record of the transactions, because at some point you are going to have to fix some problems or find out what went wrong for a user.

[judgej]

@andy1547 The demo I put together for a talk is here:

https://github.com/academe/OmniPay-SagePay-Demo

That's running here (in Sage Pay Server mode):

http://acadweb.co.uk/omnipay-demo/authorize.php

Use the test cards here:

http://www.sagepay.co.uk/support/12/36/test-card-details-for-your-test-transactions

I'll fix it at some point so you don't need to create a database for it to run.

[andy1547]

@judgej I agree with you that checking the IP address is Sagepay's would not be unnecessary if you're doing the md5 check. Thanks very much for the demo 👍

[andy1547]

Is there support for recurring card payments using tokens in this API?

[judgej]

No, not yet. I'm not sure how other gateways support it, but it would be nice for it to be supported across the range in a consistent way. It may be something that becomes a lot easier in OmniPay 3.0 with the CC details split from the payor and payee details.

[alexrussell]

At my company we have recently extended this driver to support recurring (repeat) payments with Sagepay Direct. Unfortunately we have not yet tidied the code up to release (nor had any kind of sign-off that we can) but suffice it to say, it's pretty easy to add another Request class in with a couple of extra methods.

[judgej]

This just came in - token payments is a common need across gateways.

[judgej]

According to the docs here, the createCard() gateway method would be used to get a card reference that can be used later to take payment. I'm not sure how flexible it is and how many gateways use it though.

https://github.com/thephpleague/omnipay#token-billing

[delatbabel]

Many but not all gateways implement createCard().

Some gateways implement something similar by returning a card reference when a purchase() or authorize() transaction is made. For examples of this see omnipay-paymentwall and omnipay-multicards.

[delatbabel]

Personally, I store everything that goes out, and everything that comes back, as JSON strings

I follow this approach too. You are going to need to store different types of data for different gateways, and putting all of those data together as a JSON string makes sense.

I don't even intend to change gateways on my projects!

Famous last words, right up there with "they couldn't hit an elephant at this dist...". I never intended to change gateways on my first projects either, until $MANAGEMENT walked in and said "we've signed this new deal with this other credit card company and it saves us 0.01% on our transaction fees, implement it by 3pm because that's when we're going live".

[andy1547]

@delatbabel With Sagepay we had to request for the card remembering feature to be turned on for an additional charge per month (they offer standard and bespoke pricing on this). The standard Sagepay Plus package offers 500 single-click purchases per month, I'm not sure what happens when you go over this limit... I'd guess either an error is returned in the status detail field during the registration request or the user is redirected to Sagepay but made to re-input their card details.
Does anyone have any experience with going over the limit?

[vekien]

Hello!

Is there any updates on documentation, or possibly a fork/branch that is active in development? The current documentation leads to stress and frustration and does not match up to what Omnipay documentation states (eg: Card References, no clue how they work on this Gateway...)

Would really appreciate a good source to read.

[judgej]

@viion if you can take us through what you are trying to do and where you are getting stuck, then we can keep adding to the documentation as we solve those problems. The unfortunate thing is that documentation takes effort, but with many people chipping in we can get it up to speed. It is likely also that updating the documentation will also involve updating the code too, as we discover where discrepancies are.

So, Card Reference, could you point me at what you have read, and we'll get them aligned.

[vekien]

So I am reading: http://omnipay.thephpleague.com/api/token-billing/

It says

Once you have a cardReference, you can use it instead of the card
parameter when creating a charge:

$gateway->purchase(['amount' => '10.00', 'cardReference' => 'abc']);

Many places online suggest getCardReference(), however this does not exist in the registerToken (the use of Token vs Card is confusing too). It exists on the $gateway, but why would I be getting a card reference before I've purchased/authorised? So I do getToken(), I do not know how to use the token to call another purchase?

card is required, but the omnipay documentation suggests it isn't required, Isn't the whole point to have a consistent interface/api? Providing the token to either card or cardReference when performing a purchase does nothing, just an error because it's not a valid CreditCard

This is just the main blocker I'm facing right now.

[judgej]

Okay, I'll take a look if this is possible. Sage Pay did not used to support card references, but you could create a new transaction based on an existing transaction. The "token billing" is relatively new, as is the Omnipay token billing, and whether they are compatible, I'm not sure. But I'll find out for you now.

[judgej]

Both Direct and Server have a createCard() function, and Direct has a deleteCard() function. There is no updateCard() function in Sage Pay (that's probably a Stripe thing - to update, best to just delete and recreate).

When you send a createCard() request, assuming it is successful, you will get a card reference in $response->getToken(). It looks like we need a $response->getCardReference() as an alias for this.

To use that token aka card reference, leave the credit card details blank when making a payment, and pass in a token parameter with the request - $request->setToken('your-saved-card-token'); That should use the card token.

Now this is where I am a little unclear. The documentation you pointed me to talks about "token billing", but lists a cardReference as the token for a card. The AbstractRequest has parameters for both token and cardReference, and I am really not sure which one we should be used here. I suspect it should be cardReference as it is a more permanent reference for a card. The token feels more like a fleeting tokenised card used in JavaScript front ends to send card details direct to the gateway using AJAX. So I think that needs fixing here. Maybe the title "Tokenised Billing" is the wrong wording, since it leads us to this very ambiguous outcome.

Edit: having said that, the doc page is very clear that cardReference is what should be passed in, so that's a fix needed.

[judgej]

The Sage Pay gateway will also allow you to set a flag to tell it to generate a token for the card when making a payment or authorisation. This feature does not have an Omnipay equivalent (so no common parameter name that I know of), though is shared by a number of other gateways.

[judgej]

@viion I have created a PR #90 to rename some of those functions, e.g. createToken to createCardReference, and to support setCardReference() when creating a payment. This should be a lot more consistent with the core documentation. Let me know if it makes more sense. I do realise additional docs would be good, but they will come (hopefully with your help when we get this working for you).

The PR branch is here, so give it a try (at least take a look at the README):

https://github.com/academe/omnipay-sagepay/tree/issue89

There is quite a lot of refactoring in there, but the aim has always been not to bread anything, which I hope I haven't.

[judgej]

Is there anything left to do on this issue now? The documentation has been expanded substantially and new features have been added and improved. Any examples or use-cases that need to be added?

[judgej]

Please re-open or raise new issues if you believe anything here still needs to be addressed. I'll close this issue in the meantime.