Check build complete (#12289)

* Display non-field errors in table field

* Refactor validation for build output completion

- Run validation BEFORE trying to complete (serializer validation)
- Refactor and simplify common code

* Adjust unit tests

* Fix typing

* Remove debug

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
@@ -52,6 +53,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(
@@ -260,11 +262,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,
@@ -292,6 +302,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
 
 
@@ -527,7 +541,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(),
@@ -554,30 +570,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/frontend/src/components/forms/fields/TableField.tsx

@@ -1,6 +1,7 @@
 import { t } from '@lingui/core/macro';
 import { Trans } from '@lingui/react/macro';
 import {
+  ActionIcon,
   Alert,
   Container,
   Group,
@@ -9,7 +10,10 @@ import {
   Table,
   Text
 } from '@mantine/core';
-import { IconExclamationCircle } from '@tabler/icons-react';
+import {
+  IconCornerDownRight,
+  IconExclamationCircle
+} from '@tabler/icons-react';
 import {
   type ReactNode,
   memo,
@@ -63,13 +67,33 @@ function TableFieldRow({
     );
   }
 
-  return modelRenderer({
-    item: item,
-    rowId: rowId,
-    rowErrors: rowErrors,
-    changeFn: changeFn,
-    removeFn: removeFn
-  });
+  const nonFieldErrors = rowErrors?.non_field_errors;
+
+  return (
+    <>
+      {modelRenderer({
+        item: item,
+        rowId: rowId,
+        rowErrors: rowErrors,
+        changeFn: changeFn,
+        removeFn: removeFn
+      })}
+      {nonFieldErrors && (
+        <Table.Tr key={`table-row-${rowId}-non-field-errors`}>
+          <Table.Td colSpan={columnCount}>
+            <Group gap='xs'>
+              <ActionIcon size='sm' variant='transparent' c='red'>
+                <IconCornerDownRight />
+              </ActionIcon>
+              <Text size='xs' c='red'>
+                {nonFieldErrors.message ?? nonFieldErrors}
+              </Text>
+            </Group>
+          </Table.Td>
+        </Table.Tr>
+      )}
+    </>
+  );
 }
 
 // Memoize each table field row, so that we don't re-render the entire table when a single row is updated