Fixes #23043: Correct Front Port position validation for bulk creation (#23055)

docs/models/dcim/frontport.md

@@ -1,6 +1,6 @@
 # Front Ports
 
-Front ports are pass-through ports which represent physical cable connections that comprise part of a longer path. For example, the ports on the front face of a UTP patch panel would be modeled in NetBox as front ports. Each port is assigned a physical type, and must be mapped to a specific [rear port](./rearport.md) on the same device. A single rear port may be mapped to multiple front ports, using numeric positions to annotate the specific alignment of each.
+Front ports are pass-through ports which represent physical cable connections that comprise part of a longer path. For example, the ports on the front face of a UTP patch panel would be modeled in NetBox as front ports. Each port is assigned a physical type, and must be mapped to one or more [rear port](./rearport.md) positions on the same device. A single rear port may be mapped to multiple front ports, using numeric positions to annotate the specific alignment of each.
 
 !!! tip
     Like most device components, front ports are instantiated automatically from [front port templates](./frontporttemplate.md) assigned to the selected device type when a device is created.
@@ -27,12 +27,16 @@ An alternative physical label identifying the port.
 
 The port's termination type.
 
+### Positions
+
+The number of [rear port](./rearport.md) positions to which this front port maps. For a front port which passes through to a single rear port position, set this to `1`.
+
 ### Rear Ports
 
 The rear port and position to which this front port maps.
 
 !!! tip
-    When creating multiple front ports using a patterned name (e.g. `Port [1-12]`), you may select the equivalent number of rear port-position mappings from the list.
+    When creating multiple front ports using a patterned name (e.g. `Port [1-12]`), select one rear port-position mapping for every position of every front port being created. For example, 12 front ports with two positions each requires 24 mappings, which are assigned to the generated ports in order.
 
 ### Color
 

netbox/dcim/forms/mixins.py

@@ -183,19 +183,26 @@ class FrontPortFormMixin(forms.Form):
     def clean(self):
         super().clean()
 
-        # Check that the total number of FrontPorts and positions matches the selected number of RearPort:position
-        # mappings. Note that `name` will be a list under FrontPortCreateForm, in which cases we multiply the number of
-        # FrontPorts being creation by the number of positions.
-        positions = self.cleaned_data['positions']
-        frontport_count = len(self.cleaned_data['name']) if type(self.cleaned_data['name']) is list else 1
-        rearport_count = len(self.cleaned_data['rear_ports'])
-        if frontport_count * positions != rearport_count:
+        # All three are required fields, so bail out if any of them failed its own validation
+        positions = self.cleaned_data.get('positions')
+        name = self.cleaned_data.get('name')
+        rear_ports = self.cleaned_data.get('rear_ports')
+        if not (positions and name and rear_ports):
+            return
+
+        # `name` is a list under FrontPortCreateForm, and each generated FrontPort consumes `positions` mappings
+        frontport_count = len(name) if isinstance(name, list) else 1
+        frontport_position_count = frontport_count * positions
+        rearport_count = len(rear_ports)
+
+        # {frontport_count} receives the position total. Its name is unchanged to keep existing translations valid.
+        if frontport_position_count != rearport_count:
             raise forms.ValidationError({
                 'rear_ports': _(
                     "The total number of front port positions ({frontport_count}) must match the selected number of "
                     "rear port positions ({rearport_count})."
                 ).format(
-                    frontport_count=frontport_count,
+                    frontport_count=frontport_position_count,
                     rearport_count=rearport_count
                 )
             })

netbox/dcim/forms/object_create.py

@@ -62,18 +62,20 @@ class ComponentCreateForm(forms.Form):
             return
         pattern_count = len(patterns)
         for field_name in self.replication_fields:
