VirtualChassis form validation cleanup

netbox/dcim/forms.py

@@ -2277,6 +2277,57 @@ class VirtualChassisForm(BootstrapMixin, forms.ModelForm):
         }
 
 
+class BaseVCMemberFormSet(forms.BaseModelFormSet):
+
+    def clean(self):
+        super(BaseVCMemberFormSet, self).clean()
+
+        # Check for duplicate VC position values
+        vc_position_list = []
+        for form in self.forms:
+            vc_position = form.cleaned_data['vc_position']
+            if vc_position in vc_position_list:
+                error_msg = 'A virtual chassis member already exists in position {}.'.format(vc_position)
+                form.add_error('vc_position', error_msg)
+            vc_position_list.append(vc_position)
+
+
+class DeviceVCMembershipForm(forms.ModelForm):
+
+    class Meta:
+        model = Device
+        fields = ['vc_position', 'vc_priority']
+        labels = {
+            'vc_position': 'Position',
+            'vc_priority': 'Priority',
+        }
+
+    def __init__(self, *args, validate_vc_position=False, **kwargs):
+        super(DeviceVCMembershipForm, self).__init__(*args, **kwargs)
+
+        # Require VC position (only required when the Device is a VirtualChassis member)
+        self.fields['vc_position'].required = True
+
+        # Validation of vc_position is optional. This is only required when adding a new member to an existing
+        # VirtualChassis. Otherwise, vc_position validation is handled by BaseVCMemberFormSet.
+        self.validate_vc_position = validate_vc_position
+
+    def clean_vc_position(self):
+        vc_position = self.cleaned_data['vc_position']
+
+        if self.validate_vc_position:
+            conflicting_members = Device.objects.filter(
+                virtual_chassis=self.instance.virtual_chassis,
+                vc_position=vc_position
+            )
+            if conflicting_members.exists():
+                raise forms.ValidationError(
+                    'A virtual chassis member already exists in position {}.'.format(vc_position)
+                )
+
+        return vc_position
+
+
 class VCMemberSelectForm(BootstrapMixin, ChainedFieldsMixin, forms.Form):
     site = forms.ModelChoiceField(
         queryset=Site.objects.all(),
@@ -2315,27 +2366,4 @@ class VCMemberSelectForm(BootstrapMixin, ChainedFieldsMixin, forms.Form):
         device = self.cleaned_data['device']
         if device.virtual_chassis is not None:
             raise forms.ValidationError("Device {} is already assigned to a virtual chassis.".format(device))
-
-
-class DeviceVCMembershipForm(forms.ModelForm):
-
-    class Meta:
-        model = Device
-        fields = ['vc_position', 'vc_priority']
-        labels = {
-            'vc_position': 'Position',
-            'vc_priority': 'Priority',
-        }
-
-    def __init__(self, *args, **kwargs):
-        super(DeviceVCMembershipForm, self).__init__(*args, **kwargs)
-
-        # Require VC position when assigning a member
-        self.fields['vc_position'].required = True
-
-    def clean_vc_position(self):
-        vc_position = self.cleaned_data['vc_position']
-        if Device.objects.filter(virtual_chassis=self.instance.virtual_chassis, vc_position=vc_position).exists():
-            raise forms.ValidationError("A virtual chassis member already exists in this position.")
-
-        return vc_position
+        return device

netbox/dcim/views.py

@@ -857,7 +857,7 @@ class DeviceView(View):
 
         # VirtualChassis members
         if device.virtual_chassis is not None:
-            vc_members = Device.objects.filter(virtual_chassis=device.virtual_chassis)
+            vc_members = Device.objects.filter(virtual_chassis=device.virtual_chassis).order_by('vc_position')
         else:
             vc_members = []
 
@@ -2080,20 +2080,26 @@ class VirtualChassisCreateView(PermissionRequiredMixin, View):
         # Get the list of devices being added to a VirtualChassis
         pk_form = forms.DeviceSelectionForm(request.POST)
         pk_form.full_clean()
-        device_list = pk_form.cleaned_data.get('pk')
+        device_queryset = Device.objects.filter(
+            pk__in=pk_form.cleaned_data.get('pk')
+        ).select_related('rack').order_by('vc_position')
 
-        if not device_list:
+        if not device_queryset:
             messages.warning(request, "No devices were selected.")
             return redirect('dcim:device_list')
 
-        # TODO: Error if any of the devices already belong to a VC
-
-        VCMemberFormSet = modelformset_factory(model=Device, fields=('vc_position', 'vc_priority'), extra=0)
+        VCMemberFormSet = modelformset_factory(
+            model=Device,
+            formset=forms.BaseVCMemberFormSet,
+            form=forms.DeviceVCMembershipForm,
+            extra=0
+        )
 
         if '_create' in request.POST:
 
             vc_form = forms.VirtualChassisForm(request.POST)
-            formset = VCMemberFormSet(request.POST)
+            vc_form.fields['master'].queryset = device_queryset
+            formset = VCMemberFormSet(request.POST, queryset=device_queryset)
 
             if vc_form.is_valid() and formset.is_valid():
 
@@ -2111,8 +2117,8 @@ class VirtualChassisCreateView(PermissionRequiredMixin, View):
         else:
 
             vc_form = forms.VirtualChassisForm()
-            vc_form.fields['master'].queryset = Device.objects.filter(pk__in=device_list)
-            formset = VCMemberFormSet(queryset=Device.objects.filter(pk__in=device_list))
+            vc_form.fields['master'].queryset = device_queryset
+            formset = VCMemberFormSet(queryset=device_queryset)
 
         return render(request, 'dcim/virtualchassis_edit.html', {
             'pk_form': pk_form,
@@ -2128,11 +2134,17 @@ class VirtualChassisEditView(PermissionRequiredMixin, GetReturnURLMixin, View):
     def get(self, request, pk):
 
         virtual_chassis = get_object_or_404(VirtualChassis, pk=pk)
-        VCMemberFormSet = modelformset_factory(model=Device, fields=('vc_position', 'vc_priority'), extra=0)
+        VCMemberFormSet = modelformset_factory(
+            model=Device,
+            form=forms.DeviceVCMembershipForm,
+            formset=forms.BaseVCMemberFormSet,
+            extra=0
+        )
+        members_queryset = virtual_chassis.members.select_related('rack').order_by('vc_position')
 
         vc_form = forms.VirtualChassisForm(instance=virtual_chassis)
-        vc_form.fields['master'].queryset = virtual_chassis.members.all()
-        formset = VCMemberFormSet(queryset=virtual_chassis.members.all())
+        vc_form.fields['master'].queryset = members_queryset
+        formset = VCMemberFormSet(queryset=members_queryset)
 
         return render(request, 'dcim/virtualchassis_edit.html', {
             'vc_form': vc_form,
@@ -2143,11 +2155,17 @@ class VirtualChassisEditView(PermissionRequiredMixin, GetReturnURLMixin, View):
     def post(self, request, pk):
 
         virtual_chassis = get_object_or_404(VirtualChassis, pk=pk)
-        VCMemberFormSet = modelformset_factory(model=Device, fields=('vc_position', 'vc_priority'), extra=0)
+        VCMemberFormSet = modelformset_factory(
+            model=Device,
+            form=forms.DeviceVCMembershipForm,
+            formset=forms.BaseVCMemberFormSet,
+            extra=0
+        )
+        members_queryset = virtual_chassis.members.select_related('rack').order_by('vc_position')
 
         vc_form = forms.VirtualChassisForm(request.POST, instance=virtual_chassis)
-        vc_form.fields['master'].queryset = virtual_chassis.members.all()
-        formset = VCMemberFormSet(request.POST, queryset=virtual_chassis.members.all())
+        vc_form.fields['master'].queryset = members_queryset
+        formset = VCMemberFormSet(request.POST, queryset=members_queryset)
 
         if vc_form.is_valid() and formset.is_valid():
 
@@ -2207,7 +2225,7 @@ class VirtualChassisAddMemberView(PermissionRequiredMixin, GetReturnURLMixin, Vi
             device = member_select_form.cleaned_data['device']
             device.virtual_chassis = virtual_chassis
             data = {k: request.POST[k] for k in ['vc_position', 'vc_priority']}
-            membership_form = forms.DeviceVCMembershipForm(data, instance=device)
+            membership_form = forms.DeviceVCMembershipForm(data, validate_vc_position=True, instance=device)
 
             if membership_form.is_valid():