diff --git a/src/backend/InvenTree/order/tasks.py b/src/backend/InvenTree/order/tasks.py index 8950b089a5..34e2fa033b 100644 --- a/src/backend/InvenTree/order/tasks.py +++ b/src/backend/InvenTree/order/tasks.py @@ -255,14 +255,28 @@ def complete_sales_order_shipment( At this stage, the shipment is assumed to be complete, and we need to perform the required "processing" tasks. """ - # Do not handle any lookup errors here - # If the shipment cannot be found, then we want the task to fail (and retry later) - shipment = order.models.SalesOrderShipment.objects.get(pk=shipment_id) user = User.objects.filter(pk=user_id).first() if user_id else None - logger.info('Completing SalesOrderShipment <%s>', shipment) + logger.info('Completing SalesOrderShipment <%s>', shipment_id) with transaction.atomic(): + # Lock the shipment row for the duration of the update, + # and re-check that it has not already been completed. + # This guards against duplicate task execution - e.g. if the completion + # request was submitted multiple times before the first task ran. + # Do not handle any lookup errors here: + # If the shipment cannot be found, then we want the task to fail (and retry later) + shipment = order.models.SalesOrderShipment.objects.select_for_update().get( + pk=shipment_id + ) + + if shipment.is_complete(): + logger.warning( + 'SalesOrderShipment <%s> has already been completed - skipping', + shipment_id, + ) + return + for allocation in shipment.allocations.all(): allocation.complete_allocation(user=user) diff --git a/src/backend/InvenTree/order/test_sales_order.py b/src/backend/InvenTree/order/test_sales_order.py index 5e078fa1c5..aafad46e1f 100644 --- a/src/backend/InvenTree/order/test_sales_order.py +++ b/src/backend/InvenTree/order/test_sales_order.py @@ -21,7 +21,7 @@ from order.models import ( SalesOrderShipment, ) from part.models import Part -from stock.models import StockItem, StockLocation +from stock.models import StockItem, StockItemTracking, StockLocation from users.models import Owner @@ -337,6 +337,43 @@ class SalesOrderTest(InvenTreeTestCase): self.assertEqual(self.line.fulfilled_quantity(), 50) self.assertEqual(self.line.allocated_quantity(), 50) + def test_complete_shipment_task_is_idempotent(self): + """Duplicate execution of the shipment completion task must not double-ship. + + Regression test: two rapid completion requests could enqueue the background + completion task twice - each execution then completed every allocation again, + double-counting the 'shipped' quantity and re-assigning stock to the customer. + """ + self.allocate_stock(True) + + # Complete the shipment (the task runs inline during testing) + self.shipment.complete_shipment(None) + + self.shipment.refresh_from_db() + self.line.refresh_from_db() + + self.assertTrue(self.shipment.is_complete()) + self.assertEqual(self.line.shipped, 50) + + # A second completion request is rejected outright + self.assertFalse(self.shipment.check_can_complete(raise_error=False)) + + n_items = StockItem.objects.count() + n_tracking = StockItemTracking.objects.count() + + # Simulate a duplicated task execution + # (e.g. a second completion request submitted before the first task ran) + order.tasks.complete_sales_order_shipment(self.shipment.pk, None, None) + + self.line.refresh_from_db() + + # The 'shipped' quantity must not be double-counted + self.assertEqual(self.line.shipped, 50) + + # No additional stock items or tracking entries have been created + self.assertEqual(StockItem.objects.count(), n_items) + self.assertEqual(StockItemTracking.objects.count(), n_tracking) + def test_shipment_many_items(self): """Test completion of a shipment with many items.