From 255e3881ddb862dd85c84a7079767441bd733b01 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Wed, 2 Jul 2025 08:41:51 -0400 Subject: [PATCH 1/3] Fixes #19800: ModuleType import supports associating ModuleTypeProfile --- netbox/dcim/forms/bulk_import.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/dcim/forms/bulk_import.py b/netbox/dcim/forms/bulk_import.py index 3ad4ced9148..ba6491cbe79 100644 --- a/netbox/dcim/forms/bulk_import.py +++ b/netbox/dcim/forms/bulk_import.py @@ -471,7 +471,7 @@ class Meta: model = ModuleType fields = [ 'manufacturer', 'model', 'part_number', 'description', 'airflow', 'weight', 'weight_unit', 'comments', - 'tags', + 'tags', 'profile', ] From 1984f523058d425a43f1c24404d38ad6be0687a3 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Wed, 2 Jul 2025 08:46:41 -0400 Subject: [PATCH 2/3] Fixes up ModuleTypeTestCase to include bulk import testing Also includes an additional regression assertion. --- netbox/dcim/tests/test_views.py | 78 ++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 15 deletions(-) diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index 69192f0a1ed..a9c6b12484e 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -3,17 +3,18 @@ from zoneinfo import ZoneInfo import yaml -from django.test import override_settings +from django.test import override_settings, tag from django.urls import reverse from netaddr import EUI +from core.models import ObjectType from dcim.choices import * from dcim.constants import * from dcim.models import * from ipam.models import ASN, RIR, VLAN, VRF from netbox.choices import CSVDelimiterChoices, ImportFormatChoices, WeightUnitChoices from tenancy.models import Tenant -from users.models import User +from users.models import ObjectPermission, User from utilities.testing import ViewTestCases, create_tags, create_test_device, post_data from wireless.models import WirelessLAN @@ -1000,18 +1001,7 @@ def test_export_objects(self): self.assertEqual(response.get('Content-Type'), 'text/csv; charset=utf-8') -# TODO: Change base class to PrimaryObjectViewTestCase -# Blocked by absence of bulk import view for ModuleTypes -class ModuleTypeTestCase( - ViewTestCases.GetObjectViewTestCase, - ViewTestCases.GetObjectChangelogViewTestCase, - ViewTestCases.CreateObjectViewTestCase, - ViewTestCases.EditObjectViewTestCase, - ViewTestCases.DeleteObjectViewTestCase, - ViewTestCases.ListObjectsViewTestCase, - ViewTestCases.BulkEditObjectsViewTestCase, - ViewTestCases.BulkDeleteObjectsViewTestCase -): +class ModuleTypeTestCase(ViewTestCases.PrimaryObjectViewTestCase): model = ModuleType @classmethod @@ -1023,7 +1013,7 @@ def setUpTestData(cls): ) Manufacturer.objects.bulk_create(manufacturers) - ModuleType.objects.bulk_create([ + module_types = ModuleType.objects.bulk_create([ ModuleType(model='Module Type 1', manufacturer=manufacturers[0]), ModuleType(model='Module Type 2', manufacturer=manufacturers[0]), ModuleType(model='Module Type 3', manufacturer=manufacturers[0]), @@ -1031,6 +1021,8 @@ def setUpTestData(cls): tags = create_tags('Alpha', 'Bravo', 'Charlie') + fan_module_type_profile = ModuleTypeProfile.objects.get(name='Fan') + cls.form_data = { 'manufacturer': manufacturers[1].pk, 'model': 'Device Type X', @@ -1044,6 +1036,62 @@ def setUpTestData(cls): 'part_number': '456DEF', } + cls.csv_data = ( + "manufacturer,model,part_number,comments,profile", + f"Manufacturer 1,fan0,generic-fan,,{fan_module_type_profile.name}" + ) + + cls.csv_update_data = ( + "id,model", + f"{module_types[0].id},test model", + ) + + def _set_module_type_bulk_import_permissions(self, **permission_definition): + obj_perm = ObjectPermission(**permission_definition) + obj_perm.save() + obj_perm.users.add(self.user) + additional_permissions = [ + ConsolePortTemplate, ConsoleServerPortTemplate, PowerPortTemplate, PowerOutletTemplate, + InterfaceTemplate, FrontPortTemplate, RearPortTemplate, ModuleBayTemplate, + ] + for model in additional_permissions: + obj_perm.object_types.add(ObjectType.objects.get_for_model(model)) + + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) + def test_bulk_update_objects_with_permission(self): + self._set_module_type_bulk_import_permissions( + name='Expanded ModuleType bulk import add permissions', actions=['add'] + ) + + # run base test + super().test_bulk_update_objects_with_permission() + + @tag('regression') + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[]) + def test_bulk_import_objects_with_permission(self): + self._set_module_type_bulk_import_permissions( + name='Expanded ModuleType bulk import add permissions', actions=['add'] + ) + + # run base test + super().test_bulk_import_objects_with_permission() + + # extra regression asserts + fan_module_type = ModuleType.objects.get(part_number='generic-fan') + fan_module_type_profile = ModuleTypeProfile.objects.get(name='Fan') + + assert fan_module_type.profile == fan_module_type_profile + + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[]) + def test_bulk_import_objects_with_constrained_permission(self): + self._set_module_type_bulk_import_permissions( + name='Expanded ModuleType bulk import add permissions', + constraints={'pk': 0}, # Dummy permission to deny all + actions=['add'] + ) + + super().test_bulk_import_objects_with_constrained_permission() + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) def test_moduletype_consoleports(self): moduletype = ModuleType.objects.first() From ac48272bc2ed20990997dc0ee07b9b197f256f51 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Mon, 14 Jul 2025 09:48:38 -0500 Subject: [PATCH 3/3] Address PR feedback I ultimately left the extra asserts in for test_bulk_import_objects_with_permissionsince since the parent test is currently only testing against number of objects successfully imported. Will file a follow up FR to improve that test. --- netbox/dcim/forms/bulk_import.py | 4 +-- netbox/dcim/tests/test_views.py | 51 ++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/netbox/dcim/forms/bulk_import.py b/netbox/dcim/forms/bulk_import.py index ba6491cbe79..1ea7890681c 100644 --- a/netbox/dcim/forms/bulk_import.py +++ b/netbox/dcim/forms/bulk_import.py @@ -470,8 +470,8 @@ class ModuleTypeImportForm(NetBoxModelImportForm): class Meta: model = ModuleType fields = [ - 'manufacturer', 'model', 'part_number', 'description', 'airflow', 'weight', 'weight_unit', 'comments', - 'tags', 'profile', + 'manufacturer', 'model', 'part_number', 'description', 'airflow', 'weight', 'weight_unit', 'profile', + 'comments', 'tags' ] diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index a9c6b12484e..28a2c11e2bc 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -7,14 +7,13 @@ from django.urls import reverse from netaddr import EUI -from core.models import ObjectType from dcim.choices import * from dcim.constants import * from dcim.models import * from ipam.models import ASN, RIR, VLAN, VRF from netbox.choices import CSVDelimiterChoices, ImportFormatChoices, WeightUnitChoices from tenancy.models import Tenant -from users.models import ObjectPermission, User +from users.models import User from utilities.testing import ViewTestCases, create_tags, create_test_device, post_data from wireless.models import WirelessLAN @@ -1046,21 +1045,17 @@ def setUpTestData(cls): f"{module_types[0].id},test model", ) - def _set_module_type_bulk_import_permissions(self, **permission_definition): - obj_perm = ObjectPermission(**permission_definition) - obj_perm.save() - obj_perm.users.add(self.user) - additional_permissions = [ - ConsolePortTemplate, ConsoleServerPortTemplate, PowerPortTemplate, PowerOutletTemplate, - InterfaceTemplate, FrontPortTemplate, RearPortTemplate, ModuleBayTemplate, - ] - for model in additional_permissions: - obj_perm.object_types.add(ObjectType.objects.get_for_model(model)) - @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) def test_bulk_update_objects_with_permission(self): - self._set_module_type_bulk_import_permissions( - name='Expanded ModuleType bulk import add permissions', actions=['add'] + self.add_permissions( + 'dcim.add_consoleporttemplate', + 'dcim.add_consoleserverporttemplate', + 'dcim.add_powerporttemplate', + 'dcim.add_poweroutlettemplate', + 'dcim.add_interfacetemplate', + 'dcim.add_frontporttemplate', + 'dcim.add_rearporttemplate', + 'dcim.add_modulebaytemplate', ) # run base test @@ -1069,14 +1064,21 @@ def test_bulk_update_objects_with_permission(self): @tag('regression') @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[]) def test_bulk_import_objects_with_permission(self): - self._set_module_type_bulk_import_permissions( - name='Expanded ModuleType bulk import add permissions', actions=['add'] + self.add_permissions( + 'dcim.add_consoleporttemplate', + 'dcim.add_consoleserverporttemplate', + 'dcim.add_powerporttemplate', + 'dcim.add_poweroutlettemplate', + 'dcim.add_interfacetemplate', + 'dcim.add_frontporttemplate', + 'dcim.add_rearporttemplate', + 'dcim.add_modulebaytemplate', ) # run base test super().test_bulk_import_objects_with_permission() - # extra regression asserts + # TODO: remove extra regression asserts once parent test supports testing all import fields fan_module_type = ModuleType.objects.get(part_number='generic-fan') fan_module_type_profile = ModuleTypeProfile.objects.get(name='Fan') @@ -1084,10 +1086,15 @@ def test_bulk_import_objects_with_permission(self): @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[]) def test_bulk_import_objects_with_constrained_permission(self): - self._set_module_type_bulk_import_permissions( - name='Expanded ModuleType bulk import add permissions', - constraints={'pk': 0}, # Dummy permission to deny all - actions=['add'] + self.add_permissions( + 'dcim.add_consoleporttemplate', + 'dcim.add_consoleserverporttemplate', + 'dcim.add_powerporttemplate', + 'dcim.add_poweroutlettemplate', + 'dcim.add_interfacetemplate', + 'dcim.add_frontporttemplate', + 'dcim.add_rearporttemplate', + 'dcim.add_modulebaytemplate', ) super().test_bulk_import_objects_with_constrained_permission()