Closes #16290: Capture entire object in changelog data

Author: jeremystretch

netbox/extras/api/serializers_/change_logging.py

@@ -30,6 +30,16 @@ class ObjectChangeSerializer(BaseModelSerializer):
     changed_object = serializers.SerializerMethodField(
         read_only=True
     )
+    prechange_data = serializers.JSONField(
+        source='prechange_data_clean',
+        read_only=True,
+        allow_null=True
+    )
+    postchange_data = serializers.JSONField(
+        source='postchange_data_clean',
+        read_only=True,
+        allow_null=True
+    )
 
     class Meta:
         model = ObjectChange

netbox/extras/models/change_logging.py

@@ -1,12 +1,17 @@
+from functools import cached_property
+
 from django.conf import settings
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
+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 ..querysets import ObjectChangeQuerySet
 
 __all__ = (
@@ -136,6 +141,71 @@ def get_absolute_url(self):
     def get_action_color(self):
         return ObjectChangeActionChoices.colors.get(self.action)
 
-    @property
+    @cached_property
     def has_changes(self):
         return self.prechange_data != self.postchange_data
+
+    @cached_property
+    def diff_exclude_fields(self):
+        """
+        Return a set of attributes which should be ignored when calculating a diff
+        between the pre- and post-change data. (For instance, it would not make
+        sense to compare the "last updated" times as these are expected to differ.)
+        """
+        model = self.changed_object_type.model_class()
+        attrs = set()
+
+        # Exclude auto-populated change tracking fields
+        if issubclass(model, ChangeLoggingMixin):
+            attrs.update({'created', 'last_updated'})
+
+        # Exclude MPTT-internal fields
+        if issubclass(model, MPTTModel):
+            attrs.update({'level', 'lft', 'rght', 'tree_id'})
+
+        return attrs
+
+    def get_clean_data(self, prefix):
+        """
+        Return only the pre-/post-change attributes which are relevant for calculating a diff.
+        """
+        ret = {}
+        change_data = getattr(self, f'{prefix}_data') or {}
+        for k, v in change_data.items():
+            if k not in self.diff_exclude_fields and not k.startswith('_'):
+                ret[k] = v
+        return ret
+
+    @cached_property
+    def prechange_data_clean(self):
+        return self.get_clean_data('prechange')
+
+    @cached_property
+    def postchange_data_clean(self):
+        return self.get_clean_data('postchange')
+
+    def diff(self):
+        """
+        Return a dictionary of pre- and post-change values for attributes which have changed.
+        """
+        prechange_data = self.prechange_data_clean
+        postchange_data = self.postchange_data_clean
+
+        # 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:
+            # TODO: Support deep (recursive) comparison
+            changed_data = shallow_compare_dict(prechange_data, postchange_data)
+            changed_attrs = sorted(changed_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
+            },
+        }

netbox/extras/tests/test_changelog.py

@@ -75,6 +75,10 @@ def test_create_object(self):
         self.assertEqual(oc.postchange_data['custom_fields']['cf2'], form_data['cf_cf2'])
         self.assertEqual(oc.postchange_data['tags'], ['Tag 1', 'Tag 2'])
 
+        # Check that private attributes were included in raw data but not display data
+        self.assertIn('_name', oc.postchange_data)
+        self.assertNotIn('_name', oc.postchange_data_clean)
+
     def test_update_object(self):
         site = Site(name='Site 1', slug='site-1')
         site.save()
@@ -112,6 +116,12 @@ def test_update_object(self):
         self.assertEqual(oc.postchange_data['custom_fields']['cf2'], form_data['cf_cf2'])
         self.assertEqual(oc.postchange_data['tags'], ['Tag 3'])
 
