zburke / omnipay-bluepay

BluePay driver for the Omnipay PHP payment processing library
MIT License
1 stars 7 forks source link

Response with status 0 is not successful #3

Closed acarpio89 closed 6 years ago

acarpio89 commented 6 years ago

There's two issues here:

STATUS Code for the final status of the transaction. 1 for approved, 0 for declined or E for error. https://www.bluepay.com/sites/default/files/documentation/BluePay_bp10emu/BluePay%201-0%20Emulator.txt

acarpio89 commented 6 years ago

@zburke any reason why STATUS == 0 is considered a successful payment by this library?

zburke commented 6 years ago

A decline might seem like a failed payment status, but it's a valid transaction status in the sense that there were no errors. Take a look at some other OmniPay libraries and see how they handle declines. If they treat it differently, I'd be happy to change it.

You're certainly right about using strlen instead of count. If you file that in a separate PR I'd be happy to merge it.

acarpio89 commented 6 years ago

Thanks for the quick reply @zburke

After looking at a few popular omnipay repositories, I'll admit it's not exactly clear what's the meaning of isSuccessful(). I do have a few examples where only authorized transactions seem to be considered successful:

https://github.com/thephpleague/omnipay-worldpay/blob/master/src/Message/CompletePurchaseResponse.php

https://github.com/thephpleague/omnipay-eway/blob/master/src/Message/DirectResponse.php

https://github.com/thephpleague/omnipay-securepay/blob/master/src/Message/DirectPostCompletePurchaseResponse.php

I use a few omnipay libraries and have always relied on isSuccessful being false for declined transactions. Maybe that was an incorrect assumption, but this is certainly the first one I use that treats declined payments as successful. In your client code, how do you determine that payment was not declined? manually checking the code/error message? I've always thought that kind of decision is precisely what omnipay tries to encapsulate.

acarpio89 commented 6 years ago

Check out a Stripe response for a declined transaction:

array:1 [▼
  "error" => array:6 [▼
    "charge" => "ch_1ChEgX2sGmU2F3r7oboSqryH"
    "code" => "card_declined"
    "decline_code" => "generic_decline"
    "doc_url" => "https://stripe.com/docs/error-codes/card-declined"
    "message" => "Your card was declined."
    "type" => "card_error"
  ]
]

This is the omnipay/stripe implementation of isSuccesful:

public function isSuccessful()
    {
        return !isset($this->data['error']);
    }

It returns false when a transaction is declined. No doubt about it.

zburke commented 6 years ago

Note that if we implemented isSuccessful() strictly the same way as Stripe, i.e. "not an error", then declines would be treated as successful because BluePay does not necessarily consider a decline to be an error. So, Stripe considers a "decline" to be an error; BluePay does not.

The issue is what OmniPay is trying to capture with isSuccessful: is it a boolean describing the presence/absence of errors, or a boolean describing whether a transaction was approved? Is there any guidance from OmniPay in their documentation about this?

acarpio89 commented 6 years ago

Check out how BluePay's official library determines if a transaction was successful: https://github.com/BluePay/BluePay-Sample-Code/blob/7bad88513532e7314fafb84fe24d20e003fc8500/PHP/BluePay.php#L962

STATUS == DECLINED may not be considered an error, but it definitely isn't successful.

zburke commented 6 years ago

Check out how BluePay's official library determines if a transaction was successful...

Alright; that's good enough for me.