diff --git a/netbox/circuits/forms/model_forms.py b/netbox/circuits/forms/model_forms.py index 5a48455d8..446ec0eed 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 @@ -48,17 +48,42 @@ class ProviderForm(PrimaryModelForm): label=_('ASNs'), required=False ) + add_asns = DynamicModelMultipleChoiceField( + queryset=ASN.objects.all(), + 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 and (count := self.instance.asns.count()) >= M2MAddRemoveFields.THRESHOLD: + # Add/remove mode for large M2M sets + self.fields.pop('asns') + self.fields['add_asns'].widget.add_query_param('provider_id__n', self.instance.pk) + self.fields['remove_asns'].widget.add_query_param('provider_id', self.instance.pk) + self.fields['remove_asns'].help_text = _("{count} ASNs currently assigned").format(count=count) + else: + # Simple mode for new objects or small M2M sets + self.fields.pop('add_asns') + self.fields.pop('remove_asns') + if self.instance.pk: + self.initial['asns'] = list(self.instance.asns.values_list('pk', flat=True)) + class ProviderAccountForm(PrimaryModelForm): provider = DynamicModelChoiceField( diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py index 741315323..cb5809d4c 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, @@ -142,6 +142,16 @@ class SiteForm(TenancyForm, PrimaryModelForm): label=_('ASNs'), required=False ) + add_asns = DynamicModelMultipleChoiceField( + queryset=ASN.objects.all(), + label=_('Add ASNs'), + required=False + ) + remove_asns = DynamicModelMultipleChoiceField( + queryset=ASN.objects.all(), + label=_('Remove ASNs'), + required=False + ) slug = SlugField() time_zone = TimeZoneFormField( label=_('Time zone'), @@ -151,7 +161,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 +172,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 +188,21 @@ class SiteForm(TenancyForm, PrimaryModelForm): ), } + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.instance.pk and (count := self.instance.asns.count()) >= M2MAddRemoveFields.THRESHOLD: + # Add/remove mode for large M2M sets + self.fields.pop('asns') + self.fields['add_asns'].widget.add_query_param('site_id__n', self.instance.pk) + self.fields['remove_asns'].widget.add_query_param('site_id', self.instance.pk) + self.fields['remove_asns'].help_text = _("{count} ASNs currently assigned").format(count=count) + else: + # Simple mode for new objects or small M2M sets + self.fields.pop('add_asns') + self.fields.pop('remove_asns') + if self.instance.pk: + self.initial['asns'] = list(self.instance.asns.values_list('pk', flat=True)) + class LocationForm(TenancyForm, NestedGroupModelForm): site = DynamicModelChoiceField( diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py index 118c347fd..2f01525fc 100644 --- a/netbox/dcim/tests/test_forms.py +++ b/netbox/dcim/tests/test_forms.py @@ -10,7 +10,8 @@ from dcim.choices import ( ) from dcim.forms import * from dcim.models import * -from ipam.models import VLAN +from ipam.models import ASN, RIR, VLAN +from utilities.forms.rendering import M2MAddRemoveFields from utilities.testing import create_test_device from virtualization.models import Cluster, ClusterGroup, ClusterType @@ -417,3 +418,111 @@ class InterfaceTestCase(TestCase): self.assertNotIn('untagged_vlan', form.cleaned_data.keys()) self.assertNotIn('tagged_vlans', form.cleaned_data.keys()) self.assertNotIn('qinq_svlan', form.cleaned_data.keys()) + + +class SiteFormTestCase(TestCase): + """ + Tests for M2MAddRemoveFields using Site ASN assignments as the test case. + Covers both simple mode (single multi-select field) and add/remove mode (dual fields). + """ + + @classmethod + def setUpTestData(cls): + cls.rir = RIR.objects.create(name='RIR 1', slug='rir-1') + # Create 110 ASNs: 100 to pre-assign (triggering add/remove mode) plus 10 extras + ASN.objects.bulk_create([ASN(asn=i, rir=cls.rir) for i in range(1, 111)]) + cls.asns = list(ASN.objects.order_by('asn')) + + def _site_data(self, **kwargs): + data = {'name': 'Test Site', 'slug': 'test-site', 'status': 'active'} + data.update(kwargs) + return data + + def test_new_site_uses_simple_mode(self): + """A form for a new site uses the single 'asns' field (simple mode).""" + form = SiteForm(data=self._site_data()) + self.assertIn('asns', form.fields) + self.assertNotIn('add_asns', form.fields) + self.assertNotIn('remove_asns', form.fields) + + def test_existing_site_below_threshold_uses_simple_mode(self): + """A form for an existing site with fewer than THRESHOLD ASNs uses simple mode.""" + site = Site.objects.create(name='Site 1', slug='site-1') + site.asns.set(self.asns[:5]) + form = SiteForm(instance=site) + self.assertIn('asns', form.fields) + self.assertNotIn('add_asns', form.fields) + self.assertNotIn('remove_asns', form.fields) + + def test_existing_site_at_threshold_uses_add_remove_mode(self): + """A form for an existing site with THRESHOLD or more ASNs uses add/remove mode.""" + site = Site.objects.create(name='Site 2', slug='site-2') + site.asns.set(self.asns[:M2MAddRemoveFields.THRESHOLD]) + form = SiteForm(instance=site) + self.assertNotIn('asns', form.fields) + self.assertIn('add_asns', form.fields) + self.assertIn('remove_asns', form.fields) + + def test_simple_mode_assigns_asns_on_create(self): + """Saving a new site via simple mode assigns the selected ASNs.""" + asn_pks = [asn.pk for asn in self.asns[:3]] + form = SiteForm(data=self._site_data(asns=asn_pks)) + self.assertTrue(form.is_valid(), form.errors) + site = form.save() + self.assertEqual(set(site.asns.values_list('pk', flat=True)), set(asn_pks)) + + def test_simple_mode_replaces_asns_on_edit(self): + """Saving an existing site via simple mode replaces the current ASN assignments.""" + site = Site.objects.create(name='Site 3', slug='site-3') + site.asns.set(self.asns[:3]) + new_asn_pks = [asn.pk for asn in self.asns[3:6]] + form = SiteForm( + data=self._site_data(name='Site 3', slug='site-3', asns=new_asn_pks), + instance=site + ) + self.assertTrue(form.is_valid(), form.errors) + site = form.save() + self.assertEqual(set(site.asns.values_list('pk', flat=True)), set(new_asn_pks)) + + def test_add_remove_mode_adds_asns(self): + """In add/remove mode, specifying 'add_asns' appends to current assignments.""" + site = Site.objects.create(name='Site 4', slug='site-4') + site.asns.set(self.asns[:M2MAddRemoveFields.THRESHOLD]) + new_asn_pks = [asn.pk for asn in self.asns[M2MAddRemoveFields.THRESHOLD:]] + form = SiteForm( + data=self._site_data(name='Site 4', slug='site-4', add_asns=new_asn_pks), + instance=site + ) + self.assertTrue(form.is_valid(), form.errors) + site = form.save() + self.assertEqual(site.asns.count(), len(self.asns)) + + def test_add_remove_mode_removes_asns(self): + """In add/remove mode, specifying 'remove_asns' drops those assignments.""" + site = Site.objects.create(name='Site 5', slug='site-5') + site.asns.set(self.asns[:M2MAddRemoveFields.THRESHOLD]) + remove_pks = [asn.pk for asn in self.asns[:5]] + form = SiteForm( + data=self._site_data(name='Site 5', slug='site-5', remove_asns=remove_pks), + instance=site + ) + self.assertTrue(form.is_valid(), form.errors) + site = form.save() + self.assertEqual(site.asns.count(), M2MAddRemoveFields.THRESHOLD - 5) + self.assertFalse(site.asns.filter(pk__in=remove_pks).exists()) + + def test_add_remove_mode_simultaneous_add_and_remove(self): + """In add/remove mode, add and remove operations are applied together.""" + site = Site.objects.create(name='Site 6', slug='site-6') + site.asns.set(self.asns[:M2MAddRemoveFields.THRESHOLD]) + add_pks = [asn.pk for asn in self.asns[M2MAddRemoveFields.THRESHOLD:M2MAddRemoveFields.THRESHOLD + 3]] + remove_pks = [asn.pk for asn in self.asns[:3]] + form = SiteForm( + data=self._site_data(name='Site 6', slug='site-6', add_asns=add_pks, remove_asns=remove_pks), + instance=site + ) + self.assertTrue(form.is_valid(), form.errors) + site = form.save() + self.assertEqual(site.asns.count(), M2MAddRemoveFields.THRESHOLD) + self.assertTrue(site.asns.filter(pk__in=add_pks).count() == 3) + self.assertFalse(site.asns.filter(pk__in=remove_pks).exists()) diff --git a/netbox/netbox/forms/model_forms.py b/netbox/netbox/forms/model_forms.py index a500365f9..cfca17255 100644 --- a/netbox/netbox/forms/model_forms.py +++ b/netbox/netbox/forms/model_forms.py @@ -2,6 +2,7 @@ import json from django import forms from django.contrib.contenttypes.models import ContentType +from django.db.models.fields.related import ManyToManyRel from extras.choices import * from utilities.forms.fields import CommentField, SlugField @@ -71,14 +72,49 @@ class NetBoxModelForm( def _post_clean(self): """ Override BaseModelForm's _post_clean() to store many-to-many field values on the model instance. + Handles both forward and reverse M2M relationships, and supports both simple (single field) + and add/remove (dual field) modes. """ self.instance._m2m_values = {} - for field in self.instance._meta.local_many_to_many: - if field.name in self.cleaned_data: - self.instance._m2m_values[field.name] = list(self.cleaned_data[field.name]) + + # Collect names to process: local M2M fields (includes TaggableManager from django-taggit) + # plus reverse M2M relations (ManyToManyRel). + names = [field.name for field in self.instance._meta.local_many_to_many] + names += [ + field.get_accessor_name() + for field in self.instance._meta.get_fields() + if isinstance(field, ManyToManyRel) + ] + + for name in names: + if name in self.cleaned_data: + # Simple mode: single multi-select field + self.instance._m2m_values[name] = list(self.cleaned_data[name]) + elif f'add_{name}' in self.cleaned_data or f'remove_{name}' in self.cleaned_data: + # Add/remove mode: compute the effective set + current = set(getattr(self.instance, 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_{name}', []) + ) + remove_values = set( + v.pk for v in self.cleaned_data.get(f'remove_{name}', []) + ) + self.instance._m2m_values[name] = list((current | add_values) - remove_values) return super()._post_clean() + def _save_m2m(self): + """ + Save many-to-many field values that were computed in _post_clean(). This handles M2M fields + not included in Meta.fields (e.g. those managed via M2MAddRemoveFields). + """ + super()._save_m2m() + meta_fields = self._meta.fields + for field_name, values in self.instance._m2m_values.items(): + if not meta_fields or field_name not in meta_fields: + getattr(self.instance, field_name).set(values) + class PrimaryModelForm(OwnerMixin, NetBoxModelForm): """ diff --git a/netbox/utilities/forms/rendering.py b/netbox/utilities/forms/rendering.py index 723e911e6..c7bb324d8 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,27 @@ class TabbedGroups: ] +class M2MAddRemoveFields: + """ + Represents a many-to-many relationship field on a form. It supports two rendering modes: + + 1. Simple mode: A single multi-select field pre-populated with current values. This is used + for new objects or existing objects with fewer than THRESHOLD current assignments. + 2. Add/remove mode: Two separate fields for adding and removing relations. This avoids + crashing the browser when an object has a very large number of current assignments. + + The form must define three fields: '{name}', 'add_{name}', and 'remove_{name}'. The form's + __init__() method determines the mode and removes the unused fields. + + Parameters: + name: The name of the M2M field on the model (e.g. 'asns'). + """ + THRESHOLD = 100 + + 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..156bd72c2 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,20 @@ def render_fieldset(form, fieldset): ('tabs', None, tabs) ) + elif type(item) is M2MAddRemoveFields: + if item.name in form.fields: + # Simple mode: render a single multi-select field + rows.append( + ('field', None, [form[item.name]]) + ) + else: + # Add/remove mode: render separate add and remove fields + 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