From b51f710ee540eaae9ddce40c992b0f7acbf1d56c Mon Sep 17 00:00:00 2001 From: Oliver Date: Sat, 5 Sep 2026 08:30:02 +1000 Subject: [PATCH] BOM Validation Improvements (#12780) * Adjust get_bom_hash function: - Optionally skip inclusion of part names - Skip 'piece_count' if default value set * Fall back to legacy comparison * Add per-line-item check * Add regression tests --- src/backend/InvenTree/part/models.py | 34 ++++++++-- src/backend/InvenTree/part/test_bom_item.py | 74 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/src/backend/InvenTree/part/models.py b/src/backend/InvenTree/part/models.py index 9e588b5d29..998e9d964b 100644 --- a/src/backend/InvenTree/part/models.py +++ b/src/backend/InvenTree/part/models.py @@ -2069,11 +2069,15 @@ class Part( """Return the number of part BOMs that this part appears in.""" return len(self.get_used_in()) - def get_bom_hash(self) -> str: + def get_bom_hash(self, include_part_names: bool = False) -> str: """Return a checksum hash for the BOM for this part. Used to determine if the BOM has changed (and needs to be signed off!) The hash is calculated by hashing each line item in the BOM. Returns a string representation of a hash object which can be compared with a stored value + + Arguments: + include_part_names: If True, include the part names in the hash calculation (default = False, legacy support). + """ result_hash = hashlib.md5(str(self.id).encode()) @@ -2088,7 +2092,9 @@ class Part( ) for item in bom_items: - result_hash.update(str(item.get_item_hash()).encode()) + result_hash.update( + str(item.get_item_hash(include_part_names=include_part_names)).encode() + ) return str(result_hash.digest()) @@ -2106,7 +2112,11 @@ class Part( # If there is no BOM checksum, then the BOM is not valid return False - return self.get_bom_hash() == self.bom_checksum + # Fallback to legacy BOM hash if the primary calculation does not match + return ( + self.get_bom_hash() == self.bom_checksum + or self.get_bom_hash(include_part_names=True) == self.bom_checksum + ) @transaction.atomic def validate_bom(self, user, valid: bool = True): @@ -4089,12 +4099,20 @@ class BomItem(InvenTree.models.MetadataMixin, InvenTree.models.InvenTreeModel): 'allow_variants', ] - def get_item_hash(self) -> str: - """Calculate the checksum hash of this BOM line item.""" + def get_item_hash(self, include_part_names: bool = False) -> str: + """Calculate the checksum hash of this BOM line item. + + Arguments: + include_part_names: If True, include the names of the parts in the hash calculation (default = False, legacy support) + """ # Seed the hash with the ID of this BOM item result_hash = hashlib.md5(b'') for field in self.hash_fields(): + # Skip the str representation of the parts unless explicitly requested + if not include_part_names and field in ['part', 'sub_part']: + continue + # Get the value of the field value = getattr(self, field, None) @@ -4150,7 +4168,11 @@ class BomItem(InvenTree.models.MetadataMixin, InvenTree.models.InvenTreeModel): if len(self.checksum) == 0: return False - return self.get_item_hash() == self.checksum + # Fallback to legacy item hash if the primary calculation does not match + return ( + self.get_item_hash() == self.checksum + or self.get_item_hash(include_part_names=True) == self.checksum + ) @property def is_consumable(self) -> bool: diff --git a/src/backend/InvenTree/part/test_bom_item.py b/src/backend/InvenTree/part/test_bom_item.py index 98731df81e..f114063420 100644 --- a/src/backend/InvenTree/part/test_bom_item.py +++ b/src/backend/InvenTree/part/test_bom_item.py @@ -596,6 +596,80 @@ class BomItemTest(TestCase): self.assertIsNotNone(assembly.bom_checked_date) + def test_bom_hash_legacy_compatibility(self): + """Regression test for BOM checksum drift caused by unrelated Part edits. + + get_bom_hash() / get_item_hash() used to include the string representation + of the linked 'part' and 'sub_part' objects (which embeds the part's name + and description). This meant that editing a component's name or + description - with no change to the BOM itself - would silently + invalidate every assembly which referenced it. + + The fix excludes part names from the *default* hash calculation, but must + still recognize checksums calculated by the *old* algorithm (via the + 'include_part_names' fallback), so that BOMs validated before this change + are not all instantly marked invalid. + """ + assembly = Part.objects.create( + name='LegacyHashAssembly', description='An assembly part', assembly=True + ) + + sub_part = Part.objects.create( + name='LegacyHashSubPart', description='Original description', component=True + ) + + bom_item = BomItem.objects.create(part=assembly, sub_part=sub_part, quantity=1) + + # The 'include_part_names' flag must actually change the calculated hash + self.assertNotEqual( + assembly.get_bom_hash(), assembly.get_bom_hash(include_part_names=True) + ) + self.assertNotEqual( + bom_item.get_item_hash(), bom_item.get_item_hash(include_part_names=True) + ) + + # Validate the BOM, then overwrite the stored checksums with the *legacy* + # (part-name-inclusive) hash, to simulate a BOM which was validated + # before this change was introduced + assembly.validate_bom(user=None, valid=True) + assembly.bom_checksum = assembly.get_bom_hash(include_part_names=True) + assembly.save() + + bom_item.validate_hash(valid=True) + bom_item.checksum = bom_item.get_item_hash(include_part_names=True) + bom_item.save(check_lock=False) + + # A legacy checksum must still be recognized as valid, via the fallback + self.assertTrue(assembly.is_bom_valid()) + self.assertTrue(BomItem.objects.get(pk=bom_item.pk).is_line_valid) + + # Re-validating a legacy BOM must store a new-format checksum, which no + # longer depends on the part/sub_part string representation + assembly.validate_bom(user=None, valid=True) + self.assertEqual(assembly.bom_checksum, assembly.get_bom_hash()) + self.assertNotEqual( + assembly.bom_checksum, assembly.get_bom_hash(include_part_names=True) + ) + + fresh_item = BomItem.objects.get(pk=bom_item.pk) + self.assertEqual(fresh_item.checksum, fresh_item.get_item_hash()) + + # Editing the sub-part's name/description must *not* invalidate a BOM + # which has been validated under the new algorithm + sub_part.name = 'A renamed sub-part' + sub_part.description = 'An updated description' + sub_part.save() + + self.assertTrue(assembly.is_bom_valid()) + self.assertTrue(BomItem.objects.get(pk=bom_item.pk).is_line_valid) + + # Sanity check: the fix must not mask a *genuine* BOM change + bom_item.set_quantity(2) + bom_item.save() + + self.assertFalse(assembly.is_bom_valid()) + self.assertFalse(BomItem.objects.get(pk=bom_item.pk).is_line_valid) + def test_piece_count_default(self): """Test that piece_count defaults to 1 and does not change existing behavior.""" item = BomItem.objects.get(part=100, sub_part=50)