diff --git a/docs/configuration/miscellaneous.md b/docs/configuration/miscellaneous.md index f143be139d5..2582b192899 100644 --- a/docs/configuration/miscellaneous.md +++ b/docs/configuration/miscellaneous.md @@ -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) diff --git a/netbox/extras/models/change_logging.py b/netbox/extras/models/change_logging.py index 7befed09502..0155849aa8f 100644 --- a/netbox/extras/models/change_logging.py +++ b/netbox/extras/models/change_logging.py @@ -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 diff --git a/netbox/extras/signals.py b/netbox/extras/signals.py index 42204f86e44..da0b635ff49 100644 --- a/netbox/extras/signals.py +++ b/netbox/extras/signals.py @@ -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() diff --git a/netbox/extras/tests/test_changelog.py b/netbox/extras/tests/test_changelog.py index 34fd72b2bac..e144c5dee22 100644 --- a/netbox/extras/tests/test_changelog.py +++ b/netbox/extras/tests/test_changelog.py @@ -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 @@ -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): diff --git a/netbox/netbox/models/features.py b/netbox/netbox/models/features.py index 8b0b477dcd3..0cba27318ba 100644 --- a/netbox/netbox/models/features.py +++ b/netbox/netbox/models/features.py @@ -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 @@ -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): @@ -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], @@ -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 diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py index e2cf1cd8c19..59e507d28fb 100644 --- a/netbox/netbox/settings.py +++ b/netbox/netbox/settings.py @@ -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: diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index 2d11810fc51..f3f8c7c5042 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -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): @@ -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