thephpleague / omnipay-multisafepay

MultiSafepay driver for the Omnipay PHP payment processing library
MIT License
19 stars 21 forks source link

isSuccessful() should check for presence of timestamp #13

Closed elwin1234 closed 7 years ago

elwin1234 commented 7 years ago

Hallo / Hello,

Consider the class RestCompletePurchaseResponse

class RestCompletePurchaseResponse extends RestFetchTransactionResponse
{
    /**
     * {@inheritdoc}
     */
    public function isSuccessful()
    {
        return $this->getPaymentStatus() == 'completed';
    }
}

and the getPaymentStatus() function in RestFetchTransactionResponse

    /**
     * Get raw payment status.
     *
     * @return null|string
     */
    public function getPaymentStatus()
    {
        if (isset($this->data['data']['status'])) {
            return $this->data['data']['status'];
        }
    }

At this moment the payment is considered successful when the status is "completed". However, sometimes the PSP MultiSafepay sends two notifyMerchantTrans requests to the webshop with very little time in between. I contacted MSP about this behaviour and they told me that status updates that do not contain a timestamp should be ignored. I checked some 'double requests' and concluded that the first of the two request does not contain a timestamp in the returning parameter bag. So in order to preserve that the webshop is starting two parallel order finishing steps because the two requests are placed so close after another is it possible to implement a timestamp check?

barryvdh commented 7 years ago

So the first one should be status pending? Is this in the docs, or can you forward me the mail?

And are you sure it's always the first that can be skipped? Could lead to problems otherwise..

elwin1234 commented 7 years ago

Hi,

Not necessarily the first, but the one that does not contain a timestamp should not be considered as completed. I'm still in mail contact with MSP and will keep you informed here.

lukeholder commented 7 years ago

@elwin1234 any update on this? Can you paste the MSP conversation?

lukeholder commented 7 years ago

@elwin1234 ?

elwin1234 commented 7 years ago

Hi,

I think we consider this as fixed by now.