AmpersandHQ / magento2-disable-stock-reservation

This module disables the inventory reservation logic introduced as part of MSI in Magento 2.3.3
GNU Lesser General Public License v3.0
211 stars 60 forks source link

Remove plugin that will prevent double quantity refund #93

Closed markfischmann closed 1 year ago

markfischmann commented 1 year ago

Remove plugin that calls vendor/magento/module-inventory-sales/Plugin/SalesInventory/ProcessReturnQtyOnCreditMemoPlugin.php conflicting with the module's own refund process. Prevents refund of shipped quantity in addition to module's ordered qty refund.


ampersand edit

Caused by

zeeshantpp commented 1 year ago

@convenient can i kindly ask if we can merge this?

convenient commented 1 year ago

@markfischmann @zeeshantpp Currently on break for the holidays.

Can I have a few bullet points to reproduce this issue so that I can codify a test case to capture it?

For example in #81 I added this test case, a case of creating products with known stock, taking actions, then asserting on the results?

https://github.com/AmpersandHQ/magento2-disable-stock-reservation/blob/88ca4d48851094688c8c8a6f7a0f8a62fa32a138/dev/tests/acceptance/CheckoutCest.php#L114-L164

I don't expect you to have to do the codeception tests (you can if you like) but if even i get a series of steps that I can turn into code that would help massively.

Until then if you need this change urgently i recommend https://github.com/vaimo/composer-patches

markfischmann commented 1 year ago

Hello @convenient

The steps to reproduce the issue is as follows :

Without the fix from this Pull Request, the sourcededuction processor is called twice. Once by Magento, and another time by the Ampersand module. So this fix will remove the first Magento call.

I'll see if I can find the time to create a testcase but not until a few weeks I think.

Cheers and happy holidays !

sprankhub commented 1 year ago

:eyes:

convenient commented 1 year ago

I did not mean to close these as invalid, apologies I made an error, I'm continuing the work in https://github.com/AmpersandHQ/magento2-disable-stock-reservation/pull/107 thanks

convenient commented 1 year ago

@markfischmann i've done a touch of rework here to support a wider range of 2.4 versions, can you please let me know what you think

Original version of disabling the plugin properly

caused all versions of 2.4.5 and lower to have failing tests with

1) CheckoutCest: Prevent double refund quantity on shipped order
 Test  tests/acceptance/CheckoutCest.php:preventDoubleRefundQuantityOnShippedOrder
 Step  Assert equals 100,"95.0000","The quantity should be reset to 100 after the refund"
 Fail  The quantity should be reset to 100 after the refund
Failed asserting that '95.0000' matches expected 100.
Screenshot 2023-04-28 at 18 05 17

Alternative approach of disabling the plugin conditionally

All tests pass except for 2.3 which is not possible with this preference because of this error caused by the method declaration in 2.4 and higher having : void in it. I am not super happy with my methodology of detecting if we're on 2.4.5, but it does seem to work and if the tests fail on a new version in the future that should be obvious to see. Any better recommendations are welcome.

So I have updated the magento/framework requirement to ensure we're on magento 2.4 or higher.

%message% 0/8 [>---------------------------]   0% < 1 sec 77.0 MiBProxies code generation... 0/8 [>------------------------]   0% < 1 sec 77.0 MiB
Proxies code generation... 1/8 [===>---------------------]  12% < 1 sec 81.0 MiB
Repositories code generation... 1/8 [==>-----------------]  12% < 1 sec 81.0 MiB
In ErrorHandler.php line 61:

  Warning: Declaration of Ampersand\DisableStockReservation\Plugin\Preference
  \SalesInventory\ProcessReturnQtyOnCreditMemoPlugin::aroundExecute(Magento\S
  alesInventory\Model\Order\ReturnProcessor $subject, callable $proceed, Mage
  nto\Sales\Api\Data\CreditmemoInterface $creditmemo, Magento\Sales\Api\Data\
  OrderInterface $order, array $returnToStockItems = Array, bool $isAutoRetur
  n = false): void should be compatible with Magento\InventorySales\Plugin\Sa
  lesInventory\ProcessReturnQtyOnCreditMemoPlugin::aroundExecute(Magento\Sale
  sInventory\Model\Order\ReturnProcessor $subject, callable $proceed, Magento
  \Sales\Api\Data\CreditmemoInterface $creditmemo, Magento\Sales\Api\Data\Ord
  erInterface $order, array $returnToStockItems = Array, $isAutoReturn = fals
  e) in /current_extension/src/Plugin/Preference/SalesInventory/ProcessReturn
  QtyOnCreditMemoPlugin.php on line 29
Screenshot 2023-04-28 at 18 04 38