Refund and Capture on SagePay Server

lukeholder commented 8 years ago

Both supportsRefund and supportsCapture return true even though the gateway does not support either.

This is due to ServerGateway extends DirectGateway which inherits the applicable methods.

Should we add the following methods to the ServerGateway class?

public function supportsRefund()
{
    return false;
}

public function supportsCapture()
{
    return false;
}

?

Also, anyone know when are the latest master acceptNotification' changes going to cut into a tagged release?

judgej commented 8 years ago

Yes, that makes sense.

I'm putting some time aside this weekend to work on tagging and getting some further PRs merged.

lukeholder commented 8 years ago

@judgej great! How much work would it be to get refund and capture in? I have submitted a pull request that returns false both both supports methods, but it is failing the tests probably because of something in omnipay common's AbstractGatewayTest.

danleecreates commented 8 years ago

@judgej

So as I understand it SagePay Server does not support refund and capture?

Can we get support for refund and capture built in? Is it even possible with SagePay Server?
I believe that looking from the documentation refund and capture can happen from the SagePay UI, how do they communicate it back to the notification URL?

judgej commented 8 years ago

Sage Pay offers Direct features (server-to-server) and Server features (involving the user being taken offsite or put into an iframe). To support both of these are what Sage Pay describes as Shared features. The shared features are strictly server-to-server, so I would argue they belong in the Direct part of the gateway driver, and include such methods as capture and refund.

So you could use Server/Authorize to authorise a payment, then Direct/Capture to capture the authorized funds, and then Direct/Refund to refund it. We could provide a Server/Capture feature, but it would be identical to Direct/Capture, so not serve any purpose than padding out with more classes.

So yes, refund and capture are supported by Sage Pay, and it does not matter whether you made the initial payment or authorization using the Server or Direct feature - they all ultimately go to the same place in the Sage Pay gateway servers.

Refund is in the master branch here, but not tagged as stable yet. Capture has always been available.

lukeholder commented 8 years ago

@judgej so it is correct that ServerGateway extends DirectGateway and that both of those supports methods should return true.

This might be a separate ticket then but we are getting an error on refund (and I assume also capture) due to:

$reference = json_decode($this->getTransactionReference(), true);

This should decode the json stored in the transactionReference and does, but the reference is expecting a TxAuthNo which does not exist from the original transactionReference data we save on our local transaction:

        $data['RelatedVendorTxCode'] = $reference['VendorTxCode'];
        $data['RelatedVPSTxId'] = $reference['VPSTxId'];
        $data['RelatedSecurityKey'] = $reference['SecurityKey'];
        $data['RelatedTxAuthNo'] = $reference['TxAuthNo'];

VendorTxCode, VPSTxId, and SecurityKey all exist:

Error:

Is this because it was never added to the transactionReference returned in the notify request? https://github.com/thephpleague/omnipay-sagepay/blob/master/src/Message/ServerNotifyRequest.php#L200-L205

judgej commented 8 years ago

The transactionReference you will have saved would have been from the response to the initial authorize() or purchase() request. The one you are using and showing on your screen shot - where did it come from? It should always have a TxAuthNo.

lukeholder commented 8 years ago

These response I get does not include it:

See screencast of live me live xdebuggin of the initial purchase request response (which returns a redirect response) it does not include txAuthNo.

screencast: https://jumpshare.com/v/4OS6mXVIiVvMCIOTQNpH

Should I be re-saving the transactionReference on the acceptNotification request handler?

lukeholder commented 8 years ago

So yeah, looks like that is it. There is no txAuthNo in the initial purchase redirect response, only in the notification.

So resaving the transactionReference to my local transaction has the txAuthNo present:

screenshot: http://jmp.sh/UFzp6lM

lukeholder commented 8 years ago

So now when I try to do a refund on that resaved transactionReference I saved locally I get this error:

3010 : The Relatedvpstxid Is Invalid.

So it seems there is a mismatch on the data I am using to refund.

lukeholder commented 8 years ago

Purchase Request saved Transaction Reference:

