[bug] prevent double-complete of shipments (#12366)

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)
 

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.