diff --git a/docs/release-notes/version-2.10.md b/docs/release-notes/version-2.10.md index ba1d241d3..8cca1f1bf 100644 --- a/docs/release-notes/version-2.10.md +++ b/docs/release-notes/version-2.10.md @@ -4,6 +4,7 @@ ### Bug Fixes +* [#5301](https://github.com/netbox-community/netbox/issues/5301) - Fix misleading error when racking a device with invalid parameters * [#5311](https://github.com/netbox-community/netbox/issues/5311) - Update child objects when a rack group is moved to a new site * [#5518](https://github.com/netbox-community/netbox/issues/5518) - Fix persistent vertical scrollbar diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 7ecd4efd8..4cc1a532b 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -1781,9 +1781,8 @@ class DeviceForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): 'group_id': '$rack_group', } ) - position = forms.TypedChoiceField( + position = forms.IntegerField( required=False, - empty_value=None, help_text="The lowest-numbered unit occupied by the device", widget=APISelect( api_url='/api/dcim/racks/{{rack}}/elevation/', @@ -1856,6 +1855,7 @@ class DeviceForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): "config context", } widgets = { + 'face': StaticSelect2(), 'status': StaticSelect2(), 'primary_ip4': StaticSelect2(), 'primary_ip6': StaticSelect2(), @@ -1902,6 +1902,13 @@ class DeviceForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): Q(manufacturer__isnull=True) | Q(manufacturer=self.instance.device_type.manufacturer) ) + # Disable rack assignment if this is a child device installed in a parent device + if self.instance.device_type.is_child_device and hasattr(self.instance, 'parent_bay'): + self.fields['site'].disabled = True + self.fields['rack'].disabled = True + self.initial['site'] = self.instance.parent_bay.device.site_id + self.initial['rack'] = self.instance.parent_bay.device.rack_id + else: # An object that doesn't exist yet can't have any IPs assigned to it @@ -1911,31 +1918,9 @@ class DeviceForm(BootstrapMixin, TenancyForm, CustomFieldModelForm): self.fields['primary_ip6'].widget.attrs['readonly'] = True # Rack position - pk = self.instance.pk if self.instance.pk else None - try: - if self.is_bound and self.data.get('rack') and str(self.data.get('face')): - position_choices = Rack.objects.get(pk=self.data['rack']) \ - .get_rack_units(face=self.data.get('face'), exclude=pk) - elif self.initial.get('rack') and str(self.initial.get('face')): - position_choices = Rack.objects.get(pk=self.initial['rack']) \ - .get_rack_units(face=self.initial.get('face'), exclude=pk) - else: - position_choices = [] - except Rack.DoesNotExist: - position_choices = [] - self.fields['position'].choices = [('', '---------')] + [ - (p['id'], { - 'label': p['name'], - 'disabled': bool(p['device'] and p['id'] != self.initial.get('position')), - }) for p in position_choices - ] - - # Disable rack assignment if this is a child device installed in a parent device - if pk and self.instance.device_type.is_child_device and hasattr(self.instance, 'parent_bay'): - self.fields['site'].disabled = True - self.fields['rack'].disabled = True - self.initial['site'] = self.instance.parent_bay.device.site_id - self.initial['rack'] = self.instance.parent_bay.device.rack_id + position = self.data.get('position') or self.initial.get('position') + if position: + self.fields['position'].widget.choices = [(position, f'U{position}')] class BaseDeviceCSVForm(CustomFieldModelCSVForm): diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py index 5b685057e..29818ab98 100644 --- a/netbox/dcim/models/devices.py +++ b/netbox/dcim/models/devices.py @@ -640,7 +640,7 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): # Validate site/rack combination if self.rack and self.site != self.rack.site: raise ValidationError({ - 'rack': "Rack {} does not belong to site {}.".format(self.rack, self.site), + 'rack': f"Rack {self.rack} does not belong to site {self.site}.", }) if self.rack is None: @@ -650,7 +650,7 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): }) if self.position: raise ValidationError({ - 'face': "Cannot select a rack position without assigning a rack.", + 'position': "Cannot select a rack position without assigning a rack.", }) # Validate position/face combination @@ -662,7 +662,7 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): # Prevent 0U devices from being assigned to a specific position if self.position and self.device_type.u_height == 0: raise ValidationError({ - 'position': "A U0 device type ({}) cannot be assigned to a rack position.".format(self.device_type) + 'position': f"A U0 device type ({self.device_type}) cannot be assigned to a rack position." }) if self.rack: @@ -688,8 +688,8 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): ) if self.position and self.position not in available_units: raise ValidationError({ - 'position': "U{} is already occupied or does not have sufficient space to accommodate a(n) " - "{} ({}U).".format(self.position, self.device_type, self.device_type.u_height) + 'position': f"U{self.position} is already occupied or does not have sufficient space to " + f"accommodate this device type: {self.device_type} ({self.device_type.u_height}U)" }) except DeviceType.DoesNotExist: diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py index e8cb73fe4..6eeffbc96 100644 --- a/netbox/dcim/tests/test_forms.py +++ b/netbox/dcim/tests/test_forms.py @@ -82,7 +82,7 @@ class DeviceTestCase(TestCase): self.assertTrue(form.is_valid()) self.assertTrue(form.save()) - def test_non_racked_device_with_face_position(self): + def test_non_racked_device_with_face(self): form = DeviceForm(data={ 'name': 'New Device', 'device_role': DeviceRole.objects.first().pk, @@ -92,12 +92,26 @@ class DeviceTestCase(TestCase): 'site': Site.objects.first().pk, 'rack': None, 'face': DeviceFaceChoices.FACE_REAR, - 'position': 10, 'platform': None, 'status': DeviceStatusChoices.STATUS_ACTIVE, }) self.assertFalse(form.is_valid()) self.assertIn('face', form.errors) + + def test_non_racked_device_with_position(self): + form = DeviceForm(data={ + 'name': 'New Device', + 'device_role': DeviceRole.objects.first().pk, + 'tenant': None, + 'manufacturer': Manufacturer.objects.first().pk, + 'device_type': DeviceType.objects.first().pk, + 'site': Site.objects.first().pk, + 'rack': None, + 'position': 10, + 'platform': None, + 'status': DeviceStatusChoices.STATUS_ACTIVE, + }) + self.assertFalse(form.is_valid()) self.assertIn('position', form.errors)