ServerAbstract::isPaymentSuccess() is an infinite loop about sagepay HOT 4 CLOSED

Should it do this, or something more complicated?

    public function isPaymentSuccess()
    {
        $this->checkTxModel();
        return $this->getField('STATUS') == 'OK;
    }

from sagepay.

The transaction model got let out, somehow.

public function isPaymentSuccess()
{
    $this->checkTxModel();
    return $this->tx_model->isPaymentSuccess();
}

The $this->checkTxModel(); line checks if a transaction model has been injected, and raises an exception if it has not. I am thinking of changing the ServiceAbstract so that it takes a transaction abstract as a constructor parameter, then we know it will always be set and so don't need to keep checking. It seems to be the way many frameworks do it, and there are dependency injection containers that make the dependencies easy to handle (e.g. pimple).

from sagepay.

Change made, tested and pushed.

isPaymentSuccess() will check that the payment was successful after the user has done their bit to authorise the payment, so will only be valid after the notification callback has completed and the user is sent back from SagePay to the application. It includes three possible statuses, all of which mean the payment was successful, but which leave the transaction is slightly different states: OK, AUTHENTICATED, REGISTERED. They all mean slightly different things, but ultimately mean, "yes, the bank has agreed to honour this transaction".

from sagepay.

Awesome, thanks!

I'd agree, passing the storage into the constructor would be neater.

from sagepay.