From 28399bed25f16e72ebc78e384f48baad4265401a Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Tue, 7 Nov 2023 09:48:04 +1100
Subject: [PATCH] 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          | 10 +++++---
 InvenTree/part/test_api.py        | 13 +++++++++++
 InvenTree/part/test_bom_export.py |  3 +++
 InvenTree/part/test_bom_import.py |  2 ++
 InvenTree/part/test_bom_item.py   | 38 +++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py
index 7451c8cf63..745704f580 100644
--- a/InvenTree/part/models.py
+++ b/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()
 
diff --git a/InvenTree/part/test_api.py b/InvenTree/part/test_api.py
index e75709058a..219d13b7d2 100644
--- a/InvenTree/part/test_api.py
+++ b/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()
diff --git a/InvenTree/part/test_bom_export.py b/InvenTree/part/test_bom_export.py
index e2f299dcc3..1027e0c89e 100644
--- a/InvenTree/part/test_bom_export.py
+++ b/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):
diff --git a/InvenTree/part/test_bom_import.py b/InvenTree/part/test_bom_import.py
index de6c706993..ae24418499 100644
--- a/InvenTree/part/test_bom_import.py
+++ b/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',
diff --git a/InvenTree/part/test_bom_item.py b/InvenTree/part/test_bom_item.py
index 1925d00b42..fecd0abbb1 100644
--- a/InvenTree/part/test_bom_item.py
+++ b/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)