+        # Check that private attributes were included in raw data but not display data
+        self.assertIn('_name', oc.prechange_data)
+        self.assertNotIn('_name', oc.prechange_data_clean)
+        self.assertIn('_name', oc.postchange_data)
+        self.assertNotIn('_name', oc.postchange_data_clean)
+
     def test_delete_object(self):
         site = Site(
             name='Site 1',
@@ -142,6 +152,10 @@ def test_delete_object(self):
         self.assertEqual(oc.prechange_data['tags'], ['Tag 1', 'Tag 2'])
         self.assertEqual(oc.postchange_data, None)
 
+        # Check that private attributes were included in raw data but not display data
+        self.assertIn('_name', oc.prechange_data)
+        self.assertNotIn('_name', oc.prechange_data_clean)
+
     def test_bulk_update_objects(self):
         sites = (
             Site(name='Site 1', slug='site-1', status=SiteStatusChoices.STATUS_ACTIVE),
@@ -338,6 +352,10 @@ def test_create_object(self):
         self.assertEqual(oc.postchange_data['custom_fields'], data['custom_fields'])
         self.assertEqual(oc.postchange_data['tags'], ['Tag 1', 'Tag 2'])
 
+        # Check that private attributes were included in raw data but not display data
+        self.assertIn('_name', oc.postchange_data)
+        self.assertNotIn('_name', oc.postchange_data_clean)
+
     def test_update_object(self):
         site = Site(name='Site 1', slug='site-1')
         site.save()
@@ -370,6 +388,12 @@ def test_update_object(self):
         self.assertEqual(oc.postchange_data['custom_fields'], data['custom_fields'])
         self.assertEqual(oc.postchange_data['tags'], ['Tag 3'])
 
+        # Check that private attributes were included in raw data but not display data
+        self.assertIn('_name', oc.prechange_data)
+        self.assertNotIn('_name', oc.prechange_data_clean)
+        self.assertIn('_name', oc.postchange_data)
+        self.assertNotIn('_name', oc.postchange_data_clean)
+
     def test_delete_object(self):
         site = Site(
             name='Site 1',
@@ -398,6 +422,10 @@ def test_delete_object(self):
         self.assertEqual(oc.prechange_data['tags'], ['Tag 1', 'Tag 2'])
         self.assertEqual(oc.postchange_data, None)
 
+        # Check that private attributes were included in raw data but not display data
+        self.assertIn('_name', oc.prechange_data)
+        self.assertNotIn('_name', oc.prechange_data_clean)
+
     def test_bulk_create_objects(self):
         data = (
             {

netbox/extras/views.py

@@ -723,15 +723,15 @@ def get_extra_context(self, request, instance):
 
         if not instance.prechange_data and instance.action in ['update', 'delete'] and prev_change:
             non_atomic_change = True
-            prechange_data = prev_change.postchange_data
+            prechange_data = prev_change.postchange_data_clean
         else:
             non_atomic_change = False
-            prechange_data = instance.prechange_data
+            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 or dict(),
+                instance.postchange_data_clean or dict(),
                 exclude=['last_updated'],
             )
             diff_removed = {

netbox/project-static/dist/netbox.css

@@ -1,7 +1,7 @@
 // Serialized data from change records
 pre.change-data {
-  padding-right: 0;
-  padding-left: 0;
+  border-radius: 0;
+  padding: 0;
 
   // Display each line individually for highlighting
   > span {

netbox/project-static/styles/custom/_code.scss

@@ -112,7 +112,7 @@ <h5 class="card-header">{% trans "Pre-Change Data" %}</h5>
             {% if object.prechange_data %}
               {% spaceless %}
                 <pre class="change-data">
-                  {% for k, v in object.prechange_data.items %}
+                  {% for k, v in object.prechange_data_clean.items %}
                     <span{% if k in diff_removed %} class="removed"{% endif %}>{{ k }}: {{ v|json }}</span>
                   {% endfor %}
                 </pre>
@@ -132,7 +132,7 @@ <h5 class="card-header">{% trans "Post-Change Data" %}</h5>
                 {% if object.postchange_data %}
                   {% spaceless %}
                     <pre class="change-data">
-                      {% for k, v in object.postchange_data.items %}
+                      {% for k, v in object.postchange_data_clean.items %}
                         <span{% if k in diff_added %} class="added"{% endif %}>{{ k }}: {{ v|json }}</span>
                       {% endfor %}
                     </pre>

netbox/templates/extras/objectchange.html

@@ -2,7 +2,6 @@
 
 from django.contrib.contenttypes.models import ContentType
 from django.core import serializers
-from mptt.models import MPTTModel
 
 from extras.utils import is_taggable
 
@@ -16,8 +15,7 @@ def serialize_object(obj, resolve_tags=True, extra=None, exclude=None):
     """
     Return a generic JSON representation of an object using Django's built-in serializer. (This is used for things like
     change logging, not the REST API.) Optionally include a dictionary to supplement the object data. A list of keys
-    can be provided to exclude them from the returned dictionary. Private fields (prefaced with an underscore) are
-    implicitly excluded.
+    can be provided to exclude them from the returned dictionary.
 
     Args:
         obj: The object to serialize
@@ -30,11 +28,6 @@ def serialize_object(obj, resolve_tags=True, extra=None, exclude=None):
     data = json.loads(json_str)[0]['fields']
     exclude = exclude or []
 
-    # Exclude any MPTTModel fields
-    if issubclass(obj.__class__, MPTTModel):
-        for field in ['level', 'lft', 'rght', 'tree_id']:
-            data.pop(field)
-
     # Include custom_field_data as "custom_fields"
     if hasattr(obj, 'custom_field_data'):
         data['custom_fields'] = data.pop('custom_field_data')
@@ -45,9 +38,9 @@ def serialize_object(obj, resolve_tags=True, extra=None, exclude=None):
         tags = getattr(obj, '_tags', None) or obj.tags.all()
         data['tags'] = sorted([tag.name for tag in tags])
 
-    # Skip excluded and private (prefixes with an underscore) attributes
+    # Skip any excluded attributes
     for key in list(data.keys()):
-        if key in exclude or (isinstance(key, str) and key.startswith('_')):
+        if key in exclude:
             data.pop(key)
 
     # Append any extra data