Fix serialization of tags for REST API updates

netbox/extras/api/serializers.py

@@ -108,6 +108,10 @@ class TaggedObjectSerializer(serializers.Serializer):
 
     def update(self, instance, validated_data):
         tags = validated_data.pop('tags', [])
+
+        # Cache tags on instance for change logging
+        instance._tags = tags
+
         instance = super().update(instance, validated_data)
 
         return self._save_tags(instance, tags)

netbox/extras/tests/test_changelog.py

@@ -4,7 +4,7 @@ from rest_framework import status
 
 from dcim.models import Site
 from extras.choices import *
-from extras.models import CustomField, CustomFieldValue, ObjectChange
+from extras.models import CustomField, CustomFieldValue, ObjectChange, Tag
 from utilities.testing import APITestCase
 
 
@@ -23,6 +23,14 @@ class ChangeLogTest(APITestCase):
         cf.save()
         cf.obj_type.set([ct])
 
+        # Create some tags
+        tags = (
+            Tag(name='Tag 1', slug='tag-1'),
+            Tag(name='Tag 2', slug='tag-2'),
+            Tag(name='Tag 3', slug='tag-3'),
+        )
+        Tag.objects.bulk_create(tags)
+
     def test_create_object(self):
         data = {
             'name': 'Test Site 1',
@@ -30,6 +38,10 @@ class ChangeLogTest(APITestCase):
             'custom_fields': {
                 'my_field': 'ABC'
             },
+            'tags': [
+                {'name': 'Tag 1'},
+                {'name': 'Tag 2'},
+            ]
         }
         self.assertEqual(ObjectChange.objects.count(), 0)
         url = reverse('dcim-api:site-list')
@@ -39,13 +51,16 @@ class ChangeLogTest(APITestCase):
         self.assertHttpStatus(response, status.HTTP_201_CREATED)
 
         site = Site.objects.get(pk=response.data['id'])
-        oc = ObjectChange.objects.get(
+        # First OC is the creation; second is the tags update
+        oc_list = ObjectChange.objects.filter(
             changed_object_type=ContentType.objects.get_for_model(Site),
             changed_object_id=site.pk
-        )
+        ).order_by('pk')
-        self.assertEqual(oc.changed_object, site)
+        self.assertEqual(oc_list[0].changed_object, site)
-        self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_CREATE)
+        self.assertEqual(oc_list[0].action, ObjectChangeActionChoices.ACTION_CREATE)
-        self.assertEqual(oc.object_data['custom_fields'], data['custom_fields'])
+        self.assertEqual(oc_list[0].object_data['custom_fields'], data['custom_fields'])
+        self.assertEqual(oc_list[1].action, ObjectChangeActionChoices.ACTION_UPDATE)
+        self.assertEqual(oc_list[1].object_data['tags'], ['Tag 1', 'Tag 2'])
 
     def test_update_object(self):
         site = Site(name='Test Site 1', slug='test-site-1')
@@ -57,6 +72,9 @@ class ChangeLogTest(APITestCase):
             'custom_fields': {
                 'my_field': 'DEF'
             },
+            'tags': [
+                {'name': 'Tag 3'}
+            ]
         }
         self.assertEqual(ObjectChange.objects.count(), 0)
         self.add_permissions('dcim.change_site')
@@ -66,13 +84,15 @@ class ChangeLogTest(APITestCase):
         self.assertHttpStatus(response, status.HTTP_200_OK)
 
         site = Site.objects.get(pk=response.data['id'])
-        oc = ObjectChange.objects.get(
+        # Get only the most recent OC
+        oc = ObjectChange.objects.filter(
             changed_object_type=ContentType.objects.get_for_model(Site),
             changed_object_id=site.pk
-        )
+        ).first()
         self.assertEqual(oc.changed_object, site)
         self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_UPDATE)
         self.assertEqual(oc.object_data['custom_fields'], data['custom_fields'])
+        self.assertEqual(oc.object_data['tags'], ['Tag 3'])
 
     def test_delete_object(self):
         site = Site(
@@ -80,6 +100,7 @@ class ChangeLogTest(APITestCase):
             slug='test-site-1'
         )
         site.save()
+        site.tags.set(*Tag.objects.all()[:2])
         CustomFieldValue.objects.create(
             field=CustomField.objects.get(name='my_field'),
             obj=site,
@@ -98,3 +119,4 @@ class ChangeLogTest(APITestCase):
         self.assertEqual(oc.object_repr, site.name)
         self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_DELETE)
         self.assertEqual(oc.object_data['custom_fields'], {'my_field': 'ABC'})
+        self.assertEqual(oc.object_data['tags'], ['Tag 1', 'Tag 2'])

netbox/utilities/utils.py

@@ -97,9 +97,10 @@ def serialize_object(obj, extra=None, exclude=None):
             field: str(value) for field, value in obj.cf.items()
         }
 
-    # Include any tags
+    # Include any tags. Check for tags cached on the instance; fall back to using the manager.
     if is_taggable(obj):
-        data['tags'] = [tag.name for tag in obj.tags.all()]
+        tags = getattr(obj, '_tags', obj.tags.all())
+        data['tags'] = [tag.name for tag in tags]
 
     # Append any extra data
     if extra is not None: