Browse Source

Improved validation and workflow

Jeremy Stretch 7 years ago
parent
commit
70625a5cb0

+ 37 - 8
netbox/dcim/forms.py

@@ -1667,7 +1667,9 @@ class InterfaceForm(BootstrapMixin, forms.ModelForm):
             'mode': '802.1Q Mode',
         }
         help_texts = {
-            'mode': "Nullifying the mode will clear any associated VLANs."
+            'mode': "Access: One untagged VLAN<br />"
+                    "Tagged: One untagged VLAN and/or one or more tagged VLANs<br />"
+                    "Tagged All: Implies all VLANs are available (w/optional untagged VLAN)"
         }
 
     def __init__(self, *args, **kwargs):
@@ -1693,13 +1695,20 @@ class InterfaceForm(BootstrapMixin, forms.ModelForm):
         untagged_vlan = self.cleaned_data['untagged_vlan']
         tagged_vlans = self.cleaned_data['tagged_vlans']
 
+        # A VLAN cannot be both tagged and untagged
+        if untagged_vlan and untagged_vlan in tagged_vlans:
+            raise forms.ValidationError("VLAN {} cannot be both tagged and untagged.".format(untagged_vlan))
+
+        # 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."
             })
 
-        if untagged_vlan and untagged_vlan in tagged_vlans:
-            raise forms.ValidationError("VLAN {} cannot be both tagged and untagged.".format(untagged_vlan))
+        # 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(
@@ -1720,31 +1729,51 @@ class InterfaceAssignVLANsForm(BootstrapMixin, forms.ModelForm):
 
         super(InterfaceAssignVLANsForm, self).__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)
+
         # Initialize VLAN choices
         device = self.instance.device
         vlan_choices = [
-            ('Global', [(vlan.pk, vlan) for vlan in VLAN.objects.filter(site=None)]),
-            (device.site.name, [(vlan.pk, vlan) for vlan in VLAN.objects.filter(site=device.site, group=None)]),
+            ('Global', [(vlan.pk, vlan) for vlan in VLAN.objects.filter(site=None).exclude(pk__in=assigned_vlans)]),
+            (device.site.name, [(vlan.pk, vlan) for vlan in VLAN.objects.filter(site=device.site, group=None).exclude(pk__in=assigned_vlans)]),
         ]
         for group in VLANGroup.objects.filter(site=device.site):
             vlan_choices.append((
                 '{} / {}'.format(group.site.name, group.name),
-                [(vlan.pk, vlan) for vlan in VLAN.objects.filter(group=group)]
+                [(vlan.pk, vlan) for vlan in VLAN.objects.filter(group=group).exclude(pk__in=assigned_vlans)]
             ))
         self.fields['vlans'].choices = vlan_choices
 
+    def clean(self):
+
+        super(InterfaceAssignVLANsForm, self).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 = self.cleaned_data['vlans'][0]
+            self.instance.untagged_vlan_id = self.cleaned_data['vlans'][0]
 
         return super(InterfaceAssignVLANsForm, self).save(*args, **kwargs)
 
 
-class InterfaceCreateForm(ComponentForm, forms.ModelForm):
+class InterfaceCreateForm(ComponentForm, forms.Form):
     name_pattern = ExpandableNameField(label='Name')
     form_factor = forms.ChoiceField(choices=IFACE_FF_CHOICES)
     enabled = forms.BooleanField(required=False)

+ 5 - 3
netbox/dcim/models.py

@@ -1457,11 +1457,13 @@ class Interface(models.Model):
 
     def save(self, *args, **kwargs):
 
+        # Remove untagged VLAN assignment for non-802.1Q interfaces
         if self.mode is None:
             self.untagged_vlan = None
-            self.tagged_vlans = []
-        elif self.mode is IFACE_MODE_ACCESS:
-            self.tagged_vlans = []
+
+        # Only "tagged" interfaces may have tagged VLANs assigned. ("tagged all" implies all VLANs are assigned.)
+        if self.mode is not IFACE_MODE_TAGGED:
+            self.tagged_vlans.clear()
 
         return super(Interface, self).save(*args, **kwargs)
 

+ 86 - 48
netbox/templates/dcim/interface_edit.html

@@ -16,55 +16,93 @@
             {% render_field form.mode %}
         </div>
     </div>
-    {% with interface=form.instance %}
-        {% if interface.mode %}
-            <div class="panel panel-default">
-                <div class="panel-heading"><strong>802.1Q VLANs</strong></div>
-                <table class="table panel-body">
+    {% if obj.mode %}
+        <div class="panel panel-default" id="vlans_panel">
+            <div class="panel-heading"><strong>802.1Q VLANs</strong></div>
+            <table class="table panel-body">
+                <tr>
+                    <th>VID</th>
+                    <th>Name</th>
+                    <th>Untagged</th>
+                    <th>Tagged</th>
+                </tr>
+                {% if obj.untagged_vlan %}
                     <tr>
