From 321a2fbf26ebcba1beb3bcbaf110351e0f0ecd04 Mon Sep 17 00:00:00 2001 From: bctiemann Date: Tue, 15 Sep 2026 10:01:46 -0400 Subject: [PATCH] Fixes #23154: Correct required=False mismatch on L2VPN.type and RackType.form_factor (#23175) --- netbox/dcim/api/serializers_/racks.py | 3 +++ netbox/dcim/forms/bulk_import.py | 1 - netbox/dcim/tests/test_api.py | 22 ++++++++++++++++++ netbox/dcim/tests/test_views.py | 32 +++++++++++++++++++++++---- netbox/vpn/api/serializers_/l2vpn.py | 2 +- netbox/vpn/tests/test_api.py | 20 +++++++++++++++++ 6 files changed, 74 insertions(+), 6 deletions(-) diff --git a/netbox/dcim/api/serializers_/racks.py b/netbox/dcim/api/serializers_/racks.py index a088333e2..83b147607 100644 --- a/netbox/dcim/api/serializers_/racks.py +++ b/netbox/dcim/api/serializers_/racks.py @@ -86,6 +86,9 @@ class RackBaseSerializer(PrimaryModelSerializer): class RackTypeSerializer(RackBaseSerializer): + # Unlike Rack.form_factor (optional & nullable), RackType.form_factor is required + # (blank=False, no default), so override RackBaseSerializer's optional declaration. + form_factor = ChoiceField(choices=RackFormFactorChoices, required=True) manufacturer = ManufacturerSerializer(nested=True) rack_count = serializers.IntegerField(read_only=True) diff --git a/netbox/dcim/forms/bulk_import.py b/netbox/dcim/forms/bulk_import.py index edb416782..0097b2ffa 100644 --- a/netbox/dcim/forms/bulk_import.py +++ b/netbox/dcim/forms/bulk_import.py @@ -225,7 +225,6 @@ class RackTypeImportForm(PrimaryModelImportForm): form_factor = CSVChoiceField( label=_('Type'), choices=RackFormFactorChoices, - required=False, help_text=_('Form factor') ) starting_unit = forms.IntegerField( diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index 5ba406ee5..3e7287597 100644 --- a/netbox/dcim/tests/test_api.py +++ b/netbox/dcim/tests/test_api.py @@ -1206,6 +1206,28 @@ class RackTypeTestCase(APIViewTestCases.APIViewTestCase): }, ] + def test_form_factor_required(self): + """ + form_factor must be reported as required by OPTIONS, and a POST omitting it + must be rejected with a normal "required" validation error rather than a + model-level "cannot be blank" error. + """ + self.add_permissions('dcim.add_racktype') + + response = self.client.options(self._get_list_url(), **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + self.assertTrue(response.data['actions']['POST']['form_factor']['required']) + + manufacturer = Manufacturer.objects.first() + data = { + 'manufacturer': manufacturer.pk, + 'model': 'Rack Type Missing Form Factor', + 'slug': 'rack-type-missing-form-factor', + } + response = self.client.post(self._get_list_url(), data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data['form_factor'][0].code, 'required') + class RackTestCase(APIViewTestCases.APIViewTestCase): model = Rack diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index 374ad4a40..686e3e0a0 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -503,10 +503,10 @@ class RackTypeTestCase(ViewTestCases.PrimaryObjectViewTestCase): } cls.csv_data = ( - "manufacturer,model,slug,width,u_height,weight,max_weight,weight_unit", - "Manufacturer 1,RackType 4,rack-type-4,19,42,100,2000,kg", - "Manufacturer 1,RackType 5,rack-type-5,19,42,100,2000,kg", - "Manufacturer 1,RackType 6,rack-type-6,19,42,100,2000,kg", + "manufacturer,model,slug,form_factor,width,u_height,weight,max_weight,weight_unit", + f"Manufacturer 1,RackType 4,rack-type-4,{RackFormFactorChoices.TYPE_CABINET},19,42,100,2000,kg", + f"Manufacturer 1,RackType 5,rack-type-5,{RackFormFactorChoices.TYPE_CABINET},19,42,100,2000,kg", + f"Manufacturer 1,RackType 6,rack-type-6,{RackFormFactorChoices.TYPE_CABINET},19,42,100,2000,kg", ) cls.csv_update_data = ( @@ -531,6 +531,30 @@ class RackTypeTestCase(ViewTestCases.PrimaryObjectViewTestCase): 'comments': 'New comments', } + def test_bulk_import_objects_without_form_factor(self): + """ + A CSV import row omitting form_factor must be rejected, not silently saved + with form_factor=''. + """ + obj_perm = ObjectPermission(name='Test permission', actions=['add']) + obj_perm.save() + obj_perm.users.add(self.user) + obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + + initial_count = self._get_queryset().count() + csv_data = ( + "manufacturer,model,slug,width,u_height,weight,max_weight,weight_unit", + "Manufacturer 1,RackType Missing Form Factor,rack-type-missing-form-factor,19,42,100,2000,kg", + ) + data = { + 'data': '\n'.join(csv_data), + 'format': ImportFormatChoices.CSV, + 'csv_delimiter': CSVDelimiterChoices.AUTO, + } + response = self.client.post(self._get_url('bulk_import'), data) + self.assertHttpStatus(response, 200) + self.assertEqual(self._get_queryset().count(), initial_count) + class RackTestCase(ViewTestCases.PrimaryObjectViewTestCase): model = Rack diff --git a/netbox/vpn/api/serializers_/l2vpn.py b/netbox/vpn/api/serializers_/l2vpn.py index 874ae342e..b02fa958e 100644 --- a/netbox/vpn/api/serializers_/l2vpn.py +++ b/netbox/vpn/api/serializers_/l2vpn.py @@ -16,7 +16,7 @@ __all__ = ( class L2VPNSerializer(PrimaryModelSerializer): - type = ChoiceField(choices=L2VPNTypeChoices, required=False) + type = ChoiceField(choices=L2VPNTypeChoices, required=True) import_targets = SerializedPKRelatedField( queryset=RouteTarget.objects.all(), serializer=RouteTargetSerializer, diff --git a/netbox/vpn/tests/test_api.py b/netbox/vpn/tests/test_api.py index 25695bbf0..849eae3c5 100644 --- a/netbox/vpn/tests/test_api.py +++ b/netbox/vpn/tests/test_api.py @@ -601,6 +601,26 @@ class L2VPNTestCase(APIViewTestCases.APIViewTestCase): self.assertHttpStatus(response, status.HTTP_200_OK) self.assertEqual(response_data['count'], 1) + def test_type_required(self): + """ + type must be reported as required by OPTIONS, and a POST omitting it must + be rejected with a normal "required" validation error rather than a + model-level "cannot be blank" error. + """ + self.add_permissions('vpn.add_l2vpn') + + response = self.client.options(self._get_list_url(), **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + self.assertTrue(response.data['actions']['POST']['type']['required']) + + data = { + 'name': 'L2VPN Missing Type', + 'slug': 'l2vpn-missing-type', + } + response = self.client.post(self._get_list_url(), data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data['type'][0].code, 'required') + class L2VPNTerminationTestCase(APIViewTestCases.APIViewTestCase): model = L2VPNTermination