Closes #16290: Capture entire object in changelog data

This commit is contained in:
Jeremy Stretch 2024-05-28 14:09:41 -04:00
parent 418389c577
commit a094719d23
8 changed files with 119 additions and 18 deletions

View File

@ -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

View File

@ -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
},
}

View File

@ -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 = (
{

View File

@ -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 = {

Binary file not shown.

View File

@ -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 {

View File

@ -112,7 +112,7 @@
{% 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 @@
{% 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>

View File

@ -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