diff --git a/netbox/dcim/forms/common.py b/netbox/dcim/forms/common.py index 94b97255f..330913de9 100644 --- a/netbox/dcim/forms/common.py +++ b/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) diff --git a/netbox/dcim/models/device_component_templates.py b/netbox/dcim/models/device_component_templates.py index 682b15be1..40fc6fca8 100644 --- a/netbox/dcim/models/device_component_templates.py +++ b/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 diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index f788181e8..912e537fd 100644 --- a/netbox/dcim/tests/test_models.py +++ b/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):