Przeglądaj źródła

fix(circuits): Preserve circuit terminations on ancestor deletion

Use SET_NULL for the cached region and site group references, so that
deleting an ancestor keeps the terminations of its sites and locations.
A GenericRelation on Region and SiteGroup still deletes terminations
attached to them directly.

Fixes #23185
Martin Hauser 1 dzień temu
rodzic
commit
218c171351

+ 34 - 0
netbox/circuits/migrations/0061_circuitterminations_cached_ancestors_on_delete.py

@@ -0,0 +1,34 @@
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('circuits', '0060_denormalization_triggers'),
+        ('dcim', '0251_fix_ltree_cascade_triggers'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='circuittermination',
+            name='_region',
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name='circuit_terminations',
+                to='dcim.region',
+            ),
+        ),
+        migrations.AlterField(
+            model_name='circuittermination',
+            name='_site_group',
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name='circuit_terminations',
+                to='dcim.sitegroup',
+            ),
+        ),
+    ]

+ 3 - 2
netbox/circuits/models/circuits.py

@@ -346,16 +346,17 @@ class CircuitTermination(
         blank=True,
         blank=True,
         null=True
         null=True
     )
     )
+    # SET_NULL, not CASCADE: may cache an ancestor. Direct terminations cascade via direct_circuit_terminations.
     _region = models.ForeignKey(
     _region = models.ForeignKey(
         to='dcim.Region',
         to='dcim.Region',
-        on_delete=models.CASCADE,
+        on_delete=models.SET_NULL,
         related_name='circuit_terminations',
         related_name='circuit_terminations',
         blank=True,
         blank=True,
         null=True
         null=True
     )
     )
     _site_group = models.ForeignKey(
     _site_group = models.ForeignKey(
         to='dcim.SiteGroup',
         to='dcim.SiteGroup',
-        on_delete=models.CASCADE,
+        on_delete=models.SET_NULL,
         related_name='circuit_terminations',
         related_name='circuit_terminations',
         blank=True,
         blank=True,
         null=True
         null=True

+ 66 - 1
netbox/circuits/tests/test_models.py

@@ -8,11 +8,12 @@ from circuits.models import Circuit, CircuitTermination, CircuitType, Provider,
 from circuits.models.circuits import _set_circuit_terminations
 from circuits.models.circuits import _set_circuit_terminations
 from core.choices import ObjectChangeActionChoices
 from core.choices import ObjectChangeActionChoices
 from core.models import ObjectChange
 from core.models import ObjectChange
-from dcim.models import Location, Region, Site, SiteGroup
+from dcim.models import Cable, Interface, Location, Region, Site, SiteGroup
 from extras.choices import CustomFieldStatusChoices, CustomFieldTypeChoices
 from extras.choices import CustomFieldStatusChoices, CustomFieldTypeChoices
 from extras.models import CustomField
 from extras.models import CustomField
 from netbox.context_managers import event_tracking
 from netbox.context_managers import event_tracking
 from users.models import User
 from users.models import User
+from utilities.testing import create_test_device
 
 
 
 
 class CircuitTerminationTestCase(TestCase):
 class CircuitTerminationTestCase(TestCase):
@@ -198,6 +199,53 @@ class CircuitTerminationTestCase(TestCase):
         self.assertIn('Please select a Provider Network.', errors[NON_FIELD_ERRORS])
         self.assertIn('Please select a Provider Network.', errors[NON_FIELD_ERRORS])
         self.assertNotIn('termination_id', errors)
         self.assertNotIn('termination_id', errors)
 
 
+    @tag('regression')  # Ref: #23185
+    def test_deleting_site_group_keeps_terminations_of_its_sites(self):
+        """Deleting a SiteGroup deletes only the terminations attached to it directly."""
+        site_group = SiteGroup.objects.create(name='Site Group 1', slug='site-group-1')
+        site = Site.objects.create(name='Site 3', slug='site-3', group=site_group)
+        location = Location.objects.create(name='Location 1', slug='location-1', site=site)
+        direct = CircuitTermination.objects.create(circuit=self.circuits[0], term_side='A', termination=site_group)
+        kept = (
+            CircuitTermination.objects.create(circuit=self.circuits[0], term_side='Z', termination=site),
+            CircuitTermination.objects.create(circuit=self.circuits[1], term_side='A', termination=location),
+        )
+        interface = Interface.objects.create(device=create_test_device('Device 1'), name='Interface 1')
+        cable = Cable(a_terminations=[interface], b_terminations=[kept[0]])
+        cable.save()
+
+        site_group.delete()
+
+        self.assertFalse(CircuitTermination.objects.filter(pk=direct.pk).exists())
+        for termination in kept:
+            termination.refresh_from_db()
+            self.assertIsNone(termination._site_group_id)
+        self.assertEqual(kept[0].cable, cable)
+        self.assertTrue(kept[0].cable_terminations.exists())
+
+    @tag('regression')  # Ref: #23185
+    def test_deleting_region_keeps_terminations_of_its_sites(self):
+        """Deleting a Region and its child deletes only the terminations attached to them directly."""
+        parent = Region.objects.create(name='Region 1', slug='region-1')
+        region = Region.objects.create(name='Region 2', slug='region-2', parent=parent)
+        site = Site.objects.create(name='Site 3', slug='site-3', region=region)
+        location = Location.objects.create(name='Location 1', slug='location-1', site=site)
+        direct = (
+            CircuitTermination.objects.create(circuit=self.circuits[0], term_side='A', termination=parent),
+            CircuitTermination.objects.create(circuit=self.circuits[0], term_side='Z', termination=region),
+        )
+        kept = (
+            CircuitTermination.objects.create(circuit=self.circuits[1], term_side='A', termination=site),
+            CircuitTermination.objects.create(circuit=self.circuits[1], term_side='Z', termination=location),
+        )
+
+        parent.delete()
+
+        self.assertFalse(CircuitTermination.objects.filter(pk__in=[t.pk for t in direct]).exists())
+        for termination in kept:
+            termination.refresh_from_db()
+            self.assertIsNone(termination._region_id)
+
 
 
 class CircuitTerminationDenormalizationTriggerTestCase(TestCase):
 class CircuitTerminationDenormalizationTriggerTestCase(TestCase):
     """
     """
@@ -567,6 +615,23 @@ class CircuitTerminationChangeLoggingTestCase(TestCase):
         )
         )
         self.assertLess(changes[0].pk, termination_delete.pk)
         self.assertLess(changes[0].pk, termination_delete.pk)
 
 
