Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel order and restore quote methods increase stocks twice #9969

Closed
simpleadm opened this issue Jun 16, 2017 · 10 comments
Closed

Cancel order and restore quote methods increase stocks twice #9969

simpleadm opened this issue Jun 16, 2017 · 10 comments
Assignees
Labels
bug report Component: Payment Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@simpleadm
Copy link
Contributor

simpleadm commented Jun 16, 2017

Some payment methods in cancel payment action try to cancel current order and restore quote and as the result product qty is increased by 2.

Affected payment methods

  1. Paypal Hosted Pro (CE, EE)
  2. Paypal Payflow (CE, EE)
  3. Worldpay (EE)
  4. Eway (EE)
  5. PayEx (Third party payment gateway)

Preconditions

  1. Magento 2.1.* CE and EE
  2. Configured payment methods

Steps to reproduce

  1. Suppose you have product with stock quantity: 1
  2. Add product to cart and Checkout
  3. Select affected payment method
  4. The order is placed and stock is reduced to 0
  5. Click cancel on Payment gateway (e.g. press "Cancel and return to store" button on PayPal page)

Expected result

  1. Order was canceled
  2. Product stock quantity: 1

Actual result

  1. Order was canceled
  2. Product stock quantity: 2 (increase to 2)

Additional information

For example Paypal Hosted Pro Cancel action
\Magento\Paypal\Controller\Hostedpro\Cancel

   /**
     * Customer canceled payment on gateway side.
     *
     * @return void
     */
    public function execute()
    {
        $this->checkoutHelper->cancelCurrentOrder('');
        $this->checkoutHelper->restoreQuote();

        $this->_redirect('checkout', ['_fragment' => 'payment']);
    }

Method cancelCurrentOrder will execute $order->registerCancellation($comment)->save(); that will cancel all order items and dispatch 'sales_order_item_cancel' event for each one. \Magento\CatalogInventory\Observer\CancelOrderItemObserver observer listens this event and increases product qty.

Next \Magento\Checkout\Model\Session::restoreQuote() will be executed to restore customer quote. That method will update and save a quote and dispatch restore_quote event.
\Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver listens to this event and product qty is increased the second time.

The problem is that both statements increase quantity of product.

The same issue in Payflow cancel action \Magento\Paypal\Controller\Payflow:

     /**
     * Cancel order, return quote to customer
     *
     * @param string $errorMsg
     * @return false|string
     */
    protected function _cancelPayment($errorMsg = '')
    {
        $errorMsg = trim(strip_tags($errorMsg));

        $gotoSection = false;
        $this->_checkoutHelper->cancelCurrentOrder($errorMsg);
        if ($this->_checkoutSession->restoreQuote()) {
            //Redirect to payment step
            $gotoSection = 'paymentMethod';
        }

        return $gotoSection;
    }
@biclope
Copy link

biclope commented Jun 16, 2017

you found it, you patched it

@IlnitskiyArtem
Copy link

@simpleadm, Thanks for reporting this issue.
We've created internal ticket MAGETWO-70440 to address this issue.

@veloraven veloraven added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Jul 4, 2017
@ttraskback
Copy link

ttraskback commented Sep 1, 2017

Would it not work to just check in \Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver::execute if order is canceled don't add items back to stock?

$order = $observer->getEvent()->getOrder();
if($order->getState() == \Magento\Sales\Model\Order::STATE_CANCELED){
    // Clear flag, so if order placement retried again with success - it will be processed
    $quote->setInventoryProcessed(false);
    return;
}

@simpleadm
Copy link
Contributor Author

@ttraskback it should work.

Currently will be better to add following plugin:

di.xml

<type name="Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver">
        <plugin name="CatalogInventoryPlugin::RevertQuoteInventoryObserver" type="VendorName\ModuleName\Plugin\CatalogInventory\RevertQuoteInventoryObserver" sortOrder="100" />
</type>

RevertQuoteInventoryObserver.php

    /**
     * Skip revert products sale for canceled orders
     *
     * @param \Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver $subject
     * @param \Closure $proceed
     * @param \Magento\Framework\Event\Observer $observer
     * @return void
     * @see \Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver::execute
     */
    public function aroundExecute(
        \Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver $subject,
        \Closure $proceed,
        \Magento\Framework\Event\Observer $observer
    ) {
        /** @var \Magento\Sales\Model\Order $order */
        $order = $observer->getOrder();

        if ($order && $order->isCanceled()) {
            return;
        }

        $proceed($observer);
    }

@ttraskback
Copy link

Hi @simpleadm,
Yea i also made a plugin to fix the problem in our projects.
Tough i still do the $quote->setInventoryProcessed(false); so that the buy process executes correctly.

@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Payment Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Dec 1, 2017
@magento-engcom-team
Copy link
Contributor

@simpleadm, thank you for your report.
We've created internal ticket(s) MAGETWO-70440 to track progress on the issue.

@magento-engcom-team magento-engcom-team added 2.2.x Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Dec 1, 2017
@adrian-martinez-interactiv4
Copy link
Contributor

For anyone interested: #12668 (comment)

JonkRuud added a commit to MultiSafepay/Magento2Msp that referenced this issue Jan 4, 2018
…ts can now choose to not restore the cart, avoiding any stock issues because magento processes stock not correctly (magento/magento2#9969)
@magento-team
Copy link
Contributor

Hi @simpleadm. Thank you for your report.
The issue has been fixed in #12668 by @dverkade in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming patch release.

@magento-team magento-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Jan 11, 2018
@magento-team
Copy link
Contributor

Hi @simpleadm. Thank you for your report.
The issue has been fixed in #12949 by @dverkade in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming patch release.

@magento-team magento-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Jan 11, 2018
@magento-team
Copy link
Contributor

Hi @simpleadm. Thank you for your report.
The issue has been fixed in #12952 by @dverkade in 2.1-develop branch
Related commit(s):

The fix will be available with the upcoming patch release.

@magento-team magento-team added the Fixed in 2.1.x The issue has been fixed in 2.1 release line label Jan 11, 2018
Jasper-MultiSafepay pushed a commit to Jasper-MultiSafepay/Magento2Msp that referenced this issue Jun 8, 2018
…o master

* commit '80491c0ac1e1fd3e9efe01b71eecdec5e1e4ccee':
  PLGMAGTWOS-227 - Updated version to 1.4.8
  PLGMAGTWOS-226 - Processed codeformatting tool
  PLGMAGTWOS-221. Custom "pending" state can now be used.
  PLGMAGTWOS-205. Added an option to reset the payment method for when a payment link was created when an order was added through the Magento backend.
  PLGMAGTWOS-220. Added BTW0 alternate tax rule for FPT
  PLGMAGTWOS-220. FPT are now processed within the transaction request.
  PLGMAGTWOS-219. Keep cart alive function is now configurable. Merchants can now choose to not restore the cart, avoiding any stock issues because magento processes stock not correctly (magento/magento2#9969)
  PLGMAGTWOS-223, fixes Id required error when creating an order from the backend i.c.m. a MultiSafepay payment method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Payment Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

9 participants