Merge pull request #21778 from netbox-community/21763-m2m-form-fields

Fixes #21763: Replace M2M selection field with separate add/remove fields

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(

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(

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())

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:
+        # Collect names to process: local M2M fields (includes TaggableManager from django-taggit)
-                self.instance._m2m_values[field.name] = list(self.cleaned_data[field.name])
+        # 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):
     """

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

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