limegrow / ingenico-m2-payments

2 stars 15 forks source link

Error: Undefined class constant 'CORE_CODE' #23

Closed bramstroker closed 2 years ago

bramstroker commented 3 years ago

On cancelling an order in Magento the following error will arise when the order was made using another payment method than one of the Ingenico methods. Error: Undefined class constant 'CORE_CODE'

This issue originates at https://github.com/limegrow/ingenico-m2-payments/blob/master/Model/Connector.php#L1251.

Here the constant CORE_CODE is directly called, but this constant is part of the ingenico module and only available on Ingenico payment methods, so this will result in a fatal error when calling on a payment method from another PSP.

This same problem can happen in getQuotePaymentMethod, and maybe other places.

There are also several code smells in this method.

The "clean" fix imo would be to add a method getCoreCode() (not sure what this code is anyway) to AbstractMethod, of better yet introduce a interface for this.

Than the method getOrderPaymentMethod can be refactored like this:

public function getOrderPaymentMethod($orderId)
{
    try {
        $order = $this->processor->getOrderByIncrementId($orderId);
        $method = $order->getPayment()->getMethodInstance();
        if ($method instanceof AbstractMethod) {
            return $method->getCoreCode();
        } else {
            return $method->getCode();
        }
    } catch (\Exception $exception) {
        return false;
    }
}
olegisk commented 3 years ago

@bramstroker Thank you for your comment. I think we can refactor it in the future. Btw, do you think somebody will use getOrderPaymentMethod() for non-Ingenico orders? :-)

bramstroker commented 3 years ago

Well it is called for non ingenico orders, that's why I create this issue in the first place, as it breaks the cancel flow at the moment.

The ingenico module has an observer on order_cancel_after. Which will trigger the following calls.

Nowhere in this flow is checked that the particular order is payed with Ingenico.

This could potentially be a problem not only with the cancel flow, because the method IngenicoClient\IngenicoCoreLibrary::getOrder is called from a lot of places.

olegisk commented 3 years ago

@bramstroker Thank you very much. We will fix it

bramstroker commented 3 years ago

Thanks for the quick headsup

bramstroker commented 3 years ago

@olegisk Any update on this one?