Bug fixes (#12365)

* Bug fixes:

- Enforce serial number requirements when receiving items
- Stricter merge checking
- Fix count-into-location bug
- Unit testing

* Additional unit tests

src/backend/InvenTree/order/serializers.py

@@ -1010,6 +1010,13 @@ class PurchaseOrderReceiveSerializer(serializers.Serializer):
         # Ensure barcodes are unique
         unique_barcodes = set()
 
+        # Ensure serial numbers are unique across all line items in this request
+        # (each line is only validated against the *database* individually)
+        unique_serials = set()
+        serials_globally_unique = get_global_setting(
+            'SERIAL_NUMBER_GLOBALLY_UNIQUE', False
+        )
+
         # Check if the location is not specified for any particular item
         for item in items:
             line = item['line_item']
@@ -1035,6 +1042,25 @@ class PurchaseOrderReceiveSerializer(serializers.Serializer):
                 else:
                     unique_barcodes.add(barcode)
 
+            if serials := item.get('serials'):
+                if line.part:
+                    # Scope uniqueness the same way as the database check:
+                    # per part tree, or globally (if so configured)
+                    for serial in serials:
+                        key = (
+                            str(serial)
+                            if serials_globally_unique
+                            else (line.part.part.tree_id, str(serial))
+                        )
+
+                        if key in unique_serials:
+                            raise ValidationError(
+                                _('Supplied serial numbers must be unique')
+                                + f': {serial}'
+                            )
+
+                        unique_serials.add(key)
+
         return data
 
     def save(self) -> list[stock.models.StockItem]:

src/backend/InvenTree/order/test_api.py

@@ -1369,6 +1369,43 @@ class PurchaseOrderReceiveTest(OrderTest):
         self.assertEqual(item.quantity, 10)
         self.assertEqual(item.batch, 'B-xyz-789')
 
+    def test_duplicate_serial_numbers_across_items(self):
+        """Duplicate serial numbers across line items in a single request are rejected.
+
+        Regression test: each line's serials used to be validated against the
+        database only, so two lines could claim the same serial number and create
+        duplicate serialized stock items.
+        """
+        data = {
+            'items': [
+                {'line_item': 1, 'quantity': 3, 'serial_numbers': '100-102'},
+                {'line_item': 1, 'quantity': 3, 'serial_numbers': '102-104'},
+            ],
+            'location': 1,
+        }
+
+        # Serial 102 is claimed by both entries - request must be rejected
+        response = self.post(self.url, data, expected_code=400)
+
+        self.assertIn('Supplied serial numbers must be unique', str(response.data))
+
+        # No new stock items have been created
+        self.assertEqual(self.n, StockItem.objects.count())
+
+        # Non-overlapping serial numbers are accepted
+        data['items'][1]['serial_numbers'] = '103-105'
+
+        self.post(self.url, data, expected_code=201, max_query_count=250)
+
+        self.assertEqual(self.n + 6, StockItem.objects.count())
+
+        for serial in range(100, 106):
+            self.assertEqual(
+                StockItem.objects.filter(serial=str(serial)).count(),
+                1,
+                f'Expected exactly one stock item with serial {serial}',
+            )
+
     def test_receive_large_quantity(self):
         """Test receipt of a large number of items."""
         from stock.status_codes import StockStatus

src/backend/InvenTree/stock/models.py

@@ -2491,6 +2491,11 @@ class StockItem(
         if location is None:
             return None
 
+        # This item must itself be in a state which allows merging
+        # (e.g. not serialized, in production, installed, or assigned to an order / customer)
+        if not self.can_merge():
+            return None
+
         candidates = list(
             StockItem.objects
             .filter(part=self.part, location=location)
@@ -2567,10 +2572,14 @@ class StockItem(
             pricing_data.append([self.purchase_price, self.quantity])
 
         for other in other_items:
-            # If the stock item cannot be merged, return
+            # Check the merge in both directions, so that the generic state checks
-            if not self.can_merge(other, raise_error=raise_error, **kwargs):
+            # (serialized, in production, installed, assigned to an order / customer)
+            # are applied to the incoming items as well as this one
+            if not self.can_merge(
+                other, raise_error=raise_error, **kwargs
+            ) or not other.can_merge(self, raise_error=raise_error, **kwargs):
                 logger.warning(
-                    'Stock item <%s> could not be merge into <%s>', other.pk, self.pk
+                    'Stock item <%s> could not be merged into <%s>', other.pk, self.pk
                 )
                 return
 
@@ -3060,6 +3069,15 @@ class StockItem(
 
         status = self._resolve_status_kwarg(kwargs)
         self._apply_status_change(status, tracking_info)
+
+        # Optional fields which can be supplied in a 'stocktake' call
+        self._apply_optional_transfer_fields(kwargs, tracking_info)
+
+        # Have any fields (other than quantity) been updated?
+        # Must be determined *before* model reference deltas are extracted,
+        # as those only affect the tracking entry (not the model instance)
+        fields_updated = len(tracking_info) > 0
+
         self._apply_model_reference_fields(kwargs, tracking_info)
 
         quantity_updated = self.serialized or self.updateQuantity(count)
@@ -3068,13 +3086,13 @@ class StockItem(
         # (self.quantity is updated by updateQuantity() even if the item was deleted)
         tracking_info['quantity'] = 1 if self.serialized else float(self.quantity)
 
-        if quantity_updated:
+        # Save if the quantity or any other field was changed.
+        # Note that updateQuantity() may have *deleted* the item (depleted to zero),
+        # in which case there is nothing left to save.
+        if self.pk and (quantity_updated or fields_updated):
             self.stocktake_date = InvenTree.helpers.current_date()
             self.stocktake_user = user
 
-            # Optional fields which can be supplied in a 'stocktake' call
-            self._apply_optional_transfer_fields(kwargs, tracking_info)
-
             self.save(add_note=False)
 
             trigger_event(

src/backend/InvenTree/stock/test_api.py

@@ -2945,6 +2945,56 @@ class StocktakeTest(StockAPITestCase):
             str(response.data['location']),
         )
 
+    def test_count_unchanged_quantity_saves_field_changes(self):
+        """Location / status changes are saved even if the counted quantity is unchanged.
+
+        Regression test: counting an item to its *existing* quantity used to skip
+        the model save entirely, losing any location / status change while still
+        recording the change in stock history.
+        """
+        item = StockItem.objects.get(pk=1234)
+
+        quantity = item.quantity
+        self.assertEqual(item.location.pk, 5)
+        self.assertEqual(item.status, StockStatus.OK.value)
+
+        # Clear any existing stocktake date so we can verify it gets set
+        item.stocktake_date = None
+        item.save()
+
+        self.post(
+            reverse('api-stock-count'),
+            {
+                'items': [
+                    {
+                        'pk': item.pk,
+                        'quantity': float(quantity),
+                        'status': StockStatus.DAMAGED.value,
+                    }
+                ],
+                'location': 1,
+            },
+            expected_code=201,
+        )
+
+        item.refresh_from_db()
+
+        # Quantity is unchanged, but the location and status changes must be saved
+        self.assertEqual(item.quantity, quantity)
+        self.assertEqual(item.location.pk, 1)
+        self.assertEqual(item.status, StockStatus.DAMAGED.value)
+        self.assertIsNotNone(item.stocktake_date)
+
+        # The stock history entry must agree with the saved state
+        entry = StockItemTracking.objects.filter(
+            item=item, tracking_type=StockHistoryCode.STOCK_COUNT
+        ).latest('date')
+        self.assertEqual(entry.deltas.get('quantity'), float(quantity))
+        self.assertEqual(entry.deltas.get('location'), 1)
+        self.assertEqual(entry.deltas.get('old_location'), 5)
+        self.assertEqual(entry.deltas.get('status'), StockStatus.DAMAGED.value)
+        self.assertEqual(entry.deltas.get('old_status'), StockStatus.OK.value)
+
     def test_bulk_count_query_benchmark(self):
         """Benchmark: measure the number of DB queries required to count 100 stock items at once."""
         InvenTreeSetting.set_setting('ENABLE_PLUGINS_EVENTS', True, change_user=None)
@@ -3271,6 +3321,51 @@ class StockTransferMergeTest(StockAPITestCase):
             ).exists()
         )
 
+    def test_transfer_merge_skips_protected_source(self):
+        """Merge-on-transfer must not absorb items in a protected state.
+
+        Regression test: only the *target* item used to be validated, so a
+        transfer with merge=True could absorb (and delete) an in-production
+        build output. Such items must be moved as a separate lot instead.
+        """
+        existing = StockItem.objects.create(
+            part=self.part, location=self.dest, quantity=100
+        )
+
+        # An "in production" build output
+        self.part.assembly = True
+        self.part.save()
+
+        bo = build.models.Build.objects.create(
+            reference='BO-9999', part=self.part, title='Merge test build', quantity=50
+        )
+
+        building = StockItem.objects.create(
+            part=self.part,
+            location=self.source_loc,
+            quantity=50,
+            build=bo,
+            is_building=True,
+        )
+
+        self.post(
+            self.url,
+            {
+                'items': [{'pk': building.pk, 'quantity': 50, 'merge': True}],
+                'location': self.dest.pk,
+            },
+            expected_code=201,
+        )
+
+        # The build output must survive - transferred as a separate lot instead
+        building.refresh_from_db()
+        self.assertTrue(building.is_building)
+        self.assertEqual(building.location, self.dest)
+        self.assertEqual(building.quantity, 50)
+
+        existing.refresh_from_db()
+        self.assertEqual(existing.quantity, 100)
+
 
 class StockItemDeletionTest(StockAPITestCase):
     """Tests for stock item deletion via the API."""

src/backend/InvenTree/stock/tests.py

@@ -1074,6 +1074,54 @@ class StockTest(StockTestBase):
         # Final purchase price should be the weighted average
         self.assertAlmostEqual(s1.purchase_price.amount, 16.875, places=3)
 
+    def test_merge_protected_items(self):
+        """Stock items in a protected state cannot be absorbed by a merge.
+
+        Regression test: merge_stock_items() used to run the generic state checks
+        (in production, assigned to customer, etc.) against the *target* item only,
+        allowing e.g. a build output to be merged away and deleted.
+        """
+        part = Part.objects.first()
+        part.stock_items.all().delete()
+
+        target = StockItem.objects.create(part=part, quantity=10)
+
+        # The incoming item is "in production" (a build output)
+        part.assembly = True
+        part.save()
+
+        bo = Build.objects.create(
+            reference='BO-9998', part=part, title='Merge test build', quantity=20
+        )
+
+        building = StockItem.objects.create(
+            part=part, quantity=20, build=bo, is_building=True
+        )
+
+        # Without raise_error, the merge is refused silently
+        target.merge_stock_items([building])
+
+        target.refresh_from_db()
+        building.refresh_from_db()
+
+        self.assertEqual(part.stock_items.count(), 2)
+        self.assertEqual(target.quantity, 10)
+        self.assertEqual(building.quantity, 20)
+
+        # With raise_error, the merge raises a ValidationError
+        with self.assertRaises(ValidationError):
+            target.merge_stock_items([building], raise_error=True)
+
+        # An item assigned to a customer is likewise protected
+        customer = Company.objects.create(name='MergeCust', is_customer=True)
+        assigned = StockItem.objects.create(part=part, quantity=5, customer=customer)
+
+        target.merge_stock_items([assigned])
+
+        target.refresh_from_db()
+        self.assertEqual(target.quantity, 10)
+        self.assertTrue(StockItem.objects.filter(pk=assigned.pk).exists())
+
     def test_notify_low_stock(self):
         """Test that the 'notify_low_stock' task is triggered correctly."""
         FUNC_NAME = 'part.tasks.notify_low_stock_if_required'