oc-shopaholic / oc-shopaholic-plugin

🛍️ No. 1 e-commerce platform for October CMS
https://shopaholic.one
GNU General Public License v3.0
426 stars 52 forks source link

[Orders for Shopaholic] Unable to change redirect destination on payment failure #196

Closed pikanji closed 4 years ago

pikanji commented 5 years ago

I'm currently implementing payment gateway plugin for Shopaholic. I like to let my gateway plugin to handle which page to transit for the case that purchase process has failed. Current behavior of MakeOrder component is always to go back to the previous page where form is submitted.

This cannot be overridden, because \Lovata\Toolbox\Classes\Component\ComponentSubmitForm::getResponseModeForm redirects back whenever Result::status() is false.

The cause is this part below in \Lovata\OrdersShopaholic\Components\MakeOrder::onRun or the similar part in onCreate.

$sRedirectURL = Event::fire(OrderProcessor::EVENT_ORDER_GET_REDIRECT_URL, $this->obOrder, true);
if (empty($this->obPaymentGateway)) {
    return $this->getResponseModeForm($sRedirectURL);
}

If modification is allowed, here are some solutions I thought of;

  1. Add method like getRedirectURLOnFailure in PaymentGatewayInterface to get redirect URL and prioritize it in ComponentSubmitForm::getResponseModeForm over Redirect::back.

  2. Prioritize $sRedirectURL in ComponentSubmitForm::getResponseModeForm, and pass $obPaymentGateway parameter with OrderProcessor::EVENT_ORDER_GET_REDIRECT_URL event. (I want $obPaymentGateway as parameter because $obOrder is null when purchase process failed. This way, it can allow other classes like gateway classes to handle redirect based on the payment status.

  3. Remove the condition checking the result in if (empty($this->obPaymentGateway) || !Result::status()) { in \Lovata\OrdersShopaholic\Components\MakeOrder::onRun. Then, payment gateway object can get the control over it. If we can add method like getRedirectURLOnFailure to PaymentGatewayInterface and call it in onRun, it would look even better.

The first one looks the most reasonable, but all solutions might break existing payment gateway plugins. Though there is a way to avoid breaking existing plugins, the code would become a bit less comprehensible.

kharanenka commented 5 years ago

Hi!

What do you think? Would it be correct to add such logic to MakeOrder component?

        $this->create($arOrderData, $arUserData);

        //Fire event and get redirect URL
        $sRedirectURL = Event::fire(OrderProcessor::EVENT_ORDER_GET_REDIRECT_URL, $this->obOrder, true);
        if (!Result::status() && !empty($this->obPaymentGateway) && $this->obPaymentGateway->isRedirect()) {
            $sRedirectURL = $this->obPaymentGateway->getRedirectURL();

            return Redirect::to($sRedirectURL);
        } else if (empty($this->obPaymentGateway) || !Result::status()) {
            return $this->getResponseModeForm($sRedirectURL);
        }
kharanenka commented 5 years ago

Now our partners are actively developing integration with payment gateways. We help them develop and we review plugin code. Are you interested to publish your plugin in the marketplace? We can continue discussion in OctoberCMS slack.

P.S. We have partnership agreement. We help partners with publication of plugins, get icons and banners, publish news in social networks. Affiliate plugin will be added to documentation.

pikanji commented 5 years ago

Hi @kharanenka, Thank you for your suggestion! Yes, it satisfies my need.

However, I have some concerns.

Since the modification will change the behavior, it will affect existing plugins that rely on $this->getResponseModeForm($sRedirectURL) when payment has failed and $this->obPaymentGateway->isRedirect() returns true.

Also, I wonder if it is ok that the url returned from $this->obPaymentGateway->getRedirectURL() has precedence over one returned from event handler.

Regarding publishing my payment gateway plugin in the marketplace, I can't do it. Although I'm very much willing to publish it, the payment gateway company (GMO Payment Gateway) does not allow us to make their API specification public. The source code of the plugin must be accessible from only their clients. Besides that, because they have more than 20 payment methods, it would take me quite some time to implement supports for all of them.

Anyway, please let me know if there's something else that I can help.

kharanenka commented 5 years ago

I need to think again about changes. You are right, changes should not break all existing solutions.

kharanenka commented 5 years ago

We already have several partners who have begun to develop of plugins. Some plugins are already published in marketplace. We review plugins and help with publishing. If you have any ideas for any plugins, then we will be happy to cooperate with you.

kharanenka commented 4 years ago

I have analyzed your request again. I decided to add logic that I suggested above. https://github.com/oc-shopaholic/oc-orders-shopaholic-plugin/blob/develop/components/MakeOrder.php#L94 https://github.com/oc-shopaholic/oc-orders-shopaholic-plugin/blob/develop/components/MakeOrder.php#L131

Now I see no problems with this logic. Perhaps in the future we will get negative feedback and imrove this logic.