-                        <th>VID</th>
-                        <th>Name</th>
-                        <th>Untagged</th>
-                        <th>Tagged</th>
+                        <td>
+                            <a href="{{ obj.untagged_vlan.get_absolute_url }}">{{ obj.untagged_vlan.vid }}</a>
+                        </td>
+                        <td>{{ obj.untagged_vlan.name }}</td>
+                        <td>
+                            <input type="radio" name="untagged_vlan" value="{{ obj.untagged_vlan.pk }}" checked="true" />
+                        </td>
+                        <td>
+                            <input type="checkbox" name="tagged_vlans" value="{{ obj.untagged_vlan.pk }}" />
+                        </td>
                     </tr>
-                    {% if interface.untagged_vlan %}
-                        <tr>
-                            <td>{{ interface.untagged_vlan.vid }}</td>
-                            <td>{{ interface.untagged_vlan.name }}</td>
-                            <td>
-                                <input type="radio" name="untagged_vlan" value="{{ interface.untagged_vlan.pk }}" checked="true" />
-                            </td>
-                            <td>
-                                <input type="checkbox" name="tagged_vlans" value="{{ interface.untagged_vlan.pk }}" />
-                            </td>
-                        </tr>
-                    {% endif %}
-                    {% for vlan in interface.tagged_vlans.all %}
-                        <tr>
-                            <td>{{ vlan.vid }}</td>
-                            <td>{{ vlan.name }}</td>
-                            <td>
-                                <input type="radio" name="untagged_vlan" value="{{ vlan.pk }}" />
-                            </td>
-                            <td>
-                                <input type="checkbox" name="tagged_vlans" value="{{ vlan.pk }}" checked="true" />
-                            </td>
-                        </tr>
-                    {% endfor %}
-                    {% if not interface.untagged_vlan and not interface.tagged_vlans.exists %}
-                        <tr>
-                            <td colspan="4">
-                                <span class="text-muted">No VLANs assigned</span>
-                            </td>
-                        </tr>
-                    {% endif %}
-                </table>
-                <div class="panel-footer text-right">
-                    <a href="{% url 'dcim:interface_assign_vlans' pk=interface.pk %}?return_url={% url 'dcim:interface_edit' pk=interface.pk %}" class="btn btn-primary btn-xs">
-                        <i class="glyphicon glyphicon-plus"></i> Add VLANs
-                    </a>
-                </div>
+                {% endif %}
+                {% for vlan in obj.tagged_vlans.all %}
+                    <tr>
+                        <td>
+                            <a href="{{ vlan.get_absolute_url }}">{{ vlan.vid }}</a>
+                        </td>
+                        <td>{{ vlan.name }}</td>
+                        <td>
+                            <input type="radio" name="untagged_vlan" value="{{ vlan.pk }}" />
+                        </td>
+                        <td>
+                            <input type="checkbox" name="tagged_vlans" value="{{ vlan.pk }}" checked="true" />
+                        </td>
+                    </tr>
+                {% endfor %}
+                {% if not obj.untagged_vlan and not obj.tagged_vlans.exists %}
+                    <tr>
+                        <td colspan="4" class="text-muted text-center">
+                            No VLANs assigned
+                        </td>
+                    </tr>
+                {% else %}
+                    <tr>
+                        <td colspan="2"></td>
+                        <td>
+                            <a href="#" id="clear_untagged_vlan" class="btn btn-warning btn-xs">Clear</a>
+                        </td>
+                        <td>
+                            <a href="#" id="clear_tagged_vlans" class="btn btn-warning btn-xs">Clear All</a>
+                        </td>
+                    </tr>
+                {% endif %}
+            </table>
+            <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>
-        {% endif %}
-    {% endwith %}
+        </div>
+    {% endif %}
+{% endblock %}
+
+{% block buttons %}
+    {% if obj.pk %}
+        <button type="submit" name="_update" class="btn btn-primary">Update</button>
+        <button type="submit" formaction="?return_url={% url 'dcim:interface_edit' pk=obj.pk %}" class="btn btn-primary">Update and Continue Editing</button>
+    {% else %}
+        <button type="submit" name="_create" class="btn btn-primary">Create</button>
+        <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 %}

+ 9 - 7
netbox/templates/utilities/obj_edit.html

@@ -31,13 +31,15 @@
         </div>
         <div class="row">
             <div class="col-md-6 col-md-offset-3 text-right">
-                {% if obj.pk %}
-                    <button type="submit" name="_update" class="btn btn-primary">Update</button>
-                {% else %}
-                    <button type="submit" name="_create" class="btn btn-primary">Create</button>
-                    <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>
+                {% block buttons %}
+                    {% if obj.pk %}
+                        <button type="submit" name="_update" class="btn btn-primary">Update</button>
+                    {% else %}
+                        <button type="submit" name="_create" class="btn btn-primary">Create</button>
+                        <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 %}
             </div>
         </div>
     </form>