Fixes #14329: Improve diffs for custom fields

netbox/extras/models/change_logging.py

@@ -11,7 +11,7 @@ from mptt.models import MPTTModel
 from core.models import ObjectType
 from extras.choices import *
 from netbox.models.features import ChangeLoggingMixin
-from utilities.data import shallow_compare_dict
+from utilities.data import deep_compare_dict
 from ..querysets import ObjectChangeQuerySet
 
 __all__ = (
@@ -198,7 +198,7 @@ class ObjectChange(models.Model):
             changed_attrs = sorted(prechange_data.keys())
         else:
             # TODO: Support deep (recursive) comparison
-            changed_data = shallow_compare_dict(prechange_data, postchange_data)
+            changed_data = deep_compare_dict(prechange_data, postchange_data)
             changed_attrs = sorted(changed_data.keys())
 
         return {

netbox/extras/views.py

@@ -19,7 +19,7 @@ from extras.dashboard.utils import get_widget_class
 from netbox.constants import DEFAULT_ACTION_PERMISSIONS
 from netbox.views import generic
 from netbox.views.generic.mixins import TableMixin
-from utilities.data import shallow_compare_dict
+from utilities.data import deep_compare_dict
 from utilities.forms import ConfirmationForm, get_field_value
 from utilities.htmx import htmx_partial
 from utilities.paginator import EnhancedPaginator, get_paginate_count
@@ -729,21 +729,28 @@ class ObjectChangeView(generic.ObjectView):
             prechange_data = instance.prechange_data_clean
 
         if prechange_data and instance.postchange_data:
-            diff_added = shallow_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 {}
+            diff_added, diff_removed = deep_compare_dict(prechange_data, instance.postchange_data, exclude=('last_updated'))
+            custom_fields_added = diff_added['custom_fields'] if 'custom_fields' in diff_added else None
+            custom_fields_removed = diff_removed['custom_fields'] if 'custom_fields' in diff_removed else None
+            cfr_list = []
+            if custom_fields_added:
+                for cf, cf_value in prechange_data['custom_fields'].items():
+                    cfr_list.append((cf, cf_value, cf in custom_fields_added))
+            cfa_list = []
+            if custom_fields_removed:
+                for cf, cf_value in instance.postchange_data['custom_fields'].items():
+                    cfa_list.append((cf, cf_value, cf in custom_fields_removed))
         else:
             diff_added = None
             diff_removed = None
+            cfa_list = None
+            cfr_list = None
 
         return {
             'diff_added': diff_added,
             'diff_removed': diff_removed,
+            "cfa_list": cfa_list,
+            "cfr_list": cfr_list,
             'next_change': next_change,
             'prev_change': prev_change,
             'related_changes_table': related_changes_table,

netbox/templates/extras/objectchange.html

@@ -112,10 +112,14 @@
             <div class="card-body">
             {% if object.prechange_data %}
               {% spaceless %}
-                <pre class="change-data">
-                  {% for k, v in object.prechange_data_clean.items %}
-                    <span{% if k in diff_removed %} class="removed"{% endif %}>{{ k }}: {{ v|json }}</span>
-                  {% endfor %}
+                <pre class="change-data">{% for k, v in object.prechange_data.items %}{% spaceless %}
+                    {% if k != 'custom_fields' or not cfr_list %}
+                        <span{% if k in diff_removed %} class="removed"{% endif %}>{{ k }}: {{ v|json }}</span>
+                    {% else %}
+                    <span>{{ k }}: {</span>{% for cfr_data in cfr_list %}<span{% if cfr_data.2 %} class="removed"{% endif %}>    {{ cfr_data.0|json }}: {{ cfr_data.1|json|fixindent }}</span>
+                    {% endfor %}<span>}</span>
+                    {% endif %}
+                {% endspaceless %}{% endfor %}
                 </pre>
               {% endspaceless %}
             {% elif non_atomic_change %}
@@ -132,10 +136,14 @@
             <div class="card-body">
                 {% if object.postchange_data %}
                   {% spaceless %}
-                    <pre class="change-data">
-                      {% for k, v in object.postchange_data_clean.items %}
-                        <span{% if k in diff_added %} class="added"{% endif %}>{{ k }}: {{ v|json }}</span>
-                      {% endfor %}
+                    <pre class="change-data">{% for k, v in object.postchange_data.items %}{% spaceless %}
+                        {% if k != 'custom_fields' or not cfa_list %}
+                            <span{% if k in diff_added %} class="added"{% endif %}>{{ k }}: {{ v|json }}</span>
+                        {% else %}
+                            <span>{{ k }}: {</span>{% for cfa_data in cfa_list %}<span{% if cfa_data.2 %} class="added"{% endif %}>    {{ cfa_data.0|json }}: {{ cfa_data.1|json|fixindent }}</span>
+{% endfor %}<span>}</span>
+                        {% endif %}
+                        {% endspaceless %}{% endfor %}
                     </pre>
                   {% endspaceless %}
                 {% else %}

netbox/utilities/data.py

@@ -7,7 +7,7 @@ __all__ = (
     'deepmerge',
     'drange',
     'flatten_dict',
-    'shallow_compare_dict',
+    'deep_compare_dict',
 )
 
 
@@ -46,20 +46,34 @@ def flatten_dict(d, prefix='', separator='.'):
     return ret
 
 
-def shallow_compare_dict(source_dict, destination_dict, exclude=tuple()):
+def deep_compare_dict(old, new, exclude=tuple()):
     """
-    Return a new dictionary of the different keys. The values of `destination_dict` are returned. Only the equality of
-    the first layer of keys/values is checked. `exclude` is a list or tuple of keys to be ignored.
+    Return a tuple of two dictionaries `(removed_diffs, added_diffs)` in a format
+    that is compatible with the requirements of `ObjectChangeView`.
+    `exclude` is a list or tuple of keys to be ignored.
     """
-    difference = {}
+    added_diffs = {}
+    removed_diffs = {}
 
-    for key, value in destination_dict.items():
+    for key in old:
         if key in exclude:
             continue
-        if source_dict.get(key) != value:
-            difference[key] = value
 
-    return difference
+        old_data = old[key]
+        new_data = new[key]
+
+        if old_data != new_data:
+            if isinstance(old_data, dict) and isinstance(new_data, dict):
+                (sub_added, sub_removed) = deep_compare_dict(old_data, new_data, exclude=exclude)
+                if len(sub_removed) > 0:
+                    removed_diffs[key] = sub_removed
+                if len(sub_added) > 0:
+                    added_diffs[key] = sub_added
+            else:
+                removed_diffs[key] = old_data
+                added_diffs[key] = new_data
+
+    return added_diffs, removed_diffs
 
 
 #

netbox/utilities/templatetags/helpers.py

@@ -13,6 +13,7 @@ __all__ = (
     'applied_filters',
     'as_range',
     'divide',
+    'fixindent',
     'get_item',
     'get_key',
     'humanize_megabytes',
@@ -303,3 +304,19 @@ def applied_filters(context, model, form, query_params):
         'applied_filters': applied_filters,
         'save_link': save_link,
     }
+
+
+@register.filter
+def fixindent(value: str) -> str:
+    """
+    Fixes the indentation of multiline strings so they align well
+    within the changelog view.
+    """
+    lines = value.splitlines(keepends=True)
+    # For 1 line, the indentation doesn't need to be fixed
+    if len(lines) == 1:
+        return value
+    ret = lines[0]
+    for line in lines[1:]:
+        ret += f'    {line}'
+    return ret