From b032742418842cbaf74479c8179ba249ed6611d0 Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Mon, 3 Apr 2023 16:26:07 -0400 Subject: [PATCH] Closes #12133: Move any instance mutations inside clean() to save() --- netbox/dcim/models/cables.py | 6 ++++-- netbox/dcim/models/device_components.py | 14 ++++++++++---- netbox/dcim/models/racks.py | 6 ++++-- netbox/ipam/models/ip.py | 3 --- netbox/virtualization/models/virtualmachines.py | 10 ++++++++-- netbox/virtualization/tests/test_models.py | 2 +- 6 files changed, 27 insertions(+), 14 deletions(-) diff --git a/netbox/dcim/models/cables.py b/netbox/dcim/models/cables.py index 062734355..cf5f30ee4 100644 --- a/netbox/dcim/models/cables.py +++ b/netbox/dcim/models/cables.py @@ -152,8 +152,6 @@ class Cable(PrimaryModel): # Validate length and length_unit if self.length is not None and not self.length_unit: raise ValidationError("Must specify a unit when setting a cable length") - elif self.length is None: - self.length_unit = '' if self.pk is None and (not self.a_terminations or not self.b_terminations): raise ValidationError("Must define A and B terminations when creating a new cable.") @@ -187,6 +185,10 @@ class Cable(PrimaryModel): else: self._abs_length = None + # Clear length_unit if no length is defined + if self.length is None: + self.length_unit = '' + super().save(*args, **kwargs) # Update the private pk used in __str__ in case this is a new object (i.e. just got its pk) diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py index 26a6ade98..b879b77d3 100644 --- a/netbox/dcim/models/device_components.py +++ b/netbox/dcim/models/device_components.py @@ -794,8 +794,6 @@ class Interface(ModularComponentModel, BaseInterface, CabledObjectModel, PathEnd raise ValidationError({ 'rf_channel_frequency': "Cannot specify custom frequency with channel selected.", }) - elif self.rf_channel: - self.rf_channel_frequency = get_channel_attr(self.rf_channel, 'frequency') # Validate channel width against interface type and selected channel (if any) if self.rf_channel_width: @@ -803,8 +801,6 @@ class Interface(ModularComponentModel, BaseInterface, CabledObjectModel, PathEnd raise ValidationError({'rf_channel_width': "Channel width may be set only on wireless interfaces."}) if self.rf_channel and self.rf_channel_width != get_channel_attr(self.rf_channel, 'width'): raise ValidationError({'rf_channel_width': "Cannot specify custom width with channel selected."}) - elif self.rf_channel: - self.rf_channel_width = get_channel_attr(self.rf_channel, 'width') # VLAN validation @@ -815,6 +811,16 @@ class Interface(ModularComponentModel, BaseInterface, CabledObjectModel, PathEnd f"interface's parent device, or it must be global." }) + def save(self, *args, **kwargs): + + # Set absolute channel attributes from selected options + if self.rf_channel and not self.rf_channel_frequency: + self.rf_channel_frequency = get_channel_attr(self.rf_channel, 'frequency') + if self.rf_channel and not self.rf_channel_width: + self.rf_channel_width = get_channel_attr(self.rf_channel, 'width') + + super().save(*args, **kwargs) + @property def _occupied(self): return super()._occupied or bool(self.wireless_link_id) diff --git a/netbox/dcim/models/racks.py b/netbox/dcim/models/racks.py index 03be2fdb3..e61e0f2a3 100644 --- a/netbox/dcim/models/racks.py +++ b/netbox/dcim/models/racks.py @@ -222,8 +222,6 @@ class Rack(PrimaryModel, WeightMixin): # Validate outer dimensions and unit if (self.outer_width is not None or self.outer_depth is not None) and not self.outer_unit: raise ValidationError("Must specify a unit when setting an outer width/depth") - elif self.outer_width is None and self.outer_depth is None: - self.outer_unit = '' # Validate max_weight and weight_unit if self.max_weight and not self.weight_unit: @@ -259,6 +257,10 @@ class Rack(PrimaryModel, WeightMixin): else: self._abs_max_weight = None + # Clear unit if outer width & depth are not set + if self.outer_width is None and self.outer_depth is None: + self.outer_unit = '' + super().save(*args, **kwargs) @property diff --git a/netbox/ipam/models/ip.py b/netbox/ipam/models/ip.py index e8bf13375..9effc3add 100644 --- a/netbox/ipam/models/ip.py +++ b/netbox/ipam/models/ip.py @@ -178,9 +178,6 @@ class Aggregate(GetAvailablePrefixesMixin, PrimaryModel): if self.prefix: - # Clear host bits from prefix - self.prefix = self.prefix.cidr - # /0 masks are not acceptable if self.prefix.prefixlen == 0: raise ValidationError({ diff --git a/netbox/virtualization/models/virtualmachines.py b/netbox/virtualization/models/virtualmachines.py index cc39044f9..6e9cc5664 100644 --- a/netbox/virtualization/models/virtualmachines.py +++ b/netbox/virtualization/models/virtualmachines.py @@ -169,8 +169,6 @@ class VirtualMachine(PrimaryModel, ConfigContextModel): raise ValidationError({ 'cluster': f'The selected cluster ({self.cluster}) is not assigned to this site ({self.site}).' }) - elif self.cluster: - self.site = self.cluster.site # Validate assigned cluster device if self.device and not self.cluster: @@ -201,6 +199,14 @@ class VirtualMachine(PrimaryModel, ConfigContextModel): field: f"The specified IP address ({ip}) is not assigned to this VM.", }) + def save(self, *args, **kwargs): + + # Assign site from cluster if not set + if self.cluster and not self.site: + self.site = self.cluster.site + + super().save(*args, **kwargs) + def get_status_color(self): return VirtualMachineStatusChoices.colors.get(self.status) diff --git a/netbox/virtualization/tests/test_models.py b/netbox/virtualization/tests/test_models.py index f7fa4cb39..782b9f07f 100644 --- a/netbox/virtualization/tests/test_models.py +++ b/netbox/virtualization/tests/test_models.py @@ -72,7 +72,7 @@ class VirtualMachineTestCase(TestCase): # VM with cluster site but no direct site should have its site set automatically vm = VirtualMachine(name='vm1', site=None, cluster=clusters[0]) - vm.full_clean() + vm.save() self.assertEqual(vm.site, sites[0]) def test_vm_name_case_sensitivity(self):