Selaa lähdekoodia

Address sigprof's Jan 20 feedback

1. Add validation to reject mixing {module} and {module_path} in same attribute
2. Refactor resolve_position() to match resolve_name()/resolve_label() pattern
   - Moved to ModuleBayTemplate where it can access self.position directly
   - No longer takes position as argument
3. Added test for mixed placeholder validation
Mark Coleman 3 viikkoa sitten
vanhempi
commit
898fe8b3d8

+ 6 - 0
netbox/dcim/forms/common.py

@@ -130,6 +130,12 @@ class ModuleCommonForm(forms.Form):
                             _("Cannot install module with placeholder values in a module bay with no position defined.")
                         )
 
+                    # Cannot mix {module} and {module_path} in the same attribute
+                    if has_module_token and has_module_path_token:
+                        raise forms.ValidationError(
+                            _("Cannot mix {module} and {module_path} placeholders in the same template attribute.")
+                        )
+
                     # Validate {module_path} - can only appear once
                     if has_module_path_token:
                         path_token_count = template.name.count(MODULE_PATH_TOKEN)

+ 15 - 15
netbox/dcim/models/device_component_templates.py

@@ -184,20 +184,6 @@ class ModularComponentTemplateModel(ComponentTemplateModel):
             return resolve_module_placeholders(self.label, positions)
         return self.label
 
-    def resolve_position(self, position, module):
-        """
-        Resolve {module} placeholder in position field.
-
-        This is used by ModuleBayTemplate to resolve positions like "{module}/1"
-        to actual values like "A/1" when the parent module is installed in bay "A".
-
-        Fixes Issue #20467.
-        """
-        if module:
-            positions = [m.module_bay.position for m in self._get_module_tree(module)]
-            return resolve_module_placeholders(position, positions)
-        return position
-
 
 class ConsolePortTemplate(ModularComponentTemplateModel):
     """
@@ -726,12 +712,26 @@ class ModuleBayTemplate(ModularComponentTemplateModel):
         verbose_name = _('module bay template')
         verbose_name_plural = _('module bay templates')
 
+    def resolve_position(self, module):
+        """
+        Resolve {module} and {module_path} placeholders in position field.
+
+        This allows positions like "{module}/1" to resolve to "A/1" when
+        the parent module is installed in bay "A".
+
+        Fixes Issue #20467.
+        """
+        if module:
+            positions = [m.module_bay.position for m in self._get_module_tree(module)]
+            return resolve_module_placeholders(self.position, positions)
+        return self.position
+
     def instantiate(self, **kwargs):
         module = kwargs.get('module')
         return self.component_model(
             name=self.resolve_name(module),
             label=self.resolve_label(module),
-            position=self.resolve_position(self.position, module),
+            position=self.resolve_position(module),
             **kwargs
         )
     instantiate.do_not_call_in_templates = True

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

@@ -2045,6 +2045,61 @@ class ModuleBayTestCase(TestCase):
 
         self.assertFalse(form.is_valid())
 
+    def test_mixed_module_and_module_path_fails_validation(self):
+        """
+        Test that mixing {module} and {module_path} in the same template attribute
+        fails validation. Per sigprof's feedback, these cannot be combined.
+        """
+        from dcim.forms import ModuleForm
+
+        manufacturer = Manufacturer.objects.first()
+        site = Site.objects.first()
+        device_role = DeviceRole.objects.first()
+
+        device_type = DeviceType.objects.create(
+            manufacturer=manufacturer,
+            model='Mixed Placeholder Chassis',
+            slug='mixed-placeholder-chassis'
+        )
+        ModuleBayTemplate.objects.create(
+            device_type=device_type,
+            name='Bay 1',
+            position='1'
+        )
+
+        # Module type with both {module} and {module_path} - should fail
+        bad_module_type = ModuleType.objects.create(
+            manufacturer=manufacturer,
+            model='Bad Mixed Module'
+        )
+        InterfaceTemplate.objects.create(
+            module_type=bad_module_type,
+            name='{module_path}-{module}',
+            type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS
+        )
+
+        device = Device.objects.create(
+            name='Mixed Placeholder Device',
+            device_type=device_type,
+            role=device_role,
+            site=site
+        )
+
+        bay = device.modulebays.get(name='Bay 1')
+
+        form = ModuleForm(data={
+            'device': device.pk,
+            'module_bay': bay.pk,
+            'module_type': bad_module_type.pk,
+            'status': 'active',
+            'replicate_components': True,
+            'adopt_components': False,
+        })
+
+        self.assertFalse(form.is_valid())
+        # Verify it's specifically the mixed placeholder error
+        self.assertIn('Cannot mix', str(form.errors))
+
 
 class CableTestCase(TestCase):