From 9694bacb69c82c5ed543754f436f2e09c4be890c Mon Sep 17 00:00:00 2001 From: kobayashi Date: Sat, 18 Jan 2020 07:37:06 -0500 Subject: [PATCH 1/2] 3950 not retain device type --- docs/release-notes/version-2.7.md | 1 + netbox/dcim/forms.py | 1 + netbox/dcim/models/__init__.py | 2 +- netbox/utilities/utils.py | 22 +++++++++++++++++++--- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/release-notes/version-2.7.md b/docs/release-notes/version-2.7.md index 5ca486137..2e5804c08 100644 --- a/docs/release-notes/version-2.7.md +++ b/docs/release-notes/version-2.7.md @@ -26,6 +26,7 @@ * [#3721](https://github.com/netbox-community/netbox/issues/3721) - Allow Unicode characters in tag slugs * [#3923](https://github.com/netbox-community/netbox/issues/3923) - Indicate validation failure when using SSH-style RSA keys * [#3951](https://github.com/netbox-community/netbox/issues/3951) - Fix exception in webhook worker due to missing constant +* [#3950](https://github.com/netbox-community/netbox/issues/3950) - Fix "Create and Add Another" not keep Manufacturer and Device Type after device creation * [#3953](https://github.com/netbox-community/netbox/issues/3953) - Fix validation error when creating child devices * [#3960](https://github.com/netbox-community/netbox/issues/3960) - Fix legacy device status choice * [#3962](https://github.com/netbox-community/netbox/issues/3962) - Fix display of unnamed devices in rack elevations diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 84d83a94c..2edf93857 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -1636,6 +1636,7 @@ class DeviceForm(BootstrapMixin, TenancyForm, CustomFieldForm): instance = kwargs.get('instance') if 'initial' not in kwargs: kwargs['initial'] = {} + # Using hasattr() instead of "is not None" to avoid RelatedObjectDoesNotExist on required field if instance and hasattr(instance, 'device_type'): kwargs['initial']['manufacturer'] = instance.device_type.manufacturer diff --git a/netbox/dcim/models/__init__.py b/netbox/dcim/models/__init__.py index 1c9c8682d..3827716bd 100644 --- a/netbox/dcim/models/__init__.py +++ b/netbox/dcim/models/__init__.py @@ -1409,7 +1409,7 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): 'site', 'rack_group', 'rack_name', 'position', 'face', 'comments', ] clone_fields = [ - 'device_type', 'device_role', 'tenant', 'platform', 'site', 'rack', 'status', 'cluster', + 'device_type', 'device_type__manufacturer', 'device_role', 'tenant', 'platform', 'site', 'rack', 'status', 'cluster', 'cluster__group', ] STATUS_CLASS_MAP = { diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index dc2185988..82db56c10 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -181,15 +181,31 @@ def render_jinja2(template_code, context): return Environment().from_string(source=template_code).render(**context) +def get_field_value(instance, field_name): + """ + Retrieve appropriate field name & value from object recursively. + """ + if '__' in field_name: + fn = field_name.split('__') + attr = getattr(instance, fn[0]) + return get_field_value(attr, fn[1]) + + field = instance._meta.get_field(field_name) if instance else None + field_value = field.value_from_object(instance) if field else None + + if field_name == 'group': + field_name = 'cluster_group' + return field_name, field_value + + def prepare_cloned_fields(instance): """ Compile an object's `clone_fields` list into a string of URL query parameters. Tags are automatically cloned where applicable. """ params = {} - for field_name in getattr(instance, 'clone_fields', []): - field = instance._meta.get_field(field_name) - field_value = field.value_from_object(instance) + for field in getattr(instance, 'clone_fields', []): + field_name, field_value = get_field_value(instance, field) # Swap out False with URL-friendly value if field_value is False: From 66d5cc47a5c17387f805968f361ec28cfc6c0cfc Mon Sep 17 00:00:00 2001 From: kobayashi Date: Fri, 24 Jan 2020 03:11:17 -0500 Subject: [PATCH 2/2] Fixes #3950: Cloned Device Form does not retain device type --- docs/release-notes/version-2.7.md | 2 +- netbox/dcim/fixtures/dcim.json | 58 ++++++ netbox/dcim/forms.py | 11 +- netbox/dcim/models/__init__.py | 2 +- netbox/dcim/tests/test_forms.py | 14 +- netbox/utilities/utils.py | 22 +-- .../fixtures/virtualization.json | 170 ++++++++++++++++++ 7 files changed, 256 insertions(+), 23 deletions(-) create mode 100644 netbox/virtualization/fixtures/virtualization.json diff --git a/docs/release-notes/version-2.7.md b/docs/release-notes/version-2.7.md index 2e5804c08..5e0b5d744 100644 --- a/docs/release-notes/version-2.7.md +++ b/docs/release-notes/version-2.7.md @@ -7,6 +7,7 @@ ## Bug Fixes +* [#3950](https://github.com/netbox-community/netbox/issues/3950) - Fix "Create and Add Another" not keep Manufacturer and Device Type after device creation * [#3983](https://github.com/netbox-community/netbox/issues/3983) - Permit the creation of multiple unnamed devices * [#3989](https://github.com/netbox-community/netbox/issues/3989) - Correct HTTP content type assignment for webhooks * [#3999](https://github.com/netbox-community/netbox/issues/3999) - Do not filter child results by null if non-required parent fields are blank @@ -26,7 +27,6 @@ * [#3721](https://github.com/netbox-community/netbox/issues/3721) - Allow Unicode characters in tag slugs * [#3923](https://github.com/netbox-community/netbox/issues/3923) - Indicate validation failure when using SSH-style RSA keys * [#3951](https://github.com/netbox-community/netbox/issues/3951) - Fix exception in webhook worker due to missing constant -* [#3950](https://github.com/netbox-community/netbox/issues/3950) - Fix "Create and Add Another" not keep Manufacturer and Device Type after device creation * [#3953](https://github.com/netbox-community/netbox/issues/3953) - Fix validation error when creating child devices * [#3960](https://github.com/netbox-community/netbox/issues/3960) - Fix legacy device status choice * [#3962](https://github.com/netbox-community/netbox/issues/3962) - Fix display of unnamed devices in rack elevations diff --git a/netbox/dcim/fixtures/dcim.json b/netbox/dcim/fixtures/dcim.json index b9f41edb5..2b379b9ff 100644 --- a/netbox/dcim/fixtures/dcim.json +++ b/netbox/dcim/fixtures/dcim.json @@ -66,6 +66,14 @@ "slug": "servertech" } }, +{ + "model": "dcim.manufacturer", + "pk": 4, + "fields": { + "name": "Dell", + "slug": "dell" + } +}, { "model": "dcim.devicetype", "pk": 1, @@ -144,6 +152,19 @@ "is_full_depth": false } }, +{ + "model": "dcim.devicetype", + "pk": 7, + "fields": { + "created": "2016-06-23", + "last_updated": "2016-06-23T03:19:56.521Z", + "manufacturer": 4, + "model": "PowerEdge R640", + "slug": "poweredge-r640", + "u_height": 1, + "is_full_depth": false + } +}, { "model": "dcim.consoleporttemplate", "pk": 1, @@ -1880,6 +1901,15 @@ "color": "yellow" } }, +{ + "model": "dcim.devicerole", + "pk": 7, + "fields": { + "name": "Server", + "slug": "server", + "color": "grey" + } +}, { "model": "dcim.platform", "pk": 1, @@ -2127,6 +2157,34 @@ "comments": "" } }, +{ + "model": "dcim.device", + "pk": 13, + "fields": { + "local_context_data": null, + "created": "2016-06-23", + "last_updated": "2016-06-23T03:19:56.521Z", + "device_type": 7, + "device_role": 6, + "tenant": null, + "platform": null, + "name": "test1-server1", + "serial": "", + "asset_tag": null, + "site": 1, + "rack": 2, + "position": null, + "face": "", + "status": true, + "primary_ip4": null, + "primary_ip6": null, + "cluster": 4, + "virtual_chassis": null, + "vc_position": null, + "vc_priority": null, + "comments": "" + } +}, { "model": "dcim.consoleport", "pk": 1, diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 2edf93857..79fe13a9a 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -1636,13 +1636,22 @@ class DeviceForm(BootstrapMixin, TenancyForm, CustomFieldForm): instance = kwargs.get('instance') if 'initial' not in kwargs: kwargs['initial'] = {} - # Using hasattr() instead of "is not None" to avoid RelatedObjectDoesNotExist on required field if instance and hasattr(instance, 'device_type'): kwargs['initial']['manufacturer'] = instance.device_type.manufacturer if instance and instance.cluster is not None: kwargs['initial']['cluster_group'] = instance.cluster.group + if 'device_type' in kwargs['initial'] and 'manufacturer' not in kwargs['initial']: + device_type_id = kwargs['initial']['device_type'] + manufacturer_id = DeviceType.objects.filter(pk=device_type_id).values_list('manufacturer__pk', flat=True).first() + kwargs['initial']['manufacturer'] = manufacturer_id + + if 'cluster' in kwargs['initial'] and 'cluster_group' not in kwargs['initial']: + cluster_id = kwargs['initial']['cluster'] + cluster_group_id = Cluster.objects.filter(pk=cluster_id).values_list('group__pk', flat=True).first() + kwargs['initial']['cluster_group'] = cluster_group_id + super().__init__(*args, **kwargs) if self.instance.pk: diff --git a/netbox/dcim/models/__init__.py b/netbox/dcim/models/__init__.py index 3827716bd..1c9c8682d 100644 --- a/netbox/dcim/models/__init__.py +++ b/netbox/dcim/models/__init__.py @@ -1409,7 +1409,7 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): 'site', 'rack_group', 'rack_name', 'position', 'face', 'comments', ] clone_fields = [ - 'device_type', 'device_type__manufacturer', 'device_role', 'tenant', 'platform', 'site', 'rack', 'status', 'cluster', 'cluster__group', + 'device_type', 'device_role', 'tenant', 'platform', 'site', 'rack', 'status', 'cluster', ] STATUS_CLASS_MAP = { diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py index d7a946568..5bbe36716 100644 --- a/netbox/dcim/tests/test_forms.py +++ b/netbox/dcim/tests/test_forms.py @@ -10,7 +10,7 @@ def get_id(model, slug): class DeviceTestCase(TestCase): - fixtures = ['dcim', 'ipam'] + fixtures = ['dcim', 'ipam', 'virtualization'] def test_racked_device(self): test = DeviceForm(data={ @@ -78,3 +78,15 @@ class DeviceTestCase(TestCase): }) self.assertTrue(test.is_valid()) self.assertTrue(test.save()) + + def test_cloned_cluster_device_initial_data(self): + test = DeviceForm(initial={ + 'device_type': get_id(DeviceType, 'poweredge-r640'), + 'device_role': get_id(DeviceRole, 'server'), + 'status': DeviceStatusChoices.STATUS_ACTIVE, + 'site': get_id(Site, 'test1'), + "cluster": Cluster.objects.get(id=4).id, + }) + self.assertEqual(test.initial['manufacturer'], get_id(Manufacturer, 'dell')) + self.assertIn('cluster_group', test.initial) + self.assertEqual(test.initial['cluster_group'], get_id(ClusterGroup, 'vm-host')) diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index 82db56c10..dc2185988 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -181,31 +181,15 @@ def render_jinja2(template_code, context): return Environment().from_string(source=template_code).render(**context) -def get_field_value(instance, field_name): - """ - Retrieve appropriate field name & value from object recursively. - """ - if '__' in field_name: - fn = field_name.split('__') - attr = getattr(instance, fn[0]) - return get_field_value(attr, fn[1]) - - field = instance._meta.get_field(field_name) if instance else None - field_value = field.value_from_object(instance) if field else None - - if field_name == 'group': - field_name = 'cluster_group' - return field_name, field_value - - def prepare_cloned_fields(instance): """ Compile an object's `clone_fields` list into a string of URL query parameters. Tags are automatically cloned where applicable. """ params = {} - for field in getattr(instance, 'clone_fields', []): - field_name, field_value = get_field_value(instance, field) + for field_name in getattr(instance, 'clone_fields', []): + field = instance._meta.get_field(field_name) + field_value = field.value_from_object(instance) # Swap out False with URL-friendly value if field_value is False: diff --git a/netbox/virtualization/fixtures/virtualization.json b/netbox/virtualization/fixtures/virtualization.json new file mode 100644 index 000000000..3c9537802 --- /dev/null +++ b/netbox/virtualization/fixtures/virtualization.json @@ -0,0 +1,170 @@ +[ +{ + "model": "virtualization.clustertype", + "pk": 1, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "Public Cloud", + "slug": "public-cloud" + } +}, +{ + "model": "virtualization.clustertype", + "pk": 2, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "vSphere", + "slug": "vsphere" + } +}, +{ + "model": "virtualization.clustertype", + "pk": 3, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "Hyper-V", + "slug": "hyper-v" + } +}, +{ + "model": "virtualization.clustertype", + "pk": 4, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "libvirt", + "slug": "libvirt" + } +}, +{ + "model": "virtualization.clustertype", + "pk": 5, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "LXD", + "slug": "lxd" + } +}, +{ + "model": "virtualization.clustertype", + "pk": 6, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "Docker", + "slug": "docker" + } +}, +{ + "model": "virtualization.clustergroup", + "pk": 1, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "VM Host", + "slug": "vm-host" + } +}, +{ + "model": "virtualization.cluster", + "pk": 1, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "Digital Ocean", + "type": 1, + "group": 1, + "tenant": null, + "site": null, + "comments": "" + } +}, +{ + "model": "virtualization.cluster", + "pk": 2, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "Amazon EC2", + "type": 1, + "group": 1, + "tenant": null, + "site": null, + "comments": "" + } +}, +{ + "model": "virtualization.cluster", + "pk": 3, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "Microsoft Azure", + "type": 1, + "group": 1, + "tenant": null, + "site": null, + "comments": "" + } +}, +{ + "model": "virtualization.cluster", + "pk": 4, + "fields": { + "created": "2016-08-01", + "last_updated": "2016-08-01T15:22:42.289Z", + "name": "vSphere Cluster", + "type": 2, + "group": 1, + "tenant": null, + "site": null, + "comments": "" + } +}, +{ + "model": "virtualization.virtualmachine", + "pk": 1, + "fields": { + "local_context_data": null, + "created": "2019-12-19", + "last_updated": "2019-12-19T05:24:19.146Z", + "cluster": 2, + "tenant": null, + "platform": null, + "name": "vm1", + "status": "active", + "role": null, + "primary_ip4": null, + "primary_ip6": null, + "vcpus": null, + "memory": null, + "disk": null, + "comments": "" + } +}, +{ + "model": "virtualization.virtualmachine", + "pk": 2, + "fields": { + "local_context_data": null, + "created": "2019-12-19", + "last_updated": "2019-12-19T05:24:41.478Z", + "cluster": 1, + "tenant": null, + "platform": null, + "name": "vm2", + "status": "active", + "role": null, + "primary_ip4": null, + "primary_ip6": null, + "vcpus": null, + "memory": null, + "disk": null, + "comments": "" + } +} +]