handle cascade delete

netbox/circuits/models/circuits.py

@@ -241,13 +241,14 @@ class CircuitGroupAssignment(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin,
         return reverse('circuits:circuitgroupassignment', args=[self.pk])
 
 
-def _set_circuit_termination(circuit, field_name, value):
+def _set_circuit_terminations(circuit, fields):
     """
-    Set or clear a Circuit's cached `termination_a`/`termination_z` field, recording the change.
+    Set or clear a Circuit's cached `termination_a`/`termination_z` fields, recording the change.
     """
     circuit.snapshot()
-    setattr(circuit, field_name, value)
-    circuit.save(update_fields=[field_name, 'last_updated'])
+    for field_name, value in fields.items():
+        setattr(circuit, field_name, value)
+    circuit.save(update_fields=[*fields, 'last_updated'])
 
 
 class CircuitTermination(
@@ -416,28 +417,34 @@ class CircuitTermination(
                 pk=self._orig_circuit_id, **{old_termination_name: self.pk}
             ).first()
             if circuit is not None:
-                _set_circuit_termination(circuit, old_termination_name, None)
+                _set_circuit_terminations(circuit, {old_termination_name: None})
 
         # Update the cache if this is a new termination or circuit/term_side changed
         if is_new or circuit_changed or term_side_changed:
             # Update the new circuit's termination reference
             termination_name = f'termination_{self.term_side.lower()}'
-            _set_circuit_termination(Circuit.objects.get(pk=self.circuit_id), termination_name, self)
+            _set_circuit_terminations(Circuit.objects.get(pk=self.circuit_id), {termination_name: self})
 
             # Update cached values for subsequent saves
             self._orig_circuit_id = self.circuit_id
             self._orig_term_side = self.term_side
 
-    def delete(self, *args, **kwargs):
-        # Clear the circuit's reference here; on_delete=SET_NULL is not change-logged
-        termination_name = f'termination_{self.term_side.lower()}'
-        circuit = Circuit.objects.filter(pk=self.circuit_id, **{termination_name: self.pk}).first()
-        if circuit is not None:
-            _set_circuit_termination(circuit, termination_name, None)
-
-        return super().delete(*args, **kwargs)
-
-    delete.alters_data = True
+    @classmethod
+    def clear_cached_references(cls, instances, collector):
+        # Called by CustomCollector ahead of the DELETE, for explicit and cascaded deletions alike
+        pks = {instance.pk for instance in instances}
+        doomed_circuits = {circuit.pk for circuit in collector.data.get(Circuit, ())}
+
+        circuits = Circuit.objects.filter(
+            models.Q(termination_a__in=pks) | models.Q(termination_z__in=pks)
+        ).exclude(pk__in=doomed_circuits)
+
+        for circuit in circuits:
+            _set_circuit_terminations(circuit, {
+                name: None
+                for name in ('termination_a', 'termination_z')
+                if getattr(circuit, f'{name}_id') in pks
+            })
 
     def cache_related_objects(self):
         self._provider_network = self._region = self._site_group = self._site = self._location = None

netbox/circuits/tests/test_models.py

@@ -507,8 +507,9 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
         self.assertIsNone(changes[0].postchange_data['termination_a'])
 
     @tag('regression')  # Ref: #23134
-    def test_deletion_leaves_another_terminations_pointer_alone(self):
-        # A pointer referencing a different termination must never be cleared
+    def test_deletion_resolves_the_pointer_from_the_database(self):
+        # The pointer cleared is the one which references this termination, not the one named by
+        # a stale in-memory term_side
         termination_a = self._tracked(lambda: CircuitTermination.objects.create(
             circuit=self.circuits[0], term_side='A', termination=self.sites[0],
         ))
@@ -516,12 +517,52 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
             circuit=self.circuits[0], term_side='Z', termination=self.sites[1],
         ))
         ObjectChange.objects.all().delete()
+        termination_z_pk = termination_z.pk
 
         termination_z.term_side = 'A'
         self._tracked(termination_z.delete)
 
         self.circuits[0].refresh_from_db()
         self.assertEqual(self.circuits[0].termination_a_id, termination_a.pk)
+        self.assertIsNone(self.circuits[0].termination_z_id)
+
+        changes = self._circuit_changes(self.circuits[0])
+        self.assertEqual(changes.count(), 1)
+        self.assertEqual(changes[0].prechange_data['termination_z'], termination_z_pk)
+        self.assertIsNone(changes[0].postchange_data['termination_z'])
+        self.assertEqual(changes[0].postchange_data['termination_a'], termination_a.pk)
+
+    @tag('regression')  # Ref: #23134
+    def test_cascade_deletion_records_circuit_update(self):
+        # Deleting the terminating Site reaches the termination through the collector, which does
+        # not call delete(). Both sides go in one record.
+        termination_a = self._tracked(lambda: CircuitTermination.objects.create(
+            circuit=self.circuits[0], term_side='A', termination=self.sites[0],
+        ))
+        termination_z = self._tracked(lambda: CircuitTermination.objects.create(
+            circuit=self.circuits[0], term_side='Z', termination=self.sites[0],
+        ))
+        ObjectChange.objects.all().delete()
+        termination_a_pk, termination_z_pk = termination_a.pk, termination_z.pk
+
+        self._tracked(self.sites[0].delete)
+
+        self.circuits[0].refresh_from_db()
+        self.assertIsNone(self.circuits[0].termination_a_id)
+        self.assertIsNone(self.circuits[0].termination_z_id)
+
+        changes = self._circuit_changes(self.circuits[0])
+        self.assertEqual(changes.count(), 1)
+        self.assertEqual(changes[0].prechange_data['termination_a'], termination_a_pk)
+        self.assertEqual(changes[0].prechange_data['termination_z'], termination_z_pk)
+        self.assertIsNone(changes[0].postchange_data['termination_a'])
+        self.assertIsNone(changes[0].postchange_data['termination_z'])
+
+        # The clear must precede the DELETEs which the cascade emits for the terminations
+        termination_delete = self._termination_change(
+            termination_a_pk, ObjectChangeActionChoices.ACTION_DELETE
+        )
+        self.assertLess(changes[0].pk, termination_delete.pk)
 
     def test_circuit_deletion_records_no_pointer_update(self):
         self._tracked(lambda: CircuitTermination.objects.create(

netbox/netbox/models/deletion.py

@@ -1,7 +1,7 @@
 import logging
 
 from django.contrib.contenttypes.fields import GenericRelation
-from django.db import router
+from django.db import router, transaction
 from django.db.models.deletion import CASCADE, Collector
 from django.utils.translation import gettext as _
 
@@ -126,6 +126,18 @@ class CustomCollector(Collector):
                         # Add the model that the generic relation points to as a dependency
                         self.add_dependency(field.related_model, instance, reverse_dependency=True)
 
+    def delete(self):
+        # Clear any cached references to the objects being deleted first, so that each clear is
+        # recorded and precedes the DELETE. Django nulls a SET_NULL column with a bulk UPDATE,
+        # which emits no post_save and so is never change-logged. Models opt in by defining
+        # clear_cached_references(); cascaded objects reach this the same as explicit deletions.
+        with transaction.atomic(using=self.using):
+            for model, instances in self.data.items():
+                if clear_references := getattr(model, 'clear_cached_references', None):
+                    clear_references(instances, self)
+
+            return super().delete()
+
 
 class DeleteMixin:
     """