mirror of
https://github.com/netbox-community/netbox.git
synced 2025-07-21 11:37:21 -06:00
Fixes #5301: Fix misleading error when racking a device with invalid parameters
This commit is contained in:
Jeremy Stretch
parent
396b0dace8
commit
fce61295c9
@ -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
|
||||
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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)
|
||||
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user