From 2b25a46eaacd646afe463691b1e416cfb910ed68 Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 14 Jul 2026 12:06:53 +1000 Subject: [PATCH] Prevent duplication completion of ReturnOrder (#12386) --- src/backend/InvenTree/order/models.py | 6 +++++ src/backend/InvenTree/order/test_api.py | 36 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/backend/InvenTree/order/models.py b/src/backend/InvenTree/order/models.py index 53caaa16d1..9f8023b212 100644 --- a/src/backend/InvenTree/order/models.py +++ b/src/backend/InvenTree/order/models.py @@ -3189,6 +3189,12 @@ class ReturnOrder(TotalPriceMixin, Order): def _action_complete(self, *args, **kwargs): """Complete this ReturnOrder (if not already completed).""" + # Lock this order against concurrent completion, and re-read the status + # from the database. Without this, two simultaneous completion requests + # can both observe status=IN_PROGRESS, and each would run the completion + # side effects (duplicate events and notifications). + self.status = ReturnOrder.objects.select_for_update().get(pk=self.pk).status + if self.status == ReturnOrderStatus.IN_PROGRESS.value: self.status = ReturnOrderStatus.COMPLETE.value self.complete_date = InvenTree.helpers.current_date() diff --git a/src/backend/InvenTree/order/test_api.py b/src/backend/InvenTree/order/test_api.py index 2911978c70..3942000de2 100644 --- a/src/backend/InvenTree/order/test_api.py +++ b/src/backend/InvenTree/order/test_api.py @@ -5,6 +5,7 @@ import io import json from datetime import date, datetime, timedelta from typing import Optional +from unittest import mock from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError @@ -3244,6 +3245,41 @@ class ReturnOrderTests(InvenTreeAPITestCase): stock_item.refresh_from_db() self.assertEqual(stock_item.quantity, 6) + def test_complete_stale_instance_is_noop(self): + """A second completion attempt with a stale instance must be a no-op. + + Regression test: _action_complete() checked 'status' on the caller's + (potentially stale) instance, so two concurrent completion requests + could both run the completion side effects (duplicate COMPLETED events). + The status is now re-read (under lock) from the database before the check. + """ + company = Company.objects.get(pk=4) + + rma = models.ReturnOrder.objects.create( + customer=company, description='A return order' + ) + rma.issue_order() + + # Two "concurrent" requests each hold their own instance of the order + order_a = models.ReturnOrder.objects.get(pk=rma.pk) + order_b = models.ReturnOrder.objects.get(pk=rma.pk) + + order_a.complete_order() + + rma.refresh_from_db() + self.assertEqual(rma.status, ReturnOrderStatus.COMPLETE.value) + + # The second (stale) instance still believes the order is IN_PROGRESS - + # completion must be skipped based on the database state + self.assertEqual(order_b.status, ReturnOrderStatus.IN_PROGRESS.value) + + with mock.patch('order.models.trigger_event') as trigger: + order_b.complete_order() + trigger.assert_not_called() + + rma.refresh_from_db() + self.assertEqual(rma.status, ReturnOrderStatus.COMPLETE.value) + def test_ro_calendar(self): """Test the calendar export endpoint.""" # Full test is in test_po_calendar. Since these use the same backend, test only