Fix response#to_transaction_info_plugin

killbill / killbill-plugin-framework-ruby

Framework to write Kill Bill plugins in Ruby

http://killbill.io

8 stars 11 forks source link

Fix response#to_transaction_info_plugin #43

Open pierre opened 9 years ago

pierre commented 9 years ago

We should take a look at response#status if there is no transaction row, to check whether we need to return an UNDEFINED status or an ERROR one.

See https://github.com/killbill/killbill-plugin-framework-ruby/blob/master/lib/killbill/helpers/active_merchant/active_record/models/response.rb#L106.

sbrossie commented 9 years ago

We should define what those mean first:

ERROR: The plugin knows for SURE that payment did NOT happen
UNDEFINED: The plugin DOES NOT know if the payment happened

ERROR would be returned for instance if failure occurred PRIOR contacting the gateway or if the establishment of the connection failed. As soon as the plugin were able to connect to the gateway and send some bytes we are in UNKNOWN territory until gateway replies with a meaningful status.

A simplification would be to remove ERROR and ONLY deal with unknown (as we don't guarantee that plugin implementors will correctly return ERROR or UNKNOWN in the right scenario, and therefore assume we can treat those differently).

pierre commented 9 years ago

ERROR: The plugin knows for SURE that payment did NOT happen

[...]

ERROR would be returned for instance if failure occurred PRIOR contacting the gateway or if the establishment of the connection failed. As soon as the plugin were able to connect to the gateway and send some bytes we are in UNKNOWN territory until gateway replies with a meaningful status.

Actually, I think we should raise an exception for payments which did not go through at all (connection error, bugs, etc.) vs the ones which were declined (ERROR), so that we can distinguish between PAYMENT_FAILURE (we tried, but not enough funds, etc.) and PLUGIN_FAILURE (didn't manage to touch the payment method) in analytics.