Merge commit '6865b0b6e6ed5464c25b4ccca7e3c523ce1876c5' into block-notes

src/backend/InvenTree/build/models.py

@@ -37,10 +37,7 @@ from build.validators import (
     validate_build_order_reference,
 )
 from common.models import ProjectCode
-from common.settings import (
-    get_global_setting,
-    prevent_build_output_complete_on_incompleted_tests,
-)
+from common.settings import get_global_setting
 from generic.enums import StringEnum
 from generic.states import StateTransitionMixin, StatusCodeMixin
 from plugin.events import trigger_event
@@ -1093,6 +1090,61 @@ class Build(
             },
         )
 
+    def can_complete_output(
+        self,
+        output: stock.models.StockItem,
+        quantity: Optional[decimal.Decimal] = None,
+        required_tests=None,
+    ) -> bool:
+        """Determine if the given build output can be completed.
+
+        Arguments:
+            output: The StockItem instance (build output) to check
+            quantity: The quantity to complete (defaults to entire output quantity)
+            required_tests: Optional list of required tests to check against (defaults to the part's required tests)
+
+        Returns:
+            True if the build output can be completed, False otherwise
+
+        Raises:
+            ValidationError: If the build output cannot be completed, with an appropriate message
+        """
+        prevent_incomplete = get_global_setting(
+            'PREVENT_BUILD_COMPLETION_HAVING_INCOMPLETED_TESTS'
+        )
+
+        if prevent_incomplete and not output.passedAllRequiredTests(
+            required_tests=required_tests
+        ):
+            raise ValidationError(_('Build output has not passed all required tests'))
+
+        # Ensure that none of the allocated items are themselves still "in production"
+        allocated_items = output.items_to_install.all().filter(
+            stock_item__is_building=True
+        )
+
+        if allocated_items.exists():
+            raise ValidationError(_('Allocated stock items are still in production'))
+
+        if quantity is not None and quantity != output.quantity:
+            # Cannot split a build output with allocated items
+            if output.items_to_install.exists():
+                raise ValidationError({
+                    'quantity': _(
+                        'Cannot partially complete a build output with allocated items'
+                    )
+                })
+
+            if quantity <= 0:
+                raise ValidationError({
+                    'quantity': _('Quantity must be greater than zero')
+                })
+
+            if quantity > output.quantity:
+                raise ValidationError({
+                    'quantity': _('Quantity cannot be greater than the output quantity')
+                })
+
     @transaction.atomic
     def complete_build_output(
         self,
@@ -1118,52 +1170,20 @@ class Build(
         notes = kwargs.get('notes', '')
 
         required_tests = kwargs.get('required_tests', output.part.getRequiredTests())
-        prevent_on_incomplete = kwargs.get(
-            'prevent_on_incomplete',
-            prevent_build_output_complete_on_incompleted_tests(),
+
+        self.can_complete_output(
+            output, quantity=quantity, required_tests=required_tests
         )
 
-        if prevent_on_incomplete and not output.passedAllRequiredTests(
-            required_tests=required_tests
-        ):
-            msg = _('Build output has not passed all required tests')
-
-            if serial := output.serial:
-                msg = _(f'Build output {serial} has not passed all required tests')
-
-            raise ValidationError(msg)
-
-        # List the allocated BuildItem objects for the given output
-        allocated_items = output.items_to_install.all()
-
-        # Ensure that none of the allocated items are themselves still "in production"
-        for build_item in allocated_items:
-            if build_item.stock_item.is_building:
-                raise ValidationError(
-                    _('Allocated stock items are still in production')
-                )
-
         # If a partial quantity is provided, split the stock output
         if quantity is not None and quantity != output.quantity:
-            # Cannot split a build output with allocated items
-            if allocated_items.count() > 0:
-                raise ValidationError(
-                    _('Cannot partially complete a build output with allocated items')
-                )
-
-            if quantity <= 0:
-                raise ValidationError({
-                    'quantity': _('Quantity must be greater than zero')
-                })
-
-            if quantity > output.quantity:
-                raise ValidationError({
-                    'quantity': _('Quantity cannot be greater than the output quantity')
-                })
-
             # Split the stock item
             output = output.splitStock(quantity, user=user, allow_production=True)
 
+        allocated_items = output.items_to_install.all().select_related(
+            'stock_item', 'stock_item__part'
+        )
+
         for build_item in allocated_items:
             # Complete the allocation of stock for that item
             build_item.complete_allocation(user=user)

src/backend/InvenTree/build/serializers.py

@@ -1,6 +1,8 @@
 """JSON serializers for Build API."""
 
+from collections.abc import Callable
 from decimal import Decimal
+from typing import Optional
 
 from django.core.exceptions import ValidationError as DjangoValidationError
 from django.db import models, transaction
@@ -22,7 +24,6 @@ from rest_framework import serializers
 from rest_framework.serializers import ValidationError
 
 import common.filters
-import common.settings
 import company.serializers
 import InvenTree.helpers
 import part.filters
@@ -51,6 +52,7 @@ from users.serializers import OwnerSerializer, UserSerializer
 
 from .models import Build, BuildItem, BuildLine
 from .status_codes import BuildStatus
+from .validators import check_build_output
 
 
 class BuildSerializer(
@@ -257,11 +259,19 @@ class BuildOutputSerializer(serializers.Serializer):
 class BuildOutputQuantitySerializer(BuildOutputSerializer):
     """Build output with quantity field."""
 
+    # Optional callable to validate the output field, if required
+    output_validator: Optional[Callable] = None
+
     class Meta:
         """Serializer metaclass."""
 
         fields = [*BuildOutputSerializer.Meta.fields, 'quantity']
 
+    def __init__(self, *args, **kwargs):
+        """Initialize the serializer."""
+        self.output_validator = kwargs.pop('output_validator', None)
+        super().__init__(*args, **kwargs)
+
     quantity = serializers.DecimalField(
         max_digits=15,
         decimal_places=5,
@@ -289,6 +299,10 @@ class BuildOutputQuantitySerializer(BuildOutputSerializer):
                     'quantity': _('Quantity cannot be greater than the output quantity')
                 })
 
+        if self.output_validator:
+            # Call the parent serializer's output validator, if provided
+            self.output_validator(output, quantity=quantity)
+
         return data
 
 
@@ -524,7 +538,9 @@ class BuildOutputCompleteSerializer(serializers.Serializer):
             'notes',
         ]
 
-    outputs = BuildOutputQuantitySerializer(many=True, required=True)
+    outputs = BuildOutputQuantitySerializer(
+        many=True, required=True, output_validator=check_build_output
+    )
 
     location = serializers.PrimaryKeyRelatedField(
         queryset=StockLocation.objects.all(),
@@ -551,30 +567,6 @@ class BuildOutputCompleteSerializer(serializers.Serializer):
 
         outputs = data.get('outputs', [])
 
-        if common.settings.prevent_build_output_complete_on_incompleted_tests():
-            errors = []
-            for output in outputs:
-                stock_item = output['output']
-                if (
-                    stock_item.hasRequiredTests()
-                    and not stock_item.passedAllRequiredTests()
-                ):
-                    serial = stock_item.serial
-
-                    if serial:
-                        errors.append(
-                            _(
-                                f'Build output {serial} has not passed all required tests'
-                            )
-                        )
-                    else:
-                        errors.append(
-                            _('Build output has not passed all required tests')
-                        )
-
-            if errors:
-                raise ValidationError(errors)
-
         if len(outputs) == 0:
             raise ValidationError(_('A list of build outputs must be provided'))
 

src/backend/InvenTree/build/test_api.py

@@ -11,7 +11,7 @@ from build.models import Build, BuildItem, BuildLine
 from build.status_codes import BuildStatus
 from common.settings import set_global_setting
 from InvenTree.unit_test import InvenTreeAPITestCase
-from part.models import BomItem, BomItemSubstitute, Part
+from part.models import BomItem, BomItemSubstitute, Part, PartTestTemplate
 from stock.models import StockItem, StockLocation, StockSortOrder
 from stock.status_codes import StockStatus
 
@@ -1531,10 +1531,15 @@ class BuildOutputScrapTest(BuildAPITest):
             'notes': 'Partial complete',
         }
 
-        # Ensure that an invalid quantity raises an error
-        for q in [-4, 0, 999]:
+        # Ensure that an invalid quantity raises an error, with the expected message
+        for q, expected_message in [
+            (-4, 'Ensure this value is greater than or equal to 0'),
+            (0, 'Quantity must be greater than zero'),
+            (999, 'Quantity cannot be greater than the output quantity'),
+        ]:
             data['outputs'][0]['quantity'] = q
-            self.post(url, data, expected_code=400)
+            response = self.post(url, data, expected_code=400)
+            self.assertIn(expected_message, str(response.data))
 
         # Partially complete the output (with a valid quantity)
         data['outputs'][0]['quantity'] = 4
@@ -1552,6 +1557,94 @@ class BuildOutputScrapTest(BuildAPITest):
         self.assertEqual(completed_output.status, StockStatus.OK)
         self.assertFalse(completed_output.is_building)
 
+    def test_complete_with_required_tests(self):
+        """Test that build output completion is blocked if required tests have not passed."""
+        build = Build.objects.get(pk=1)
+        output = build.create_build_output(1).first()
+
+        template = PartTestTemplate.objects.create(
+            part=build.part, test_name='Required test', required=True
+        )
+
+        set_global_setting(
+            'PREVENT_BUILD_COMPLETION_HAVING_INCOMPLETED_TESTS', True, change_user=None
+        )
+
+        url = reverse('api-build-output-complete', kwargs={'pk': build.pk})
+
+        data = {'outputs': [{'output': output.pk}], 'location': 1}
+
+        response = self.post(url, data, expected_code=400)
+
+        self.assertIn(
+            'Build output has not passed all required tests', str(response.data)
+        )
+
+        # Add a passing test result - the output should now be able to be completed
+        output.add_test_result(template=template, result=True)
+
+        self.post(url, data, expected_code=200)
+
+    def test_complete_still_in_production(self):
+        """Test that build output completion is blocked if an allocated item is still in production."""
+        build = Build.objects.get(pk=1)
+        output = build.create_build_output(1).first()
+
+        build.create_build_line_items()
+        line = build.build_lines.first()
+
+        sub_build = Build.objects.create(
+            part=line.bom_item.sub_part,
+            quantity=1,
+            title='Sub-build',
+            reference='BO-9998',
+        )
+
+        in_production = StockItem.objects.create(
+            part=line.bom_item.sub_part, quantity=1, is_building=True, build=sub_build
+        )
+
+        BuildItem.objects.create(
+            build_line=line, stock_item=in_production, quantity=1, install_into=output
+        )
+
+        url = reverse('api-build-output-complete', kwargs={'pk': build.pk})
+
+        response = self.post(
+            url, {'outputs': [{'output': output.pk}], 'location': 1}, expected_code=400
+        )
+
+        self.assertIn(
+            'Allocated stock items are still in production', str(response.data)
+        )
+
+    def test_partial_complete_with_allocated_items(self):
+        """Test that a build output with allocated items cannot be partially completed."""
+        build = Build.objects.get(pk=1)
+        output = build.create_build_output(10).first()
+
+        build.create_build_line_items()
+        line = build.build_lines.first()
+
+        stock_item = StockItem.objects.create(part=line.bom_item.sub_part, quantity=10)
+
+        BuildItem.objects.create(
+            build_line=line, stock_item=stock_item, quantity=1, install_into=output
+        )
+
+        url = reverse('api-build-output-complete', kwargs={'pk': build.pk})
+
+        response = self.post(
+            url,
+            {'outputs': [{'output': output.pk, 'quantity': 4}], 'location': 1},
+            expected_code=400,
+        )
+
+        self.assertIn(
+            'Cannot partially complete a build output with allocated items',
+            str(response.data),
+        )
+
 
 class BuildOutputCancelTest(BuildAPITest):
     """Test cancellation of build outputs."""

src/backend/InvenTree/build/test_build.py

@@ -650,11 +650,15 @@ class BuildTest(BuildTestBase):
             'PREVENT_BUILD_COMPLETION_HAVING_INCOMPLETED_TESTS', True, change_user=None
         )
 
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(ValidationError) as exc:
             self.build_w_tests_trackable.complete_build_output(
                 self.stockitem_with_required_test, None
             )
 
+        self.assertIn(
+            'Build output has not passed all required tests', str(exc.exception)
+        )
+
         # let's complete the required test and see if it could be saved
         StockItemTestResult.objects.create(
             stock_item=self.stockitem_with_required_test,
@@ -671,6 +675,59 @@ class BuildTest(BuildTestBase):
             self.stockitem_wo_required_test, None
         )
 
+    def test_complete_output_still_in_production(self):
+        """Test that a build output cannot be completed if allocated stock is still in production."""
+        # Create a stock item of the tracked sub-part, which is itself still "in production"
+        sub_build = Build.objects.create(
+            reference=generate_next_build_reference(),
+            title='Building a sub-part',
+            part=self.sub_part_3,
+            quantity=2,
+            issued_by=get_user_model().objects.get(pk=1),
+        )
+
+        in_production = StockItem.objects.create(
+            part=self.sub_part_3, quantity=2, is_building=True, build=sub_build
+        )
+
+        self.allocate_stock(self.output_1, {in_production: 2})
+
+        with self.assertRaises(ValidationError) as exc:
+            self.build.complete_build_output(self.output_1, None)
+
+        self.assertIn(
+            'Allocated stock items are still in production', str(exc.exception)
+        )
+
+    def test_partial_complete_with_allocated_items(self):
+        """Test that a build output with tracked allocations cannot be partially completed."""
+        # Allocate tracked stock against output_1 (quantity=3)
+        self.allocate_stock(self.output_1, {self.stock_3_1: 6})
+
+        with self.assertRaises(ValidationError) as exc:
+            self.build.complete_build_output(self.output_1, None, quantity=1)
+
+        self.assertIn(
+            'Cannot partially complete a build output with allocated items',
+            str(exc.exception),
+        )
+
+    def test_complete_output_invalid_quantity(self):
+        """Test that invalid quantities are rejected when completing a build output directly."""
+        with self.assertRaises(ValidationError) as exc:
+            self.build.complete_build_output(self.output_1, None, quantity=0)
+
+        self.assertIn('Quantity must be greater than zero', str(exc.exception))
+
+        with self.assertRaises(ValidationError) as exc:
+            self.build.complete_build_output(
+                self.output_1, None, quantity=self.output_1.quantity + 1
+            )
+
+        self.assertIn(
+            'Quantity cannot be greater than the output quantity', str(exc.exception)
+        )
+
     def test_overdue_notification(self):
         """Test sending of notifications when a build order is overdue."""
         self.ensurePluginsLoaded()

src/backend/InvenTree/build/validators.py

@@ -21,3 +21,13 @@ def validate_build_order_reference(value):
 
     # If we get to here, run the "default" validation routine
     Build.validate_reference_field(value)
+
+
+def check_build_output(output, quantity=None):
+    """Run a validation check against each output before accepting it for completion.
+
+    Arguments:
+        output (StockItem): The build output to check
+        quantity (Decimal, optional): The quantity to complete. If None, the full output quantity is assumed.
+    """
+    output.build.can_complete_output(output, quantity=quantity)

src/backend/InvenTree/common/settings.py

@@ -142,12 +142,3 @@ def stock_expiry_enabled():
     from common.models import InvenTreeSetting
 
     return InvenTreeSetting.get_setting('STOCK_ENABLE_EXPIRY', False, create=False)
-
-
-def prevent_build_output_complete_on_incompleted_tests():
-    """Returns True if the completion of the build outputs is disabled until the required tests are passed."""
-    from common.models import InvenTreeSetting
-
-    return InvenTreeSetting.get_setting(
-        'PREVENT_BUILD_COMPLETION_HAVING_INCOMPLETED_TESTS', False, create=False
-    )

src/backend/InvenTree/stock/serializers.py

@@ -1769,7 +1769,7 @@ class StockCountSerializer(StockAdjustmentSerializer):
         fields = ['items', 'notes', 'location']
 
     location = serializers.PrimaryKeyRelatedField(
-        queryset=StockLocation.objects.filter(structural=False),
+        queryset=StockLocation.objects.filter(),
         many=False,
         required=False,
         allow_null=True,
@@ -1777,6 +1777,15 @@ class StockCountSerializer(StockAdjustmentSerializer):
         help_text=_('Set stock location for counted items (optional)'),
     )
 
+    def validate_location(self, location):
+        """Validate the provided location."""
+        if location and location.structural:
+            raise ValidationError(
+                _('Structural locations cannot be assigned stock items')
+            )
+
+        return location
+
     def save(self):
         """Count stock."""
         request = self.context['request']
@@ -1873,7 +1882,7 @@ class StockTransferSerializer(StockAdjustmentSerializer):
     items = StockAdjustmentItemSerializer(many=True, require_non_zero=True)
 
     location = serializers.PrimaryKeyRelatedField(
-        queryset=StockLocation.objects.filter(structural=False),
+        queryset=StockLocation.objects.filter(),
         many=False,
         required=True,
         allow_null=False,
@@ -1881,6 +1890,15 @@ class StockTransferSerializer(StockAdjustmentSerializer):
         help_text=_('Destination stock location'),
     )
 
+    def validate_location(self, location):
+        """Validate the provided location."""
+        if location and location.structural:
+            raise ValidationError(
+                _('Structural locations cannot be assigned stock items')
+            )
+
+        return location
+
     def save(self):
         """Transfer stock."""
         request = self.context['request']
@@ -1964,7 +1982,7 @@ class StockReturnSerializer(StockAdjustmentSerializer):
     )
 
     location = serializers.PrimaryKeyRelatedField(
-        queryset=StockLocation.objects.filter(structural=False),
+        queryset=StockLocation.objects.filter(),
         many=False,
         required=True,
         allow_null=False,
@@ -1972,6 +1990,15 @@ class StockReturnSerializer(StockAdjustmentSerializer):
         help_text=_('Destination stock location'),
     )
 
+    def validate_location(self, location):
+        """Validate the provided location."""
+        if location and location.structural:
+            raise ValidationError(
+                _('Structural locations cannot be assigned stock items')
+            )
+
+        return location
+
     merge = serializers.BooleanField(
         default=False,
         required=False,

src/backend/InvenTree/stock/test_api.py

@@ -2435,7 +2435,10 @@ class StocktakeTest(StockAPITestCase):
             {'items': [{'pk': item_a.pk, 'quantity': 1}], 'location': structural.pk},
             expected_code=400,
         )
-        self.assertIn('does not exist', str(response.data['location']))
+        self.assertIn(
+            'Structural locations cannot be assigned stock items',
+            str(response.data['location']),
+        )
 
 
 class StockTransferMergeTest(StockAPITestCase):