academe / SagePay-Integration

HTTP Messages for the Sage Pay REST (Pi) gateway.
GNU General Public License v3.0
9 stars 5 forks source link

Support endpoint to retrieve an existing transaction #25

Closed judgej closed 8 years ago

judgej commented 8 years ago

New endpoint https://test.sagepay.com/api/v1/transactions/<transactionId> to get a transaction.

It may also be worth trying GET https://test.sagepay.com/api/v1/transactions to see if transaction listings are available too.

judgej commented 8 years ago

Wondering also whether there is some kind of REST library we can plug into, with an adapter to map objects and responses.

judgej commented 8 years ago

A rethink perhaps on the way the API is structured. Now that "transactions" is a resource, with POST (to create a new transaction) and GET (to retrieve an existing transaction) and with child resources (GET 3d-secure) then we maybe need to think more along the lines of the resources than the functions that can be performed over the API. Learning every day!

judgej commented 8 years ago

At the moment, each message class we have, specifies a single resource (e.g. transaction) with a single verb (e.g. GET, POST). The first step would be to turn these into resources without an implied verb. The resource would also make no reference to what authentication is needed, or what the URL is.

The messages then link those resources with endpoints, verbs, authentication and HTTP headers. The messages will likely be extended from a single message, with not much more than metadata for the details. I'll have a ponder over this tonight and see how this fits in with other projects and design patterns.

The idea of this separation already exists in the Model and Message classes.

judgej commented 8 years ago

Some resources that I'm finding useful:

http://blog.madewithlove.be/post/birdseye-view-on-api/ http://docs.httplug.io/en/latest/tutorial/

judgej commented 8 years ago

One problem I see with a more generic REST framework is that the REST API does not strictly model the resources by path. For example, to make a request for a transaction, i.e. to create a transaction on Sage Pay, you need to send the full transaction details to POST /transactions. Those details include the card token, amount and all personal details of the purchaser and recipient.

What you get back is the result of authorisation, just details of that (3D Secure result, back references etc.) and not the full transaction. By fetching from GET /transactions/<transactionId> you would expect to get back the transaction resource you originally POSTed, albeit with additional details. But you don't. Instead you are returned the transaction request results, the same as you got back when POSTing the original transaction.

That doesn't seem right. I have suggested that GET /transactions/<transactionId> should return the same resource that POST /transactions sent at the start, with the current authorisation result being a child resource, perhaps GET /transactions/<transactionId>/auth-result.

Note also the authorisation result will change - it is not necessarily fixed. Its state will change between the first transaction request, and the sending of the 3D Secure PaRes, assumign 3D Secure is in operation.

judgej commented 8 years ago

The 3D Secure resource has an end-point similar to what I'm suggesting: /transactions/<transactionId>/3d-secure - works for POST at least (not tried GET, but a REST API ought to support that eventually).

judgej commented 8 years ago

So, at the moment, different resources share the same REST end points, but are accessed using different HTTP verbs. Will have to keep an eye on that.

judgej commented 8 years ago

The TransactionResultRequest message is used to request the results of a transaction, given the transactionId. It returns data for a TransactionResponse message. That latter message should probably be renamed TransactionResultResponse for consistency, but I want to see what Sage Pay do next before I do that.