Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended payment status constants in NotificationInterface #153

Open
aimeos opened this issue May 1, 2017 · 3 comments
Open

Extended payment status constants in NotificationInterface #153

aimeos opened this issue May 1, 2017 · 3 comments

Comments

@aimeos
Copy link
Contributor

aimeos commented May 1, 2017

Currently, the NotificationInterface only supports, "success", "pending" and "failed" status values:
https://github.com/thephpleague/omnipay-common/blob/master/src/Common/Message/NotificationInterface.php

But Omnipay itself has support for authorize, refund and void methods as well:
https://github.com/thephpleague/omnipay-common/blob/master/src/Common/AbstractGateway.php#L184

To have the same level of support in both, NotificationInterface and in the gateways, we need at least these additional payment status constants:

  • "authorized"
  • "refunded"
  • "void" (or "cancelled")

"pending" is not the same as "authorized" as gateways return "pending" if the payment status isn't clear yet while "authorized" if the money can be captured now. There are also payment status values sent by gateways that can't be mapped to one of those status values. Thus, getTranscationsStatus() should return a value for "don't change current status", maybe NULL. We've seen in Jason's PayOne gateway (https://github.com/academe/OmniPay-Payone/blob/master/src/Message/ShopTransactionStatusServerRequest.php#L191) that mapping every unknown payment status to "failed" will lead to a false status because PayOne sends several updates with different notification values after the initial payment.

@lukeholder
Copy link

This is a major issue with omnipay. We need someone to take some architectural leadership and fix this across all gateways.

thephpleague/omnipay#433

@lukeholder
Copy link

Anyone interested in having a google hangout on this. Whats the plan with Omnipay 2?

@judgej
Copy link
Member

judgej commented May 13, 2017

Playing with Wirecard now, and notifications are being received for all sorts - config errors, session timeouts, etc. The paymentState does just fall into one of four states in the notification though: success, failure, cancel and pending. They are all payment results though, so evem there, there is a mix of purposes for the paymentState.

With findal statuses such as "refunded", it's a little more complicated. Is that a final state on the unitial transaction? I don't think so, because to be refunded, the opriginal transaction must have been a success, and it ended there. The refund is a completely new transaction (referencing the original) and that has its own state diagram. So the refund can be a success or a failure too.

In some gateways, void is handled the same way. In others it is a void against the original transaction. The SagePay REST API accepts void as an "instruction", something I haven't quite got to the bottom of yet - is the instruction another transaction, or is it something that can take the original transaction to a new state? The transaction may have been successful at noon, but it can still be voided later on that day, before it is cleared by the bank.

So I think we have some complex state diagrams to deal with, which can be more complex than we think at first sight ("success" isn't the end of it until clearing at midnight). And we have to understand that some things which appear to be actions on existing transactions, are actually completely new transactions (e.g. refunds). And I'm not sure Omnipay makes it entrely clear enough how it fits into those models.

Owch, my head is spinnning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants