thephpleague / omnipay-sagepay

Sage Pay driver for the Omnipay PHP payment processing library
MIT License
55 stars 78 forks source link

Possible missing step in Readme #115

Closed pmhou closed 6 years ago

pmhou commented 6 years ago

I'm working on a Server integration and have been following the steps in the readme doc but hit a snag when it came to implementing the notification handler.

// Returns instance of \Omnipay\SagePay\Message\ServerNotifyRequest
$request = $gateway->acceptNotification();

Following the readme, it states that the request variable is used to respond with invalid, error or confirm

if (! $request->isValid()) {
    // Respond to Sage Pay indicating we are not accepting anything about this message.
    // You might want to log `$request->getData()` first, for later analysis.

    $request->invalid($nextUrl, 'Signature not valid - goodbye');
}

However those methods exist within \Omnipay\SagePay\Message\ServerNotifyResponse. It appears you need to do the following, which is missed out of the readme.

// Returns instance of \Omnipay\SagePay\Message\ServerNotifyResponse
$response = $request->sendData($request->getData());

// Now you can do $response->invalid|error|confirm()

Could you confirm whether this is correct? Thanks.

judgej commented 6 years ago

If you are using the older Omnipay 2.x (or 3.0.x) then what you describe will still be the case, and the documentation for that branch reflects that.

For the newer 3.0+ 3.1.0+ release the send() step is optional, as there is only a ServerNotifyRequest class present, and that does everything that previously the ServerNotifyRequest and ServerNotifyResponse did between them. This is more consistent with Omnipay Common, which does not consider the notification handler to be a request that must be "sent".

Which version are you using? Maybe some dependencies are holding it back?

pmhou commented 6 years ago

Thanks for the quick response. My composer.lock is referencing 3.0.1

judgej commented 6 years ago

Actually, it's 3.1.0 the notify handler changed - not 3.0.x - sorry for misleading. 3.1.0 was released last night, so you may be comparing the 3.1.0 documentation with a slightly older release. Try 3.1.0 :-)

pmhou commented 6 years ago

Yep, that's done it! Thanks for confirming 👍

judgej commented 6 years ago

np - let me know if you find any other issues, as this is a new release with lots of changes, but is stable. Your name is familiar - PHPNE, FrontendNE?

pmhou commented 6 years ago

Will do. And yes, PHPNE - small world!