From 1df6eee4675902d39e25f7d574f0030694739ba1 Mon Sep 17 00:00:00 2001 From: Mark Coleman Date: Mon, 19 Jan 2026 15:54:49 +0100 Subject: [PATCH] Add position field resolution for module bays (fixes #20467) - Add resolve_position() method to ModularComponentTemplateModel - Update ModuleBayTemplate.instantiate() to resolve {module} in position field - Add test_module_bay_position_resolves_placeholder test This completes the fix for nested module placeholder issues by ensuring the position field also resolves {module} placeholders, which is required for building correct full paths in 3+ level hierarchies. --- .../dcim/models/device_component_templates.py | 33 +++++++- netbox/dcim/tests/test_models.py | 77 +++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/netbox/dcim/models/device_component_templates.py b/netbox/dcim/models/device_component_templates.py index 558a53761..6ee43647c 100644 --- a/netbox/dcim/models/device_component_templates.py +++ b/netbox/dcim/models/device_component_templates.py @@ -207,6 +207,32 @@ class ModularComponentTemplateModel(ComponentTemplateModel): return label 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 not position or MODULE_TOKEN not in position: + return position + + if module: + modules = self._get_module_tree(module) + token_count = position.count(MODULE_TOKEN) + if token_count == 1: + # Single token: substitute with full path + full_path = '/'.join([m.module_bay.position for m in modules]) + position = position.replace(MODULE_TOKEN, full_path, 1) + else: + # Multiple tokens: substitute level-by-level + for m in modules: + position = position.replace(MODULE_TOKEN, m.module_bay.position, 1) + return position + return position + class ConsolePortTemplate(ModularComponentTemplateModel): """ @@ -736,10 +762,11 @@ class ModuleBayTemplate(ModularComponentTemplateModel): verbose_name_plural = _('module bay templates') def instantiate(self, **kwargs): + module = kwargs.get('module') return self.component_model( - name=self.resolve_name(kwargs.get('module')), - label=self.resolve_label(kwargs.get('module')), - position=self.position, + name=self.resolve_name(module), + label=self.resolve_label(module), + position=self.resolve_position(self.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 dd038ae56..1e5d5c8d5 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -969,6 +969,83 @@ class ModuleBayTestCase(TestCase): self.assertEqual(interface_2.name, 'SFP 1/2') self.assertEqual(interface_2.label, '1/2') + def test_module_bay_position_resolves_placeholder(self): + """ + Test that the position field of instantiated module bays resolves {module} placeholder. + + Issue #20467: When a module type has module bay templates with position="{module}/1", + the instantiated module bay should have position="A/1" (not literal "{module}/1"). + + This test should: + - FAIL on main branch (bug present: position contains "{module}") + - PASS after fix (position is resolved to actual value) + """ + manufacturer = Manufacturer.objects.first() + site = Site.objects.first() + device_role = DeviceRole.objects.first() + + # Create device type with module bay at position 'A' + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Position Test Chassis', + slug='position-test-chassis' + ) + ModuleBayTemplate.objects.create( + device_type=device_type, + name='Bay A', + position='A' + ) + + # Create module type with nested bays using {module} in POSITION field + extension_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Position Test Extension' + ) + ModuleBayTemplate.objects.create( + module_type=extension_type, + name='Sub Bay {module}-1', + label='{module}-1', + position='{module}/1' # This should resolve to "A/1" + ) + ModuleBayTemplate.objects.create( + module_type=extension_type, + name='Sub Bay {module}-2', + label='{module}-2', + position='{module}/2' # This should resolve to "A/2" + ) + + # Create device + device = Device.objects.create( + name='Position Test Device', + device_type=device_type, + role=device_role, + site=site + ) + + # Install extension module in Bay A + parent_bay = device.modulebays.get(name='Bay A') + module = Module.objects.create( + device=device, + module_bay=parent_bay, + module_type=extension_type + ) + + # Verify the nested bays have resolved names (this already works) + nested_bay_1 = module.modulebays.get(name='Sub Bay A-1') + nested_bay_2 = module.modulebays.get(name='Sub Bay A-2') + + # Verify labels are resolved (this already works) + self.assertEqual(nested_bay_1.label, 'A-1') + self.assertEqual(nested_bay_2.label, 'A-2') + + # Verify POSITION field is resolved (Issue #20467 - this currently fails) + self.assertEqual(nested_bay_1.position, 'A/1') + self.assertEqual(nested_bay_2.position, 'A/2') + + # Also verify no {module} literal remains + self.assertNotIn('{module}', nested_bay_1.position) + self.assertNotIn('{module}', nested_bay_2.position) + def test_single_placeholder_direct_install_depth_1(self): """ Test that installing a module directly at depth=1 with a single {module}