Changes requested to tests and update comments/doctstrings

netbox/dcim/models/cables.py

@@ -520,7 +520,7 @@ class CablePath(models.Model):
 
             # All mid-span terminations must all be attached to the same device
             if not isinstance(terminations[0], PathEndpoint):
-                assert all(t.device == terminations[0].device for t in terminations[1:])
+                assert all(t.parent == terminations[0].parent for t in terminations[1:])
 
             # Check for a split path (e.g. rear port fanning out to multiple front ports with
             # different cables attached)

netbox/dcim/svg/cables.py

@@ -32,6 +32,8 @@ class Node(Hyperlink):
         color: Box fill color (RRGGBB format)
         labels: An iterable of text strings. Each label will render on a new line within the box.
         radius: Box corner radius, for rounded corners (default: 10)
+        object: A copy of the object to allow reference when drawing cables to determine which cables are connected to
+                which terminations.
     """
 
     object = None
@@ -39,6 +41,7 @@ class Node(Hyperlink):
     def __init__(self, position, width, url, color, labels, radius=10, object=object, **extra):
         super(Node, self).__init__(href=url, target='_parent', **extra)
 
+        # Save object for reference by cable systems
         self.object = object
 
         x, y = position
@@ -246,15 +249,31 @@ class CableTraceSVG:
         ))
 
     def draw_cable(self, cable, terminations, cable_count=0):
+        """
+        Draw a single cable.  Terminations and cable count are passed for determining position and padding
+
+        :param cable: The cable to draw
+        :param terminations: List of terminations to build positioning data off of
+        :param cable_count: Count of all cables on this layer for determining whether to collapse description into a
+                            tooltip.
+        """
+
+        # If the cable count is higher than 2, collapse the description into a tooltip
         if cable_count > 2:
+            # Use the cable __str__ function to denote the cable
             labels = [f'{cable}']
+
+            # Include the label and the status description in the tooltip
             description = [
                 f'Cable {cable}',
                 cable.get_status_display()
             ]
+
             if cable.type:
+                # Include the cable type in the tooltip
                 description.append(cable.get_type_display())
             if cable.length and cable.length_unit:
+                # Include the cable length in the tooltip
                 description.append(f'{cable.length} {cable.get_length_unit_display()}')
         else:
             labels = [
@@ -265,13 +284,20 @@ class CableTraceSVG:
             if cable.type:
                 labels.append(cable.get_type_display())
             if cable.length and cable.length_unit:
+                # Include the cable length in the tooltip
                 labels.append(f'{cable.length} {cable.get_length_unit_display()}')
 
+        # If there is only one termination, center on that termination
+        # Otherwise average the center across the terminations
         if len(terminations) == 1:
             center = terminations[0].bottom_center[0]
         else:
+            # Get a list of termination centers
             termination_centers = [term.bottom_center[0] for term in terminations]
+            # Average the centers
             center = sum(termination_centers) / len(termination_centers)
+
+        # Create the connector
         connector = Connector(
             start=(center, self.cursor),
             color=cable.color or '000000',
@@ -280,6 +306,7 @@ class CableTraceSVG:
             description=description
         )
 
+        # Set the cursor position
         self.cursor += connector.height
 
         return connector
@@ -374,10 +401,15 @@ class CableTraceSVG:
                 for link in links:
                     # Cable
                     if type(link) is Cable and not link_cables.get(link.pk):
+                        # Reset cursor
                         self.cursor = cursor
+                        # Generate a list of terminations connected to this cable
                         near_end_link_terminations = [term for term in terminations if term.object.cable == link]
+                        # Draw the cable
                         cable = self.draw_cable(link, near_end_link_terminations, cable_count=len(links))
+                        # Add cable to the list of cables
                         link_cables.update({link.pk: cable})
+                        # Add cable to drawing
                         self.connectors.append(cable)
 
                         # Draw fan-ins

netbox/dcim/tests/test_cablepaths.py

@@ -1891,6 +1891,126 @@ class CablePathTestCase(TestCase):
         )
         self.assertEqual(CablePath.objects.count(), 3)
 
+    def test_221_non_symmetric_paths(self):
+        """
+        [IF1] --C1-- [FP1] [RP1] --C2-- [RP2] [FP2] --C3-- -------------------------------------- [IF2]
+        [IF2] --C5-- [FP3] [RP3] --C4-- [RP4] [FP4] --C6-- [FP5] [RP5] --C7-- [RP6] [FP6] --C3---/
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        interface3 = Interface.objects.create(device=self.device, name='Interface 3')
+        rearport1 = RearPort.objects.create(device=self.device, name='Rear Port 1', positions=1)
+        rearport2 = RearPort.objects.create(device=self.device, name='Rear Port 2', positions=1)
+        rearport3 = RearPort.objects.create(device=self.device, name='Rear Port 3', positions=1)
+        rearport4 = RearPort.objects.create(device=self.device, name='Rear Port 4', positions=1)
+        rearport5 = RearPort.objects.create(device=self.device, name='Rear Port 5', positions=1)
+        rearport6 = RearPort.objects.create(device=self.device, name='Rear Port 6', positions=1)
+        frontport1 = FrontPort.objects.create(
+            device=self.device, name='Front Port 1', rear_port=rearport1, rear_port_position=1
+        )
+        frontport2 = FrontPort.objects.create(
+            device=self.device, name='Front Port 2', rear_port=rearport2, rear_port_position=1
+        )
+        frontport3 = FrontPort.objects.create(
+            device=self.device, name='Front Port 3', rear_port=rearport3, rear_port_position=1
+        )
+        frontport4 = FrontPort.objects.create(
+            device=self.device, name='Front Port 4', rear_port=rearport4, rear_port_position=1
+        )
+        frontport5 = FrontPort.objects.create(
+            device=self.device, name='Front Port 5', rear_port=rearport5, rear_port_position=1
+        )
+        frontport6 = FrontPort.objects.create(
+            device=self.device, name='Front Port 6', rear_port=rearport6, rear_port_position=1
+        )
+
+        cable2 = Cable(
+            a_terminations=[rearport1],
+            b_terminations=[rearport2],
+            label='C2'
+        )
+        cable2.save()
+        cable4 = Cable(
+            a_terminations=[rearport3],
+            b_terminations=[rearport4],
+            label='C4'
+        )
+        cable4.save()
+        cable6 = Cable(
+            a_terminations=[frontport4],
+            b_terminations=[frontport5],
+            label='C6'
+        )
+        cable6.save()
+        cable7 = Cable(
+            a_terminations=[rearport5],
+            b_terminations=[rearport6],
+            label='C7'
+        )
+        cable7.save()
+        self.assertEqual(CablePath.objects.count(), 0)
+
+        # Create cable1
+        cable1 = Cable(
+            a_terminations=[interface1],
+            b_terminations=[frontport1],
+            label='C1'
+        )
+        cable1.save()
+        self.assertPathExists(
+            (
+                interface1, cable1, frontport1, rearport1, cable2, rearport2, frontport2
+            ),
+            is_complete=False
+        )
+        # Create cable1
+        cable5 = Cable(
+            a_terminations=[interface3],
+            b_terminations=[frontport3],
+            label='C5'
+        )
+        cable5.save()
+        self.assertPathExists(
+            (
+                interface3, cable5, frontport3, rearport3, cable4, rearport4, frontport4, cable6, frontport5, rearport5,
+                cable7, rearport6, frontport6
+            ),
+            is_complete=False
+        )
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        # Create cable 3
+        cable3 = Cable(
+            a_terminations=[frontport2, frontport6],
+            b_terminations=[interface2],
+            label='C3'
+        )
+        cable3.save()
+        self.assertPathExists(
+            (
+                interface2, cable3, (frontport2, frontport6), (rearport2, rearport6), (cable2, cable7),
+                (rearport1, rearport5), (frontport1, frontport5), (cable1, cable6)
+            ),
+            is_complete=False,
+            is_split=True
+        )
+        self.assertPathExists(
+            (
+                interface1, cable1, frontport1, rearport1, cable2, rearport2, frontport2, cable3, interface2
+            ),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertPathExists(
+            (
+                interface3, cable5, frontport3, rearport3, cable4, rearport4, frontport4, cable6, frontport5, rearport5,
+                cable7, rearport6, frontport6, cable3, interface2
+            ),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertEqual(CablePath.objects.count(), 3)
+
     def test_301_create_path_via_existing_cable(self):
         """
         [IF1] --C1-- [FP1] [RP1] --C2-- [RP2] [FP2] --C3-- [IF2]
@@ -2042,130 +2162,10 @@ class CablePathTestCase(TestCase):
             is_active=True
         )
 
-    def test_401_non_symmetric_paths(self):
+    def test_401_exclude_midspan_devices(self):
         """
-        [IF1] --C1-- [FP1] [RP1] --C2-- [RP2] [FP2] --C3-- -------------------------------------- [IF2]
+        [IF1] --C1-- [FP1][Test Device][RP1] --C2-- [RP2][Test Device][FP2] --C3-- [IF2]
-        [IF2] --C5-- [FP3] [RP3] --C4-- [RP4] [FP4] --C6-- [FP5] [RP5] --C7-- [RP6] [FP6] --C3---/
+                     [FP3][Test mid-span Device][RP3] --C4-- [RP4][Test mid-span Device][FP4] /
-        """
-        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
-        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
-        interface3 = Interface.objects.create(device=self.device, name='Interface 3')
-        rearport1 = RearPort.objects.create(device=self.device, name='Rear Port 1', positions=1)
-        rearport2 = RearPort.objects.create(device=self.device, name='Rear Port 2', positions=1)
-        rearport3 = RearPort.objects.create(device=self.device, name='Rear Port 3', positions=1)
-        rearport4 = RearPort.objects.create(device=self.device, name='Rear Port 4', positions=1)
-        rearport5 = RearPort.objects.create(device=self.device, name='Rear Port 5', positions=1)
-        rearport6 = RearPort.objects.create(device=self.device, name='Rear Port 6', positions=1)
-        frontport1 = FrontPort.objects.create(
-            device=self.device, name='Front Port 1', rear_port=rearport1, rear_port_position=1
-        )
-        frontport2 = FrontPort.objects.create(
-            device=self.device, name='Front Port 2', rear_port=rearport2, rear_port_position=1
-        )
-        frontport3 = FrontPort.objects.create(
-            device=self.device, name='Front Port 3', rear_port=rearport3, rear_port_position=1
-        )
-        frontport4 = FrontPort.objects.create(
-            device=self.device, name='Front Port 4', rear_port=rearport4, rear_port_position=1
-        )
-        frontport5 = FrontPort.objects.create(
-            device=self.device, name='Front Port 5', rear_port=rearport5, rear_port_position=1
-        )
-        frontport6 = FrontPort.objects.create(
-            device=self.device, name='Front Port 6', rear_port=rearport6, rear_port_position=1
-        )
-
-        cable2 = Cable(
-            a_terminations=[rearport1],
-            b_terminations=[rearport2],
-            label='C2'
-        )
-        cable2.save()
-        cable4 = Cable(
-            a_terminations=[rearport3],
-            b_terminations=[rearport4],
-            label='C4'
-        )
-        cable4.save()
-        cable6 = Cable(
-            a_terminations=[frontport4],
-            b_terminations=[frontport5],
-            label='C6'
-        )
-        cable6.save()
-        cable7 = Cable(
-            a_terminations=[rearport5],
-            b_terminations=[rearport6],
-            label='C7'
-        )
-        cable7.save()
-        self.assertEqual(CablePath.objects.count(), 0)
-
-        # Create cable1
-        cable1 = Cable(
-            a_terminations=[interface1],
-            b_terminations=[frontport1],
-            label='C1'
-        )
-        cable1.save()
-        self.assertPathExists(
-            (
-                interface1, cable1, frontport1, rearport1, cable2, rearport2, frontport2
-            ),
-            is_complete=False
-        )
-        # Create cable1
-        cable5 = Cable(
-            a_terminations=[interface3],
-            b_terminations=[frontport3],
-            label='C5'
-        )
-        cable5.save()
-        self.assertPathExists(
-            (
-                interface3, cable5, frontport3, rearport3, cable4, rearport4, frontport4, cable6, frontport5, rearport5,
-                cable7, rearport6, frontport6
-            ),
-            is_complete=False
-        )
-        self.assertEqual(CablePath.objects.count(), 2)
-
-        # Create cable 3
-        cable3 = Cable(
-            a_terminations=[frontport2, frontport6],
-            b_terminations=[interface2],
-            label='C3'
-        )
-        cable3.save()
-        self.assertPathExists(
-            (
-                interface2, cable3, (frontport2, frontport6), (rearport2, rearport6), (cable2, cable7),
-                (rearport1, rearport5), (frontport1, frontport5), (cable1, cable6)
-            ),
-            is_complete=False,
-            is_split=True
-        )
-        self.assertPathExists(
-            (
-                interface1, cable1, frontport1, rearport1, cable2, rearport2, frontport2, cable3, interface2
-            ),
-            is_complete=True,
-            is_active=True
-        )
-        self.assertPathExists(
-            (
-                interface3, cable5, frontport3, rearport3, cable4, rearport4, frontport4, cable6, frontport5, rearport5,
-                cable7, rearport6, frontport6, cable3, interface2
-            ),
-            is_complete=True,
-            is_active=True
-        )
-        self.assertEqual(CablePath.objects.count(), 3)
-
-    def test_402_exclude_midspan_devices(self):
-        """
-        [IF1] --C1-- [FP1] [RP1] --C2-- [RP2] [FP2] --C3-- [IF2]
-                     [FP3] [RP3] --C4-- [RP4] [FP4] /
         """
         device = Device.objects.create(
             site=self.site,
@@ -2212,10 +2212,9 @@ class CablePathTestCase(TestCase):
             b_terminations=[frontport1, frontport3],
             label='C1'
         )
-        try:
+        with self.assertRaises(AssertionError):
             cable1.save()
-        except AssertionError:
+
-            pass
         self.assertPathNotExists(
             (
                 interface1, cable1, (frontport1, frontport3), (rearport1, rearport3), (cable2, cable4),
@@ -2232,10 +2231,9 @@ class CablePathTestCase(TestCase):
             label='C3'
         )
 
-        try:
+        with self.assertRaises(AssertionError):
             cable3.save()
-        except AssertionError:
+
-            pass
         self.assertPathNotExists(
             (
                 interface2, cable3, (frontport2, frontport4), (rearport2, rearport4), (cable2, cable4),