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

* 14147 Prevent logging to Change Log when no changes are made

* 14147 add test

* 14147 add exclude_fields to serialize_object

* 14147 make skip empty default to True

* 14147 remove override of to_objectchange

* Misc cleanup

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>

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)

netbox/extras/models/change_logging.py

@@ -135,3 +135,7 @@ class ObjectChange(models.Model):
 
     def get_action_color(self):
         return ObjectChangeActionChoices.colors.get(self.action)
+
+    @property
+    def has_changes(self):
+        return self.prechange_data != self.postchange_data

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
+            if objectchange and objectchange.has_changes:
-            objectchange.request_id = request.id
+                objectchange.user = request.user
-            objectchange.save()
+                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()

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 @@ class ChangeLogViewTest(ModelViewTestCase):
         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):
 

netbox/netbox/models/features.py

@@ -15,6 +15,7 @@ from core.choices import JobStatusChoices
 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 @@ class ChangeLoggingMixin(models.Model):
         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 @@ class ChangeLoggingMixin(models.Model):
         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
 

netbox/netbox/settings.py

@@ -177,6 +177,7 @@ STORAGE_CONFIG = getattr(configuration, 'STORAGE_CONFIG', {})
 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:

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