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

Adds plugin to ensure InventoryInStorePickupSales shipment can be sent with qty is 0, solves issue #69 and #108 #109

Closed waleedbasit closed 1 year ago

waleedbasit commented 1 year ago

This approach was suggested in issue 69, and it solves issues 108 and 69.

This error happens when you try to "notify order for pick up" for an order that happens created with the store pickup method and the whole qty of any product has been ordered. To reproduce this error:

  1. Order a product via store pickup and order the whole qty of that product so that it is out of stock after your order.
  2. Check that the product is out of stock and has qty 0 in the backend.
  3. Open the order in the backend and click the “Notify Order is Ready for Pickup” button.
  4. You should get an error message “The order is not ready for pickup“.

Expected: The customer has been notified and shipment created. Actual: An error message "Order is not ready for pickup." is displayed and no shipment is created.

Versions: Magento: 2.4.5-p2 PHP: 8.1

convenient commented 1 year ago

Thanks for this. Can you please merge in this branch to see how your changes fare with the updated CI pipeline and testing infrastructure that I'm getting in place?

https://github.com/AmpersandHQ/magento2-disable-stock-reservation/pull/103

Also if you could either attempt to write a test that goes via the API, or give some bullet point steps on how to repro this that would be much appreciated

convenient commented 1 year ago

Thanks for this. Can you please merge in this branch to see how your changes fare with the updated CI pipeline and testing infrastructure that I'm getting in place?

https://github.com/AmpersandHQ/magento2-disable-stock-reservation/pull/103

Also if you could either attempt to write a test that goes via the API, or give some bullet point steps on how to repro this that would be much appreciated

sprankhub commented 1 year ago

@convenient, the new plugin now looks fine to me.

convenient commented 1 year ago

Hey @sprankhub @waleedbasit

The command "PHP_CS_FIXER_IGNORE_ENV=1 ./vendor/bin/php-cs-fixer fix --dry-run --rules=@PSR2 --diff src/" failed and exited with 8 during .

If you could fix the code styling issue being reported in Travis that would be helpful thanks

convenient commented 1 year ago

@sprankhub @waleedbasit Would these be the test notes

Expected

Actual

waleedbasit commented 1 year ago

@sprankhub @waleedbasit Would these be the test notes

  • Create product with qty=5
  • Place order with store pickup for qty=5

    • Product is_in_stock=0 and qty=0
  • Open the order in the backend and click the “Notify Order is Ready for Pickup” button.

    • (I really pray there is an API for this sweat for my testing)

Expected

  • ?????

Actual

  • The order is not ready for pickup

Yes, that seems right and the expected results would be: "The customer has been notified and shipment created."

convenient commented 1 year ago

@sprankhub @waleedbasit in another branch I changed the plugin to be return $result and we can see the tests there all failing

https://app.travis-ci.com/github/AmpersandHQ/magento2-disable-stock-reservation/builds/262564162

There was 1 failure:
1) Ampersand\DisableStockReservation\Test\Integration\InventoryInStorePickupSalesAdminUi\NotifyPickupControllerTest::testNotifyProducts
Session messages do not meet expectations array (
  0 => 'The order is not ready for pickup',
)
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'The customer has been notified and shipment created.'
+    0 => 'The order is not ready for pickup'
 )

Thats the exact error you reported, and with the plugin fix we correctly see The customer has been notified and shipment created. so to me this seems very successful :)

thanks