Merge pull request #3925 from hSaria/3840-limit-vlan-choices

Fixes #3840: Only show valid interface VLAN choices

netbox/dcim/forms.py

@@ -2832,7 +2832,10 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm):
         widget=APISelect(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tagged_vlans = DynamicModelMultipleChoiceField(
@@ -2842,7 +2845,10 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm):
         widget=APISelectMultiple(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tags = TagField(
@@ -2871,18 +2877,20 @@ class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        # Limit LAG choices to interfaces belonging to this device (or VC master)
         if self.is_bound:
             device = Device.objects.get(pk=self.data['device'])
-            self.fields['lag'].queryset = Interface.objects.filter(
-                device__in=[device, device.get_vc_master()],
-                type=InterfaceTypeChoices.TYPE_LAG
-            )
         else:
-            self.fields['lag'].queryset = Interface.objects.filter(
-                device__in=[self.instance.device, self.instance.device.get_vc_master()],
-                type=InterfaceTypeChoices.TYPE_LAG
-            )
+            device = self.instance.device
+
+        # Limit LAG choices to interfaces belonging to this device (or VC master)
+        self.fields['lag'].queryset = Interface.objects.filter(
+            device__in=[device, device.get_vc_master()],
+            type=InterfaceTypeChoices.TYPE_LAG
+        )
+
+        # Add current site to VLANs query params
+        self.fields['untagged_vlan'].widget.add_additional_query_param('site_id', device.site.pk)
+        self.fields['tagged_vlans'].widget.add_additional_query_param('site_id', device.site.pk)
 
 
 class InterfaceCreateForm(BootstrapMixin, InterfaceCommonForm, forms.Form):
@@ -2942,7 +2950,10 @@ class InterfaceCreateForm(BootstrapMixin, InterfaceCommonForm, forms.Form):
         widget=APISelect(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tagged_vlans = DynamicModelMultipleChoiceField(
@@ -2951,7 +2962,10 @@ class InterfaceCreateForm(BootstrapMixin, InterfaceCommonForm, forms.Form):
         widget=APISelectMultiple(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
 
@@ -2967,6 +2981,10 @@ class InterfaceCreateForm(BootstrapMixin, InterfaceCommonForm, forms.Form):
             type=InterfaceTypeChoices.TYPE_LAG
         )
 
+        # Add current site to VLANs query params
+        self.fields['untagged_vlan'].widget.add_additional_query_param('site_id', device.site.pk)
+        self.fields['tagged_vlans'].widget.add_additional_query_param('site_id', device.site.pk)
+
 
 class InterfaceCSVForm(forms.ModelForm):
     device = FlexibleModelChoiceField(
@@ -3090,7 +3108,10 @@ class InterfaceBulkEditForm(BootstrapMixin, AddRemoveTagsForm, BulkEditForm):
         widget=APISelect(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tagged_vlans = DynamicModelMultipleChoiceField(
@@ -3099,7 +3120,10 @@ class InterfaceBulkEditForm(BootstrapMixin, AddRemoveTagsForm, BulkEditForm):
         widget=APISelectMultiple(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
 
@@ -3118,6 +3142,10 @@ class InterfaceBulkEditForm(BootstrapMixin, AddRemoveTagsForm, BulkEditForm):
                 device__in=[device, device.get_vc_master()],
                 type=InterfaceTypeChoices.TYPE_LAG
             )
+
+            # Add current site to VLANs query params
+            self.fields['untagged_vlan'].widget.add_additional_query_param('site_id', device.site.pk)
+            self.fields['tagged_vlans'].widget.add_additional_query_param('site_id', device.site.pk)
         else:
             self.fields['lag'].choices = ()
             self.fields['lag'].widget.attrs['disabled'] = True

netbox/project-static/js/forms.js

@@ -190,15 +190,18 @@ $(document).ready(function() {
                 $.each(element.attributes, function(index, attr){
                     if (attr.name.includes("data-additional-query-param-")){
                         var param_name = attr.name.split("data-additional-query-param-")[1];
-                        if (param_name in parameters) {
-                            if (Array.isArray(parameters[param_name])) {
-                                parameters[param_name].push(attr.value)
+
+                        $.each($.parseJSON(attr.value), function(index, value) {
+                            if (param_name in parameters) {
+                                if (Array.isArray(parameters[param_name])) {
+                                    parameters[param_name].push(value);
+                                } else {
+                                    parameters[param_name] = [parameters[param_name], value];
+                                }
                             } else {
-                                parameters[param_name] = [parameters[param_name], attr.value]
+                                parameters[param_name] = value;
                             }
-                        } else {
-                            parameters[param_name] = attr.value;
-                        }
+                        });
                     }
                 });
 

netbox/utilities/forms.py

@@ -309,12 +309,17 @@ class APISelect(SelectWithDisabled):
 
     def add_additional_query_param(self, name, value):
         """
-        Add details for an additional query param in the form of a data-* attribute.
+        Add details for an additional query param in the form of a data-* JSON-encoded list attribute.
 
         :param name: The name of the query param
         :param value: The value of the query param
         """
-        self.attrs['data-additional-query-param-{}'.format(name)] = value
+        key = 'data-additional-query-param-{}'.format(name)
+
+        values = json.loads(self.attrs.get(key, '[]'))
+        values.append(value)
+
+        self.attrs[key] = json.dumps(values)
 
     def add_conditional_query_param(self, condition, value):
         """

netbox/virtualization/forms.py

@@ -659,7 +659,10 @@ class InterfaceForm(BootstrapMixin, forms.ModelForm):
         widget=APISelect(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tagged_vlans = DynamicModelMultipleChoiceField(
@@ -668,7 +671,10 @@ class InterfaceForm(BootstrapMixin, forms.ModelForm):
         widget=APISelectMultiple(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tags = TagField(
@@ -696,35 +702,12 @@ class InterfaceForm(BootstrapMixin, forms.ModelForm):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        # Limit VLan choices to those in: global vlans, global groups, the current site's group, the current site
-        vlan_choices = []
-        global_vlans = VLAN.objects.filter(site=None, group=None)
-        vlan_choices.append(
-            ('Global', [(vlan.pk, vlan) for vlan in global_vlans])
-        )
-        for group in VLANGroup.objects.filter(site=None):
-            global_group_vlans = VLAN.objects.filter(group=group)
-            vlan_choices.append(
-                (group.name, [(vlan.pk, vlan) for vlan in global_group_vlans])
-            )
-
+        # Add current site to VLANs query params
         site = getattr(self.instance.parent, 'site', None)
         if site is not None:
-
-            # Add non-grouped site VLANs
-            site_vlans = VLAN.objects.filter(site=site, group=None)
-            vlan_choices.append((site.name, [(vlan.pk, vlan) for vlan in site_vlans]))
-
-            # Add grouped site VLANs
-            for group in VLANGroup.objects.filter(site=site):
-                site_group_vlans = VLAN.objects.filter(group=group)
-                vlan_choices.append((
-                    '{} / {}'.format(group.site.name, group.name),
-                    [(vlan.pk, vlan) for vlan in site_group_vlans]
-                ))
-
-        self.fields['untagged_vlan'].choices = [(None, '---------')] + vlan_choices
-        self.fields['tagged_vlans'].choices = vlan_choices
+            # Add current site to VLANs query params
+            self.fields['untagged_vlan'].widget.add_additional_query_param('site_id', site.pk)
+            self.fields['tagged_vlans'].widget.add_additional_query_param('site_id', site.pk)
 
     def clean(self):
         super().clean()
@@ -785,7 +768,10 @@ class InterfaceCreateForm(BootstrapMixin, forms.Form):
         widget=APISelect(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tagged_vlans = DynamicModelMultipleChoiceField(
@@ -794,7 +780,10 @@ class InterfaceCreateForm(BootstrapMixin, forms.Form):
         widget=APISelectMultiple(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tags = TagField(
@@ -808,35 +797,11 @@ class InterfaceCreateForm(BootstrapMixin, forms.Form):
             pk=self.initial.get('virtual_machine') or self.data.get('virtual_machine')
         )
 
-        # Limit VLAN choices to those in: global vlans, global groups, the current site's group, the current site
-        vlan_choices = []
-        global_vlans = VLAN.objects.filter(site=None, group=None)
-        vlan_choices.append(
-            ('Global', [(vlan.pk, vlan) for vlan in global_vlans])
-        )
-        for group in VLANGroup.objects.filter(site=None):
-            global_group_vlans = VLAN.objects.filter(group=group)
-            vlan_choices.append(
-                (group.name, [(vlan.pk, vlan) for vlan in global_group_vlans])
-            )
-
         site = getattr(virtual_machine.cluster, 'site', None)
         if site is not None:
-
-            # Add non-grouped site VLANs
-            site_vlans = VLAN.objects.filter(site=site, group=None)
-            vlan_choices.append((site.name, [(vlan.pk, vlan) for vlan in site_vlans]))
-
-            # Add grouped site VLANs
-            for group in VLANGroup.objects.filter(site=site):
-                site_group_vlans = VLAN.objects.filter(group=group)
-                vlan_choices.append((
-                    '{} / {}'.format(group.site.name, group.name),
-                    [(vlan.pk, vlan) for vlan in site_group_vlans]
-                ))
-
-        self.fields['untagged_vlan'].choices = [(None, '---------')] + vlan_choices
-        self.fields['tagged_vlans'].choices = vlan_choices
+            # Add current site to VLANs query params
+            self.fields['untagged_vlan'].widget.add_additional_query_param('site_id', site.pk)
+            self.fields['tagged_vlans'].widget.add_additional_query_param('site_id', site.pk)
 
 
 class InterfaceBulkEditForm(BootstrapMixin, BulkEditForm):
@@ -873,7 +838,10 @@ class InterfaceBulkEditForm(BootstrapMixin, BulkEditForm):
         widget=APISelect(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
     tagged_vlans = DynamicModelMultipleChoiceField(
@@ -882,7 +850,10 @@ class InterfaceBulkEditForm(BootstrapMixin, BulkEditForm):
         widget=APISelectMultiple(
             api_url="/api/ipam/vlans/",
             display_field='display_name',
-            full=True
+            full=True,
+            additional_query_params={
+                'site_id': 'null',
+            },
         )
     )
 
@@ -898,35 +869,11 @@ class InterfaceBulkEditForm(BootstrapMixin, BulkEditForm):
         if 'virtual_machine' in self.initial:
             parent_obj = VirtualMachine.objects.filter(pk=self.initial['virtual_machine']).first()
 
-            # Limit VLAN choices to global VLANs, VLANs in global groups, the current site's group, the current site
-            vlan_choices = []
-            global_vlans = VLAN.objects.filter(site=None, group=None)
-            vlan_choices.append(
-                ('Global', [(vlan.pk, vlan) for vlan in global_vlans])
-            )
-            for group in VLANGroup.objects.filter(site=None):
-                global_group_vlans = VLAN.objects.filter(group=group)
-                vlan_choices.append(
-                    (group.name, [(vlan.pk, vlan) for vlan in global_group_vlans])
-                )
-            if parent_obj.cluster is not None:
-                site = getattr(parent_obj.cluster, 'site', None)
-                if site is not None:
-
-                    # Add non-grouped site VLANs
-                    site_vlans = VLAN.objects.filter(site=site, group=None)
-                    vlan_choices.append((site.name, [(vlan.pk, vlan) for vlan in site_vlans]))
-
-                    # Add grouped site VLANs
-                    for group in VLANGroup.objects.filter(site=site):
-                        site_group_vlans = VLAN.objects.filter(group=group)
-                        vlan_choices.append((
-                            '{} / {}'.format(group.site.name, group.name),
-                            [(vlan.pk, vlan) for vlan in site_group_vlans]
-                        ))
-
-            self.fields['untagged_vlan'].choices = [(None, '---------')] + vlan_choices
-            self.fields['tagged_vlans'].choices = vlan_choices
+            site = getattr(parent_obj.cluster, 'site', None)
+            if site is not None:
+                # Add current site to VLANs query params
+                self.fields['untagged_vlan'].widget.add_additional_query_param('site_id', site.pk)
+                self.fields['tagged_vlans'].widget.add_additional_query_param('site_id', site.pk)
 
 
 #