thephpleague / omnipay-mollie

Mollie driver for the Omnipay PHP payment processing library
MIT License
62 stars 38 forks source link

Support for $response->isPending #66

Open Mark-H opened 5 years ago

Mark-H commented 5 years ago

Related to #18

My OmniPay implementation (2.5.2 / mollie 3.2.0) supports a wide range of gateways from a single integration, as OmniPay was meant to be used for.

When the customer returns to the checkout, the code looks somewhat like this:


try {
    $response = $gateway->completePurchase($data);

    if ($response->isSuccessful()) {
        // handle successful payment, completing their order
    }
    elseif ($response->isRedirect()) {
        // redirect the user to payment page
    }
    elseif ($response->isCancelled()) {
        //  handle cancelled payment, allowing customer to choose something else
    }
    elseif ($response->isPending()) {
        // show the payment is still awaiting (async) conformation, optionally with the payment page link
    }
    else {
        // show unknown state error
    }

} catch (\Exception $e) {
    // handle critical exception telling customer "something went seriously wrong"
}

I also ran into #18 in this situation, where the slower payment methods offered by mollie (e.g. bank transfer) would cause the customers to see a "something went seriously wrong" because the gateway threw the exception about the transactionReference not being set. In reality, the payment confirmation would come a second or two later (or a couple days later for bank transfer), so the integration is telling the customer something incorrect. I've worked around that particular issue by specifically setting the transactionReference from the stored transaction ID, but it's still not working great.

On further inspection it appears that $response->isPending() is not implemented, so while I can redirect the user back to their payment page ($response->isRedirect() is still true for open transactions, at least in test mode), I can't show the payment pending page on my own page. I think that ideally a payment that is in status "open" should cause isPending() to be true. Probably on the FetchTransactionResponse, but there might be something to say for implementing that only on the CompletePurchaseResponse

There already is an isOpen() method on the FetchTransactionResponse, but as that's specific to Mollie and doesn't exist on the abstractresponse/interface, I can't quite as easily incorporate that into my general checkout logic.

Basically, 2 questions:

  1. Is there any particular reason the gateway implements an isOpen but not an isPending with the same logic that I'm overlooking?
  2. Would a PR that adds that targeting the (admittedly fairly old) 3.2 version be accepted, or would I be better off managing my own fork until I can update my core package to use OmniPay 3?
nfourtythree commented 4 years ago

We are running into something similar. If people are taking payments via Bank Transfer on mollie, these payments often take time but people want the purchase to be complete.

At the moment Mollie returns status of open with a method of banktransfer. With this data the response continues to have: isSuccessful() return false and isRedirect() being true. This could be a prime example for the introduction of isPending().

As a crude example in the CompletePurchaseResponse class could be:

/**
 * @return bool
 */
public function isPending()
{
    // Return as pending if the purchase is a bank transfer with and `open` status.
    if (!$this->isSuccessful() && $this->isOpen() && isset($this->data['method']) && 'banktransfer' === $this->data['method']) {
        return true;
    }

    return parent::isPending();
}

Unless there is something I am missing here and there is a different way of handling these types of payments?

Mark-H commented 4 years ago

Why would the method specifically need to be banktransfer for the result to be pending?

I've since moved the Mollie payment integration off from OmniPay (because their own api is ❤️ ), but just the isOpen check should be sufficient to determine that, you know, the payment is pending. Wether that's because iDeal is taking 30 more seconds today or it's a bank transfer that will take another week - pending = pending.

nfourtythree commented 4 years ago

The thought behind that is because a credit card can also have a status of open. But those (as far as my knowledge on Mollie goes) wouldn't want to be completing orders.

I think the likelihood is that maybe moving away from the omnipay library to avoid getting "lost" in the abstraction and dealing with the mollie API directly.