Răsfoiți Sursa

fix(dcim): Prevent path rebuild when Cable Terminations unchanged

Compare Cable Terminations against stored values instead of empty cache
when checking for modifications. Freshly loaded cables have no cached
terminations, causing false positives that unnecessarily rebuild paths.
Add regression test coverage for resaving Cables and Termination
reassignment.

Fixes #23097
Martin Hauser 1 zi în urmă
părinte
comite
f019d16843

+ 2 - 1
netbox/dcim/models/cables.py

@@ -244,7 +244,8 @@ class Cable(PrimaryModel):
                 ct.termination for ct in CableTermination.objects.filter(pk__in=value).prefetch_related('termination')
             ]
 
-        if not self.pk or getattr(self, _attr, []) != list(value):
+        # The private cache is empty on a freshly loaded cable, so compare against the stored terminations
+        if not self.pk or self._get_x_terminations(side) != list(value):
             self._terminations_modified = True
 
         setattr(self, _attr, value)

+ 46 - 0
netbox/dcim/tests/test_cablepaths.py

@@ -2892,6 +2892,52 @@ class LegacyCablePathTestCase(BaseCablePathTestCase):
         interface3.refresh_from_db()
         self.assertPathIsNotSet(interface3)
 
+    def test_304_resave_cable_with_unchanged_terminations(self):
+        """
+        [IF1] --C1-- [IF2]
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+
+        cable1 = Cable(
+            a_terminations=[interface1],
+            b_terminations=[interface2]
+        )
+        cable1.save()
+
+        path_pks = set(CablePath.objects.values_list('pk', flat=True))
+        termination_pks = set(CableTermination.objects.filter(cable=cable1).values_list('pk', flat=True))
+        self.assertEqual(len(path_pks), 2)
+        self.assertEqual(len(termination_pks), 2)
+
+        # Reassign the same terminations, as the edit form does on every submission
+        cable1 = Cable.objects.get(pk=cable1.pk)
+        cable1.a_terminations = [interface1]
+        cable1.b_terminations = [interface2]
+        cable1.label = 'Renamed'
+        cable1.save()
+
+        self.assertEqual(set(CablePath.objects.values_list('pk', flat=True)), path_pks)
+        self.assertEqual(
+            set(CableTermination.objects.filter(cable=cable1).values_list('pk', flat=True)),
+            termination_pks
+        )
+
+        path1 = self.assertPathExists(
+            (interface1, cable1, interface2),
+            is_complete=True,
+            is_active=True
+        )
+        path2 = self.assertPathExists(
+            (interface2, cable1, interface1),
+            is_complete=True,
+            is_active=True
+        )
+        interface1.refresh_from_db()
+        interface2.refresh_from_db()
+        self.assertPathIsSet(interface1, path1)
+        self.assertPathIsSet(interface2, path2)
+
     def test_401_exclude_midspan_devices(self):
         """
         [IF1] --C1-- [FP1][Test Device][RP1] --C2-- [RP2][Test Device][FP2] --C3-- [IF2]

+ 27 - 0
netbox/dcim/tests/test_models.py

@@ -2413,6 +2413,33 @@ class CableTestCase(TestCase):
         with self.assertRaises(ValidationError):
             cable.clean()
 
+    def test_reassigning_unchanged_terminations_does_not_flag_a_change(self):
+        """
+        Assigning the stored terminations to a freshly loaded cable must leave them unflagged.
+        """
+        interface1 = Interface.objects.get(device__name='TestDevice1', name='eth0')
+        interface2 = Interface.objects.get(device__name='TestDevice2', name='eth0')
+
+        # A cable loaded from the database has no cached terminations
+        cable = Cable.objects.first()
+        cable.a_terminations = [interface1]
+        cable.b_terminations = [interface2]
+
+        self.assertFalse(cable._terminations_modified)
+
+    def test_reassigning_different_terminations_flags_a_change(self):
+        """
+        Assigning a different termination to a freshly loaded cable must flag the change.
+        """
+        interface1 = Interface.objects.get(device__name='TestDevice1', name='eth0')
+        interface3 = Interface.objects.get(device__name='TestDevice2', name='eth1')
+
+        cable = Cable.objects.first()
+        cable.a_terminations = [interface1]
+        cable.b_terminations = [interface3]
+
+        self.assertTrue(cable._terminations_modified)
+
     def test_partial_save_does_not_apply_an_unwritten_profile(self):
         """
         A save excluding profile must leave the terminations alone but keep the change pending.

+ 37 - 0
netbox/dcim/tests/test_views.py

@@ -5271,6 +5271,43 @@ class CableTestCase(
             [(1, interfaces[1]), (2, interfaces[0])]
         )
 
+    @tag('regression')  # Issue #23097
+    def test_edit_with_unchanged_terminations_preserves_paths(self):
+        """
+        Editing a cable without changing its terminations must leave its paths in place.
+        """
+        # The form's termination fields are restricted by view permission
+        self.add_permissions('dcim.change_cable', 'dcim.view_interface')
+
+        interface_a = Interface.objects.get(
+            device__name='Device 1', device__site__name='Site 1', name='Interface 1'
+        )
+        cable = interface_a.cable
+        interface_b = cable.b_terminations[0]
+        path_pks = set(CablePath.objects.filter(_nodes__contains=cable).values_list('pk', flat=True))
+        self.assertEqual(len(path_pks), 2)
+
+        data = {
+            'a_terminations': [interface_a.pk],
+            'b_terminations': [interface_b.pk],
+            'type': CableTypeChoices.TYPE_CAT6,
+            'status': LinkStatusChoices.STATUS_CONNECTED,
+            'label': 'Renamed',
+            'color': 'c0c0c0',
+        }
+        request = {
+            'path': self._get_url('edit', cable),
+            'data': post_data(data),
+        }
+        self.assertHttpStatus(self.client.post(**request), 302)
+
+        cable.refresh_from_db()
+        self.assertEqual(cable.label, 'Renamed')
+        self.assertEqual(
+            set(CablePath.objects.filter(_nodes__contains=cable).values_list('pk', flat=True)),
+            path_pks
+        )
+
 
 #
 # Connections