Fix for reverting stock twice for cancelled orders

[dverkade]

Description

Fix for reverting stock twice for cancelled orders:

• Removed cataloginventory event observer restore_quote, that increased the stock when rebuilding a quote. Reverting quote inventory should be responsibility either of sales_model_service_quote_submit_failure or sales_order_item_cancel events;

Fixed Issues

1. Cancel order and restore quote methods increase stocks twice #9969: Cancel order and restore quote methods increase stocks twice

[adrian-martinez-interactiv4]

Hi @dverkade , we are discussing internally the best way to solve related issue, I will keep you updated

[adrian-martinez-interactiv4]

Hi again @dverkade , it seems that reverting stock is performed twice due to wrong event legacy observer in Magento/CatalogInventory/etc/events.xml:

<?xml version="1.0"?>
<!--
/**
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Event/etc/events.xsd">
    (...)
    <event name="sales_model_service_quote_submit_failure">
        <observer name="inventory" instance="Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver"/>
    </event>
    <event name="restore_quote">
        <observer name="inventory" instance="Magento\CatalogInventory\Observer\RevertQuoteInventoryObserver"/>
    </event>
    <event name="sales_order_item_cancel">
        <observer name="inventory" instance="Magento\CatalogInventory\Observer\CancelOrderItemObserver"/>
    </event>
    (...)
</config>

Reverting quote inventory should be responsibility either of sales_model_service_quote_submit_failure or sales_order_item_cancel events; it seems restore_quote event, using the same observer as sales_model_service_quote_submit_failure, should be removed, as apparently is there as consequence of some refactoring over legacy code and this task is out of its responsibility, for further info see c6856aa#diff-83d4d8c646cab3e9d92bbf93acc36b47L60 :

This should solve the problem without hardcoding and tying order states to available states to revert the stock, as at some point there could be more valid states to revert the stock, and order state does not necessarily defines if a quote item inventory can be reverted or not.

My suggestion is revert the applied changes and remove restore_quote event observer from Magento/CatalogInventory/etc/events.xml.

[dverkade]

Hi @adrian-martinez-interactiv4,

Thank you for your feedback. I have reverted the commits and took a look at the restore_quote event observer. This event is called when Magento needs to create a new quote with a different quote_id from an order. Nothing happens to the status of the original order, so we should not update the inventory at that moment. So I have removed this event observer from the code. I did not change it to paypal_payment_cancel, since this is event is never dispatched and a paypal event observer should be placed within the paypal module not in the cataloginventory module.

[adrian-martinez-interactiv4]

Hi @dverkade , the paypal_payment_cancel event in the image was only for ilustrating that the change that introduced this observer into cataloginventory module came from legacy code, changes look ok now, as last one thing, could you squash the seven commits into one single clean commit and push force your branch? Thank you in advance.