sezzle / sezzle-magento2

Apache License 2.0
5 stars 3 forks source link

Order view plugin does not call proceed so prevents after plugins added to the execute method #18

Closed LRotherfield closed 2 years ago

LRotherfield commented 2 years ago

We have a custom plugin that runs afterExecute on the order view controller. However your module adds a plugin aroundExecute on the same class that does not call proceed and thus stops our plugin from firing.

When reading this code it seems like the only requirement is setting a specific flag on the order before the page is rendered.
https://github.com/sezzle/sezzle-magento2/blob/master/Plugin/Sales/Controller/Adminhtml/Order/ViewPlugin.php

If that is the case, there is no need to us an around plugin, and especially not to have the plugin also extend the class it is plugging into. Could you not instead use a before plugin like so?:

<?php

namespace Sezzle\Sezzlepay\Plugin\Sales\Controller\Adminhtml\Order;

use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Sales\Api\OrderRepositoryInterface;
use Magento\Sales\Controller\Adminhtml\Order\View;
use Magento\Sales\Model\Order;
use Sezzle\Sezzlepay\Model\Sezzle;
use Sezzle\Sezzlepay\Model\System\Config\Container\SezzleConfigInterface;

class CheckAuthExpiry
{
    private $sezzleModel;
    private $sezzleConfig;
    private $orderRepository;

    /**
     * @param OrderRepositoryInterface $orderRepository
     * @param Sezzle $sezzleModel
     * @param SezzleConfigInterface $sezzleConfig
     */
    public function __construct(
        OrderRepositoryInterface $orderRepository,
        Sezzle $sezzleModel,
        SezzleConfigInterface $sezzleConfig
    ) {
        $this->orderRepository = $orderRepository;
        $this->sezzleModel = $sezzleModel;
        $this->sezzleConfig = $sezzleConfig;
    }

    public function beforeExecute(
        View $subject
    ) {
        try {
            $order = $this->orderRepository->get($subject->getRequest()->getParam('order_id'));
        } catch (NoSuchEntityException $e) {
            //Early exit, this exception will be thrown later in the request and exit before any of this
            //plugin's modifications are required
            return;
        }
        $isTokenizedAllowed = $this->sezzleConfig->isTokenizationAllowed();
        $order->setActionFlag(
            Order::ACTION_FLAG_INVOICE,
            $this->sezzleModel->canInvoice($order)
            || $isTokenizedAllowed
        );
    }
}

This would set the flag at the correct time without requiring you to extend the view controller or block other plugins. Thoughts?

Cheers Luke

arijit-sezzle commented 2 years ago

@LRotherfield Thanks for reporting the issue and sorry for the inconvenience. I am investigating this and will let you know with further updates. Thanks.

arijit-sezzle commented 2 years ago

Fix has been released in v6.0.3.

LRotherfield commented 2 years ago

Thanks Arijit