Kaynağa Gözat

Fixes #23154: Correct required=False mismatch on L2VPN.type and RackType.form_factor (#23175)

bctiemann 10 saat önce
ebeveyn
işleme
321a2fbf26

+ 3 - 0
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)
 

+ 0 - 1
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(

+ 22 - 0
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

+ 28 - 4
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

+ 1 - 1
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,

+ 20 - 0
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