Bug Report

Issue Description

Cancel an already shipped order results in incorrect pending inventory amount if there are other pending orders.

Steps To Reproduce

• Set product inventory for product A to 10
• Let user order 5 amounts of product A in order1
• Let user order 2 amounts of product A in order2

now product inventory for A is still at 10 pending inventory for A is 7 (-> available amount A is 10 - 7 = 3)

• Create Shipment for order1 of 5 amounts of product A

now product inventory for A is at 5 pending inventory for A is 2 (-> available amount A is 5 - 2 = 3)

• Cancel the order1

Expected Result

product inventory for A is at 10 pending inventory for A is 2

The cancel action should just increase the product inventory for A and leave the pending inventory as is. The reason is, that the order1 already got shipped, so pending inventory was already reduced as well as normal product inventory.

So as end result available amount for A should be 8 (10 - 2).

Actual Result

product inventory for A is at 10 pending inventory for A is 0

So as end result available amount for A currently is 10 (10 - 0).

Reason: The pending inventory is changed for A even if it should not, as there already was a shipment for order1. This is due to this line: https://github.com/bagisto/bagisto/blob/28a81fb1ad3c6427f58ebec5b6c8564e83d92cee/packages/Webkul/Sales/src/Repositories/OrderItemRepository.php#L172

The returnQtyToProductInventory function in the OrderItemRepository correctly updates the inventory sources for A. It takes into account the shipment items. But the function needs to also account the shipment amounts to correctly calculate the orderedInventory qty for A.

As a sidenote, this part is a bit strange also: $orderItem->qty_ordered ? $orderItem->qty_to_cancel : $orderItem->parent->qty_ordered Why is the qty_to_cancel been taken if qty_ordered is existing? Wouldn’t it be logical to take qty_ordered instead? Anyway, this line probably needs to be changed as qty_ordered as well as qty_to_cancel do not reflect the correct amount to reduce the orderedInventory if there already was a shipment (as described above).

Bagisto Version

1.4.1

I can only advertise and stress the importance of unit tests for these kind of things. Please create unit tests for core functionality like this! Please consider unit tests to have high priority! Such tests are very suitable to find such kind of problems as presented here and also prevent regressions when refactoring happens.

best Martin