-            value_count = len(self.cleaned_data[field_name])
-            if self.cleaned_data[field_name]:
-                if value_count == 1:
-                    # If the field resolves to a single value (because no pattern was used), multiply it by the number
-                    # of expected values. This allows us to reuse the same label when creating multiple components.
-                    self.cleaned_data[field_name] = self.cleaned_data[field_name] * pattern_count
-                elif value_count != pattern_count:
-                    raise forms.ValidationError({
-                        field_name: _(
-                            "The provided pattern specifies {value_count} values, but {pattern_count} are expected."
-                        ).format(value_count=value_count, pattern_count=pattern_count)
-                    }, code='label_pattern_mismatch')
+            # A field is absent from cleaned_data if it failed its own validation, e.g. an inverted numeric range
+            if not (values := self.cleaned_data.get(field_name)):
+                continue
+            value_count = len(values)
+            if value_count == 1:
+                # If the field resolves to a single value (because no pattern was used), multiply it by the number
+                # of expected values. This allows us to reuse the same label when creating multiple components.
+                self.cleaned_data[field_name] = values * pattern_count
+            elif value_count != pattern_count:
+                raise forms.ValidationError({
+                    field_name: _(
+                        "The provided pattern specifies {value_count} values, but {pattern_count} are expected."
+                    ).format(value_count=value_count, pattern_count=pattern_count)
+                }, code='label_pattern_mismatch')
 
 
 #

netbox/dcim/tests/test_forms.py

@@ -319,6 +319,13 @@ class FrontPortTestCase(TestCase):
             RearPort(name='RearPort4', device=cls.device, type=PortTypeChoices.TYPE_8P8C),
         )
         RearPort.objects.bulk_create(cls.rear_ports)
