Bladeren bron

Fixes #22967: Repair CircuitTermination cached scope fields on Location move

handle_location_site_change() repaired the cached scope fields of Location-
scoped Prefixes, Clusters and WirelessLANs, but not CircuitTerminations,
which cache the same ancestry under their own termination_type/termination_id
generic FK rather than CachedScopeMixin.scope. That made them invisible both
to the repair loop and to sync_cached_scope_fields().

Two cases were left wrong. A termination at a descendant Location kept its
_site, _region and _site_group entirely, since descendants are moved by a
queryset update() which fires no post_save. A termination at the moved
Location itself had _site refreshed by the denormalized-field registry, but
not _region or _site_group: those are mapped off the separate _site
registration, which requires a Site save.

Repair both cases by selecting through the generic termination fields over
the Location and its descendants alike. These columns back the site, region
and site group filters for Circuit and CircuitTermination, so a stale value
drops the circuit out of filtered lists and leaves it showing under its
former site.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Jeremy Stretch 3 dagen geleden
bovenliggende
commit
40a9df14a8
2 gewijzigde bestanden met toevoegingen van 62 en 1 verwijderingen
  1. 19 1
      netbox/dcim/signals.py
  2. 43 0
      netbox/dcim/tests/test_signals.py

+ 19 - 1
netbox/dcim/signals.py

@@ -6,6 +6,7 @@ from django.db.models import F, Q
 from django.db.models.signals import post_delete, post_save, pre_save
 from django.dispatch import receiver
 
+from circuits.models import CircuitTermination
 from dcim.choices import CableEndChoices, LinkStatusChoices
 from ipam.models import Prefix
 from netbox.search.backends import search_backend
@@ -90,7 +91,7 @@ def cache_presave_scope_fields(instance, raw=False, using=None, **kwargs):
 
 
 @receiver(post_save, sender=Location)
-def handle_location_site_change(instance, created, **kwargs):
+def handle_location_site_change(instance, created, using=None, **kwargs):
     """
     Update child objects when a Location is saved. All updates are queryset update() calls,
     which fire no signals and generate no change records for the affected objects.
@@ -140,6 +141,23 @@ def handle_location_site_change(instance, created, **kwargs):
                         _site_group_id=site['group_id'],
                     )
 
+                # CircuitTermination caches the same ancestry under its own generic
+                # termination field rather than CachedScopeMixin.scope, so it is invisible to
+                # both the loop above and sync_cached_scope_fields(). Its own rows are
+                # refreshed by the denormalized-field registry, but only their _site: _region
+                # and _site_group are mapped off the separate _site registration, which fires
+                # on a Site save. Rows scoped to descendant Locations get nothing at all, as
+                # the get_descendants() update above fires no post_save — which is why the
+                # include_self=True membership is load-bearing here.
+                CircuitTermination.objects.using(using).filter(
+                    termination_type=location_ct, termination_id__in=locations
+                ).update(
+                    _location_id=F('termination_id'),
+                    _site_id=instance.site_id,
+                    _region_id=site['region_id'],
+                    _site_group_id=site['group_id'],
+                )
+
 
 @receiver(post_save, sender=Rack)
 def handle_rack_site_change(instance, created, **kwargs):

+ 43 - 0
netbox/dcim/tests/test_signals.py

@@ -6,6 +6,7 @@ from django.db import connection, transaction
 from django.test import SimpleTestCase, TestCase, TransactionTestCase
 from django.test.utils import CaptureQueriesContext
 
+from circuits.models import Circuit, CircuitTermination, CircuitType, Provider
 from dcim import signals
 from dcim.choices import CableEndChoices, CableProfileChoices, LinkStatusChoices
 from dcim.models import (
@@ -76,6 +77,48 @@ class LocationSiteChangeSignalTestCase(TestCase):
         self.assertEqual(interface._site, self.site_b)
         self.assertEqual(power_panel.site, self.site_b)
 
+    def test_changing_location_site_updates_circuittermination_caches(self):
+        # CircuitTermination caches its scope ancestry under termination_type/termination_id
+        # rather than under CachedScopeMixin's scope field, so sync_cached_scope_fields does
+        # not cover it and the denormalized-field registry refreshes only _site. Both the
+        # moved Location's own terminations and those of its descendants must be repaired
+        # here, region and site group included. Origin and destination Sites are given
+        # distinct regions and groups so a value left stale is distinguishable from one that
+        # was never set.
+        origin_region = Region.objects.create(name='Region C', slug='region-c')
+        origin_group = SiteGroup.objects.create(name='Group C', slug='group-c')
+        origin = Site.objects.create(
+            name='Site C', slug='site-c', region=origin_region, group=origin_group
+        )
+        region = Region.objects.create(name='Region D', slug='region-d')
+        group = SiteGroup.objects.create(name='Group D', slug='group-d')
+        site = Site.objects.create(name='Site D', slug='site-d', region=region, group=group)
+        parent_location = Location.objects.create(name='Parent', slug='parent', site=origin)
+        child_location = Location.objects.create(name='Child', slug='child', site=origin, parent=parent_location)
+        provider = Provider.objects.create(name='Provider', slug='provider')
+        circuit_type = CircuitType.objects.create(name='Circuit Type', slug='circuit-type')
+        circuit = Circuit.objects.create(cid='Circuit 1', provider=provider, type=circuit_type)
+        termination_a = CircuitTermination.objects.create(
+            circuit=circuit, term_side='A', termination=parent_location
+        )
+        termination_z = CircuitTermination.objects.create(
+            circuit=circuit, term_side='Z', termination=child_location
+        )
+        for termination in (termination_a, termination_z):
+            self.assertEqual(termination._site, origin)
+            self.assertEqual(termination._region, origin_region)
+            self.assertEqual(termination._site_group, origin_group)
+
+        parent_location.site = site
+        parent_location.save()
+
+        for termination, location in ((termination_a, parent_location), (termination_z, child_location)):
+            termination.refresh_from_db()
+            self.assertEqual(termination._location, location)
+            self.assertEqual(termination._site, site)
+            self.assertEqual(termination._region, region)
+            self.assertEqual(termination._site_group, group)
+
     def test_creating_location_does_not_attempt_to_propagate(self):
         # Should not raise — newly-created locations have no descendants.
         Location.objects.create(name='New', slug='new', site=self.site_a)