frankmullenger / silverstripe-payment

SilverStripe Payment Module
6 stars 5 forks source link

Payment Status inconsitent #3

Open Zauberfisch opened 10 years ago

Zauberfisch commented 10 years ago

from Payment.php Line 14 to 24:

/**
 * Method: The payment method used
 * Status:
 * - Incomplete (default): Payment created but nothing confirmed as successful
 * - Success: Payment successful
 * - Failure: Payment failed during process
 * - Pending: Payment awaiting receipt/bank transfer etc
 * - Incomplete: Payment cancelled
 * Amount: The payment amount amd currency
 * HTTPStatus: Status code of the HTTP response
 */

note the inconsistency of status Incomplete. It is first described as the default, with meaning that nothing happened yet. A few lines below it says that Incomplete means cancelled.

the documentation needs to be fixed, but first we need to clarify what Incomplete actually means.

Zauberfisch commented 10 years ago

it looks like Incomplete is not actually the default value. PaymentProcessor->setup() is where a new Payment object is created, and the status there is set to Pending

Zauberfisch commented 10 years ago

futher investigation on this matter reveals that in fact the Incomplete status is not used in the payment module at all.

the cancel handler in PaymentProcessor.php Line 310 actually creates a PaymentGateway_Failure, which in turn sets the status for the cancelled payment to Failure.

Zauberfisch commented 10 years ago

Further inconsistencies can be found in the payment modules:

Zauberfisch commented 10 years ago

to sum it up: we need to clearly define which status means what.
and fix all modules according to that definition.

frankmullenger commented 10 years ago

@Zauberfisch thanks for picking this up. I believe this line should not have been in the docs:

As you say we definitely need to clarify, initially a payment is created as Pending (awaiting actual payment to be made, appropriate for payment methods such as cheque/bank transfer).

The PayPal and PaymentExpress modules check the payment was completed by querying the API again and (iirc) this is where a payment might be marked as "Incomplete".

Zauberfisch commented 10 years ago

what would Incomplete mean than? is it another form of pending? or does Incomplete mean some kind of failure?

frankmullenger commented 10 years ago

I guess "Incomplete" is comparable to "Failure". The payment might have been made, but has not been confirmed. Perhaps it is similar to "Pending" in that sense.

Zauberfisch commented 10 years ago

You just made it sound like its both ^^

also, what if the gate way returns status pending? some gateways also handle bank transfer or bank withdraw for you. so that means the payment gateway process might have been successful, but the payment is pending, because the gateway has to withdraw the money from the bank.

so the status is still pending. but the gateway process has to be logged as successful, to know that this part was successful. could this be considered incomplete?

frankmullenger commented 10 years ago

Yes I think it could be considered "Incomplete" in that case.

Zauberfisch commented 10 years ago

ok, so how about we define it as follows:

should we add an additional status "Cancelled" as a separation between cancel and failure? Some Gateway providers support separation of those 2 states.

frankmullenger commented 10 years ago

Sounds good, if a gateway has the additional "Cancel" status maybe that could be added to the module for that particular gateway rather than the Payment module itself.