From b049678457af419f64517b088b2b4c0da11356ef Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 27 Sep 2022 11:09:44 -0700 Subject: [PATCH] 9654 changes from code review --- netbox/dcim/api/serializers.py | 6 +++--- netbox/dcim/choices.py | 2 +- netbox/dcim/forms/bulk_edit.py | 10 +++++----- netbox/dcim/forms/filtersets.py | 16 ++++++++-------- netbox/dcim/models/devices.py | 6 +++--- netbox/dcim/models/mixins.py | 6 +++--- netbox/dcim/models/racks.py | 4 ++-- netbox/dcim/tests/test_filtersets.py | 24 ++++++++++++------------ netbox/utilities/utils.py | 21 +++++++-------------- 9 files changed, 44 insertions(+), 51 deletions(-) diff --git a/netbox/dcim/api/serializers.py b/netbox/dcim/api/serializers.py index cf92eed29..2b2d85e3b 100644 --- a/netbox/dcim/api/serializers.py +++ b/netbox/dcim/api/serializers.py @@ -203,7 +203,7 @@ class RackSerializer(NetBoxModelSerializer): outer_unit = ChoiceField(choices=RackDimensionUnitChoices, allow_blank=True, required=False) device_count = serializers.IntegerField(read_only=True) powerfeed_count = serializers.IntegerField(read_only=True) - weight_unit = ChoiceField(choices=DeviceWeightUnitChoices, allow_blank=True, required=False) + weight_unit = ChoiceField(choices=WeightUnitChoices, allow_blank=True, required=False) class Meta: model = Rack @@ -318,7 +318,7 @@ class DeviceTypeSerializer(NetBoxModelSerializer): subdevice_role = ChoiceField(choices=SubdeviceRoleChoices, allow_blank=True, required=False) airflow = ChoiceField(choices=DeviceAirflowChoices, allow_blank=True, required=False) device_count = serializers.IntegerField(read_only=True) - weight_unit = ChoiceField(choices=DeviceWeightUnitChoices, allow_blank=True, required=False) + weight_unit = ChoiceField(choices=WeightUnitChoices, allow_blank=True, required=False) class Meta: model = DeviceType @@ -333,7 +333,7 @@ class ModuleTypeSerializer(NetBoxModelSerializer): url = serializers.HyperlinkedIdentityField(view_name='dcim-api:moduletype-detail') manufacturer = NestedManufacturerSerializer() # module_count = serializers.IntegerField(read_only=True) - weight_unit = ChoiceField(choices=DeviceWeightUnitChoices, allow_blank=True, required=False) + weight_unit = ChoiceField(choices=WeightUnitChoices, allow_blank=True, required=False) class Meta: model = ModuleType diff --git a/netbox/dcim/choices.py b/netbox/dcim/choices.py index d0aab4068..8466d4861 100644 --- a/netbox/dcim/choices.py +++ b/netbox/dcim/choices.py @@ -1314,7 +1314,7 @@ class CableLengthUnitChoices(ChoiceSet): ) -class DeviceWeightUnitChoices(ChoiceSet): +class WeightUnitChoices(ChoiceSet): # Metric UNIT_KILOGRAM = 'kg' diff --git a/netbox/dcim/forms/bulk_edit.py b/netbox/dcim/forms/bulk_edit.py index 28629fb65..72407b6dd 100644 --- a/netbox/dcim/forms/bulk_edit.py +++ b/netbox/dcim/forms/bulk_edit.py @@ -290,7 +290,7 @@ class RackBulkEditForm(NetBoxModelBulkEditForm): required=False ) weight_unit = forms.ChoiceField( - choices=add_blank_choice(DeviceWeightUnitChoices), + choices=add_blank_choice(WeightUnitChoices), required=False, initial='', widget=StaticSelect() @@ -301,7 +301,7 @@ class RackBulkEditForm(NetBoxModelBulkEditForm): ('Rack', ('status', 'role', 'tenant', 'serial', 'asset_tag')), ('Location', ('region', 'site_group', 'site', 'location')), ('Hardware', ('type', 'width', 'u_height', 'desc_units', 'outer_width', 'outer_depth', 'outer_unit')), - ('Attributes', ('weight', 'weight_unit')), + ('Weight', ('weight', 'weight_unit')), ) nullable_fields = ( 'location', 'tenant', 'role', 'serial', 'asset_tag', 'outer_width', 'outer_depth', 'outer_unit', 'comments', 'weight' @@ -382,7 +382,7 @@ class DeviceTypeBulkEditForm(NetBoxModelBulkEditForm): required=False ) weight_unit = forms.ChoiceField( - choices=add_blank_choice(DeviceWeightUnitChoices), + choices=add_blank_choice(WeightUnitChoices), required=False, initial='', widget=StaticSelect() @@ -391,7 +391,7 @@ class DeviceTypeBulkEditForm(NetBoxModelBulkEditForm): model = DeviceType fieldsets = ( (None, ('manufacturer', 'part_number', 'u_height', 'is_full_depth', 'airflow')), - ('Attributes', ('weight', 'weight_unit')), + ('Weight', ('weight', 'weight_unit')), ) nullable_fields = ('part_number', 'airflow', 'weight') @@ -420,7 +420,7 @@ class ModuleTypeBulkEditForm(NetBoxModelBulkEditForm): required=False ) weight_unit = forms.ChoiceField( - choices=add_blank_choice(DeviceWeightUnitChoices), + choices=add_blank_choice(WeightUnitChoices), required=False, initial='', widget=StaticSelect() diff --git a/netbox/dcim/forms/filtersets.py b/netbox/dcim/forms/filtersets.py index 15b62b71b..9647b9c26 100644 --- a/netbox/dcim/forms/filtersets.py +++ b/netbox/dcim/forms/filtersets.py @@ -282,11 +282,11 @@ class RackFilterForm(TenancyFilterForm, ContactModelFilterForm, NetBoxModelFilte required=False ) tag = TagFilterField(model) - weight = forms.IntegerField( + weight = forms.DecimalField( required=False ) weight_unit = forms.ChoiceField( - choices=add_blank_choice(DeviceWeightUnitChoices), + choices=add_blank_choice(WeightUnitChoices), required=False ) @@ -378,7 +378,7 @@ class DeviceTypeFilterForm(NetBoxModelFilterSetForm): 'console_ports', 'console_server_ports', 'power_ports', 'power_outlets', 'interfaces', 'pass_through_ports', 'device_bays', 'module_bays', 'inventory_items', )), - ('Attributes', ('weight', 'weight_unit')), + ('Weight', ('weight', 'weight_unit')), ) manufacturer_id = DynamicModelMultipleChoiceField( queryset=Manufacturer.objects.all(), @@ -474,11 +474,11 @@ class DeviceTypeFilterForm(NetBoxModelFilterSetForm): ) ) tag = TagFilterField(model) - weight = forms.IntegerField( + weight = forms.DecimalField( required=False ) weight_unit = forms.ChoiceField( - choices=add_blank_choice(DeviceWeightUnitChoices), + choices=add_blank_choice(WeightUnitChoices), required=False ) @@ -492,7 +492,7 @@ class ModuleTypeFilterForm(NetBoxModelFilterSetForm): 'console_ports', 'console_server_ports', 'power_ports', 'power_outlets', 'interfaces', 'pass_through_ports', )), - ('Attributes', ('weight', 'weight_unit')), + ('Weight', ('weight', 'weight_unit')), ) manufacturer_id = DynamicModelMultipleChoiceField( queryset=Manufacturer.objects.all(), @@ -546,11 +546,11 @@ class ModuleTypeFilterForm(NetBoxModelFilterSetForm): ) ) tag = TagFilterField(model) - weight = forms.IntegerField( + weight = forms.DecimalField( required=False ) weight_unit = forms.ChoiceField( - choices=add_blank_choice(DeviceWeightUnitChoices), + choices=add_blank_choice(WeightUnitChoices), required=False ) diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py index ce80537c6..d72073b84 100644 --- a/netbox/dcim/models/devices.py +++ b/netbox/dcim/models/devices.py @@ -20,7 +20,7 @@ from netbox.models import OrganizationalModel, NetBoxModel from utilities.choices import ColorChoices from utilities.fields import ColorField, NaturalOrderingField from .device_components import * -from .mixins import DeviceWeightMixin +from .mixins import WeightMixin __all__ = ( @@ -71,7 +71,7 @@ class Manufacturer(OrganizationalModel): return reverse('dcim:manufacturer', args=[self.pk]) -class DeviceType(NetBoxModel, DeviceWeightMixin): +class DeviceType(NetBoxModel, WeightMixin): """ A DeviceType represents a particular make (Manufacturer) and model of device. It specifies rack height and depth, as well as high-level functional role(s). @@ -309,7 +309,7 @@ class DeviceType(NetBoxModel, DeviceWeightMixin): return self.subdevice_role == SubdeviceRoleChoices.ROLE_CHILD -class ModuleType(NetBoxModel, DeviceWeightMixin): +class ModuleType(NetBoxModel, WeightMixin): """ A ModuleType represents a hardware element that can be installed within a device and which houses additional components; for example, a line card within a chassis-based switch such as the Cisco Catalyst 6500. Like a diff --git a/netbox/dcim/models/mixins.py b/netbox/dcim/models/mixins.py index 4655c1ec3..98cc4016f 100644 --- a/netbox/dcim/models/mixins.py +++ b/netbox/dcim/models/mixins.py @@ -4,7 +4,7 @@ from dcim.choices import * from utilities.utils import to_kilograms -class DeviceWeightMixin(models.Model): +class WeightMixin(models.Model): weight = models.DecimalField( max_digits=8, decimal_places=2, @@ -13,7 +13,7 @@ class DeviceWeightMixin(models.Model): ) weight_unit = models.CharField( max_length=50, - choices=DeviceWeightUnitChoices, + choices=WeightUnitChoices, blank=True, ) # Stores the normalized weight (in kilograms) for database ordering @@ -29,7 +29,7 @@ class DeviceWeightMixin(models.Model): def save(self, *args, **kwargs): - # Store the given weight (if any) in meters for use in database ordering + # Store the given weight (if any) in kilograms for use in database ordering if self.weight and self.weight_unit: self._abs_weight = to_kilograms(self.weight, self.weight_unit) else: diff --git a/netbox/dcim/models/racks.py b/netbox/dcim/models/racks.py index bc3a52772..90efb3e02 100644 --- a/netbox/dcim/models/racks.py +++ b/netbox/dcim/models/racks.py @@ -20,7 +20,7 @@ from utilities.fields import ColorField, NaturalOrderingField from utilities.utils import array_to_string, drange from .device_components import PowerOutlet, PowerPort from .devices import Device, Module -from .mixins import DeviceWeightMixin +from .mixins import WeightMixin from .power import PowerFeed __all__ = ( @@ -64,7 +64,7 @@ class RackRole(OrganizationalModel): return reverse('dcim:rackrole', args=[self.pk]) -class Rack(NetBoxModel, DeviceWeightMixin): +class Rack(NetBoxModel, WeightMixin): """ Devices are housed within Racks. Each rack has a defined height measured in rack units, and a front and rear face. Each Rack is assigned to a Site and (optionally) a Location. diff --git a/netbox/dcim/tests/test_filtersets.py b/netbox/dcim/tests/test_filtersets.py index 024a870ff..6536a501f 100644 --- a/netbox/dcim/tests/test_filtersets.py +++ b/netbox/dcim/tests/test_filtersets.py @@ -409,9 +409,9 @@ class RackTestCase(TestCase, ChangeLoggedFilterSetTests): Tenant.objects.bulk_create(tenants) racks = ( - Rack(name='Rack 1', facility_id='rack-1', site=sites[0], location=locations[0], tenant=tenants[0], status=RackStatusChoices.STATUS_ACTIVE, role=rack_roles[0], serial='ABC', asset_tag='1001', type=RackTypeChoices.TYPE_2POST, width=RackWidthChoices.WIDTH_19IN, u_height=42, desc_units=False, outer_width=100, outer_depth=100, outer_unit=RackDimensionUnitChoices.UNIT_MILLIMETER, weight=10, weight_unit=DeviceWeightUnitChoices.UNIT_POUND), - Rack(name='Rack 2', facility_id='rack-2', site=sites[1], location=locations[1], tenant=tenants[1], status=RackStatusChoices.STATUS_PLANNED, role=rack_roles[1], serial='DEF', asset_tag='1002', type=RackTypeChoices.TYPE_4POST, width=RackWidthChoices.WIDTH_21IN, u_height=43, desc_units=False, outer_width=200, outer_depth=200, outer_unit=RackDimensionUnitChoices.UNIT_MILLIMETER, weight=20, weight_unit=DeviceWeightUnitChoices.UNIT_POUND), - Rack(name='Rack 3', facility_id='rack-3', site=sites[2], location=locations[2], tenant=tenants[2], status=RackStatusChoices.STATUS_RESERVED, role=rack_roles[2], serial='GHI', asset_tag='1003', type=RackTypeChoices.TYPE_CABINET, width=RackWidthChoices.WIDTH_23IN, u_height=44, desc_units=True, outer_width=300, outer_depth=300, outer_unit=RackDimensionUnitChoices.UNIT_INCH, weight=30, weight_unit=DeviceWeightUnitChoices.UNIT_KILOGRAM), + Rack(name='Rack 1', facility_id='rack-1', site=sites[0], location=locations[0], tenant=tenants[0], status=RackStatusChoices.STATUS_ACTIVE, role=rack_roles[0], serial='ABC', asset_tag='1001', type=RackTypeChoices.TYPE_2POST, width=RackWidthChoices.WIDTH_19IN, u_height=42, desc_units=False, outer_width=100, outer_depth=100, outer_unit=RackDimensionUnitChoices.UNIT_MILLIMETER, weight=10, weight_unit=WeightUnitChoices.UNIT_POUND), + Rack(name='Rack 2', facility_id='rack-2', site=sites[1], location=locations[1], tenant=tenants[1], status=RackStatusChoices.STATUS_PLANNED, role=rack_roles[1], serial='DEF', asset_tag='1002', type=RackTypeChoices.TYPE_4POST, width=RackWidthChoices.WIDTH_21IN, u_height=43, desc_units=False, outer_width=200, outer_depth=200, outer_unit=RackDimensionUnitChoices.UNIT_MILLIMETER, weight=20, weight_unit=WeightUnitChoices.UNIT_POUND), + Rack(name='Rack 3', facility_id='rack-3', site=sites[2], location=locations[2], tenant=tenants[2], status=RackStatusChoices.STATUS_RESERVED, role=rack_roles[2], serial='GHI', asset_tag='1003', type=RackTypeChoices.TYPE_CABINET, width=RackWidthChoices.WIDTH_23IN, u_height=44, desc_units=True, outer_width=300, outer_depth=300, outer_unit=RackDimensionUnitChoices.UNIT_INCH, weight=30, weight_unit=WeightUnitChoices.UNIT_KILOGRAM), ) Rack.objects.bulk_create(racks) @@ -522,7 +522,7 @@ class RackTestCase(TestCase, ChangeLoggedFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) def test_weight_unit(self): - params = {'weight_unit': DeviceWeightUnitChoices.UNIT_POUND} + params = {'weight_unit': WeightUnitChoices.UNIT_POUND} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) @@ -696,9 +696,9 @@ class DeviceTypeTestCase(TestCase, ChangeLoggedFilterSetTests): Manufacturer.objects.bulk_create(manufacturers) device_types = ( - DeviceType(manufacturer=manufacturers[0], model='Model 1', slug='model-1', part_number='Part Number 1', u_height=1, is_full_depth=True, front_image='front.png', rear_image='rear.png', weight=10, weight_unit=DeviceWeightUnitChoices.UNIT_POUND), - DeviceType(manufacturer=manufacturers[1], model='Model 2', slug='model-2', part_number='Part Number 2', u_height=2, is_full_depth=True, subdevice_role=SubdeviceRoleChoices.ROLE_PARENT, airflow=DeviceAirflowChoices.AIRFLOW_FRONT_TO_REAR, weight=20, weight_unit=DeviceWeightUnitChoices.UNIT_POUND), - DeviceType(manufacturer=manufacturers[2], model='Model 3', slug='model-3', part_number='Part Number 3', u_height=3, is_full_depth=False, subdevice_role=SubdeviceRoleChoices.ROLE_CHILD, airflow=DeviceAirflowChoices.AIRFLOW_REAR_TO_FRONT, weight=30, weight_unit=DeviceWeightUnitChoices.UNIT_KILOGRAM), + DeviceType(manufacturer=manufacturers[0], model='Model 1', slug='model-1', part_number='Part Number 1', u_height=1, is_full_depth=True, front_image='front.png', rear_image='rear.png', weight=10, weight_unit=WeightUnitChoices.UNIT_POUND), + DeviceType(manufacturer=manufacturers[1], model='Model 2', slug='model-2', part_number='Part Number 2', u_height=2, is_full_depth=True, subdevice_role=SubdeviceRoleChoices.ROLE_PARENT, airflow=DeviceAirflowChoices.AIRFLOW_FRONT_TO_REAR, weight=20, weight_unit=WeightUnitChoices.UNIT_POUND), + DeviceType(manufacturer=manufacturers[2], model='Model 3', slug='model-3', part_number='Part Number 3', u_height=3, is_full_depth=False, subdevice_role=SubdeviceRoleChoices.ROLE_CHILD, airflow=DeviceAirflowChoices.AIRFLOW_REAR_TO_FRONT, weight=30, weight_unit=WeightUnitChoices.UNIT_KILOGRAM), ) DeviceType.objects.bulk_create(device_types) @@ -852,7 +852,7 @@ class DeviceTypeTestCase(TestCase, ChangeLoggedFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) def test_weight_unit(self): - params = {'weight_unit': DeviceWeightUnitChoices.UNIT_POUND} + params = {'weight_unit': WeightUnitChoices.UNIT_POUND} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) @@ -871,9 +871,9 @@ class ModuleTypeTestCase(TestCase, ChangeLoggedFilterSetTests): Manufacturer.objects.bulk_create(manufacturers) module_types = ( - ModuleType(manufacturer=manufacturers[0], model='Model 1', part_number='Part Number 1', weight=10, weight_unit=DeviceWeightUnitChoices.UNIT_POUND), - ModuleType(manufacturer=manufacturers[1], model='Model 2', part_number='Part Number 2', weight=20, weight_unit=DeviceWeightUnitChoices.UNIT_POUND), - ModuleType(manufacturer=manufacturers[2], model='Model 3', part_number='Part Number 3', weight=30, weight_unit=DeviceWeightUnitChoices.UNIT_KILOGRAM), + ModuleType(manufacturer=manufacturers[0], model='Model 1', part_number='Part Number 1', weight=10, weight_unit=WeightUnitChoices.UNIT_POUND), + ModuleType(manufacturer=manufacturers[1], model='Model 2', part_number='Part Number 2', weight=20, weight_unit=WeightUnitChoices.UNIT_POUND), + ModuleType(manufacturer=manufacturers[2], model='Model 3', part_number='Part Number 3', weight=30, weight_unit=WeightUnitChoices.UNIT_KILOGRAM), ) ModuleType.objects.bulk_create(module_types) @@ -964,7 +964,7 @@ class ModuleTypeTestCase(TestCase, ChangeLoggedFilterSetTests): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) def test_weight_unit(self): - params = {'weight_unit': DeviceWeightUnitChoices.UNIT_POUND} + params = {'weight_unit': WeightUnitChoices.UNIT_POUND} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index 1d2de4768..263da38a7 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -12,7 +12,7 @@ from django.http import QueryDict from jinja2.sandbox import SandboxedEnvironment from mptt.models import MPTTModel -from dcim.choices import CableLengthUnitChoices, DeviceWeightUnitChoices +from dcim.choices import CableLengthUnitChoices, WeightUnitChoices from extras.plugins import PluginConfig from extras.utils import is_taggable from netbox.config import get_config @@ -272,7 +272,7 @@ def to_meters(length, unit): def to_kilograms(weight, unit): """ - Convert the given length to kilograms. + Convert the given weight to kilograms. """ try: if weight < 0: @@ -280,24 +280,17 @@ def to_kilograms(weight, unit): except TypeError: raise TypeError(f"Invalid value '{weight}' for weight (must be a number)") - valid_units = DeviceWeightUnitChoices.values() + valid_units = WeightUnitChoices.values() if unit not in valid_units: raise ValueError(f"Unknown unit {unit}. Must be one of the following: {', '.join(valid_units)}") - UNIT_KILOGRAM = 'kg' - UNIT_GRAM = 'g' - - # Imperial - UNIT_POUND = 'lb' - UNIT_OUNCE = 'oz' - - if unit == DeviceWeightUnitChoices.UNIT_KILOGRAM: + if unit == WeightUnitChoices.UNIT_KILOGRAM: return weight - if unit == DeviceWeightUnitChoices.UNIT_GRAM: + if unit == WeightUnitChoices.UNIT_GRAM: return weight * 1000 - if unit == DeviceWeightUnitChoices.UNIT_POUND: + if unit == WeightUnitChoices.UNIT_POUND: return weight * Decimal(0.453592) - if unit == DeviceWeightUnitChoices.UNIT_OUNCE: + if unit == WeightUnitChoices.UNIT_OUNCE: return weight * Decimal(0.0283495) raise ValueError(f"Unknown unit {unit}. Must be 'kg', 'g', 'lb', 'oz'.")