{“SecurityKey":"TGWLORDGDK","VPSTxId":"{4D77DE4F-F4DC-40B3-E7C3-0E54517A8D1C}","VendorTxCode":"203"}

Accept Notification Request saved Transaction Reference:

{"SecurityKey":"TGWLORDGDK","TxAuthNo":"12520609","VPSTxId":"{4D77DE4F-F4DC-40B3-E7C3-0E54517A8D1C}","VendorTxCode":"203"}

Refund Transaction Reference set on refund request:

{"SecurityKey":"TGWLORDGDK","TxAuthNo":"12520609","VPSTxId":"{4D77DE4F-F4DC-40B3-E7C3-0E54517A8D1C}","VendorTxCode":"203"}

Getting 3010 : The Relatedvpstxid Is Invalid. on the above.

judgej commented 8 years ago

The mystery one to me is the initial transaction reference. I'm wondering - is this with 3D Secure enabled? I don't think Sage Pay get the TxAuthNo until the transaction is authorised by the bank, which does not happen until the user passes the 3D Secure stage.

The docs say about TxAuthNo on the initial transaction:

Only present if Status is OK.

With 3D Secure, the status will be "3DAUTH" so not "OK". Let me have a play to see if I can get the documentation a little more robust.

judgej commented 8 years ago

Here is a simple Direct payment and refund that works for me as a starting point:

$gateway = OmniPay::create('SagePay\Direct');u
// Then set up credentials here.

// Make a direct payment. Set up the card details. No 3D Secure is enabled.
$transactionId = 'txn-id-' . rand(1000000000, 9999999999);
$request = $gateway->purchase(array(
    'amount' => '9.99',
    'currency' => 'GBP',
    'card' => $card,
    'transactionId' => $transactionId,
    'description' => 'Demo payment',
));
$response = $request->send();

$transaction_reference = $response->getTransactionReference();

$refund_transactionId = 'rfd-id-' . rand(1000000000, 9999999999);
$response = $gateway->refund([
    'amount' => 9.99,
    'currency' => 'GBP',
    'transactionReference' => $transaction_reference,
    'transactionId' => $refund_transactionId,
    'description' => 'Some kind of refund',
])->send();

echo "<p>Refund transaction reference:</p>";
echo "<p>" . $transaction_reference . "</p>";

echo "<p>Transaction reference for the refund:</p>";
echo "<p>" . $response->getTransactionReference() . "</p>";

I guess we need examples with Direct and Server, and for both, with and without 3D Secure. The full transactionReference will then be available at different stages in each of those four circumstances, which I want to document.

lukeholder commented 8 years ago

OK, sorry to waste your time. Looks like this is due to the sagepay test account I was using. Both 'refunds' and 'authorize' was turned off for the test account... using another test account let me refund a transaction.

One issue I have found though, when capturing an authed transaction, saving the transaction reference of the capture response does not contain all the data to do a refund on that captured transaction. I needed to get the data from the original auth response in order to do a refund.

Would be good if the capture response->getTransactionReference was populated with the same data as the original authorization.

lukeholder commented 8 years ago

Also a side note, the SagePay documentation says:

ERROR should be used very rarely, and should ONLY be sent if something unforeseen has happened on your server or database (if you receive a notification POST for a transaction you cannot find, for instance).

But in the example in the README, error was returned when we had already marked the payment as successful or 'already paid'. I was finding, if for some reason sagepay hit my acceptNotification endpoint twice, the second time I would return $response->error('already paid') and they would thus mark the payment as failed on their side:

screenshot: http://jmp.sh/JtZAKcW

judgej commented 8 years ago

Don't worry about it - happened to me too. The Sage Pay admin login does not tell you ALL the services that are enabled. I just asked them to turn on everything, which they did pretty quickly :-)

I'm still going to look into the auth number a little further, to see which circumstances it is available in the initial response, and which circumstances it is not available until the notify callback.

judgej commented 8 years ago

The multiple notifications are strange. I guess if you get an identical notification to one you have already processed, it makes sense to return the same result as last time, assuming the result somehow got lost the first time. You are right, the "error" result is for when your application says, "WTF is this; agh my database is melting?!"

judgej commented 8 years ago

Okay, I've done some tests, and this are the conclusions wrt the transactionReference. The pattern in the response to the initial transaction registration is:

VendorTxCode is always in the transactionReference. This is hardly surprising since the merchant site provides it.
Both authorize and purchase behave the same.
The TXAuthNo is never available if the initial transaction registration fails.
The TXAuthNo is never available if a redirect is required. This is with Direct+3D Secure, or Server in all cases.

So the initial transactionReference is the final transactionReference only if registering a success valid transaction through the Direct interface with NO 3D Secure. In all other [successful] cases the transactionReference must be fetched at a later stage. That is then the final reference to be used for capture, refund, void etc. These stages are:

In all Server transactions through the acceptNotification callback.
In Direct transactions with 3D Secure through the response to the completeAuthorize request.

I'll get some documentation in place to explain this. In short, the developer needs to know when to save the transactionReference, and when it can (and must) be safely updated with a more complete transactionReference. The very essence of this is that if the response or notification from Sage Pay contains a TXAuthNo field, that that message contains the transactionReference that must be saved for future use (capture, void, refund, repeat, etc.).

judgej commented 8 years ago

One thing to note that we have never supported in this gateway: Sage Pay also integrates with PayPal. This is handled as a redirect in a very similar way in which 3D Secure is handled. I would expect behaviour of the gateway to be very similar when using PayPal to pay instead of a credit card.

judgej commented 7 years ago

Check out the latest documentation for the notification handler. I've pushed a number of changes, and it can be amended if it still does not properly describe the process of events in a payment:

https://github.com/thephpleague/omnipay-sagepay#notification-handler

If you are all happy with that, and the shared support for refund and capture (and void and abort) on both Direct and Server, then I think this issue can be closed. It sill needs to be flagged as a release though (should we close issues before that happens, or just tag them as "pending release" - hmmm).

lukeholder commented 7 years ago

Closing, refund and capture working, you are right, they use the direct driver fine.

thephpleague / omnipay-sagepay

Refund and Capture on SagePay Server #76