Misc cleanup

docs/configuration/miscellaneous.md

@@ -84,10 +84,10 @@ changes in the database indefinitely.
 
 Default: True
 
-Enables skipping the creation of logged changes on updates if there were no modifications to the object.
+If enabled, a change log record will not be created when an object is updated without any changes to its existing field values.
 
 !!! note
-    As a side-effect of turning this on, the `last_updated` field will not be included in the change log record.
+    The object's `last_updated` field will always reflect the time of the most recent update, regardless of this parameter.
 
 ---
 

netbox/extras/tests/test_changelog.py

@@ -210,7 +210,8 @@ class ChangeLogViewTest(ModelViewTestCase):
 
     @override_settings(CHANGELOG_SKIP_EMPTY_CHANGES=False)
     def test_update_object_change(self):
-        site = Site(
+        # Create a Site
+        site = Site.objects.create(
             name='Site 1',
             slug='site-1',
             status=SiteStatusChoices.STATUS_PLANNED,
@@ -219,15 +220,13 @@ class ChangeLogViewTest(ModelViewTestCase):
                 'cf2': None
             }
         )
-        site.save()
 
+        # Update it with the same field values
         form_data = {
             'name': site.name,
             'slug': site.slug,
             'status': SiteStatusChoices.STATUS_PLANNED,
         }
-
-        oc_count = ObjectChange.objects.count()
         request = {
             'path': self._get_url('edit', instance=site),
             'data': post_data(form_data),
@@ -235,11 +234,14 @@ class ChangeLogViewTest(ModelViewTestCase):
         self.add_permissions('dcim.change_site', 'extras.view_tag')
         response = self.client.post(**request)
         self.assertHttpStatus(response, 302)
-        self.assertNotEqual(oc_count, ObjectChange.objects.count())
+
+        # 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):
-        site = Site(
+        # Create a Site
+        site = Site.objects.create(
             name='Site 1',
             slug='site-1',
             status=SiteStatusChoices.STATUS_PLANNED,
@@ -248,15 +250,13 @@ class ChangeLogViewTest(ModelViewTestCase):
                 'cf2': None
             }
         )
-        site.save()
 
+        # Update it with the same field values
         form_data = {
             'name': site.name,
             'slug': site.slug,
             'status': SiteStatusChoices.STATUS_PLANNED,
         }
-
-        oc_count = ObjectChange.objects.count()
         request = {
             'path': self._get_url('edit', instance=site),
             'data': post_data(form_data),
@@ -264,7 +264,9 @@ class ChangeLogViewTest(ModelViewTestCase):
         self.add_permissions('dcim.change_site', 'extras.view_tag')
         response = self.client.post(**request)
         self.assertHttpStatus(response, 302)
-        self.assertEqual(oc_count, ObjectChange.objects.count())
+
+        # Check that no ObjectChange records have been created
+        self.assertEqual(ObjectChange.objects.count(), 0)
 
 
 class ChangeLogAPITest(APITestCase):

netbox/netbox/models/features.py

@@ -64,12 +64,15 @@ class ChangeLoggingMixin(models.Model):
     class Meta:
         abstract = True
 
-    def serialize_object(self, exclude_fields=[]):
+    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, exclude_fields=exclude_fields)
+        return serialize_object(self, exclude=exclude or [])
 
     def snapshot(self):
         """
@@ -80,7 +83,7 @@ class ChangeLoggingMixin(models.Model):
         if get_config().CHANGELOG_SKIP_EMPTY_CHANGES:
             exclude_fields = ['last_updated',]
 
-        self._prechange_snapshot = self.serialize_object(exclude_fields=exclude_fields)
+        self._prechange_snapshot = self.serialize_object(exclude=exclude_fields)
     snapshot.alters_data = True
 
     def to_objectchange(self, action):
@@ -90,9 +93,9 @@ class ChangeLoggingMixin(models.Model):
         """
         from extras.models import ObjectChange
 
-        exclude_fields = []
+        exclude = []
         if get_config().CHANGELOG_SKIP_EMPTY_CHANGES:
-            exclude_fields = ['last_updated',]
+            exclude = ['last_updated']
 
         objectchange = ObjectChange(
             changed_object=self,
@@ -102,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(exclude_fields=exclude_fields)
+            objectchange.postchange_data = self.serialize_object(exclude=exclude)
 
         return objectchange
 

netbox/utilities/utils.py

@@ -144,26 +144,29 @@ def count_related(model, field):
     return Coalesce(subquery, 0)
 
 
-def serialize_object(obj, resolve_tags=True, extra=None, exclude_fields=[]):
+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):
         for field in ['level', 'lft', 'rght', 'tree_id']:
             data.pop(field)
 
-    if exclude_fields:
-        for field in exclude_fields:
-            if field in data:
-                data.pop(field)
-
     # Include custom_field_data as "custom_fields"
     if hasattr(obj, 'custom_field_data'):
         data['custom_fields'] = data.pop('custom_field_data')
@@ -174,16 +177,15 @@ def serialize_object(obj, resolve_tags=True, extra=None, exclude_fields=[]):
         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