#19680 fix deletion dependency order for GenericRelations

netbox/core/signals.py

@@ -162,6 +162,12 @@ def handle_deleted_object(sender, instance, **kwargs):
                 getattr(obj, related_field_name).remove(instance)
             elif type(relation) is ManyToOneRel and relation.field.null is True:
                 setattr(obj, related_field_name, None)
+                # make sure the object hasn't been deleted - in case of
+                # deletion chaining of related objects
+                try:
+                    obj.refresh_from_db()
+                except DoesNotExist:
+                    continue
                 obj.save()
 
     # Enqueue the object for event processing

netbox/core/tests/test_changelog.py

@@ -6,12 +6,13 @@
 from core.choices import ObjectChangeActionChoices
 from core.models import ObjectChange, ObjectType
 from dcim.choices import SiteStatusChoices
-from dcim.models import Site
+from dcim.models import Site, CableTermination, Device, DeviceType, DeviceRole, Interface, Cable
 from extras.choices import *
 from extras.models import CustomField, CustomFieldChoiceSet, Tag
 from utilities.testing import APITestCase
 from utilities.testing.utils import create_tags, post_data
 from utilities.testing.views import ModelViewTestCase
+from dcim.models import Manufacturer
 
 
 class ChangeLogViewTest(ModelViewTestCase):
@@ -270,6 +271,81 @@ def test_update_object_nochange(self):
         # Check that no ObjectChange records have been created
         self.assertEqual(ObjectChange.objects.count(), 0)
 
+    def test_ordering_genericrelation(self):
+        # Create required objects first
+        manufacturer = Manufacturer.objects.create(name='Manufacturer 1')
+        device_type = DeviceType.objects.create(
+            manufacturer=manufacturer,
+            model='Model 1',
+            slug='model-1'
+        )
+        device_role = DeviceRole.objects.create(
+            name='Role 1',
+            slug='role-1'
+        )
+        site = Site.objects.create(
+            name='Site 1',
+            slug='site-1'
+        )
+
+        # Create two devices
+        device1 = Device.objects.create(
+            name='Device 1',
+            device_type=device_type,
+            role=device_role,
+            site=site
+        )
+        device2 = Device.objects.create(
+            name='Device 2',
+            device_type=device_type,
+            role=device_role,
+            site=site
+        )
+
+        # Create interfaces on both devices
+        interface1 = Interface.objects.create(
+            device=device1,
+            name='eth0',
+            type='1000base-t'
+        )
+        interface2 = Interface.objects.create(
+            device=device2,
+            name='eth0',
+            type='1000base-t'
+        )
+
+        # Create a cable between the interfaces
+        _ = Cable.objects.create(
+            a_terminations=[interface1],
+            b_terminations=[interface2],
+            status='connected'
+        )
+
+        # Delete device1
+        request = {
+            'path': reverse('dcim:device_delete', kwargs={'pk': device1.pk}),
+            'data': post_data({'confirm': True}),
+        }
+        self.add_permissions(
+            'dcim.delete_device',
+            'dcim.delete_interface',
+            'dcim.delete_cable',
+            'dcim.delete_cabletermination'
+        )
+        response = self.client.post(**request)
+        self.assertHttpStatus(response, 302)
+
+        # Get the ObjectChange records for delete actions ordered by time
+        changes = ObjectChange.objects.filter(
+            action=ObjectChangeActionChoices.ACTION_DELETE
+        ).order_by('time')[:3]
+
+        # Verify the order of deletion
+        self.assertEqual(len(changes), 3)
+        self.assertEqual(changes[0].changed_object_type, ContentType.objects.get_for_model(CableTermination))
+        self.assertEqual(changes[1].changed_object_type, ContentType.objects.get_for_model(Interface))
+        self.assertEqual(changes[2].changed_object_type, ContentType.objects.get_for_model(Device))
+
 
 class ChangeLogAPITest(APITestCase):
 

netbox/netbox/models/deletion.py

@@ -0,0 +1,90 @@
+import logging
+
+from django.contrib.contenttypes.fields import GenericRelation
+from django.db import router
+from django.db.models.deletion import Collector
+
+logger = logging.getLogger("netbox.models.deletion")
+
+
+class CustomCollector(Collector):
+    """
+    Custom collector that handles GenericRelations correctly.
+    """
+
+    def collect(
+        self,
+        objs,
+        source=None,
+        nullable=False,
+        collect_related=True,
+        source_attr=None,
+        reverse_dependency=False,
+        keep_parents=False,
+        fail_on_restricted=True,
+    ):
+        """
+        Override collect to first collect standard dependencies,
+        then add GenericRelations to the dependency graph.
+        """
+        # Call parent collect first to get all standard dependencies
+        super().collect(
+            objs,
+            source=source,
+            nullable=nullable,
+            collect_related=collect_related,
+            source_attr=source_attr,
+            reverse_dependency=reverse_dependency,
+            keep_parents=keep_parents,
+            fail_on_restricted=fail_on_restricted,
+        )
+
+        # Track which GenericRelations we've already processed to prevent infinite recursion
+        processed_relations = set()
+
+        # Now add GenericRelations to the dependency graph
+        for _, instances in list(self.data.items()):
+            for instance in instances:
+                # Get all GenericRelations for this model
+                for field in instance._meta.private_fields:
+                    if isinstance(field, GenericRelation):
+                        # Create a unique key for this relation
+                        relation_key = f"{instance._meta.model_name}.{field.name}"
+                        if relation_key in processed_relations:
+                            continue
+                        processed_relations.add(relation_key)
+
+                        # Add the model that the generic relation points to as a dependency
+                        self.add_dependency(field.related_model, instance, reverse_dependency=True)
+
+
+class DeleteMixin:
+    """
+    Mixin to override the model delete function to use our custom collector.
+    """
+
+    def delete(self, using=None, keep_parents=False):
+        """
+        Override delete to use our custom collector.
+        """
+        using = using or router.db_for_write(self.__class__, instance=self)
+        assert self._get_pk_val() is not None, "%s object can't be deleted because its %s attribute is set to None." % (
+            self._meta.object_name,
+            self._meta.pk.attname,
+        )
+
+        collector = CustomCollector(using=using)
+        collector.collect([self], keep_parents=keep_parents)
+
+        return collector.delete()
+
+    delete.alters_data = True
+
+    @classmethod
+    def verify_mro(cls, instance):
+        """
+        Verify that this mixin is first in the MRO.
+        """
+        mro = instance.__class__.__mro__
+        if mro.index(cls) != 0:
+            raise RuntimeError(f"{cls.__name__} must be first in the MRO. Current MRO: {mro}")

netbox/netbox/models/features.py

@@ -16,6 +16,7 @@
 from extras.constants import CUSTOMFIELD_EMPTY_VALUES
 from extras.utils import is_taggable
 from netbox.config import get_config
+from netbox.models.deletion import DeleteMixin
 from netbox.registry import registry
 from netbox.signals import post_clean
 from utilities.json import CustomFieldJSONEncoder
@@ -45,7 +46,7 @@
 # Feature mixins
 #
 
-class ChangeLoggingMixin(models.Model):
+class ChangeLoggingMixin(DeleteMixin, models.Model):
     """
     Provides change logging support for a model. Adds the `created` and `last_updated` fields.
     """