From 1fb6507cc1cb4c0980be9c2324076a2c59b6f642 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 17 Mar 2026 09:44:01 -0700 Subject: [PATCH] #14329 Improve diffs for custom_fields --- netbox/core/models/change_logging.py | 24 +++++----- netbox/core/views.py | 4 +- netbox/templates/core/objectchange.html | 4 +- netbox/utilities/tests/test_data.py | 59 +++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 17 deletions(-) diff --git a/netbox/core/models/change_logging.py b/netbox/core/models/change_logging.py index 09da4fe4d..4409994da 100644 --- a/netbox/core/models/change_logging.py +++ b/netbox/core/models/change_logging.py @@ -199,20 +199,18 @@ class ObjectChange(models.Model): # Determine which attributes have changed if self.action == ObjectChangeActionChoices.ACTION_CREATE: changed_attrs = sorted(postchange_data.keys()) - elif self.action == ObjectChangeActionChoices.ACTION_DELETE: - changed_attrs = sorted(prechange_data.keys()) - else: - diff_added, diff_removed = deep_compare_dict(prechange_data, postchange_data) return { - 'pre': dict(sorted(diff_removed.items())), - 'post': dict(sorted(diff_added.items())), + 'pre': {k: prechange_data.get(k) for k in changed_attrs}, + 'post': {k: postchange_data.get(k) for k in changed_attrs}, } - + if self.action == ObjectChangeActionChoices.ACTION_DELETE: + changed_attrs = sorted(prechange_data.keys()) + return { + 'pre': {k: prechange_data.get(k) for k in changed_attrs}, + 'post': {k: postchange_data.get(k) for k in changed_attrs}, + } + diff_added, diff_removed = deep_compare_dict(prechange_data, postchange_data) return { - 'pre': { - k: prechange_data.get(k) for k in changed_attrs - }, - 'post': { - k: postchange_data.get(k) for k in changed_attrs - }, + 'pre': dict(sorted(diff_removed.items())), + 'post': dict(sorted(diff_added.items())), } diff --git a/netbox/core/views.py b/netbox/core/views.py index bf767d18b..2b8eb4c52 100644 --- a/netbox/core/views.py +++ b/netbox/core/views.py @@ -274,8 +274,8 @@ class ObjectChangeView(generic.ObjectView): if prechange_data and instance.postchange_data: diff_added, diff_removed = deep_compare_dict( - prechange_data or dict(), - instance.postchange_data_clean or dict(), + prechange_data, + instance.postchange_data_clean, exclude=['last_updated'], ) else: diff --git a/netbox/templates/core/objectchange.html b/netbox/templates/core/objectchange.html index d08af4db7..1864e2332 100644 --- a/netbox/templates/core/objectchange.html +++ b/netbox/templates/core/objectchange.html @@ -124,7 +124,7 @@ {% if subdiff.items %} {{ k }}: { {% for sub_k, sub_v in v.items %} - {{ sub_k }}: {{ sub_v|json }} + {{ sub_k }}: {{ sub_v|json }} {% endfor %} } {% else %} @@ -154,7 +154,7 @@ {% if subdiff.items %} {{ k }}: { {% for sub_k, sub_v in v.items %} - {{ sub_k }}: {{ sub_v|json }} + {{ sub_k }}: {{ sub_v|json }} {% endfor %} } {% else %} diff --git a/netbox/utilities/tests/test_data.py b/netbox/utilities/tests/test_data.py index 0d4bd95df..957f09ed7 100644 --- a/netbox/utilities/tests/test_data.py +++ b/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,64 @@ class RangeFunctionsTestCase(TestCase): ) +class DeepCompareDictTestCase(TestCase): + + def test_no_changes(self): + source = {'a': 1, 'b': 'foo', 'c': {'x': 1, 'y': 2}} + added, removed = deep_compare_dict(source, source) + 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'}} + added, removed = deep_compare_dict(source, source) + self.assertEqual(added, {}) + self.assertEqual(removed, {}) + + def test_mixed_flat_and_nested(self): + source = {'name': 'old', 'custom_fields': {'cf1': 'old', 'cf2': 'same'}} + dest = {'name': 'new', 'custom_fields': {'cf1': 'new', 'cf2': 'same'}} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {'name': 'new', 'custom_fields': {'cf1': 'new'}}) + self.assertEqual(removed, {'name': 'old', 'custom_fields': {'cf1': 'old'}}) + + 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}) + + class GetConfigValueCITestCase(TestCase): def test_exact_match(self):