mirror of
https://github.com/netbox-community/netbox.git
synced 2026-01-19 18:18:43 -06:00
Refactor: centralize module token substitution logic
Per sigprof's review feedback, extract the duplicated token substitution logic into a single resolve_module_token() helper in constants.py. This addresses two review comments: 1. Duplication between ModuleCommonForm.clean() and resolve_name() 2. Duplication between resolve_name() and resolve_label() Benefits: - Single source of truth for substitution logic - MODULE_TOKEN_SEPARATOR constant for future configurability - Cleaner, more maintainable code (-7 net lines) - Easier to modify separator handling in one place
This commit is contained in:
@@ -79,6 +79,41 @@ NONCONNECTABLE_IFACE_TYPES = VIRTUAL_IFACE_TYPES + WIRELESS_IFACE_TYPES
|
||||
#
|
||||
|
||||
MODULE_TOKEN = '{module}'
|
||||
MODULE_TOKEN_SEPARATOR = '/'
|
||||
|
||||
|
||||
def resolve_module_token(text, positions):
|
||||
"""
|
||||
Substitute {module} tokens in text with position values.
|
||||
|
||||
Args:
|
||||
text: String potentially containing {module} tokens
|
||||
positions: List of position strings from the module tree (root to leaf)
|
||||
|
||||
Returns:
|
||||
Text with {module} tokens replaced according to these rules:
|
||||
- Single token: replaced with full path (positions joined by MODULE_TOKEN_SEPARATOR)
|
||||
- Multiple tokens: replaced level-by-level (first token gets first position, etc.)
|
||||
|
||||
This centralizes the substitution logic used by both ModuleCommonForm.clean()
|
||||
and ModularComponentTemplateModel.resolve_*() methods.
|
||||
"""
|
||||
if not text or MODULE_TOKEN not in text:
|
||||
return text
|
||||
|
||||
token_count = text.count(MODULE_TOKEN)
|
||||
|
||||
if token_count == 1:
|
||||
# Single token: substitute with full path (e.g., "1/1" for depth 2)
|
||||
full_path = MODULE_TOKEN_SEPARATOR.join(positions)
|
||||
return text.replace(MODULE_TOKEN, full_path, 1)
|
||||
else:
|
||||
# Multiple tokens: substitute level-by-level (existing behavior)
|
||||
result = text
|
||||
for pos in positions:
|
||||
result = result.replace(MODULE_TOKEN, pos, 1)
|
||||
return result
|
||||
|
||||
|
||||
MODULAR_COMPONENT_TEMPLATE_MODELS = Q(
|
||||
app_label='dcim',
|
||||
|
||||
@@ -139,14 +139,9 @@ class ModuleCommonForm(forms.Form):
|
||||
)
|
||||
)
|
||||
|
||||
if token_count == 1:
|
||||
# Single token: substitute with full path (e.g., "1/1" for depth 2)
|
||||
full_path = '/'.join([mb.position for mb in module_bays])
|
||||
resolved_name = resolved_name.replace(MODULE_TOKEN, full_path, 1)
|
||||
else:
|
||||
# Multiple tokens: substitute level-by-level (existing behavior)
|
||||
for mb in module_bays:
|
||||
resolved_name = resolved_name.replace(MODULE_TOKEN, mb.position, 1)
|
||||
# Use centralized helper for token substitution
|
||||
positions = [mb.position for mb in module_bays]
|
||||
resolved_name = resolve_module_token(resolved_name, positions)
|
||||
|
||||
existing_item = installed_components.get(resolved_name)
|
||||
|
||||
|
||||
@@ -170,41 +170,17 @@ class ModularComponentTemplateModel(ComponentTemplateModel):
|
||||
return modules
|
||||
|
||||
def resolve_name(self, module):
|
||||
if MODULE_TOKEN not in self.name:
|
||||
return self.name
|
||||
|
||||
"""Resolve {module} placeholder(s) in component name."""
|
||||
if module:
|
||||
modules = self._get_module_tree(module)
|
||||
token_count = self.name.count(MODULE_TOKEN)
|
||||
name = self.name
|
||||
if token_count == 1:
|
||||
# Single token: substitute with full path (e.g., "1/1" for depth 2)
|
||||
full_path = '/'.join([m.module_bay.position for m in modules])
|
||||
name = name.replace(MODULE_TOKEN, full_path, 1)
|
||||
else:
|
||||
# Multiple tokens: substitute level-by-level (existing behavior)
|
||||
for m in modules:
|
||||
name = name.replace(MODULE_TOKEN, m.module_bay.position, 1)
|
||||
return name
|
||||
positions = [m.module_bay.position for m in self._get_module_tree(module)]
|
||||
return resolve_module_token(self.name, positions)
|
||||
return self.name
|
||||
|
||||
def resolve_label(self, module):
|
||||
if MODULE_TOKEN not in self.label:
|
||||
return self.label
|
||||
|
||||
"""Resolve {module} placeholder(s) in component label."""
|
||||
if module:
|
||||
modules = self._get_module_tree(module)
|
||||
token_count = self.label.count(MODULE_TOKEN)
|
||||
label = self.label
|
||||
if token_count == 1:
|
||||
# Single token: substitute with full path (e.g., "1/1" for depth 2)
|
||||
full_path = '/'.join([m.module_bay.position for m in modules])
|
||||
label = label.replace(MODULE_TOKEN, full_path, 1)
|
||||
else:
|
||||
# Multiple tokens: substitute level-by-level (existing behavior)
|
||||
for m in modules:
|
||||
label = label.replace(MODULE_TOKEN, m.module_bay.position, 1)
|
||||
return label
|
||||
positions = [m.module_bay.position for m in self._get_module_tree(module)]
|
||||
return resolve_module_token(self.label, positions)
|
||||
return self.label
|
||||
|
||||
def resolve_position(self, position, module):
|
||||
@@ -216,21 +192,9 @@ class ModularComponentTemplateModel(ComponentTemplateModel):
|
||||
|
||||
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
|
||||
positions = [m.module_bay.position for m in self._get_module_tree(module)]
|
||||
return resolve_module_token(position, positions)
|
||||
return position
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user