mirror of
https://github.com/inventree/InvenTree.git
synced 2026-10-14 14:11:55 +00:00
Fix MPTT check for BOM validity (#13008)
Use refreshed DB values rather than validating against in-memory values
This commit is contained in:
1 parent
fa04bcc455
commit
76a21b7899
2 files changed
+69
-1
No files matched your search
@@ -540,6 +540,12 @@ class Part(
|
||||
# For legacy reasons the 'variant_of' field is used to indicate the MPTT parent
|
||||
parent_attr = 'variant_of'
|
||||
|
||||
# Do not order the tree by name (overrides InvenTreeTree.MPTTMeta).
|
||||
# Ordered insertion of a new root node shifts the tree_id of every following
|
||||
# tree in the database, which is expensive (O(N) row updates per new Part),
|
||||
# and leaves the tree_id of any Part instances held in memory stale.
|
||||
order_insertion_by = []
|
||||
|
||||
@staticmethod
|
||||
def get_api_url():
|
||||
"""Return the list API endpoint URL associated with the Part model."""
|
||||
@@ -686,7 +692,16 @@ class Part(
|
||||
)
|
||||
})
|
||||
|
||||
if self.tree_id == parent.tree_id:
|
||||
# Read the tree_id values from the database, as in-memory values may be stale
|
||||
tree_ids = dict(
|
||||
Part.objects.filter(pk__in=[self.pk, parent.pk]).values_list(
|
||||
'pk', 'tree_id'
|
||||
)
|
||||
)
|
||||
|
||||
if tree_ids.get(self.pk, self.tree_id) == tree_ids.get(
|
||||
parent.pk, parent.tree_id
|
||||
):
|
||||
raise ValidationError({
|
||||
'sub_part': _(
|
||||
f"Part '{self}' cannot be used in BOM for '{parent}' (recursive)"
|
||||
|
||||
@@ -378,6 +378,59 @@ class BomItemTest(TestCase):
|
||||
with self.assertRaises(django_exceptions.ValidationError):
|
||||
BomItem.objects.create(part=part_v, sub_part=part_a, quantity=10)
|
||||
|
||||
def test_bom_unrelated_root_parts(self):
|
||||
"""Unrelated top-level parts created in succession can be added to each other's BOM.
|
||||
|
||||
Regression test: ordered (by name) insertion of a new root Part shifted the tree_id
|
||||
of following trees in the database, leaving instances held in memory with a stale
|
||||
tree_id - which could match the new part's tree_id, falsely flagging the BOM as recursive.
|
||||
"""
|
||||
# Part 'Y' is inserted directly before Part 'Z' when ordered by name
|
||||
part_z = Part.objects.create(
|
||||
name='zzzz assembly', description='Z', assembly=True, component=True
|
||||
)
|
||||
part_y = Part.objects.create(
|
||||
name='zzzy assembly', description='Y', assembly=True, component=True
|
||||
)
|
||||
|
||||
# In-memory tree_id values match the database
|
||||
self.assertEqual(part_z.tree_id, Part.objects.get(pk=part_z.pk).tree_id)
|
||||
self.assertNotEqual(part_z.tree_id, part_y.tree_id)
|
||||
|
||||
# No refresh_from_db() required
|
||||
BomItem.objects.create(part=part_y, sub_part=part_z, quantity=1)
|
||||
|
||||
def test_root_part_create_preserves_tree_ids(self):
|
||||
"""Creating a new top-level part does not modify the tree_id of any other part."""
|
||||
tree_ids = dict(Part.objects.values_list('pk', 'tree_id'))
|
||||
|
||||
# Name sorts before every existing part
|
||||
Part.objects.create(name='!!! first part', description='first')
|
||||
|
||||
self.assertEqual(
|
||||
dict(Part.objects.filter(pk__in=tree_ids).values_list('pk', 'tree_id')),
|
||||
tree_ids,
|
||||
)
|
||||
|
||||
def test_check_add_to_bom_stale_tree_id(self):
|
||||
"""check_add_to_bom() uses database tree_id values, not stale in-memory values."""
|
||||
part_a = Part.objects.create(
|
||||
name='Stale A', description='A', assembly=True, is_template=True
|
||||
)
|
||||
part_b = Part.objects.create(name='Stale B', description='B', component=True)
|
||||
|
||||
# Simulate a stale in-memory tree_id which matches the other part
|
||||
part_b.tree_id = part_a.tree_id
|
||||
|
||||
self.assertTrue(part_b.check_add_to_bom(part_a, raise_error=True))
|
||||
|
||||
# A genuine variant relationship is still detected
|
||||
part_v = Part.objects.create(
|
||||
name='Stale V', description='V', variant_of=part_a, component=True
|
||||
)
|
||||
|
||||
self.assertFalse(part_v.check_add_to_bom(part_a))
|
||||
|
||||
def test_locked_assembly(self):
|
||||
"""Test that BomItem objects work correctly for a 'locked' assembly."""
|
||||
assembly = Part.objects.create(
|
||||
|
||||
Reference in new issue
Block a user