Additional validation cleanup

netbox/dcim/forms.py

@@ -2310,6 +2310,11 @@ class VCMemberSelectForm(BootstrapMixin, ChainedFieldsMixin, forms.Form):
         )
     )
 
+    def clean_device(self):
+        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):
 
@@ -2321,6 +2326,12 @@ class DeviceVCMembershipForm(forms.ModelForm):
             '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():

netbox/dcim/models.py

@@ -1007,7 +1007,7 @@ class Device(CreatedUpdatedModel, CustomFieldModel):
             })
 
         # Validate virtual chassis assignment
-        if self.virtual_chassis and not self.vc_position:
+        if self.virtual_chassis and self.vc_position is None:
             raise ValidationError({
                 'vc_position': "A device assigned to a virtual chassis must have its position defined."
             })

netbox/dcim/views.py

@@ -2102,13 +2102,17 @@ class VirtualChassisCreateView(PermissionRequiredMixin, View):
             formset = VCMemberFormSet(request.POST)
 
             if vc_form.is_valid() and formset.is_valid():
+
                 with transaction.atomic():
+
+                    # Assign each device to the VirtualChassis before saving
                     virtual_chassis = vc_form.save()
                     devices = formset.save(commit=False)
                     for device in devices:
                         device.virtual_chassis = virtual_chassis
                         device.save()
-                    return redirect(vc_form.cleaned_data['master'].get_absolute_url())
+
+                return redirect(vc_form.cleaned_data['master'].get_absolute_url())
 
         else:
 
@@ -2153,8 +2157,17 @@ class VirtualChassisEditView(PermissionRequiredMixin, GetReturnURLMixin, View):
 
         if vc_form.is_valid() and formset.is_valid():
 
-            vc_form.save()
-            formset.save()
+            with transaction.atomic():
+
+                # Save the VirtualChassis
+                vc_form.save()
+
+                # Nullify the vc_position of each member first to allow reordering without raising an IntegrityError on
+                # duplicate positions. Then save each member instance.
+                members = formset.save(commit=False)
+                Device.objects.filter(pk__in=[m.pk for m in members]).update(vc_position=None)
+                for member in members:
+                    member.save()
 
             return redirect(vc_form.cleaned_data['master'].get_absolute_url())