diff --git a/netbox/extras/api/serializers_/change_logging.py b/netbox/extras/api/serializers_/change_logging.py index 32585637c..46fb901ff 100644 --- a/netbox/extras/api/serializers_/change_logging.py +++ b/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 diff --git a/netbox/extras/models/change_logging.py b/netbox/extras/models/change_logging.py index ebcebc09a..8451a0d15 100644 --- a/netbox/extras/models/change_logging.py +++ b/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 @@ class ObjectChange(models.Model): 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 + }, + } diff --git a/netbox/extras/tests/test_changelog.py b/netbox/extras/tests/test_changelog.py index d9d6f1f45..aac526e0f 100644 --- a/netbox/extras/tests/test_changelog.py +++ b/netbox/extras/tests/test_changelog.py @@ -75,6 +75,10 @@ class ChangeLogViewTest(ModelViewTestCase): 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 @@ class ChangeLogViewTest(ModelViewTestCase): 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 @@ class ChangeLogViewTest(ModelViewTestCase): 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 @@ class ChangeLogAPITest(APITestCase): 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 @@ class ChangeLogAPITest(APITestCase): 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 @@ class ChangeLogAPITest(APITestCase): 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 = ( { diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 3a82539fb..82f519c00 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -723,15 +723,15 @@ class ObjectChangeView(generic.ObjectView): 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 = { diff --git a/netbox/project-static/dist/netbox.css b/netbox/project-static/dist/netbox.css index a1179a319..082a5b64d 100644 Binary files a/netbox/project-static/dist/netbox.css and b/netbox/project-static/dist/netbox.css differ diff --git a/netbox/project-static/styles/custom/_code.scss b/netbox/project-static/styles/custom/_code.scss index 4b068b94d..1be563b21 100644 --- a/netbox/project-static/styles/custom/_code.scss +++ b/netbox/project-static/styles/custom/_code.scss @@ -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 { diff --git a/netbox/templates/extras/objectchange.html b/netbox/templates/extras/objectchange.html index 0aee6185b..368a71821 100644 --- a/netbox/templates/extras/objectchange.html +++ b/netbox/templates/extras/objectchange.html @@ -112,7 +112,7 @@ {% if object.prechange_data %} {% spaceless %}
- {% for k, v in object.prechange_data.items %}
+ {% for k, v in object.prechange_data_clean.items %}
{{ k }}: {{ v|json }}
{% endfor %}
@@ -132,7 +132,7 @@
{% if object.postchange_data %}
{% spaceless %}
- {% for k, v in object.postchange_data.items %}
+ {% for k, v in object.postchange_data_clean.items %}
{{ k }}: {{ v|json }}
{% endfor %}
diff --git a/netbox/utilities/serialization.py b/netbox/utilities/serialization.py
index f7a2002d1..af1169e97 100644
--- a/netbox/utilities/serialization.py
+++ b/netbox/utilities/serialization.py
@@ -2,7 +2,6 @@ import json
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