diff --git a/docs/release-notes/version-2.11.md b/docs/release-notes/version-2.11.md index c548d53579d..7ab536c6733 100644 --- a/docs/release-notes/version-2.11.md +++ b/docs/release-notes/version-2.11.md @@ -13,6 +13,7 @@ ### Bug Fixes (from Beta) +* [#5583](https://github.com/netbox-community/netbox/issues/5583) - Eliminate redundant change records when adding/removing tags * [#6100](https://github.com/netbox-community/netbox/issues/6100) - Fix VM interfaces table "add interfaces" link * [#6104](https://github.com/netbox-community/netbox/issues/6104) - Fix location column on racks table * [#6105](https://github.com/netbox-community/netbox/issues/6105) - Hide checkboxes for VMs under cluster VMs view diff --git a/netbox/extras/signals.py b/netbox/extras/signals.py index 0d6295e5b27..63e8f07b770 100644 --- a/netbox/extras/signals.py +++ b/netbox/extras/signals.py @@ -22,23 +22,35 @@ def _handle_changed_object(request, sender, instance, **kwargs): """ Fires when an object is created or updated. """ - # Queue the object for processing once the request completes + m2m_changed = False + + # Determine the type of change being made if kwargs.get('created'): action = ObjectChangeActionChoices.ACTION_CREATE elif 'created' in kwargs: action = ObjectChangeActionChoices.ACTION_UPDATE elif kwargs.get('action') in ['post_add', 'post_remove'] and kwargs['pk_set']: # m2m_changed with objects added or removed + m2m_changed = True action = ObjectChangeActionChoices.ACTION_UPDATE else: return # Record an ObjectChange if applicable if hasattr(instance, 'to_objectchange'): - objectchange = instance.to_objectchange(action) - objectchange.user = request.user - objectchange.request_id = request.id - objectchange.save() + if m2m_changed: + ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(instance), + changed_object_id=instance.pk, + request_id=request.id + ).update( + postchange_data=instance.to_objectchange(action).postchange_data + ) + else: + objectchange = instance.to_objectchange(action) + objectchange.user = request.user + objectchange.request_id = request.id + objectchange.save() # Enqueue webhooks enqueue_webhooks(instance, request.user, request.id, action) diff --git a/netbox/extras/tests/test_changelog.py b/netbox/extras/tests/test_changelog.py index 5e44c83d140..91868832c19 100644 --- a/netbox/extras/tests/test_changelog.py +++ b/netbox/extras/tests/test_changelog.py @@ -56,19 +56,18 @@ def test_create_object(self): response = self.client.post(**request) self.assertHttpStatus(response, 302) + # Verify the creation of a new ObjectChange record site = Site.objects.get(name='Site 1') - # First OC is the creation; second is the tags update - oc_list = ObjectChange.objects.filter( + oc = ObjectChange.objects.get( changed_object_type=ContentType.objects.get_for_model(Site), changed_object_id=site.pk - ).order_by('pk') - self.assertEqual(oc_list[0].changed_object, site) - self.assertEqual(oc_list[0].action, ObjectChangeActionChoices.ACTION_CREATE) - self.assertEqual(oc_list[0].prechange_data, None) - self.assertEqual(oc_list[0].postchange_data['custom_fields']['my_field'], form_data['cf_my_field']) - self.assertEqual(oc_list[0].postchange_data['custom_fields']['my_field_select'], form_data['cf_my_field_select']) - self.assertEqual(oc_list[1].action, ObjectChangeActionChoices.ACTION_UPDATE) - self.assertEqual(oc_list[1].postchange_data['tags'], ['Tag 1', 'Tag 2']) + ) + self.assertEqual(oc.changed_object, site) + self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_CREATE) + self.assertEqual(oc.prechange_data, None) + self.assertEqual(oc.postchange_data['custom_fields']['my_field'], form_data['cf_my_field']) + self.assertEqual(oc.postchange_data['custom_fields']['my_field_select'], form_data['cf_my_field_select']) + self.assertEqual(oc.postchange_data['tags'], ['Tag 1', 'Tag 2']) def test_update_object(self): site = Site(name='Site 1', slug='site-1') @@ -93,8 +92,8 @@ def test_update_object(self): response = self.client.post(**request) self.assertHttpStatus(response, 302) + # Verify the creation of a new ObjectChange record site.refresh_from_db() - # Get only the most recent OC oc = ObjectChange.objects.filter( changed_object_type=ContentType.objects.get_for_model(Site), changed_object_id=site.pk @@ -259,17 +258,15 @@ def test_create_object(self): self.assertHttpStatus(response, status.HTTP_201_CREATED) site = Site.objects.get(pk=response.data['id']) - # First OC is the creation; second is the tags update - oc_list = ObjectChange.objects.filter( + oc = ObjectChange.objects.get( changed_object_type=ContentType.objects.get_for_model(Site), changed_object_id=site.pk - ).order_by('pk') - self.assertEqual(oc_list[0].changed_object, site) - self.assertEqual(oc_list[0].action, ObjectChangeActionChoices.ACTION_CREATE) - self.assertEqual(oc_list[0].prechange_data, None) - self.assertEqual(oc_list[0].postchange_data['custom_fields'], data['custom_fields']) - self.assertEqual(oc_list[1].action, ObjectChangeActionChoices.ACTION_UPDATE) - self.assertEqual(oc_list[1].postchange_data['tags'], ['Tag 1', 'Tag 2']) + ) + self.assertEqual(oc.changed_object, site) + self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_CREATE) + self.assertEqual(oc.prechange_data, None) + self.assertEqual(oc.postchange_data['custom_fields'], data['custom_fields']) + self.assertEqual(oc.postchange_data['tags'], ['Tag 1', 'Tag 2']) def test_update_object(self): site = Site(name='Site 1', slug='site-1') @@ -294,11 +291,10 @@ def test_update_object(self): self.assertHttpStatus(response, status.HTTP_200_OK) site = Site.objects.get(pk=response.data['id']) - # Get only the most recent OC - oc = ObjectChange.objects.filter( + oc = ObjectChange.objects.get( 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.postchange_data['custom_fields'], data['custom_fields']) diff --git a/netbox/utilities/testing/api.py b/netbox/utilities/testing/api.py index f4f4ffefe20..132eea2ae40 100644 --- a/netbox/utilities/testing/api.py +++ b/netbox/utilities/testing/api.py @@ -6,6 +6,8 @@ from rest_framework import status from rest_framework.test import APIClient +from extras.choices import ObjectChangeActionChoices +from extras.models import ObjectChange from users.models import ObjectPermission, Token from .utils import disable_warnings from .views import ModelTestCase @@ -223,13 +225,23 @@ def test_create_object(self): response = self.client.post(self._get_list_url(), self.create_data[0], format='json', **self.header) self.assertHttpStatus(response, status.HTTP_201_CREATED) self.assertEqual(self._get_queryset().count(), initial_count + 1) + instance = self._get_queryset().get(pk=response.data['id']) self.assertInstanceEqual( - self._get_queryset().get(pk=response.data['id']), + instance, self.create_data[0], exclude=self.validation_excluded_fields, api=True ) + # Verify ObjectChange creation + if hasattr(self.model, 'to_objectchange'): + objectchanges = ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(instance), + changed_object_id=instance.pk + ) + self.assertEqual(len(objectchanges), 1) + self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_CREATE) + def test_bulk_create_objects(self): """ POST a set of objects in a single request. @@ -304,6 +316,15 @@ def test_update_object(self): api=True ) + # Verify ObjectChange creation + if hasattr(self.model, 'to_objectchange'): + objectchanges = ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(instance), + changed_object_id=instance.pk + ) + self.assertEqual(len(objectchanges), 1) + self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_UPDATE) + def test_bulk_update_objects(self): """ PATCH a set of objects in a single request. @@ -367,6 +388,15 @@ def test_delete_object(self): self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) self.assertFalse(self._get_queryset().filter(pk=instance.pk).exists()) + # Verify ObjectChange creation + if hasattr(self.model, 'to_objectchange'): + objectchanges = ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(instance), + changed_object_id=instance.pk + ) + self.assertEqual(len(objectchanges), 1) + self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_DELETE) + def test_bulk_delete_objects(self): """ DELETE a set of objects in a single request. diff --git a/netbox/utilities/testing/views.py b/netbox/utilities/testing/views.py index 703780f9523..6b1f4f8a9c5 100644 --- a/netbox/utilities/testing/views.py +++ b/netbox/utilities/testing/views.py @@ -10,7 +10,8 @@ from netaddr import IPNetwork from taggit.managers import TaggableManager -from extras.models import Tag +from extras.choices import ObjectChangeActionChoices +from extras.models import ObjectChange, Tag from users.models import ObjectPermission from utilities.permissions import resolve_permission_ct from .utils import disable_warnings, extract_form_failures, post_data @@ -323,7 +324,16 @@ def test_create_object_with_permission(self): } self.assertHttpStatus(self.client.post(**request), 302) self.assertEqual(initial_count + 1, self._get_queryset().count()) - self.assertInstanceEqual(self._get_queryset().order_by('pk').last(), self.form_data) + instance = self._get_queryset().order_by('pk').last() + self.assertInstanceEqual(instance, self.form_data) + + # Verify ObjectChange creation + objectchanges = ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(instance), + changed_object_id=instance.pk + ) + self.assertEqual(len(objectchanges), 1) + self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_CREATE) @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) def test_create_object_with_constrained_permission(self): @@ -410,6 +420,14 @@ def test_edit_object_with_permission(self): self.assertHttpStatus(self.client.post(**request), 302) self.assertInstanceEqual(self._get_queryset().get(pk=instance.pk), self.form_data) + # Verify ObjectChange creation + objectchanges = ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(instance), + changed_object_id=instance.pk + ) + self.assertEqual(len(objectchanges), 1) + self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_UPDATE) + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) def test_edit_object_with_constrained_permission(self): instance1, instance2 = self._get_queryset().all()[:2] @@ -489,6 +507,14 @@ def test_delete_object_with_permission(self): with self.assertRaises(ObjectDoesNotExist): self._get_queryset().get(pk=instance.pk) + # Verify ObjectChange creation + objectchanges = ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(instance), + changed_object_id=instance.pk + ) + self.assertEqual(len(objectchanges), 1) + self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_DELETE) + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) def test_delete_object_with_constrained_permission(self): instance1, instance2 = self._get_queryset().all()[:2]