+        cls.rear_port_templates = (
+            RearPortTemplate(name='RearPort1', device_type=cls.device.device_type, type=PortTypeChoices.TYPE_8P8C),
+            RearPortTemplate(name='RearPort2', device_type=cls.device.device_type, type=PortTypeChoices.TYPE_8P8C),
+            RearPortTemplate(name='RearPort3', device_type=cls.device.device_type, type=PortTypeChoices.TYPE_8P8C),
+            RearPortTemplate(name='RearPort4', device_type=cls.device.device_type, type=PortTypeChoices.TYPE_8P8C),
+        )
+        RearPortTemplate.objects.bulk_create(cls.rear_port_templates)
 
     def test_front_port_label_count_valid(self):
         """
@@ -353,6 +360,124 @@ class FrontPortTestCase(TestCase):
         self.assertFalse(form.is_valid())
         self.assertIn('label', form.errors)
 
+    def test_front_port_position_count_valid(self):
+        """
+        Test that generating front ports with multiple positions each passes form validation.
+        """
+        front_port_data = {
+            'device': self.device.pk,
+            'name': 'FrontPort[1-2]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 2,
+            'rear_ports': [f'{rear_port.pk}:1' for rear_port in self.rear_ports],
+        }
+        form = FrontPortCreateForm(front_port_data)
+
+        self.assertTrue(form.is_valid(), form.errors)
+
+    def test_front_port_position_count_mismatch(self):
+        """
+        Check that the mismatch error reports the total number of front port positions, not the port count.
+        """
+        bad_front_port_data = {
+            'device': self.device.pk,
+            'name': 'FrontPort[1-2]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 2,
+            'rear_ports': [f'{rear_port.pk}:1' for rear_port in self.rear_ports[:2]],
+        }
+        form = FrontPortCreateForm(bad_front_port_data)
+
+        self.assertFalse(form.is_valid())
+        self.assertIn(
+            'The total number of front port positions (4) must match the selected number of rear port '
+            'positions (2).',
+            form.errors['rear_ports']
+        )
+
+    def test_front_port_template_position_count_mismatch(self):
+        """
+        Check that the front port template form reports the same corrected position total.
+        """
+        bad_front_port_template_data = {
+            'device_type': self.device.device_type.pk,
+            'name': 'FrontPort[1-2]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 2,
+            'rear_ports': [f'{rear_port_template.pk}:1' for rear_port_template in self.rear_port_templates[:2]],
+        }
+        form = FrontPortTemplateCreateForm(bad_front_port_template_data)
+
+        self.assertFalse(form.is_valid())
+        self.assertIn(
+            'The total number of front port positions (4) must match the selected number of rear port '
+            'positions (2).',
+            form.errors['rear_ports']
+        )
+
+    def test_front_port_missing_rear_ports(self):
+        """
+        Check that omitting the rear port selection reports a field error rather than raising an exception.
+        """
+        bad_front_port_data = {
+            'device': self.device.pk,
+            'name': 'FrontPort[1-2]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 1,
+        }
+        form = FrontPortCreateForm(bad_front_port_data)
+
+        self.assertFalse(form.is_valid())
+        self.assertIn('rear_ports', form.errors)
+
+    def test_front_port_invalid_positions(self):
+        """
+        Check that a non-numeric position count reports a field error rather than raising an exception.
+        """
+        bad_front_port_data = {
+            'device': self.device.pk,
+            'name': 'FrontPort[1-2]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 'two',
+            'rear_ports': [f'{rear_port.pk}:1' for rear_port in self.rear_ports[:2]],
+        }
+        form = FrontPortCreateForm(bad_front_port_data)
+
+        self.assertFalse(form.is_valid())
+        self.assertIn('positions', form.errors)
+
+    def test_front_port_template_missing_rear_ports(self):
+        """
+        Check that the front port template form also reports a field error rather than raising an exception.
+        """
+        bad_front_port_template_data = {
+            'device_type': self.device.device_type.pk,
+            'name': 'FrontPort[1-2]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 1,
+        }
+        form = FrontPortTemplateCreateForm(bad_front_port_template_data)
+
+        self.assertFalse(form.is_valid())
+        self.assertIn('rear_ports', form.errors)
+
+    def test_front_port_invalid_label_range(self):
+        """
+        Check that an inverted label range reports a field error rather than raising an exception.
+        """
+        bad_front_port_data = {
+            'device': self.device.pk,
+            'name': 'FrontPort[1-2]',
+            'label': 'Port[2-1]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 1,
+            'rear_ports': [f'{rear_port.pk}:1' for rear_port in self.rear_ports[:2]],
+        }
+        form = FrontPortCreateForm(bad_front_port_data)
+
+        self.assertFalse(form.is_valid())
+        self.assertIn('label', form.errors)
+
 
 class InterfaceTestCase(TestCase):
 

netbox/dcim/tests/test_views.py

@@ -3658,6 +3658,43 @@ class FrontPortTestCase(ViewTestCases.DeviceComponentViewTestCase):
         self.assertEqual(response.status_code, 200)
         self.assertFalse(FrontPort.objects.filter(name='Front Port 10').exists())
 
+    def test_create_multiple_objects_with_multiple_positions(self):
+        """
+        Check that bulk creation gives each generated front port its own slice of the selected mappings.
+        """
+        device = Device.objects.get(name='Device 1')
+        rear_ports = (
+            RearPort(device=device, name='Rear Port 7', positions=2),
+            RearPort(device=device, name='Rear Port 8', positions=2),
+        )
+        RearPort.objects.bulk_create(rear_ports)
+        self.add_permissions('dcim.add_frontport')
+
+        response = self.client.post(self._get_url('add'), post_data({
+            'device': device.pk,
+            'name': 'Multi Port [1-2]',
+            'type': PortTypeChoices.TYPE_8P8C,
+            'positions': 2,
+            'rear_ports': [
+                f'{rear_ports[0].pk}:1',
+                f'{rear_ports[0].pk}:2',
+                f'{rear_ports[1].pk}:1',
+                f'{rear_ports[1].pk}:2',
+            ],
+        }))
+
+        self.assertHttpStatus(response, 302)
+        for front_port_name, rear_port in (('Multi Port 1', rear_ports[0]), ('Multi Port 2', rear_ports[1])):
+            front_port = FrontPort.objects.get(device=device, name=front_port_name)
+            self.assertEqual(front_port.positions, 2)
+            self.assertEqual(
+                [
+                    (m.front_port_position, m.rear_port_id, m.rear_port_position)
+                    for m in front_port.mappings.order_by('front_port_position')
+                ],
+                [(1, rear_port.pk, 1), (2, rear_port.pk, 2)]
+            )
+
     def test_trace(self):
         self.add_permissions(
             'dcim.view_frontport',