diff --git a/netbox/dcim/models/modules.py b/netbox/dcim/models/modules.py index 5f37b7ee1..313766aae 100644 --- a/netbox/dcim/models/modules.py +++ b/netbox/dcim/models/modules.py @@ -1,3 +1,5 @@ +from collections.abc import Iterable, Mapping + import jsonschema import yaml from django.core.exceptions import ValidationError @@ -170,7 +172,10 @@ class ModuleType(ImageAttachmentsMixin, PrimaryModel, WeightMixin): attrs = {} for name, options in self.profile.schema.get('properties', {}).items(): key = options.get('title', title(name)) - attrs[key] = self.attribute_data.get(name) + value = self.attribute_data.get(name) + if isinstance(value, Iterable) and not isinstance(value, (str, bytes, Mapping)): + value = ', '.join(str(v) for v in value) + attrs[key] = value return dict(sorted(attrs.items())) def clean(self): diff --git a/netbox/dcim/tests/query_counts.json b/netbox/dcim/tests/query_counts.json index 7861ee6b7..f2f2d2f6b 100644 --- a/netbox/dcim/tests/query_counts.json +++ b/netbox/dcim/tests/query_counts.json @@ -42,7 +42,7 @@ "modulebay:list_objects_with_permission": 21, "modulebaytemplate:api_list_objects": 11, "moduletype:api_list_objects": 14, - "moduletype:list_objects_with_permission": 21, + "moduletype:list_objects_with_permission": 22, "moduletypeprofile:api_list_objects": 13, "moduletypeprofile:list_objects_with_permission": 20, "platform:api_list_objects": 13, diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py index 2bb0d241d..7bf03075e 100644 --- a/netbox/dcim/tests/test_forms.py +++ b/netbox/dcim/tests/test_forms.py @@ -1,3 +1,6 @@ +from unittest.mock import patch + +from django import forms from django.test import TestCase from dcim.choices import ( @@ -177,6 +180,51 @@ class DeviceTestCase(TestCase): self.assertIn('position', form.errors) +class ModuleTypeFormTestCase(TestCase): + + @classmethod + def setUpTestData(cls): + cls.manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1') + cls.profile = ModuleTypeProfile.objects.create( + name='Module Type Profile 1', + schema={ + 'properties': { + 'media': { + 'title': 'Media', + 'type': 'array', + 'items': { + 'type': 'string', + 'enum': ['copper', 'sfp', 'qsfp28'], + }, + }, + }, + }, + ) + + def test_enum_array_attribute_uses_multiselect_field(self): + form = ModuleTypeForm(data={ + 'manufacturer': self.manufacturer.pk, + 'model': 'Module Type 1', + 'profile': self.profile.pk, + 'attr_media': ['copper', 'qsfp28'], + }) + + self.assertIsInstance(form.fields['attr_media'], forms.MultipleChoiceField) + self.assertEqual( + list(form.fields['attr_media'].choices), + [ + ('copper', 'copper'), + ('sfp', 'sfp'), + ('qsfp28', 'qsfp28'), + ], + ) + with patch('utilities.forms.fields.dynamic.get_action_url', return_value='/'): + self.assertTrue(form.is_valid(), form.errors) + + module_type = form.save() + self.assertEqual(module_type.attribute_data, {'media': ['copper', 'qsfp28']}) + + class VCPositionTokenFormTestCase(TestCase): @classmethod diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index dc3c2bd24..6f8139835 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -179,6 +179,45 @@ class ModuleTypeTestCase(TestCase): module_type.refresh_from_db() self.assertEqual(module_type.interface_template_count, 1) + def test_attributes(self): + """ + ModuleType.attributes should normalize iterable values into strings for presentation. + """ + manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1') + profile = ModuleTypeProfile.objects.create( + name='Module Type Profile 1', + schema={ + 'properties': { + 'media': { + 'title': 'Media', + 'type': 'array', + 'items': {'type': 'string'}, + }, + 'enabled': { + 'title': 'Enabled', + 'type': 'boolean', + }, + }, + }, + ) + module_type = ModuleType.objects.create( + manufacturer=manufacturer, + model='Module Type 1', + profile=profile, + attribute_data={ + 'media': ['sfp', 'qsfp28'], + 'enabled': True, + }, + ) + + self.assertEqual( + module_type.attributes, + { + 'Enabled': True, + 'Media': 'sfp, qsfp28', + }, + ) + class RackTypeTestCase(TestCase): diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index abbb222ab..dc32c7bd9 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -1226,6 +1226,19 @@ console-ports: class ModuleTypeTestCase(ViewTestCases.PrimaryObjectViewTestCase): model = ModuleType + SCHEMA = { + 'properties': { + 'media': { + 'title': 'Media', + 'type': 'array', + 'items': { + 'type': 'string', + 'enum': ['copper', 'sfp', 'qsfp28'], + }, + }, + }, + } + @classmethod def setUpTestData(cls): @@ -1235,8 +1248,15 @@ class ModuleTypeTestCase(ViewTestCases.PrimaryObjectViewTestCase): ) Manufacturer.objects.bulk_create(manufacturers) + profile = ModuleTypeProfile.objects.create(name='Module Type Profile 1', schema=cls.SCHEMA) + module_types = ModuleType.objects.bulk_create([ - ModuleType(model='Module Type 1', manufacturer=manufacturers[0]), + ModuleType( + model='Module Type 1', + manufacturer=manufacturers[0], + profile=profile, + attribute_data={'media': ['copper', 'qsfp28']}, + ), ModuleType(model='Module Type 2', manufacturer=manufacturers[0]), ModuleType(model='Module Type 3', manufacturer=manufacturers[0]), ]) @@ -1319,6 +1339,19 @@ class ModuleTypeTestCase(ViewTestCases.PrimaryObjectViewTestCase): super().test_bulk_import_objects_with_constrained_permission() + @tag('regression') + def test_get_object_renders_profile_attribute_lists(self): + self.add_permissions( + 'dcim.view_moduletype', + 'dcim.view_moduletypeprofile', + ) + moduletype = ModuleType.objects.first() + response = self.client.get(moduletype.get_absolute_url()) + + self.assertHttpStatus(response, 200) + self.assertContains(response, 'Media') + self.assertContains(response, 'copper, qsfp28') + def test_moduletype_consoleports(self): self.add_permissions('dcim.view_moduletype', 'dcim.view_consoleporttemplate') moduletype = ModuleType.objects.first() @@ -2558,14 +2591,33 @@ class ModuleTestCase( @classmethod def setUpTestData(cls): manufacturer = Manufacturer.objects.create(name='Generic', slug='generic') - module_type_profile = ModuleTypeProfile.objects.create(name='Module Type Profile 1') + module_type_profile = ModuleTypeProfile.objects.create( + name='Module Type Profile 1', + schema={ + 'properties': { + 'media': { + 'title': 'Media', + 'type': 'array', + 'items': { + 'type': 'string', + 'enum': ['copper', 'sfp', 'qsfp28'], + }, + }, + }, + }, + ) devices = ( create_test_device('Device 1'), create_test_device('Device 2'), ) module_types = ( - ModuleType(manufacturer=manufacturer, model='Module Type 1', profile=module_type_profile), + ModuleType( + manufacturer=manufacturer, + model='Module Type 1', + profile=module_type_profile, + attribute_data={'media': ['copper', 'qsfp28']}, + ), ModuleType(manufacturer=manufacturer, model='Module Type 2'), ModuleType(manufacturer=manufacturer, model='Module Type 3'), ModuleType(manufacturer=manufacturer, model='Module Type 4'), @@ -2634,6 +2686,19 @@ class ModuleTestCase( self.assertContains(response, 'Module Type Profile 1') + @tag('regression') + def test_module_detail_renders_module_type_attribute_lists(self): + self.add_permissions( + 'dcim.view_module', + 'dcim.view_moduletype', + 'dcim.view_moduletypeprofile', + ) + response = self.client.get(self._get_queryset().first().get_absolute_url()) + + self.assertHttpStatus(response, 200) + self.assertContains(response, 'Media') + self.assertContains(response, 'copper, qsfp28') + def test_module_component_replication(self): self.add_permissions( 'dcim.view_module', diff --git a/netbox/utilities/jsonschema.py b/netbox/utilities/jsonschema.py index 708e38307..a25e24f21 100644 --- a/netbox/utilities/jsonschema.py +++ b/netbox/utilities/jsonschema.py @@ -94,7 +94,9 @@ class JSONSchemaProperty: } # Choices - if self.enum: + if self.type == PropertyTypeEnum.ARRAY.value and (item_enums := self.items.get('enum')): + field_kwargs['choices'] = [(v, v) for v in item_enums] + elif self.enum: choices = [(v, v) for v in self.enum] if not required: choices = [(None, ''), *choices] @@ -102,8 +104,9 @@ class JSONSchemaProperty: # Arrays if self.type == PropertyTypeEnum.ARRAY.value: - items_type = self.items.get('type', PropertyTypeEnum.STRING.value) - field_kwargs['base_field'] = FORM_FIELDS[items_type]() + if not self.items.get('enum'): + items_type = self.items.get('type', PropertyTypeEnum.STRING.value) + field_kwargs['base_field'] = FORM_FIELDS[items_type]() # String validation if self.type == PropertyTypeEnum.STRING.value: @@ -135,6 +138,8 @@ class JSONSchemaProperty: """ Resolve the property's type (and string format, if specified) to the appropriate field class. """ + if self.type == PropertyTypeEnum.ARRAY.value and self.items.get('enum'): + return forms.MultipleChoiceField if self.enum: if self.type == PropertyTypeEnum.ARRAY.value: return forms.MultipleChoiceField diff --git a/netbox/utilities/tests/test_jsonschema.py b/netbox/utilities/tests/test_jsonschema.py new file mode 100644 index 000000000..2526bc84d --- /dev/null +++ b/netbox/utilities/tests/test_jsonschema.py @@ -0,0 +1,46 @@ +from django import forms +from django.contrib.postgres.forms import SimpleArrayField +from django.test import TestCase + +from utilities.jsonschema import JSONSchemaProperty + + +class JSONSchemaPropertyTestCase(TestCase): + + def test_array_enum_uses_multiple_choice_field(self): + prop = JSONSchemaProperty( + type='array', + title='Media', + items={ + 'type': 'string', + 'enum': ['copper', 'sfp', 'qsfp28'], + }, + ) + + field = prop.to_form_field('media') + + self.assertIsInstance(field, forms.MultipleChoiceField) + self.assertEqual( + list(field.choices), + [ + ('copper', 'copper'), + ('sfp', 'sfp'), + ('qsfp28', 'qsfp28'), + ], + ) + self.assertEqual(field.clean(['copper', 'qsfp28']), ['copper', 'qsfp28']) + + def test_plain_array_uses_simple_array_field(self): + prop = JSONSchemaProperty( + type='array', + title='Ports', + items={ + 'type': 'string', + }, + ) + + field = prop.to_form_field('ports') + + self.assertIsInstance(field, SimpleArrayField) + self.assertIsInstance(field.base_field, forms.CharField) + self.assertEqual(field.clean('ge-0/0/0,ge-0/0/1'), ['ge-0/0/0', 'ge-0/0/1'])