Skip to content

14147 Prevent logging to Change Log when no changes are made #14477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/configuration/miscellaneous.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ changes in the database indefinitely.

---

## CHANGELOG_SKIP_EMPTY_CHANGES

Default: True

If enabled, a change log record will not be created when an object is updated without any changes to its existing field values.

!!! note
The object's `last_updated` field will always reflect the time of the most recent update, regardless of this parameter.

---

## DATA_UPLOAD_MAX_MEMORY_SIZE

Default: `2621440` (2.5 MB)
Expand Down
4 changes: 4 additions & 0 deletions netbox/extras/models/change_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,7 @@ def get_absolute_url(self):

def get_action_color(self):
return ObjectChangeActionChoices.colors.get(self.action)

@property
def has_changes(self):
return self.prechange_data != self.postchange_data
7 changes: 4 additions & 3 deletions netbox/extras/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ def handle_changed_object(sender, instance, **kwargs):
)
else:
objectchange = instance.to_objectchange(action)
objectchange.user = request.user
objectchange.request_id = request.id
objectchange.save()
if objectchange and objectchange.has_changes:
objectchange.user = request.user
objectchange.request_id = request.id
objectchange.save()

# If this is an M2M change, update the previously queued webhook (from post_save)
queue = events_queue.get()
Expand Down
61 changes: 61 additions & 0 deletions netbox/extras/tests/test_changelog.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from django.contrib.contenttypes.models import ContentType
from django.test import override_settings
from django.urls import reverse
from rest_framework import status

Expand Down Expand Up @@ -207,6 +208,66 @@ def test_bulk_delete_objects(self):
self.assertEqual(objectchange.prechange_data['slug'], sites[0].slug)
self.assertEqual(objectchange.postchange_data, None)

@override_settings(CHANGELOG_SKIP_EMPTY_CHANGES=False)
def test_update_object_change(self):
# Create a Site
site = Site.objects.create(
name='Site 1',
slug='site-1',
status=SiteStatusChoices.STATUS_PLANNED,
custom_field_data={
'cf1': None,
'cf2': None
}
)

# Update it with the same field values
form_data = {
'name': site.name,
'slug': site.slug,
'status': SiteStatusChoices.STATUS_PLANNED,
}
request = {
'path': self._get_url('edit', instance=site),
'data': post_data(form_data),
}
self.add_permissions('dcim.change_site', 'extras.view_tag')
response = self.client.post(**request)
self.assertHttpStatus(response, 302)

# Check that an ObjectChange record has been created
self.assertEqual(ObjectChange.objects.count(), 1)

@override_settings(CHANGELOG_SKIP_EMPTY_CHANGES=True)
def test_update_object_nochange(self):
# Create a Site
site = Site.objects.create(
name='Site 1',
slug='site-1',
status=SiteStatusChoices.STATUS_PLANNED,
custom_field_data={
'cf1': None,
'cf2': None
}
)

# Update it with the same field values
form_data = {
'name': site.name,
'slug': site.slug,
'status': SiteStatusChoices.STATUS_PLANNED,
}
request = {
'path': self._get_url('edit', instance=site),
'data': post_data(form_data),
}
self.add_permissions('dcim.change_site', 'extras.view_tag')
response = self.client.post(**request)
self.assertHttpStatus(response, 302)

# Check that no ObjectChange records have been created
self.assertEqual(ObjectChange.objects.count(), 0)


class ChangeLogAPITest(APITestCase):

Expand Down
21 changes: 17 additions & 4 deletions netbox/netbox/models/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from core.models import ContentType
from extras.choices import *
from extras.utils import is_taggable, register_features
from netbox.config import get_config
from netbox.registry import registry
from netbox.signals import post_clean
from utilities.json import CustomFieldJSONEncoder
Expand Down Expand Up @@ -63,19 +64,26 @@ class ChangeLoggingMixin(models.Model):
class Meta:
abstract = True

def serialize_object(self):
def serialize_object(self, exclude=None):
"""
Return a JSON representation of the instance. Models can override this method to replace or extend the default
serialization logic provided by the `serialize_object()` utility function.

Args:
exclude: An iterable of attribute names to omit from the serialized output
"""
return serialize_object(self)
return serialize_object(self, exclude=exclude or [])

def snapshot(self):
"""
Save a snapshot of the object's current state in preparation for modification. The snapshot is saved as
`_prechange_snapshot` on the instance.
"""
self._prechange_snapshot = self.serialize_object()
exclude_fields = []
if get_config().CHANGELOG_SKIP_EMPTY_CHANGES:
exclude_fields = ['last_updated',]

self._prechange_snapshot = self.serialize_object(exclude=exclude_fields)
snapshot.alters_data = True

def to_objectchange(self, action):
Expand All @@ -84,6 +92,11 @@ def to_objectchange(self, action):
by ChangeLoggingMiddleware.
"""
from extras.models import ObjectChange

exclude = []
if get_config().CHANGELOG_SKIP_EMPTY_CHANGES:
exclude = ['last_updated']

objectchange = ObjectChange(
changed_object=self,
object_repr=str(self)[:200],
Expand All @@ -92,7 +105,7 @@ def to_objectchange(self, action):
if hasattr(self, '_prechange_snapshot'):
objectchange.prechange_data = self._prechange_snapshot
if action in (ObjectChangeActionChoices.ACTION_CREATE, ObjectChangeActionChoices.ACTION_UPDATE):
objectchange.postchange_data = self.serialize_object()
objectchange.postchange_data = self.serialize_object(exclude=exclude)

return objectchange

Expand Down
1 change: 1 addition & 0 deletions netbox/netbox/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@
TIME_FORMAT = getattr(configuration, 'TIME_FORMAT', 'g:i a')
TIME_ZONE = getattr(configuration, 'TIME_ZONE', 'UTC')
ENABLE_LOCALIZATION = getattr(configuration, 'ENABLE_LOCALIZATION', False)
CHANGELOG_SKIP_EMPTY_CHANGES = getattr(configuration, 'CHANGELOG_SKIP_EMPTY_CHANGES', True)

# Check for hard-coded dynamic config parameters
for param in PARAMS:
Expand Down
21 changes: 14 additions & 7 deletions netbox/utilities/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,23 @@ def count_related(model, field):
return Coalesce(subquery, 0)


def serialize_object(obj, resolve_tags=True, extra=None):
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.

Args:
obj: The object to serialize
resolve_tags: If true, any assigned tags will be represented by their names
extra: Any additional data to include in the serialized output. Keys provided in this mapping will
override object attributes.
exclude: An iterable of attributes to exclude from the serialized output
"""
json_str = serializers.serialize('json', [obj])
data = json.loads(json_str)[0]['fields']
exclude = exclude or []

# Exclude any MPTTModel fields
if issubclass(obj.__class__, MPTTModel):
Expand All @@ -169,16 +177,15 @@ def serialize_object(obj, resolve_tags=True, extra=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
for key in list(data.keys()):
if key in exclude or (isinstance(key, str) and key.startswith('_')):
data.pop(key)

# Append any extra data
if extra is not None:
data.update(extra)

# Copy keys to list to avoid 'dictionary changed size during iteration' exception
for key in list(data):
# Private fields shouldn't be logged in the object change
if isinstance(key, str) and key.startswith('_'):
data.pop(key)

return data


Expand Down