From 3680b0ccd4167480c1f87f1f071ae2cb109b2d10 Mon Sep 17 00:00:00 2001 From: Mark Coleman Date: Mon, 19 Jan 2026 19:06:42 +0100 Subject: [PATCH] Refactor: move resolve_module_placeholders from constants.py to utils.py Constants should only contain constant values, not functions with logic. The helper function now lives in dcim/utils.py alongside other utilities like update_interface_bridges and create_port_mappings. --- netbox/dcim/constants.py | 47 ------------------ netbox/dcim/forms/common.py | 1 + .../dcim/models/device_component_templates.py | 1 + netbox/dcim/utils.py | 48 +++++++++++++++++++ 4 files changed, 50 insertions(+), 47 deletions(-) diff --git a/netbox/dcim/constants.py b/netbox/dcim/constants.py index 56c07f505..6efeeab47 100644 --- a/netbox/dcim/constants.py +++ b/netbox/dcim/constants.py @@ -82,53 +82,6 @@ MODULE_TOKEN = '{module}' MODULE_PATH_TOKEN = '{module_path}' MODULE_TOKEN_SEPARATOR = '/' - -def resolve_module_placeholders(text, positions): - """ - Substitute {module} and {module_path} placeholders in text with position values. - - Args: - text: String potentially containing {module} or {module_path} placeholders - positions: List of position strings from the module tree (root to leaf) - - Returns: - Text with placeholders replaced according to these rules: - - {module_path}: Always expands to full path (positions joined by MODULE_TOKEN_SEPARATOR). - Can only appear once in the text. - - {module}: If used once, expands to the PARENT module bay position only (last in positions). - If used multiple times, each token is replaced level-by-level. - - This design (Option 2 per sigprof's feedback) allows two approaches: - 1. Use {module_path} for automatic full-path expansion (hardcodes '/' separator) - 2. Use {module} in position fields to build custom paths with user-controlled separators - """ - if not text: - return text - - result = text - - # Handle {module_path} - always expands to full path - if MODULE_PATH_TOKEN in result: - full_path = MODULE_TOKEN_SEPARATOR.join(positions) if positions else '' - result = result.replace(MODULE_PATH_TOKEN, full_path) - - # Handle {module} - parent-only for single token, level-by-level for multiple - if MODULE_TOKEN in result: - token_count = result.count(MODULE_TOKEN) - if token_count == 1 and positions: - # Single {module}: substitute with parent (immediate) bay position only - parent_position = positions[-1] if positions else '' - result = result.replace(MODULE_TOKEN, parent_position, 1) - else: - # Multiple {module}: substitute level-by-level (existing behavior) - for pos in positions: - result = result.replace(MODULE_TOKEN, pos, 1) - - return result - - MODULAR_COMPONENT_TEMPLATE_MODELS = Q( app_label='dcim', model__in=( diff --git a/netbox/dcim/forms/common.py b/netbox/dcim/forms/common.py index ed23f7346..94b97255f 100644 --- a/netbox/dcim/forms/common.py +++ b/netbox/dcim/forms/common.py @@ -3,6 +3,7 @@ from django.utils.translation import gettext_lazy as _ from dcim.choices import * from dcim.constants import * +from dcim.utils import resolve_module_placeholders from utilities.forms import get_field_value __all__ = ( diff --git a/netbox/dcim/models/device_component_templates.py b/netbox/dcim/models/device_component_templates.py index cbcd1389a..682b15be1 100644 --- a/netbox/dcim/models/device_component_templates.py +++ b/netbox/dcim/models/device_component_templates.py @@ -8,6 +8,7 @@ from mptt.models import MPTTModel, TreeForeignKey from dcim.choices import * from dcim.constants import * from dcim.models.base import PortMappingBase +from dcim.utils import resolve_module_placeholders from dcim.models.mixins import InterfaceValidationMixin from netbox.models import ChangeLoggedModel from utilities.fields import ColorField, NaturalOrderingField diff --git a/netbox/dcim/utils.py b/netbox/dcim/utils.py index 4b9d0fb5c..fcc3ebd2c 100644 --- a/netbox/dcim/utils.py +++ b/netbox/dcim/utils.py @@ -4,6 +4,54 @@ from django.apps import apps from django.contrib.contenttypes.models import ContentType from django.db import router, transaction +from dcim.constants import MODULE_PATH_TOKEN, MODULE_TOKEN, MODULE_TOKEN_SEPARATOR + + +def resolve_module_placeholders(text, positions): + """ + Substitute {module} and {module_path} placeholders in text with position values. + + Args: + text: String potentially containing {module} or {module_path} placeholders + positions: List of position strings from the module tree (root to leaf) + + Returns: + Text with placeholders replaced according to these rules: + + {module_path}: Always expands to full path (positions joined by MODULE_TOKEN_SEPARATOR). + Can only appear once in the text. + + {module}: If used once, expands to the PARENT module bay position only (last in positions). + If used multiple times, each token is replaced level-by-level. + + This design (Option 2 per sigprof's feedback) allows two approaches: + 1. Use {module_path} for automatic full-path expansion (hardcodes '/' separator) + 2. Use {module} in position fields to build custom paths with user-controlled separators + """ + if not text: + return text + + result = text + + # Handle {module_path} - always expands to full path + if MODULE_PATH_TOKEN in result: + full_path = MODULE_TOKEN_SEPARATOR.join(positions) if positions else '' + result = result.replace(MODULE_PATH_TOKEN, full_path) + + # Handle {module} - parent-only for single token, level-by-level for multiple + if MODULE_TOKEN in result: + token_count = result.count(MODULE_TOKEN) + if token_count == 1 and positions: + # Single {module}: substitute with parent (immediate) bay position only + parent_position = positions[-1] if positions else '' + result = result.replace(MODULE_TOKEN, parent_position, 1) + else: + # Multiple {module}: substitute level-by-level (existing behavior) + for pos in positions: + result = result.replace(MODULE_TOKEN, pos, 1) + + return result + def compile_path_node(ct_id, object_id): return f'{ct_id}:{object_id}'