From 139e059a457172ee568c281c50d0ae405d711888 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Wed, 10 Aug 2022 23:37:29 +1200 Subject: [PATCH] Check for (and reduce) overallocated stock when editing build orders. (#3237) * Check for (and reduce) overallocated stock when editing build orders. After a build order is edited, check whether any bom items are now overallocated to the build, and if they are find appropriate build items to reduce the quantity of or remove from the build. Fixes: #3236 * Only trim overallocated stock if requested Turns the complete with overallocated stock option into a choice, so the user can reject (default), continue (existing option in the bool), or now additionally instead choose to have the overallocated stock removed before the build is completed. * fix style errors * Another style fix. * Add tests for overallocation handling API logic * Capitalize overallocation options. --- InvenTree/build/models.py | 28 ++++++++++ InvenTree/build/serializers.py | 29 ++++++++-- InvenTree/build/test_api.py | 99 ++++++++++++++++++++++++++++++++++ InvenTree/build/test_build.py | 69 ++++++++++++++++++++++-- 4 files changed, 217 insertions(+), 8 deletions(-) diff --git a/InvenTree/build/models.py b/InvenTree/build/models.py index c71defc40cd2..1f7303770781 100644 --- a/InvenTree/build/models.py +++ b/InvenTree/build/models.py @@ -736,6 +736,34 @@ def delete_output(self, output): # Remove the build output from the database output.delete() + @transaction.atomic + def trim_allocated_stock(self): + """Called after save to reduce allocated stock if the build order is now overallocated.""" + allocations = BuildItem.objects.filter(build=self) + + # Only need to worry about untracked stock here + for bom_item in self.untracked_bom_items: + reduce_by = self.allocated_quantity(bom_item) - self.required_quantity(bom_item) + if reduce_by <= 0: + continue # all OK + + # find builditem(s) to trim + for a in allocations.filter(bom_item=bom_item): + # Previous item completed the job + if reduce_by == 0: + break + + # Easy case - this item can just be reduced. + if a.quantity > reduce_by: + a.quantity -= reduce_by + a.save() + break + + # Harder case, this item needs to be deleted, and any remainder + # taken from the next items in the list. + reduce_by -= a.quantity + a.delete() + @transaction.atomic def subtract_allocated_stock(self, user): """Called when the Build is marked as "complete", this function removes the allocated untracked items from stock.""" diff --git a/InvenTree/build/serializers.py b/InvenTree/build/serializers.py index 74fdf599ee2d..309e63071eaf 100644 --- a/InvenTree/build/serializers.py +++ b/InvenTree/build/serializers.py @@ -473,21 +473,36 @@ def save(self): ) +class OverallocationChoice(): + """Utility class to contain options for handling over allocated stock items.""" + + REJECT = 'reject' + ACCEPT = 'accept' + TRIM = 'trim' + + OPTIONS = { + REJECT: ('Not permitted'), + ACCEPT: _('Accept as consumed by this build order'), + TRIM: _('Deallocate before completing this build order'), + } + + class BuildCompleteSerializer(serializers.Serializer): """DRF serializer for marking a BuildOrder as complete.""" - accept_overallocated = serializers.BooleanField( - label=_('Accept Overallocated'), - help_text=_('Accept stock items which have been overallocated to this build order'), + accept_overallocated = serializers.ChoiceField( + label=_('Overallocated Stock'), + choices=list(OverallocationChoice.OPTIONS.items()), + help_text=_('How do you want to handle extra stock items assigned to the build order'), required=False, - default=False, + default=OverallocationChoice.REJECT, ) def validate_accept_overallocated(self, value): """Check if the 'accept_overallocated' field is required""" build = self.context['build'] - if build.has_overallocated_parts(output=None) and not value: + if build.has_overallocated_parts(output=None) and value == OverallocationChoice.REJECT: raise ValidationError(_('Some stock items have been overallocated')) return value @@ -541,6 +556,10 @@ def save(self): request = self.context['request'] build = self.context['build'] + data = self.validated_data + if data.get('accept_overallocated', OverallocationChoice.REJECT) == OverallocationChoice.TRIM: + build.trim_allocated_stock() + build.complete_build(request.user) diff --git a/InvenTree/build/test_api.py b/InvenTree/build/test_api.py index 8af14ddb677f..02304b006609 100644 --- a/InvenTree/build/test_api.py +++ b/InvenTree/build/test_api.py @@ -717,6 +717,105 @@ def test_valid_data(self): self.assertEqual(allocation.stock_item.pk, 2) +class BuildOverallocationTest(BuildAPITest): + """Unit tests for over allocation of stock items against a build order. + + Using same Build ID=1 as allocation test above. + """ + + def setUp(self): + """Basic operation as part of test suite setup""" + super().setUp() + + self.assignRole('build.add') + self.assignRole('build.change') + + self.build = Build.objects.get(pk=1) + self.url = reverse('api-build-finish', kwargs={'pk': self.build.pk}) + + StockItem.objects.create(part=Part.objects.get(pk=50), quantity=30) + + # Keep some state for use in later assertions, and then overallocate + self.state = {} + self.allocation = {} + for i, bi in enumerate(self.build.part.bom_items.all()): + rq = self.build.required_quantity(bi, None) + i + 1 + si = StockItem.objects.filter(part=bi.sub_part, quantity__gte=rq).first() + + self.state[bi.sub_part] = (si, si.quantity, rq) + BuildItem.objects.create( + build=self.build, + stock_item=si, + quantity=rq, + ) + + # create and complete outputs + self.build.create_build_output(self.build.quantity) + outputs = self.build.build_outputs.all() + self.build.complete_build_output(outputs[0], self.user) + + # Validate expected state after set-up. + self.assertEqual(self.build.incomplete_outputs.count(), 0) + self.assertEqual(self.build.complete_outputs.count(), 1) + self.assertEqual(self.build.completed, self.build.quantity) + + def test_overallocated_requires_acceptance(self): + """Test build order cannot complete with overallocated items.""" + # Try to complete the build (it should fail due to overallocation) + response = self.post( + self.url, + {}, + expected_code=400 + ) + self.assertTrue('accept_overallocated' in response.data) + + # Check stock items have not reduced at all + for si, oq, _ in self.state.values(): + si.refresh_from_db() + self.assertEqual(si.quantity, oq) + + # Accept overallocated stock + self.post( + self.url, + { + 'accept_overallocated': 'accept', + }, + expected_code=201, + ) + + self.build.refresh_from_db() + + # Build should have been marked as complete + self.assertTrue(self.build.is_complete) + + # Check stock items have reduced in-line with the overallocation + for si, oq, rq in self.state.values(): + si.refresh_from_db() + self.assertEqual(si.quantity, oq - rq) + + def test_overallocated_can_trim(self): + """Test build order will trim/de-allocate overallocated stock when requested.""" + self.post( + self.url, + { + 'accept_overallocated': 'trim', + }, + expected_code=201, + ) + + self.build.refresh_from_db() + + # Build should have been marked as complete + self.assertTrue(self.build.is_complete) + + # Check stock items have reduced only by bom requirement (overallocation trimmed) + for bi in self.build.part.bom_items.all(): + si, oq, _ = self.state[bi.sub_part] + rq = self.build.required_quantity(bi, None) + si.refresh_from_db() + self.assertEqual(si.quantity, oq - rq) + + class BuildListTest(BuildAPITest): """Tests for the BuildOrder LIST API.""" diff --git a/InvenTree/build/test_build.py b/InvenTree/build/test_build.py index 79c7953c1de2..23925eba1731 100644 --- a/InvenTree/build/test_build.py +++ b/InvenTree/build/test_build.py @@ -7,6 +7,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import Group from django.core.exceptions import ValidationError +from django.db.models import Sum from InvenTree import status_codes as status @@ -17,6 +18,9 @@ from stock.models import StockItem from users.models import Owner +import logging +logger = logging.getLogger('inventree') + class BuildTestBase(TestCase): """Run some tests to ensure that the Build model is working properly.""" @@ -120,9 +124,9 @@ def setUp(self): self.stock_2_1 = StockItem.objects.create(part=self.sub_part_2, quantity=5) self.stock_2_2 = StockItem.objects.create(part=self.sub_part_2, quantity=5) - self.stock_2_2 = StockItem.objects.create(part=self.sub_part_2, quantity=5) - self.stock_2_2 = StockItem.objects.create(part=self.sub_part_2, quantity=5) - self.stock_2_2 = StockItem.objects.create(part=self.sub_part_2, quantity=5) + self.stock_2_3 = StockItem.objects.create(part=self.sub_part_2, quantity=5) + self.stock_2_4 = StockItem.objects.create(part=self.sub_part_2, quantity=5) + self.stock_2_5 = StockItem.objects.create(part=self.sub_part_2, quantity=5) self.stock_3_1 = StockItem.objects.create(part=self.sub_part_3, quantity=1000) @@ -375,6 +379,65 @@ def test_partial_allocation(self): self.assertTrue(self.build.are_untracked_parts_allocated()) + def test_overallocation_and_trim(self): + """Test overallocation of stock and trim function""" + + # Fully allocate tracked stock (not eligible for trimming) + self.allocate_stock( + self.output_1, + { + self.stock_3_1: 6, + } + ) + self.allocate_stock( + self.output_2, + { + self.stock_3_1: 14, + } + ) + # Fully allocate part 1 (should be left alone) + self.allocate_stock( + None, + { + self.stock_1_1: 3, + self.stock_1_2: 47, + } + ) + + extra_2_1 = StockItem.objects.create(part=self.sub_part_2, quantity=6) + extra_2_2 = StockItem.objects.create(part=self.sub_part_2, quantity=4) + + # Overallocate part 2 (30 needed) + self.allocate_stock( + None, + { + self.stock_2_1: 5, + self.stock_2_2: 5, + self.stock_2_3: 5, + self.stock_2_4: 5, + self.stock_2_5: 5, # 25 + extra_2_1: 6, # 31 + extra_2_2: 4, # 35 + } + ) + self.assertTrue(self.build.has_overallocated_parts(None)) + + self.build.trim_allocated_stock() + self.assertFalse(self.build.has_overallocated_parts(None)) + + self.build.complete_build_output(self.output_1, None) + self.build.complete_build_output(self.output_2, None) + self.assertTrue(self.build.can_complete) + + self.build.complete_build(None) + + self.assertEqual(self.build.status, status.BuildStatus.COMPLETE) + + # Check stock items are in expected state. + self.assertEqual(StockItem.objects.get(pk=self.stock_1_2.pk).quantity, 53) + self.assertEqual(StockItem.objects.filter(part=self.sub_part_2).aggregate(Sum('quantity'))['quantity__sum'], 5) + self.assertEqual(StockItem.objects.get(pk=self.stock_3_1.pk).quantity, 980) + def test_cancel(self): """Test cancellation of the build"""