diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py index 3fba0842f..fa3f21592 100644 --- a/netbox/dcim/forms/model_forms.py +++ b/netbox/dcim/forms/model_forms.py @@ -1242,6 +1242,7 @@ class VirtualChassisForm(PrimaryModelForm): label=_('Master'), queryset=Device.objects.all(), required=False, + widget=SelectWithPK(), ) class Meta: @@ -1249,9 +1250,6 @@ class VirtualChassisForm(PrimaryModelForm): fields = [ 'name', 'domain', 'master', 'description', 'owner', 'comments', 'tags', ] - widgets = { - 'master': SelectWithPK(), - } def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -2043,6 +2041,7 @@ class InterfaceForm(InterfaceCommonForm, ModularDeviceComponentForm): choices=add_blank_choice(InterfaceModeChoices), required=False, help_text=_('IEEE 802.1Q tagging strategy'), + widget=HTMXSelect(hx_target_id='dot1q-switching'), ) rf_role = TypedChoiceField( label=_('Wireless role'), @@ -2186,10 +2185,6 @@ class InterfaceForm(InterfaceCommonForm, ModularDeviceComponentForm): 'speed': NumberWithOptions( options=InterfaceSpeedChoices ), - 'mode': HTMXSelect(hx_target_id='dot1q-switching'), - } - labels = { - 'mode': '802.1Q Mode', } diff --git a/netbox/extras/forms/model_forms.py b/netbox/extras/forms/model_forms.py index 41e00f9ce..8f02f88e1 100644 --- a/netbox/extras/forms/model_forms.py +++ b/netbox/extras/forms/model_forms.py @@ -129,10 +129,6 @@ class CustomFieldForm(ChangelogMessageMixin, OwnerMixin, forms.ModelForm): model = CustomField fields = '__all__' help_texts = { - 'type': _( - "The type of data stored in this field. For object/multi-object fields, select the related object " - "type below." - ), 'description': _("This will be displayed as help text for the form field. Markdown is supported.") } @@ -641,7 +637,6 @@ class EventRuleForm(OwnerMixin, NetBoxModelForm): 'action_object_type', 'action_object_id', 'action_data', 'owner', 'comments', 'tags' ) widgets = { - 'conditions': forms.Textarea(attrs={'class': 'font-monospace'}), 'action_object_type': forms.HiddenInput, 'action_object_id': forms.HiddenInput, } diff --git a/netbox/netbox/tests/test_forms.py b/netbox/netbox/tests/test_forms.py index 8a01c393a..0d3bda991 100644 --- a/netbox/netbox/tests/test_forms.py +++ b/netbox/netbox/tests/test_forms.py @@ -1,8 +1,25 @@ +from django import forms +from django.forms.models import ALL_FIELDS from django.test import TestCase +from circuits.forms import CircuitGroupAssignmentForm, CircuitTerminationForm +from core.forms import DataSourceForm from dcim.choices import InterfaceTypeChoices -from dcim.forms import InterfaceImportForm +from dcim.forms import ( + CableForm, + FrontPortCreateForm, + InterfaceForm, + InterfaceImportForm, + ModuleTypeForm, + RackForm, +) from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site +from extras.forms import CustomFieldForm, EventRuleForm +from ipam.forms import PrefixForm, ServiceForm, VLANGroupForm +from utilities.forms.widgets import HTMXSelect +from virtualization.forms import ClusterForm, VirtualMachineForm, VMInterfaceForm +from vpn.forms import TunnelCreateForm, TunnelTerminationForm +from wireless.forms import WirelessLANForm class NetBoxModelImportFormCleanTestCase(TestCase): @@ -301,3 +318,162 @@ class NetBoxModelImportFormCleanTestCase(TestCase): ) self.assertTrue(form.is_valid(), f'Form errors: {form.errors}') self.assertIsNone(form.cleaned_data['wwn']) + + +class HTMXPartialSwapRenderingTestCase(TestCase): + """Ensure each form field's HTMX swap wiring is present on the widget bound to the field.""" + + # (form, bound field name, target fieldset id) + PARTIAL_SWAP_FIELDS = ( + (InterfaceForm, 'mode', 'dot1q-switching'), + (VMInterfaceForm, 'mode', 'dot1q-switching'), + (ModuleTypeForm, 'profile', 'profile-attributes'), + (CableForm, 'a_terminations_type', 'cable-side-a'), + (CableForm, 'b_terminations_type', 'cable-side-b'), + (VLANGroupForm, 'scope', 'scope'), + (ServiceForm, 'parent', 'service'), + (CircuitTerminationForm, 'termination', 'circuit-termination'), + (CircuitGroupAssignmentForm, 'member', 'circuit-group-assignment'), + (TunnelCreateForm, 'termination1_type', 'tunnel-termination1'), + (TunnelCreateForm, 'termination2_type', 'tunnel-termination2'), + (TunnelTerminationForm, 'type', 'tunnel-termination'), + (EventRuleForm, 'action_type', 'event-rule-action'), + (ClusterForm, 'scope', 'scope'), + (WirelessLANForm, 'scope', 'scope'), + (PrefixForm, 'scope', 'scope'), + ) + + # Fields that intentionally re-render the whole form (#form_fields) rather than a single fieldset. + FULL_FORM_FIELDS = ( + (CustomFieldForm, 'type'), + (DataSourceForm, 'type'), + (VirtualMachineForm, 'virtual_machine_type'), + (RackForm, 'rack_type'), + (FrontPortCreateForm, 'device'), + ) + + # CableForm targets template
s, not FieldSet(html_id=...), so its target can't be checked + # against form.fieldsets. + FIELDSET_ID_EXEMPT = {CableForm} + + @staticmethod + def _hx_widget(field): + """Return the HTMXSelect carrying the field's HTMX attrs, unwrapping a MultiWidget by type.""" + widget = field.widget + if isinstance(widget, forms.MultiWidget): + hx = next((w for w in widget.widgets if isinstance(w, HTMXSelect)), None) + assert hx is not None, f'{type(widget).__name__} has no HTMXSelect subwidget' + return hx + return widget + + def test_partial_swap_fields_target_their_fieldset(self): + for form_class, field_name, target_id in self.PARTIAL_SWAP_FIELDS: + with self.subTest(form=form_class.__name__, field=field_name): + form = form_class() + self.assertIn(field_name, form.fields) + attrs = self._hx_widget(form.fields[field_name]).attrs + # hx-get issues the request; hx-target/hx-select alone are inert. + self.assertIn('hx-get', attrs) + self.assertEqual(attrs.get('hx-select'), f'#{target_id}') + self.assertEqual(attrs.get('hx-target'), f'#{target_id}') + # The target must name a declared FieldSet html_id, else the swap fails silently. + if form_class not in self.FIELDSET_ID_EXEMPT: + fieldset_ids = {getattr(fs, 'html_id', None) for fs in getattr(form, 'fieldsets', ())} + self.assertIn(target_id, fieldset_ids) + + def test_interface_mode_retains_option_descriptions(self): + # Partial swap and option descriptions must coexist on the mode field. + for form_class in (InterfaceForm, VMInterfaceForm): + with self.subTest(form=form_class.__name__): + self.assertTrue(form_class().fields['mode'].widget.descriptions) + + def test_full_form_fields_do_not_partial_swap(self): + for form_class, field_name in self.FULL_FORM_FIELDS: + with self.subTest(form=form_class.__name__, field=field_name): + form = form_class() + self.assertIn(field_name, form.fields) + attrs = self._hx_widget(form.fields[field_name]).attrs + self.assertIn('hx-get', attrs) + self.assertEqual(attrs.get('hx-target'), '#form_fields') + self.assertNotIn('hx-select', attrs) + + +class MetaShadowingTestCase(TestCase): + """Ensure declared fields do not shadow supported `ModelForm.Meta` configuration.""" + # Known overlaps whose cleanup is deferred, as specific (form, Meta attribute, field) triples so + # that any new overlap on the same form is still caught. ConfigRevisionForm builds these fields + # via ConfigFormMetaclass; restoring their monospace widget is tracked separately (see #22889). + ALLOWED = { + ('ConfigRevisionForm', 'widgets', 'BANNER_LOGIN'), + ('ConfigRevisionForm', 'widgets', 'BANNER_MAINTENANCE'), + ('ConfigRevisionForm', 'widgets', 'BANNER_TOP'), + ('ConfigRevisionForm', 'widgets', 'BANNER_BOTTOM'), + ('ConfigRevisionForm', 'widgets', 'CUSTOM_VALIDATORS'), + ('ConfigRevisionForm', 'widgets', 'PROTECTION_RULES'), + } + + @staticmethod + def _all_model_forms(): + # Import every app's forms package so all ModelForm subclasses are registered before walking. + import importlib + import pkgutil + for app in ( + 'circuits', 'core', 'dcim', 'extras', 'ipam', 'tenancy', 'users', 'utilities', + 'virtualization', 'vpn', 'wireless', 'netbox', + ): + # Only tolerate an app that has no forms package; a broken import inside a package that + # does exist must surface, or the invariant could pass having skipped part of the tree. + pkg_name = f'{app}.forms' + try: + pkg = importlib.import_module(pkg_name) + except ModuleNotFoundError as e: + if e.name == pkg_name: + continue + raise + for module in getattr(pkg, '__path__', []) and pkgutil.iter_modules(pkg.__path__) or []: + submodule = f'{app}.forms.{module.name}' + try: + importlib.import_module(submodule) + except ModuleNotFoundError as e: + if e.name == submodule: + continue + raise + + seen, stack = set(), [forms.ModelForm] + while stack: + for sub in stack.pop().__subclasses__(): + if sub not in seen: + seen.add(sub) + stack.append(sub) + return seen + + @staticmethod + def _configured_field_names(configured, declared): + # Field names a Meta option targets. localized_fields may be the ALL_FIELDS sentinel + # ('__all__') meaning every field; set() of that string would yield its characters, so map + # the sentinel to the full declared set instead. + if configured == ALL_FIELDS: + return set(declared) + return set(configured or ()) + + def test_meta_config_does_not_shadow_declared_fields(self): + for form_class in self._all_model_forms(): + meta = getattr(form_class, 'Meta', None) + declared = set(getattr(form_class, 'declared_fields', {})) + if meta is None or not declared: + continue + for attr in ('widgets', 'labels', 'help_texts', 'error_messages', 'field_classes', 'localized_fields'): + configured = self._configured_field_names(getattr(meta, attr, None), declared) + overlap = declared & configured + overlap -= {f for f in overlap if (form_class.__name__, attr, f) in self.ALLOWED} + with self.subTest(form=form_class.__name__, meta=attr): + self.assertEqual( + overlap, set(), + f"{form_class.__module__}.{form_class.__name__} sets Meta.{attr} for " + f"explicitly declared field(s) {sorted(overlap)}; Django discards these. " + f"Move the config onto the declared field or drop the Meta entry." + ) + + def test_localized_fields_all_sentinel_expands_to_declared(self): + # The ALL_FIELDS sentinel must expand to the declared fields, not char-split into letters. + self.assertEqual(self._configured_field_names(ALL_FIELDS, {'name', 'status'}), {'name', 'status'}) diff --git a/netbox/utilities/forms/widgets/select.py b/netbox/utilities/forms/widgets/select.py index 4c8d4500b..841e0267d 100644 --- a/netbox/utilities/forms/widgets/select.py +++ b/netbox/utilities/forms/widgets/select.py @@ -26,6 +26,11 @@ class AttrSelectMixin: super().__init__(*args, **kwargs) self.descriptions = descriptions or {} + def __deepcopy__(self, memo): + obj = super().__deepcopy__(memo) + obj.descriptions = self.descriptions.copy() + return obj + def create_option(self, name, value, label, selected, index, subindex=None, attrs=None): option = super().create_option(name, value, label, selected, index, subindex, attrs) @@ -90,9 +95,10 @@ class ColorSelect(forms.Select): self.attrs['class'] = 'color-select' -class HTMXSelect(forms.Select): +class HTMXSelect(Select): """ - Selection widget that will re-generate the HTML form upon the selection of a new option. + Selection widget that re-generates the HTML form upon selection of a new option, and supports + per-option descriptions alongside its HTMX behavior. """ def __init__(self, method='get', hx_url='.', hx_include_id='form_fields', hx_target_id=None, attrs=None, **kwargs): method = method.lower() diff --git a/netbox/utilities/tests/test_forms.py b/netbox/utilities/tests/test_forms.py index d8ed4513e..7d880a2e7 100644 --- a/netbox/utilities/tests/test_forms.py +++ b/netbox/utilities/tests/test_forms.py @@ -1,3 +1,4 @@ +import copy import warnings from types import SimpleNamespace @@ -1042,6 +1043,13 @@ class DescriptionSelectTestCase(TestCase): self.assertInHTML('', html) self.assertInHTML('', html) + def test_deepcopy_does_not_share_descriptions(self): + widget = Select(choices=[('x', 'X')], descriptions={'x': 'Description X'}) + clone = copy.deepcopy(widget) + self.assertIsNot(widget.descriptions, clone.descriptions) + widget.descriptions['y'] = 'leaked' + self.assertNotIn('y', clone.descriptions) + def test_choices_setter_delegates_through_mro(self): """ AttrChoiceMixin must delegate to the parent field's choices setter via the MRO, not a hardcoded base, diff --git a/netbox/virtualization/forms/model_forms.py b/netbox/virtualization/forms/model_forms.py index a1c20a8cf..0912d5c8a 100644 --- a/netbox/virtualization/forms/model_forms.py +++ b/netbox/virtualization/forms/model_forms.py @@ -422,6 +422,7 @@ class VMInterfaceForm(InterfaceCommonForm, VMComponentForm): choices=add_blank_choice(InterfaceModeChoices), required=False, help_text=_('IEEE 802.1Q tagging strategy'), + widget=HTMXSelect(hx_target_id='dot1q-switching'), ) primary_mac_address = DynamicModelChoiceField( queryset=MACAddress.objects.all(), @@ -509,12 +510,6 @@ class VMInterfaceForm(InterfaceCommonForm, VMComponentForm): 'untagged_vlan', 'tagged_vlans', 'qinq_svlan', 'vlan_translation_policy', 'vrf', 'owner', 'tags', ] - labels = { - 'mode': _('802.1Q Mode'), - } - widgets = { - 'mode': HTMXSelect(hx_target_id='dot1q-switching'), - } class VirtualDiskForm(VMComponentForm): diff --git a/netbox/wireless/forms/model_forms.py b/netbox/wireless/forms/model_forms.py index 11ec61c59..851a13ee6 100644 --- a/netbox/wireless/forms/model_forms.py +++ b/netbox/wireless/forms/model_forms.py @@ -228,7 +228,3 @@ class WirelessLinkForm(DistanceValidationMixin, TenancyForm, PrimaryModelForm): attrs={'data-toggle': 'password'} ), } - labels = { - 'auth_type': 'Type', - 'auth_cipher': 'Cipher', - }