silverstripe-archive / silverstripe-payment

SilverStripe Payment Module
Other
24 stars 43 forks source link

Lookups implementation #20

Open jedateach opened 12 years ago

jedateach commented 12 years ago

Payment lookup is a common feature for gateways that allows payment status to be checked on the remote payment servers. I think we should provide an abstracted approach for making these lookups, and also clearly define and document the expected behaviour.

When to do a look up:

The lookup should not overwrite the Payment model's status if the lookup function simply failed to connect to the server. Likewise if there is some other error that doesn't relate to the actual transaction. We don't want a lookup to cause successful (or valid failed) payments to be changed for the wrong reasons. I guess a lookup is mainly really appropriate when we don't have a known completed status yet.

It may be useful to have a button in the CMS that triggers a lookup, if payment status is pending/incomplete etc.

frankmullenger commented 12 years ago

Agree that some guidelines should be set. IIRC the problem is that in our complete action that is redirected to from the gateway we mark a payment as successful which provides the opportunity for a hacker to abort payment on the gateway and request the /complete URL to mark a payment as successful.

Should we be assuming that payment is unsuccessful until the lookup is performed to confirm? Some gateways might not have a lookup API - for those gateways should we assume payment is successful in complete?

At the moment I believe the module is flexible enough for devs to do either when they implement a gateway which is good but could lead to inconsistent behaviour without those guidelines I guess.

If the lookup API does not connect being able to add a button to the CMS to trigger the lookup action would be great, but might add a bit of complexity and be outside the scope of the module? At the very least clients should be able to log in to their gateway to check a payment status directly.

jedateach commented 12 years ago

I guess we'll understand a bit better as we implement more gateways. Yes, some gateways won't have this feature, so if it is implemented, then it shouldn't clutter the standard API. Dependency injection? :P

...this also raises the question of how payments can be manually updated. Should the option to change payment status only be given when status is Incomplete or Pending, or should it always be changeable? Perhaps it's a permission thing. We should think a bit further about CMS integration of the module, or perhaps just prototype some ideas on a branch.