From f30786d8fed1acd6a064c552b253b7eec439567c Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 27 Mar 2026 16:42:43 -0400 Subject: [PATCH] Fixes #21763: Replace M2M selection field with separate add/remove fields --- netbox/circuits/forms/model_forms.py | 23 ++++++++++--- netbox/circuits/tests/test_views.py | 2 +- netbox/dcim/forms/model_forms.py | 24 +++++++++++--- netbox/dcim/tests/test_views.py | 2 +- netbox/ipam/forms/model_forms.py | 32 ++++++++++--------- netbox/netbox/forms/model_forms.py | 12 +++++++ netbox/netbox/views/generic/object_views.py | 11 +++++++ netbox/utilities/forms/rendering.py | 16 ++++++++++ netbox/utilities/templatetags/form_helpers.py | 9 +++++- 9 files changed, 103 insertions(+), 28 deletions(-) diff --git a/netbox/circuits/forms/model_forms.py b/netbox/circuits/forms/model_forms.py index 5a48455d8..a84906ffb 100644 --- a/netbox/circuits/forms/model_forms.py +++ b/netbox/circuits/forms/model_forms.py @@ -22,7 +22,7 @@ from utilities.forms.fields import ( SlugField, ) from utilities.forms.mixins import DistanceValidationMixin -from utilities.forms.rendering import FieldSet, InlineFields +from utilities.forms.rendering import FieldSet, InlineFields, M2MAddRemoveFields from utilities.forms.widgets import DatePicker, HTMXSelect, NumberWithOptions from utilities.templatetags.builtins.filters import bettertitle @@ -43,22 +43,35 @@ __all__ = ( class ProviderForm(PrimaryModelForm): slug = SlugField() - asns = DynamicModelMultipleChoiceField( + add_asns = DynamicModelMultipleChoiceField( queryset=ASN.objects.all(), - label=_('ASNs'), + label=_('Add ASNs'), + required=False + ) + remove_asns = DynamicModelMultipleChoiceField( + queryset=ASN.objects.all(), + label=_('Remove ASNs'), required=False ) fieldsets = ( - FieldSet('name', 'slug', 'asns', 'description', 'tags'), + FieldSet('name', 'slug', M2MAddRemoveFields('asns'), 'description', 'tags'), ) class Meta: model = Provider fields = [ - 'name', 'slug', 'asns', 'description', 'owner', 'comments', 'tags', + 'name', 'slug', 'description', 'owner', 'comments', 'tags', ] + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.instance.pk: + self.fields['remove_asns'].widget.add_query_param('provider_id', self.instance.pk) + else: + self.fields.pop('remove_asns') + self.fields['add_asns'].label = _('ASNs') + class ProviderAccountForm(PrimaryModelForm): provider = DynamicModelChoiceField( diff --git a/netbox/circuits/tests/test_views.py b/netbox/circuits/tests/test_views.py index 6ced9a958..3a3e2a50c 100644 --- a/netbox/circuits/tests/test_views.py +++ b/netbox/circuits/tests/test_views.py @@ -42,7 +42,7 @@ class ProviderTestCase(ViewTestCases.PrimaryObjectViewTestCase): cls.form_data = { 'name': 'Provider X', 'slug': 'provider-x', - 'asns': [asns[6].pk, asns[7].pk], + 'add_asns': [asns[6].pk, asns[7].pk], 'comments': 'Another provider', 'tags': [t.pk for t in tags], } diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py index 741315323..1507bdfe3 100644 --- a/netbox/dcim/forms/model_forms.py +++ b/netbox/dcim/forms/model_forms.py @@ -23,7 +23,7 @@ from utilities.forms.fields import ( NumericArrayField, SlugField, ) -from utilities.forms.rendering import FieldSet, InlineFields, TabbedGroups +from utilities.forms.rendering import FieldSet, InlineFields, M2MAddRemoveFields, TabbedGroups from utilities.forms.widgets import ( APISelect, ClearableFileInput, @@ -137,9 +137,14 @@ class SiteForm(TenancyForm, PrimaryModelForm): required=False, quick_add=True ) - asns = DynamicModelMultipleChoiceField( + add_asns = DynamicModelMultipleChoiceField( queryset=ASN.objects.all(), - label=_('ASNs'), + label=_('Add ASNs'), + required=False + ) + remove_asns = DynamicModelMultipleChoiceField( + queryset=ASN.objects.all(), + label=_('Remove ASNs'), required=False ) slug = SlugField() @@ -151,7 +156,8 @@ class SiteForm(TenancyForm, PrimaryModelForm): fieldsets = ( FieldSet( - 'name', 'slug', 'status', 'region', 'group', 'facility', 'asns', 'time_zone', 'description', 'tags', + 'name', 'slug', 'status', 'region', 'group', 'facility', M2MAddRemoveFields('asns'), 'time_zone', + 'description', 'tags', name=_('Site') ), FieldSet('tenant_group', 'tenant', name=_('Tenancy')), @@ -161,7 +167,7 @@ class SiteForm(TenancyForm, PrimaryModelForm): class Meta: model = Site fields = ( - 'name', 'slug', 'status', 'region', 'group', 'tenant_group', 'tenant', 'facility', 'asns', 'time_zone', + 'name', 'slug', 'status', 'region', 'group', 'tenant_group', 'tenant', 'facility', 'time_zone', 'description', 'physical_address', 'shipping_address', 'latitude', 'longitude', 'owner', 'comments', 'tags', ) widgets = { @@ -177,6 +183,14 @@ class SiteForm(TenancyForm, PrimaryModelForm): ), } + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.instance.pk: + self.fields['remove_asns'].widget.add_query_param('site_id', self.instance.pk) + else: + self.fields.pop('remove_asns') + self.fields['add_asns'].label = _('ASNs') + class LocationForm(TenancyForm, NestedGroupModelForm): site = DynamicModelChoiceField( diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index 1aac72875..3d7426013 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -160,7 +160,7 @@ class SiteTestCase(ViewTestCases.PrimaryObjectViewTestCase): 'group': groups[1].pk, 'tenant': None, 'facility': 'Facility X', - 'asns': [asns[6].pk, asns[7].pk], + 'add_asns': [asns[6].pk, asns[7].pk], 'time_zone': ZoneInfo('UTC'), 'description': 'Site description', 'physical_address': '742 Evergreen Terrace, Springfield, USA', diff --git a/netbox/ipam/forms/model_forms.py b/netbox/ipam/forms/model_forms.py index 1bdcff2d8..ff3cac503 100644 --- a/netbox/ipam/forms/model_forms.py +++ b/netbox/ipam/forms/model_forms.py @@ -21,7 +21,7 @@ from utilities.forms.fields import ( NumericArrayField, NumericRangeArrayField, ) -from utilities.forms.rendering import FieldSet, InlineFields, ObjectAttribute, TabbedGroups +from utilities.forms.rendering import FieldSet, InlineFields, M2MAddRemoveFields, ObjectAttribute, TabbedGroups from utilities.forms.utils import get_field_value from utilities.forms.widgets import DatePicker, HTMXSelect from utilities.templatetags.builtins.filters import bettertitle @@ -152,36 +152,38 @@ class ASNForm(TenancyForm, PrimaryModelForm): label=_('RIR'), quick_add=True ) - sites = DynamicModelMultipleChoiceField( + add_sites = DynamicModelMultipleChoiceField( queryset=Site.objects.all(), - label=_('Sites'), + label=_('Add sites'), + required=False + ) + remove_sites = DynamicModelMultipleChoiceField( + queryset=Site.objects.all(), + label=_('Remove sites'), required=False ) fieldsets = ( - FieldSet('asn', 'rir', 'sites', 'description', 'tags', name=_('ASN')), + FieldSet('asn', 'rir', M2MAddRemoveFields('sites'), 'description', 'tags', name=_('ASN')), FieldSet('tenant_group', 'tenant', name=_('Tenancy')), ) class Meta: model = ASN fields = [ - 'asn', 'rir', 'sites', 'tenant_group', 'tenant', 'description', 'owner', 'comments', 'tags' + 'asn', 'rir', 'tenant_group', 'tenant', 'description', 'owner', 'comments', 'tags' ] widgets = { 'date_added': DatePicker(), } - def __init__(self, data=None, instance=None, *args, **kwargs): - super().__init__(data=data, instance=instance, *args, **kwargs) - - if self.instance and self.instance.pk is not None: - self.fields['sites'].initial = self.instance.sites.all().values_list('id', flat=True) - - def save(self, *args, **kwargs): - instance = super().save(*args, **kwargs) - instance.sites.set(self.cleaned_data['sites']) - return instance + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.instance.pk: + self.fields['remove_sites'].widget.add_query_param('asn_id', self.instance.pk) + else: + self.fields.pop('remove_sites') + self.fields['add_sites'].label = _('Sites') class RoleForm(OrganizationalModelForm): diff --git a/netbox/netbox/forms/model_forms.py b/netbox/netbox/forms/model_forms.py index a500365f9..97215d55a 100644 --- a/netbox/netbox/forms/model_forms.py +++ b/netbox/netbox/forms/model_forms.py @@ -75,7 +75,19 @@ class NetBoxModelForm( self.instance._m2m_values = {} for field in self.instance._meta.local_many_to_many: if field.name in self.cleaned_data: + # Standard M2M field (set-based) self.instance._m2m_values[field.name] = list(self.cleaned_data[field.name]) + elif f'add_{field.name}' in self.cleaned_data or f'remove_{field.name}' in self.cleaned_data: + # Add/remove M2M field pair: compute the effective set + current = set(getattr(self.instance, field.name).values_list('pk', flat=True)) \ + if self.instance.pk else set() + add_values = set( + v.pk for v in self.cleaned_data.get(f'add_{field.name}', []) + ) + remove_values = set( + v.pk for v in self.cleaned_data.get(f'remove_{field.name}', []) + ) + self.instance._m2m_values[field.name] = list((current | add_values) - remove_values) return super()._post_clean() diff --git a/netbox/netbox/views/generic/object_views.py b/netbox/netbox/views/generic/object_views.py index c6630d341..2aaa92830 100644 --- a/netbox/netbox/views/generic/object_views.py +++ b/netbox/netbox/views/generic/object_views.py @@ -299,6 +299,17 @@ class ObjectEditView(GetReturnURLMixin, BaseObjectView): object_created = form.instance.pk is None obj = form.save() + # Process any add/remove M2M field pairs + for field in obj._meta.local_many_to_many: + add_key = f'add_{field.name}' + remove_key = f'remove_{field.name}' + if add_key in form.cleaned_data or remove_key in form.cleaned_data: + m2m_manager = getattr(obj, field.name) + if add_values := form.cleaned_data.get(add_key): + m2m_manager.add(*add_values) + if remove_values := form.cleaned_data.get(remove_key): + m2m_manager.remove(*remove_values) + # Check that the new object conforms with any assigned object-level permissions if not self.queryset.filter(pk=obj.pk).exists(): raise PermissionsViolation() diff --git a/netbox/utilities/forms/rendering.py b/netbox/utilities/forms/rendering.py index 723e911e6..17235e107 100644 --- a/netbox/utilities/forms/rendering.py +++ b/netbox/utilities/forms/rendering.py @@ -5,6 +5,7 @@ from functools import cached_property __all__ = ( 'FieldSet', 'InlineFields', + 'M2MAddRemoveFields', 'ObjectAttribute', 'TabbedGroups', ) @@ -73,6 +74,21 @@ class TabbedGroups: ] +class M2MAddRemoveFields: + """ + Represents an add/remove field pair for a many-to-many relationship. Rather than rendering + a single multi-select pre-populated with all current values (which can crash the browser for + large datasets), this renders two fields: one for adding new relations and one for removing + existing relations. + + Parameters: + name: The name of the M2M field on the model (e.g. 'asns'). The form must define + corresponding 'add_{name}' and 'remove_{name}' fields. + """ + def __init__(self, name): + self.name = name + + class ObjectAttribute: """ Renders the value for a specific attribute on the form's instance. This may be used to diff --git a/netbox/utilities/templatetags/form_helpers.py b/netbox/utilities/templatetags/form_helpers.py index 0a3078ccf..974fcc633 100644 --- a/netbox/utilities/templatetags/form_helpers.py +++ b/netbox/utilities/templatetags/form_helpers.py @@ -1,6 +1,6 @@ from django import template -from utilities.forms.rendering import InlineFields, ObjectAttribute, TabbedGroups +from utilities.forms.rendering import InlineFields, M2MAddRemoveFields, ObjectAttribute, TabbedGroups __all__ = ( 'getfield', @@ -80,6 +80,13 @@ def render_fieldset(form, fieldset): ('tabs', None, tabs) ) + elif type(item) is M2MAddRemoveFields: + for field_name in (f'add_{item.name}', f'remove_{item.name}'): + if field_name in form.fields: + rows.append( + ('field', None, [form[field_name]]) + ) + elif type(item) is ObjectAttribute: value = getattr(form.instance, item.name) label = value._meta.verbose_name if hasattr(value, '_meta') else item.name