From d96174cddbc2566fea53b3fe39b5d4aa5d30fbf1 Mon Sep 17 00:00:00 2001 From: Oliver Date: Fri, 30 Oct 2026 14:43:12 +1100 Subject: [PATCH] [bug] better handling of empty values for audit log (#13134) * [bug] better handling of empty values for audit log * reverse unrelated commits --- src/backend/InvenTree/audit/models.py | 27 +++++++++++++++------- src/backend/InvenTree/audit/tests.py | 33 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/backend/InvenTree/audit/models.py b/src/backend/InvenTree/audit/models.py index 1e11d481b8..70af57de20 100644 --- a/src/backend/InvenTree/audit/models.py +++ b/src/backend/InvenTree/audit/models.py @@ -119,6 +119,20 @@ def drop_orphan_currency_changes(instance, changes: dict) -> dict: return result +def is_unchanged_value(value) -> bool: + """Return True if a recorded [old, new] change pair does not represent a real change. + + None and '' are treated as equivalent, as nullable string fields (e.g. Part.IPN) + may hold either value for "empty" - e.g. a NULL value saved from a form as ''. + """ + if not (isinstance(value, (list, tuple)) and len(value) == 2): + return False + + old, new = (None if v == '' else v for v in value) + + return old == new + + def is_concrete_field(model, field_name: str) -> bool: """Return True if field_name is a concrete field on the model (or is unknown).""" try: @@ -224,9 +238,10 @@ class AuditLogEntryManager(LogEntryManager): def log_create(self, instance, force_log: bool = False, **kwargs): """Create a new log entry for the provided instance. - For updates, fields whose recorded old and new values are identical are dropped. - These occur when the in-memory value differs from the database value only in type - (e.g. an empty FieldFile vs. an empty string), and would otherwise add noise to every save. + For updates, fields whose recorded old and new values are identical (or both empty, + i.e. None or '') are dropped. These occur when the in-memory value differs from the + database value only in type (e.g. an empty FieldFile vs. an empty string, or a NULL + value saved from a form as ''), and would otherwise add noise to every save. Updates which assign a query expression (e.g. F('completed') + 1) are deferred until after the save, so that the value actually written can be recorded @@ -264,11 +279,7 @@ class AuditLogEntryManager(LogEntryManager): changes = { field: value for field, value in changes.items() - if not ( - isinstance(value, (list, tuple)) - and len(value) == 2 - and value[0] == value[1] - ) + if not is_unchanged_value(value) } if not changes and not force_log: diff --git a/src/backend/InvenTree/audit/tests.py b/src/backend/InvenTree/audit/tests.py index 86c96e55f5..bbbb71ca1c 100644 --- a/src/backend/InvenTree/audit/tests.py +++ b/src/backend/InvenTree/audit/tests.py @@ -208,6 +208,39 @@ class AuditModelTest(AuditTestMixin, TestCase): self.assertNotIn('reference_int', entry.changes_dict) self.assertNotIn('metadata', entry.changes_dict) + def test_empty_values_equivalent(self): + """Changing a field between None and '' is not recorded as a change.""" + Part.objects.filter(pk=self.part.pk).update(IPN=None) + self.part.refresh_from_db() + + # Saving an empty value from a form ('') over NULL + self.part.IPN = '' + self.part.description = 'Updated' + self.part.save() + + entry = self.entries_for(self.part, action=AuditLogEntry.Action.UPDATE).last() + self.assertEqual(entry.changes_dict, {'description': ['A part', 'Updated']}) + + # No entry is created if the only "change" is between empty values + self.part.IPN = None + self.part.save() + + self.assertEqual( + self.entries_for(self.part, action=AuditLogEntry.Action.UPDATE).count(), 1 + ) + + # A real change to or from an empty value is still recorded + self.part.IPN = 'IPN-001' + self.part.save() + self.part.IPN = '' + self.part.save() + + entries = self.entries_for(self.part, action=AuditLogEntry.Action.UPDATE) + self.assertEqual( + [e.changes_dict['IPN'] for e in entries.order_by('pk')[1:]], + [[None, 'IPN-001'], ['IPN-001', '']], + ) + def test_parent_link(self): """Child entries are linked to the parent - and are retained after deletion.""" line = PurchaseOrderLineItem.objects.create(