#14329 Improve diffs for custom_fields

netbox/core/models/change_logging.py

@@ -11,7 +11,7 @@ from mptt.models import MPTTModel
 from core.choices import ObjectChangeActionChoices
 from core.querysets import ObjectChangeQuerySet
 from netbox.models.features import ChangeLoggingMixin, has_feature
-from utilities.data import shallow_compare_dict
+from utilities.data import deep_compare_dict, shallow_compare_dict
 
 __all__ = (
     'ObjectChange',
@@ -202,9 +202,11 @@ class ObjectChange(models.Model):
         elif self.action == ObjectChangeActionChoices.ACTION_DELETE:
             changed_attrs = sorted(prechange_data.keys())
         else:
-            # TODO: Support deep (recursive) comparison
-            changed_data = shallow_compare_dict(prechange_data, postchange_data)
-            changed_attrs = sorted(changed_data.keys())
+            diff_added, diff_removed = deep_compare_dict(prechange_data, postchange_data)
+            return {
+                'pre': dict(sorted(diff_removed.items())),
+                'post': dict(sorted(diff_added.items())),
+            }
 
         return {
             'pre': {

netbox/core/views.py

@@ -30,7 +30,7 @@ from netbox.views import generic
 from netbox.views.generic.base import BaseObjectView
 from netbox.views.generic.mixins import TableMixin
 from utilities.apps import get_installed_apps
-from utilities.data import shallow_compare_dict
+from utilities.data import deep_compare_dict, shallow_compare_dict
 from utilities.forms import ConfirmationForm
 from utilities.htmx import htmx_partial
 from utilities.json import ConfigJSONEncoder
@@ -273,14 +273,11 @@ class ObjectChangeView(generic.ObjectView):
             prechange_data = instance.prechange_data_clean
 
         if prechange_data and instance.postchange_data:
-            diff_added = shallow_compare_dict(
+            diff_added, diff_removed = deep_compare_dict(
                 prechange_data or dict(),
                 instance.postchange_data_clean or dict(),
                 exclude=['last_updated'],
             )
-            diff_removed = {
-                x: prechange_data.get(x) for x in diff_added
-            } if prechange_data else {}
         else:
             diff_added = None
             diff_removed = None

netbox/utilities/data.py

@@ -7,6 +7,7 @@ __all__ = (
     'array_to_ranges',
     'array_to_string',
     'check_ranges_overlap',
+    'deep_compare_dict',
     'deepmerge',
     'drange',
     'flatten_dict',
@@ -83,6 +84,35 @@ def shallow_compare_dict(source_dict, destination_dict, exclude=tuple()):
     return difference
 
 
+def deep_compare_dict(source_dict, destination_dict, exclude=tuple()):
+    """
+    Return a two-tuple of dictionaries (added, removed) representing the differences between source_dict and
+    destination_dict. For values which are themselves dicts, the comparison is performed recursively such that only
+    the changed keys within the nested dict are included. `exclude` is a list or tuple of keys to be ignored.
+    """
+    added = {}
+    removed = {}
+
+    all_keys = set(source_dict) | set(destination_dict)
+    for key in all_keys:
+        if key in exclude:
+            continue
+        src_val = source_dict.get(key)
+        dst_val = destination_dict.get(key)
+        if src_val == dst_val:
+            continue
+        if isinstance(src_val, dict) and isinstance(dst_val, dict):
+            sub_added, sub_removed = deep_compare_dict(src_val, dst_val)
+            if sub_added or sub_removed:
+                added[key] = sub_added
+                removed[key] = sub_removed
+        else:
+            added[key] = dst_val
+            removed[key] = src_val
+
+    return added, removed
+
+
 #
 # Array utilities
 #

netbox/utilities/tests/test_data.py

@@ -3,6 +3,7 @@ from django.test import TestCase
 
 from utilities.data import (
     check_ranges_overlap,
+    deep_compare_dict,
     get_config_value_ci,
     ranges_to_string,
     ranges_to_string_list,
@@ -100,6 +101,66 @@ class RangeFunctionsTestCase(TestCase):
         )
 
 
+class DeepCompareDictTestCase(TestCase):
+
+    def test_no_changes(self):
+        source = {'a': 1, 'b': 'foo', 'c': {'x': 1, 'y': 2}}
+        dest = {'a': 1, 'b': 'foo', 'c': {'x': 1, 'y': 2}}
+        added, removed = deep_compare_dict(source, dest)
+        self.assertEqual(added, {})
+        self.assertEqual(removed, {})
+
+    def test_scalar_change(self):
+        source = {'a': 1, 'b': 'foo'}
+        dest = {'a': 2, 'b': 'foo'}
+        added, removed = deep_compare_dict(source, dest)
+        self.assertEqual(added, {'a': 2})
+        self.assertEqual(removed, {'a': 1})
+
+    def test_key_added(self):
+        source = {'a': 1}
+        dest = {'a': 1, 'b': 'new'}
+        added, removed = deep_compare_dict(source, dest)
+        self.assertEqual(added, {'b': 'new'})
+        self.assertEqual(removed, {'b': None})
+
+    def test_key_removed(self):
+        source = {'a': 1, 'b': 'old'}
+        dest = {'a': 1}
+        added, removed = deep_compare_dict(source, dest)
+        self.assertEqual(added, {'b': None})
+        self.assertEqual(removed, {'b': 'old'})
+
+    def test_nested_dict_partial_change(self):
+        """Only changed sub-keys of a nested dict are included."""
+        source = {'custom_fields': {'cf1': 'old', 'cf2': 'unchanged'}}
+        dest = {'custom_fields': {'cf1': 'new', 'cf2': 'unchanged'}}
+        added, removed = deep_compare_dict(source, dest)
+        self.assertEqual(added, {'custom_fields': {'cf1': 'new'}})
+        self.assertEqual(removed, {'custom_fields': {'cf1': 'old'}})
+
+    def test_nested_dict_no_change(self):
+        source = {'name': 'test', 'custom_fields': {'cf1': 'same'}}
+        dest = {'name': 'test', 'custom_fields': {'cf1': 'same'}}
+        added, removed = deep_compare_dict(source, dest)
+        self.assertEqual(added, {})
+        self.assertEqual(removed, {})
+
+    def test_exclude(self):
+        source = {'a': 1, 'last_updated': '2024-01-01'}
+        dest = {'a': 2, 'last_updated': '2024-06-01'}
+        added, removed = deep_compare_dict(source, dest, exclude=['last_updated'])
+        self.assertEqual(added, {'a': 2})
+        self.assertEqual(removed, {'a': 1})
+
+    def test_deeply_nested(self):
+        source = {'level1': {'level2': {'val': 'old', 'other': 'same'}}}
+        dest = {'level1': {'level2': {'val': 'new', 'other': 'same'}}}
+        added, removed = deep_compare_dict(source, dest)
+        self.assertEqual(added, {'level1': {'level2': {'val': 'new'}}})
+        self.assertEqual(removed, {'level1': {'level2': {'val': 'old'}}})
+
+
 class GetConfigValueCITestCase(TestCase):
 
     def test_exact_match(self):