+    @tag('regression')  # Ref: #23185
+    def test_site_group_deletion_keeps_terminations_of_its_sites(self):
+        """In a request, deleting a SiteGroup deletes and logs the termination attached to it and keeps the other."""
+        site_group = SiteGroup.objects.create(name='Site Group 1', slug='site-group-1')
+        site = Site.objects.create(name='Site 3', slug='site-3', group=site_group)
+        direct = CircuitTermination.objects.create(circuit=self.circuits[0], term_side='A', termination=site_group)
+        kept = CircuitTermination.objects.create(circuit=self.circuits[0], term_side='Z', termination=site)
+        interface = Interface.objects.create(device=create_test_device('Device 1'), name='Interface 1')
+        Cable(a_terminations=[interface], b_terminations=[kept]).save()
+
+        self._tracked(site_group.delete)
+
+        self._termination_change(direct.pk, ObjectChangeActionChoices.ACTION_DELETE)
+        kept.refresh_from_db()
+        self.assertIsNone(kept._site_group_id)
+        self.assertTrue(kept.cable_terminations.exists())
+
     def test_circuit_deletion_records_no_pointer_update(self):
     def test_circuit_deletion_records_no_pointer_update(self):
         self._tracked(lambda: CircuitTermination.objects.create(
         self._tracked(lambda: CircuitTermination.objects.create(
             circuit=self.circuits[0], term_side='A', termination=self.sites[0],
             circuit=self.circuits[0], term_side='A', termination=self.sites[0],

+ 12 - 0
netbox/dcim/models/sites.py

@@ -55,6 +55,12 @@ class Region(ContactsMixin, NestedLtreeGroupModel):
         object_id_field='scope_id',
         object_id_field='scope_id',
         related_query_name='region'
         related_query_name='region'
     )
     )
+    # Deletion cascade only, so no related_query_name. circuit_terminations is the reverse of the cached _region.
+    direct_circuit_terminations = GenericRelation(
+        to='circuits.CircuitTermination',
+        content_type_field='termination_type',
+        object_id_field='termination_id'
+    )
 
 
     class Meta:
     class Meta:
         ordering = ('sort_path',)
         ordering = ('sort_path',)
@@ -120,6 +126,12 @@ class SiteGroup(ContactsMixin, NestedLtreeGroupModel):
         object_id_field='scope_id',
         object_id_field='scope_id',
         related_query_name='site_group'
         related_query_name='site_group'
     )
     )
+    # Deletion cascade only, so no related_query_name. circuit_terminations is the reverse of the cached _site_group.
+    direct_circuit_terminations = GenericRelation(
+        to='circuits.CircuitTermination',
+        content_type_field='termination_type',
+        object_id_field='termination_id'
+    )
 
 
     class Meta:
     class Meta:
         ordering = ('sort_path',)
         ordering = ('sort_path',)