Add in in-line vlan editing and Bulk vlan editing (#3350)

* Fixes #3341 - Added in-line vlan editing
* Fixes #2160 - Added bulk vlan editing

Inconsequential behaviour changes:

* APISelect can now take "full=True" to return a non-brief set
* Select2 will no group by "group & site, group, site, global" if full=True is set in APISelect

netbox/dcim/forms.py

@@ -56,6 +56,25 @@ def get_device_by_name_or_pk(name):
     return device
 
 
+class InterfaceCommonForm:
+    def clean(self):
+
+        super().clean()
+
+        # Validate VLAN assignments
+        tagged_vlans = self.cleaned_data['tagged_vlans']
+
+        # Untagged interfaces cannot be assigned tagged VLANs
+        if self.cleaned_data['mode'] == IFACE_MODE_ACCESS and tagged_vlans:
+            raise forms.ValidationError({
+                'mode': "An access interface cannot have tagged VLANs assigned."
+            })
+
+        # Remove all tagged VLAN assignments from "tagged all" interfaces
+        elif self.cleaned_data['mode'] == IFACE_MODE_TAGGED_ALL:
+            self.cleaned_data['tagged_vlans'] = []
+
+
 class BulkRenameForm(forms.Form):
     """
     An extendable form to be used for renaming device components in bulk.
@@ -2110,7 +2129,26 @@ class PowerOutletBulkDisconnectForm(ConfirmationForm):
 # Interfaces
 #
 
-class InterfaceForm(BootstrapMixin, forms.ModelForm):
+class InterfaceForm(InterfaceCommonForm, BootstrapMixin, forms.ModelForm):
+    untagged_vlan = forms.ModelChoiceField(
+        queryset=VLAN.objects.all(),
+        required=False,
+        widget=APISelect(
+            api_url="/api/ipam/vlans/",
+            display_field='display_name',
+            full=True
+        )
+    )
+    tagged_vlans = forms.ModelMultipleChoiceField(
+        queryset=VLAN.objects.all(),
+        required=False,
+        widget=APISelectMultiple(
+            api_url="/api/ipam/vlans/",
+            display_field='display_name',
+            full=True
+        )
+    )
+
     tags = TagField(
         required=False
     )
@@ -2149,112 +2187,8 @@ class InterfaceForm(BootstrapMixin, forms.ModelForm):
                 device__in=[self.instance.device, self.instance.device.get_vc_master()], type=IFACE_TYPE_LAG
             )
 
-    def clean(self):
-
-        super().clean()
-
-        # Validate VLAN assignments
-        tagged_vlans = self.cleaned_data['tagged_vlans']
-
-        # Untagged interfaces cannot be assigned tagged VLANs
-        if self.cleaned_data['mode'] == IFACE_MODE_ACCESS and tagged_vlans:
-            raise forms.ValidationError({
-                'mode': "An access interface cannot have tagged VLANs assigned."
-            })
-
-        # Remove all tagged VLAN assignments from "tagged all" interfaces
-        elif self.cleaned_data['mode'] == IFACE_MODE_TAGGED_ALL:
-            self.cleaned_data['tagged_vlans'] = []
-
 
-class InterfaceAssignVLANsForm(BootstrapMixin, forms.ModelForm):
-    vlans = forms.MultipleChoiceField(
-        choices=[],
-        label='VLANs',
-        widget=StaticSelect2Multiple(
-            attrs={
-                'size': 20,
-            }
-        )
-    )
-    tagged = forms.BooleanField(
-        required=False,
-        initial=True
-    )
-
-    class Meta:
-        model = Interface
-        fields = []
-
-    def __init__(self, *args, **kwargs):
-
-        super().__init__(*args, **kwargs)
-
-        if self.instance.mode == IFACE_MODE_ACCESS:
-            self.initial['tagged'] = False
-
-        # Find all VLANs already assigned to the interface for exclusion from the list
-        assigned_vlans = [v.pk for v in self.instance.tagged_vlans.all()]
-        if self.instance.untagged_vlan is not None:
-            assigned_vlans.append(self.instance.untagged_vlan.pk)
-
-        # Compile VLAN choices
-        vlan_choices = []
-
-        # Add non-grouped global VLANs
-        global_vlans = VLAN.objects.filter(site=None, group=None).exclude(pk__in=assigned_vlans)
-        vlan_choices.append(
-            ('Global', [(vlan.pk, vlan) for vlan in global_vlans])
-        )
-
-        # Add grouped global VLANs
-        for group in VLANGroup.objects.filter(site=None):
-            global_group_vlans = VLAN.objects.filter(group=group).exclude(pk__in=assigned_vlans)
-            vlan_choices.append(
-                (group.name, [(vlan.pk, vlan) for vlan in global_group_vlans])
-            )
-
-        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).exclude(pk__in=assigned_vlans)
-            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).exclude(pk__in=assigned_vlans)
-                vlan_choices.append((
-                    '{} / {}'.format(group.site.name, group.name),
-                    [(vlan.pk, vlan) for vlan in site_group_vlans]
-                ))
-
-        self.fields['vlans'].choices = vlan_choices
-
-    def clean(self):
-
-        super().clean()
-
-        # Only untagged VLANs permitted on an access interface
-        if self.instance.mode == IFACE_MODE_ACCESS and len(self.cleaned_data['vlans']) > 1:
-            raise forms.ValidationError("Only one VLAN may be assigned to an access interface.")
-
-        # 'tagged' is required if more than one VLAN is selected
-        if not self.cleaned_data['tagged'] and len(self.cleaned_data['vlans']) > 1:
-            raise forms.ValidationError("Only one untagged VLAN may be selected.")
-
-    def save(self, *args, **kwargs):
-
-        if self.cleaned_data['tagged']:
-            for vlan in self.cleaned_data['vlans']:
-                self.instance.tagged_vlans.add(vlan)
-        else:
-            self.instance.untagged_vlan_id = self.cleaned_data['vlans'][0]
-
-        return super().save(*args, **kwargs)
-
-
-class InterfaceCreateForm(ComponentForm, forms.Form):
+class InterfaceCreateForm(InterfaceCommonForm, ComponentForm, forms.Form):
     name_pattern = ExpandableNameField(
         label='Name'
     )
@@ -2298,6 +2232,24 @@ class InterfaceCreateForm(ComponentForm, forms.Form):
     tags = TagField(
         required=False
     )
+    untagged_vlan = forms.ModelChoiceField(
+        queryset=VLAN.objects.all(),
+        required=False,
+        widget=APISelect(
+            api_url="/api/ipam/vlans/",
+            display_field='display_name',
+            full=True
+        )
+    )
+    tagged_vlans = forms.ModelMultipleChoiceField(
+        queryset=VLAN.objects.all(),
+        required=False,
+        widget=APISelectMultiple(
+            api_url="/api/ipam/vlans/",
+            display_field='display_name',
+            full=True
+        )
+    )
 
     def __init__(self, *args, **kwargs):
 
@@ -2316,7 +2268,7 @@ class InterfaceCreateForm(ComponentForm, forms.Form):
             self.fields['lag'].queryset = Interface.objects.none()
 
 
-class InterfaceBulkEditForm(BootstrapMixin, AddRemoveTagsForm, BulkEditForm):
+class InterfaceBulkEditForm(InterfaceCommonForm, BootstrapMixin, AddRemoveTagsForm, BulkEditForm):
     pk = forms.ModelMultipleChoiceField(
         queryset=Interface.objects.all(),
         widget=forms.MultipleHiddenInput()
@@ -2360,10 +2312,28 @@ class InterfaceBulkEditForm(BootstrapMixin, AddRemoveTagsForm, BulkEditForm):
         required=False,
         widget=StaticSelect2()
     )
+    untagged_vlan = forms.ModelChoiceField(
+        queryset=VLAN.objects.all(),
+        required=False,
+        widget=APISelect(
+            api_url="/api/ipam/vlans/",
+            display_field='display_name',
+            full=True
+        )
+    )
+    tagged_vlans = forms.ModelMultipleChoiceField(
+        queryset=VLAN.objects.all(),
+        required=False,
+        widget=APISelectMultiple(
+            api_url="/api/ipam/vlans/",
+            display_field='display_name',
+            full=True
+        )
+    )
 
     class Meta:
         nullable_fields = [
-            'lag', 'mac_address', 'mtu', 'description', 'mode',
+            'lag', 'mac_address', 'mtu', 'description', 'mode', 'untagged_vlan', 'tagged_vlans'
         ]
 
     def __init__(self, *args, **kwargs):

netbox/dcim/urls.py

@@ -209,7 +209,6 @@ urlpatterns = [
     path(r'interfaces/<int:termination_a_id>/connect/<str:termination_b_type>/', views.CableCreateView.as_view(), name='interface_connect', kwargs={'termination_a_type': Interface}),
     path(r'interfaces/<int:pk>/', views.InterfaceView.as_view(), name='interface'),
     path(r'interfaces/<int:pk>/edit/', views.InterfaceEditView.as_view(), name='interface_edit'),
-    path(r'interfaces/<int:pk>/assign-vlans/', views.InterfaceAssignVLANsView.as_view(), name='interface_assign_vlans'),
     path(r'interfaces/<int:pk>/delete/', views.InterfaceDeleteView.as_view(), name='interface_delete'),
     path(r'interfaces/<int:pk>/changelog/', ObjectChangeLogView.as_view(), name='interface_changelog', kwargs={'model': Interface}),
     path(r'interfaces/<int:pk>/trace/', views.CableTraceView.as_view(), name='interface_trace', kwargs={'model': Interface}),

netbox/dcim/views.py

@@ -1348,12 +1348,6 @@ class InterfaceEditView(PermissionRequiredMixin, ObjectEditView):
     template_name = 'dcim/interface_edit.html'
 
 
-class InterfaceAssignVLANsView(PermissionRequiredMixin, ObjectEditView):
-    permission_required = 'dcim.change_interface'
-    model = Interface
-    model_form = forms.InterfaceAssignVLANsForm
-
-
 class InterfaceDeleteView(PermissionRequiredMixin, ObjectDeleteView):
     permission_required = 'dcim.delete_interface'
     model = Interface

netbox/project-static/js/forms.js

@@ -143,11 +143,13 @@ $(document).ready(function() {
                 // Base query params
                 var parameters = {
                     q: params.term,
-                    brief: 1,
                     limit: 50,
                     offset: offset,
                 };
 
+                // Allow for controlling the brief setting from within APISelect
+                parameters.brief = ( $('#id_untagged_vlan').is('[data-full]') ? undefined : true );
+
                 // filter-for fields from a chain
                 var attr_name = "data-filter-for-" + $(element).attr("name");
                 var form = $(element).closest('form');
@@ -194,18 +196,41 @@ $(document).ready(function() {
 
             processResults: function (data) {
                 var element = this.$element[0];
-                // Clear any disabled options
                 $(element).children('option').attr('disabled', false);
-                var results = $.map(data.results, function (obj) {
-                    obj.text = obj[element.getAttribute('display-field')] || obj.name;
-                    obj.id = obj[element.getAttribute('value-field')] || obj.id;
+                var results = data.results;
 
-                    if(element.getAttribute('disabled-indicator') && obj[element.getAttribute('disabled-indicator')]) {
+                results = results.reduce((results,record) => {
+                    record.text = record[element.getAttribute('display-field')] || record.name;
+                    record.id = record[element.getAttribute('value-field')] || record.id;
+                    if(element.getAttribute('disabled-indicator') && record[element.getAttribute('disabled-indicator')]) {
                         // The disabled-indicator equated to true, so we disable this option
-                        obj.disabled = true;
+                        record.disabled = true;
                     }
-                    return obj;
-                });
+
+                    if( record.group !== undefined && record.group !== null && record.site !== undefined && record.site !== null ) {
+                        results[record.site.name + ":" + record.group.name] = results[record.site.name + ":" + record.group.name] || { text: record.site.name + " / " + record.group.name, children: [] }
+                        results[record.site.name + ":" + record.group.name].children.push(record);
+                    }
+                    else if( record.group !== undefined && record.group !== null ) {
+                        results[record.group.name] = results[record.group.name] || { text: record.group.name, children: [] }
+                        results[record.group.name].children.push(record);
+                    }
+                    else if( record.site !== undefined && record.site !== null ) {
+                        results[record.site.name] = results[record.site.name] || { text: record.site.name, children: [] }
+                        results[record.site.name].children.push(record);
+                    }
+                    else if ( (record.group !== undefined || record.group == null) && (record.site !== undefined || record.site === null) ) {
+                        results['global'] = results['global'] || { text: 'Global', children: [] }
+                        results['global'].children.push(record);
+                    }
+                    else {
+                        results[record.id] = record
+                    }
+
+                    return results;
+                },Object.create(null));
+
+                results = Object.values(results);
 
                 // Handle the null option, but only add it once
                 if (element.getAttribute('data-null-option') && data.previous === null) {
@@ -300,4 +325,34 @@ $(document).ready(function() {
             $('#id_tags').append(option).trigger('change');
         }
     });
+
+    if( $('select#id_mode').length > 0 ) {
+        $('select#id_mode').on('change', function () {
+            if ($(this).val() == '') {
+                $('select#id_untagged_vlan').val();
+                $('select#id_untagged_vlan').trigger('change');
+                $('select#id_tagged_vlans').val([]);
+                $('select#id_tagged_vlans').trigger('change');
+                $('select#id_untagged_vlan').parent().parent().hide();
+                $('select#id_tagged_vlans').parent().parent().hide();
+            }
+            else if ($(this).val() == 100) {
+                $('select#id_tagged_vlans').val([]);
+                $('select#id_tagged_vlans').trigger('change');
+                $('select#id_untagged_vlan').parent().parent().show();
+                $('select#id_tagged_vlans').parent().parent().hide();
+            }
+            else if ($(this).val() == 200) {
+                $('select#id_untagged_vlan').parent().parent().show();
+                $('select#id_tagged_vlans').parent().parent().show();
+            }
+            else if ($(this).val() == 300) {
+                $('select#id_tagged_vlans').val([]);
+                $('select#id_tagged_vlans').trigger('change');
+                $('select#id_untagged_vlan').parent().parent().show();
+                $('select#id_tagged_vlans').parent().parent().hide();
+            }
+        });
+        $('select#id_mode').trigger('change');
+    }
 });

netbox/templates/dcim/interface_edit.html

@@ -14,6 +14,8 @@
             {% render_field form.mgmt_only %}
             {% render_field form.description %}
             {% render_field form.mode %}
+            {% render_field form.untagged_vlan %}
+            {% render_field form.tagged_vlans %}
         </div>
     </div>
     <div class="panel panel-default">
@@ -22,21 +24,6 @@
             {% render_field form.tags %}
         </div>
     </div>
-    <div class="panel panel-default" id="vlans_panel">
-        <div class="panel-heading"><strong>802.1Q VLANs</strong></div>
-        {% if obj.mode %}
-            {% include 'dcim/inc/interface_vlans_table.html' %}
-            <div class="panel-footer text-right">
-                <a href="{% url 'dcim:interface_assign_vlans' pk=obj.pk %}?return_url={% url 'dcim:interface_edit' pk=obj.pk %}" class="btn btn-primary btn-xs{% if form.instance.mode == 100 and form.instance.untagged_vlan %} disabled{% endif %}">
-                    <i class="glyphicon glyphicon-plus"></i> Add VLANs
-                </a>
-            </div>
-        {% else %}
-            <div class="panel-body text-center text-muted">
-                <p>802.1Q mode not set</p>
-            </div>
-        {% endif %}
-    </div>
 {% endblock %}
 
 {% block buttons %}
@@ -48,19 +35,4 @@
         <button type="submit" name="_addanother" class="btn btn-primary">Create and Add Another</button>
     {% endif %}
     <a href="{{ return_url }}" class="btn btn-default">Cancel</a>
-{% endblock %}
-
-{% block javascript %}
-    <script type="text/javascript">
-        $(document).ready(function() {
-            $('#clear_untagged_vlan').click(function () {
-                $('input[name="untagged_vlan"]').prop("checked", false);
-                return false;
-            });
-            $('#clear_tagged_vlans').click(function () {
-                $('input[name="tagged_vlans"]').prop("checked", false);
-                return false;
-            });
-        });
-    </script>
-{% endblock %}
+{% endblock %}

netbox/utilities/forms.py

@@ -298,6 +298,7 @@ class APISelect(SelectWithDisabled):
         conditional_query_params=None,
         additional_query_params=None,
         null_option=False,
+        full=False,
         *args,
         **kwargs
     ):
@@ -306,6 +307,8 @@ class APISelect(SelectWithDisabled):
 
         self.attrs['class'] = 'netbox-select2-api'
         self.attrs['data-url'] = '/{}{}'.format(settings.BASE_PATH, api_url.lstrip('/'))  # Inject BASE_PATH
+        if full:
+            self.attrs['data-full'] = full
         if display_field:
             self.attrs['display-field'] = display_field
         if value_field:

netbox/utilities/views.py

@@ -7,6 +7,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ValidationError
 from django.db import transaction, IntegrityError
 from django.db.models import Count, ProtectedError
+from django.db.models.query import QuerySet
 from django.forms import CharField, Form, ModelMultipleChoiceField, MultipleHiddenInput, Textarea
 from django.http import HttpResponse, HttpResponseServerError
 from django.shortcuts import get_object_or_404, redirect, render
@@ -530,9 +531,13 @@ class BulkEditView(GetReturnURLMixin, View):
 
                             # Update standard fields. If a field is listed in _nullify, delete its value.
                             for name in standard_fields:
-                                if name in form.nullable_fields and name in nullified_fields:
+                                if name in form.nullable_fields and name in nullified_fields and isinstance(form.cleaned_data[name], QuerySet):
+                                    getattr(obj, name).set([])
+                                elif name in form.nullable_fields and name in nullified_fields:
                                     setattr(obj, name, '' if isinstance(form.fields[name], CharField) else None)
-                                elif form.cleaned_data[name] not in (None, ''):
+                                elif isinstance(form.cleaned_data[name], QuerySet) and form.cleaned_data[name]:
+                                    getattr(obj, name).set(form.cleaned_data[name])
+                                elif form.cleaned_data[name] not in (None, '') and not isinstance(form.cleaned_data[name], QuerySet):
                                     setattr(obj, name, form.cleaned_data[name])
                             obj.full_clean()
                             obj.save()