BOM tree fix (#5870)

* Extend protection against recurisve BOMs

- Prevent part variants from being use BOMs for other variants of the same part

* Add unit tests for new BOM validation checks

* Cleanup  urls.md

* Update unit tests

* Unit test update

* <ore unit test fixes

InvenTree/part/models.py

@@ -483,14 +483,18 @@ class Part(InvenTreeBarcodeMixin, InvenTreeNotesMixin, MetadataMixin, MPTTModel)
         This will fail if:
 
         a) The parent part is the same as this one
-        b) The parent part is used in the BOM for *this* part
-        c) The parent part is used in the BOM for any child parts under this one
+        b) The parent part exists in the same variant tree as this one
+        c) The parent part is used in the BOM for *this* part
+        d) The parent part is used in the BOM for any child parts under this one
         """
         result = True
 
         try:
             if self.pk == parent.pk:
-                raise ValidationError({'sub_part': _(f"Part '{self}' is  used in BOM for '{parent}' (recursive)")})
+                raise ValidationError({'sub_part': _(f"Part '{self}' cannot be used in BOM for '{parent}' (recursive)")})
+
+            if self.tree_id == parent.tree_id:
+                raise ValidationError({'sub_part': _(f"Part '{self}' cannot be used in BOM for '{parent}' (recursive)")})
 
             bom_items = self.get_bom_items()
 

InvenTree/part/test_api.py

@@ -2196,6 +2196,13 @@ class BomItemTest(InvenTreeAPITestCase):
         'part.delete',
     ]
 
+    def setUp(self):
+        """Set up the test case"""
+        super().setUp()
+
+        # Rebuild part tree so BOM items validate correctly
+        Part.objects.rebuild()
+
     def test_bom_list(self):
         """Tests for the BomItem list endpoint."""
         # How many BOM items currently exist in the database?
@@ -2357,6 +2364,7 @@ class BomItemTest(InvenTreeAPITestCase):
 
     def test_get_bom_detail(self):
         """Get the detail view for a single BomItem object."""
+
         url = reverse('api-bom-item-detail', kwargs={'pk': 3})
 
         response = self.get(url, expected_code=200)
@@ -3032,6 +3040,11 @@ class PartMetadataAPITest(InvenTreeAPITestCase):
         'part_category.change',
     ]
 
+    def setUp(self):
+        """Setup unit tets"""
+        super().setUp()
+        Part.objects.rebuild()
+
     def metatester(self, apikey, model):
         """Generic tester"""
         modeldata = model.objects.first()

InvenTree/part/test_bom_export.py

@@ -4,6 +4,7 @@ import csv
 
 from django.urls import reverse
 
+import part.models
 from InvenTree.unit_test import InvenTreeTestCase
 
 
@@ -23,6 +24,8 @@ class BomExportTest(InvenTreeTestCase):
         """Perform test setup functions"""
         super().setUp()
 
+        part.models.Part.objects.rebuild()
+
         self.url = reverse('api-bom-download', kwargs={'pk': 100})
 
     def test_bom_template(self):

InvenTree/part/test_bom_import.py

@@ -22,6 +22,8 @@ class BomUploadTest(InvenTreeAPITestCase):
         """Create BOM data as part of setup routine"""
         super().setUpTestData()
 
+        Part.objects.rebuild()
+
         cls.part = Part.objects.create(
             name='Assembly',
             description='An assembled part',

InvenTree/part/test_bom_item.py

@@ -28,6 +28,10 @@ class BomItemTest(TestCase):
 
     def setUp(self):
         """Create initial data"""
+        super().setUp()
+
+        Part.objects.rebuild()
+
         self.bob = Part.objects.get(id=100)
         self.orphan = Part.objects.get(name='Orphan')
         self.r1 = Part.objects.get(name='R_2K2_0805')
@@ -261,3 +265,37 @@ class BomItemTest(TestCase):
                 p.set_metadata(k, k)
 
             self.assertEqual(len(p.metadata.keys()), 4)
+
+    def test_invalid_bom(self):
+        """Test that ValidationError is correctly raised for an invalid BOM item"""
+
+        # First test: A BOM item which points to itself
+        with self.assertRaises(django_exceptions.ValidationError):
+            BomItem.objects.create(
+                part=self.bob,
+                sub_part=self.bob,
+                quantity=1
+            )
+
+        # Second test: A recursive BOM
+        part_a = Part.objects.create(name='Part A', description="A part which is called A", assembly=True, is_template=True, component=True)
+        part_b = Part.objects.create(name='Part B', description="A part which is called B", assembly=True, component=True)
+        part_c = Part.objects.create(name='Part C', description="A part which is called C", assembly=True, component=True)
+
+        BomItem.objects.create(part=part_a, sub_part=part_b, quantity=10)
+        BomItem.objects.create(part=part_b, sub_part=part_c, quantity=10)
+
+        with self.assertRaises(django_exceptions.ValidationError):
+            BomItem.objects.create(part=part_c, sub_part=part_a, quantity=10)
+
+        with self.assertRaises(django_exceptions.ValidationError):
+            BomItem.objects.create(part=part_c, sub_part=part_b, quantity=10)
+
+        # Third test: A recursive BOM with a variant part
+        part_v = Part.objects.create(name='Part V', description='A part which is called V', variant_of=part_a, assembly=True, component=True)
+
+        with self.assertRaises(django_exceptions.ValidationError):
+            BomItem.objects.create(part=part_a, sub_part=part_v, quantity=10)
+
+        with self.assertRaises(django_exceptions.ValidationError):
+            BomItem.objects.create(part=part_v, sub_part=part_a, quantity=10)