Просмотр исходного кода

Use add/remove fields only when assignment count is 100+

Jeremy Stretch 2 дней назад
Родитель
Сommit
d5f37d7a87

+ 12 - 2
netbox/circuits/forms/model_forms.py

@@ -43,6 +43,11 @@ __all__ = (
 
 class ProviderForm(PrimaryModelForm):
     slug = SlugField()
+    asns = DynamicModelMultipleChoiceField(
+        queryset=ASN.objects.all(),
+        label=_('ASNs'),
+        required=False
+    )
     add_asns = DynamicModelMultipleChoiceField(
         queryset=ASN.objects.all(),
         label=_('Add ASNs'),
@@ -66,11 +71,16 @@ class ProviderForm(PrimaryModelForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        if self.instance.pk:
+        if self.instance.pk and self.instance.asns.count() >= M2MAddRemoveFields.THRESHOLD:
+            # Add/remove mode for large M2M sets
+            self.fields.pop('asns')
             self.fields['remove_asns'].widget.add_query_param('provider_id', self.instance.pk)
         else:
+            # Simple mode for new objects or small M2M sets
+            self.fields.pop('add_asns')
             self.fields.pop('remove_asns')
-            self.fields['add_asns'].label = _('ASNs')
+            if self.instance.pk:
+                self.initial['asns'] = list(self.instance.asns.values_list('pk', flat=True))
 
 
 class ProviderAccountForm(PrimaryModelForm):

+ 1 - 1
netbox/circuits/tests/test_views.py

@@ -42,7 +42,7 @@ class ProviderTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         cls.form_data = {
             'name': 'Provider X',
             'slug': 'provider-x',
-            'add_asns': [asns[6].pk, asns[7].pk],
+            'asns': [asns[6].pk, asns[7].pk],
             'comments': 'Another provider',
             'tags': [t.pk for t in tags],
         }

+ 12 - 2
netbox/dcim/forms/model_forms.py

@@ -137,6 +137,11 @@ class SiteForm(TenancyForm, PrimaryModelForm):
         required=False,
         quick_add=True
     )
+    asns = DynamicModelMultipleChoiceField(
+        queryset=ASN.objects.all(),
+        label=_('ASNs'),
+        required=False
+    )
     add_asns = DynamicModelMultipleChoiceField(
         queryset=ASN.objects.all(),
         label=_('Add ASNs'),
@@ -185,11 +190,16 @@ class SiteForm(TenancyForm, PrimaryModelForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        if self.instance.pk:
+        if self.instance.pk and self.instance.asns.count() >= M2MAddRemoveFields.THRESHOLD:
+            # Add/remove mode for large M2M sets
+            self.fields.pop('asns')
             self.fields['remove_asns'].widget.add_query_param('site_id', self.instance.pk)
         else:
+            # Simple mode for new objects or small M2M sets
+            self.fields.pop('add_asns')
             self.fields.pop('remove_asns')
-            self.fields['add_asns'].label = _('ASNs')
+            if self.instance.pk:
+                self.initial['asns'] = list(self.instance.asns.values_list('pk', flat=True))
 
 
 class LocationForm(TenancyForm, NestedGroupModelForm):

+ 1 - 1
netbox/dcim/tests/test_views.py

@@ -160,7 +160,7 @@ class SiteTestCase(ViewTestCases.PrimaryObjectViewTestCase):
             'group': groups[1].pk,
             'tenant': None,
             'facility': 'Facility X',
-            'add_asns': [asns[6].pk, asns[7].pk],
+            'asns': [asns[6].pk, asns[7].pk],
             'time_zone': ZoneInfo('UTC'),
             'description': 'Site description',
             'physical_address': '742 Evergreen Terrace, Springfield, USA',

+ 12 - 2
netbox/ipam/forms/model_forms.py

@@ -152,6 +152,11 @@ class ASNForm(TenancyForm, PrimaryModelForm):
         label=_('RIR'),
         quick_add=True
     )
+    sites = DynamicModelMultipleChoiceField(
+        queryset=Site.objects.all(),
+        label=_('Sites'),
+        required=False
+    )
     add_sites = DynamicModelMultipleChoiceField(
         queryset=Site.objects.all(),
         label=_('Add sites'),
@@ -179,11 +184,16 @@ class ASNForm(TenancyForm, PrimaryModelForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        if self.instance.pk:
+        if self.instance.pk and self.instance.sites.count() >= M2MAddRemoveFields.THRESHOLD:
+            # Add/remove mode for large M2M sets
+            self.fields.pop('sites')
             self.fields['remove_sites'].widget.add_query_param('asn_id', self.instance.pk)
         else:
+            # Simple mode for new objects or small M2M sets
+            self.fields.pop('add_sites')
             self.fields.pop('remove_sites')
-            self.fields['add_sites'].label = _('Sites')
+            if self.instance.pk:
+                self.initial['sites'] = list(self.instance.sites.values_list('pk', flat=True))
 
 
 class RoleForm(OrganizationalModelForm):

+ 33 - 10
netbox/netbox/forms/model_forms.py

@@ -2,6 +2,8 @@ import json
 
 from django import forms
 from django.contrib.contenttypes.models import ContentType
+from django.db import models
+from django.db.models.fields.related import ManyToManyRel
 
 from extras.choices import *
 from utilities.forms.fields import CommentField, SlugField
@@ -71,26 +73,47 @@ 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:
-                # 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)) \
+        for field in self.instance._meta.get_fields():
+            # Determine the accessor name for this M2M relationship
+            if isinstance(field, models.ManyToManyField):
+                name = field.name
+            elif isinstance(field, ManyToManyRel):
+                name = field.get_accessor_name()
+            else:
+                continue
+
+            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_{field.name}', [])
+                    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_{field.name}', [])
+                    v.pk for v in self.cleaned_data.get(f'remove_{name}', [])
                 )
-                self.instance._m2m_values[field.name] = list((current | add_values) - remove_values)
+                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):
     """

+ 0 - 11
netbox/netbox/views/generic/object_views.py

@@ -299,17 +299,6 @@ 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()

+ 12 - 6
netbox/utilities/forms/rendering.py

@@ -76,15 +76,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.
+    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'). The form must define
-              corresponding 'add_{name}' and 'remove_{name}' fields.
+        name: The name of the M2M field on the model (e.g. 'asns').
     """
+    THRESHOLD = 100
+
     def __init__(self, name):
         self.name = name
 

+ 12 - 5
netbox/utilities/templatetags/form_helpers.py

@@ -81,11 +81,18 @@ def render_fieldset(form, fieldset):
             )
 
         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]])
-                    )
+            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)