Standardize the all-fields error key

netbox/netbox/api/serializers/bulk.py

@@ -48,9 +48,9 @@ class BulkOperationEntryErrorSerializer(serializers.Serializer):
     errors = serializers.DictField(
         help_text=_(
             "The errors for this entry, keyed by field name. Values are ordinarily arrays of "
-            "messages. Errors which do not pertain to a specific field appear under `__all__` "
-            "(model validation, protection rules, restricted tags) or `non_field_errors` (errors "
-            "concerning the shape of the entry itself)."
+            "messages. Errors which pertain to no particular field -- model validation, protection "
+            "rules, restricted tags, object-level permissions, or the shape of the entry itself -- "
+            "all appear under the single key `__all__`."
         )
     )
 

netbox/netbox/api/viewsets/mixins.py

@@ -438,7 +438,10 @@ class BulkCreateModelMixin:
             for i, item in enumerate(data):
                 if not isinstance(item, dict):
                     # Checked explicitly because get_serializer() infers many=True from a list, so a
-                    # nested list would otherwise be validated as a batch of its own
+                    # nested list would otherwise be validated as a batch of its own. This mirrors
+                    # the message REST framework itself reports for a non-dictionary item, hence its
+                    # key rather than a literal -- which NON_FIELD_ERRORS_KEY is configured to match
+                    # anyway, so that the API has a single key for non-field errors (see settings).
                     errors.append({
                         'index': i,
                         'errors': {

netbox/netbox/settings.py

@@ -805,6 +805,12 @@ REST_FRAMEWORK = {
     'DEFAULT_SCHEMA_CLASS': 'core.api.schema.NetBoxAutoSchema',
     'DEFAULT_VERSION': REST_FRAMEWORK_VERSION,
     'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.AcceptHeaderVersioning',
+    # Align REST framework's key for errors which pertain to no particular field with Django's
+    # (django.core.exceptions.NON_FIELD_ERRORS), so that the API reports such an error under one key
+    # rather than two. Model validation errors reach a response by way of full_clean(), and so are
+    # keyed by Django; errors raised by a serializer or field are keyed by REST framework. Without
+    # this, which of the two a client must read depends on the layer which rejected the request.
+    'NON_FIELD_ERRORS_KEY': '__all__',
     'SCHEMA_COERCE_METHOD_NAMES': {
         # Default mappings
         'retrieve': 'read',

netbox/netbox/tests/test_api.py

@@ -1,11 +1,13 @@
 import uuid
 
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import NON_FIELD_ERRORS
 from django.db.backends.postgresql.psycopg_any import NumericRange
 from django.test import RequestFactory, TestCase
 from django.urls import reverse
 from rest_framework.exceptions import ValidationError
 from rest_framework.request import Request
+from rest_framework.settings import api_settings
 
 from dcim.api.serializers import RackSerializer
 from dcim.models import Device, Site
@@ -51,6 +53,44 @@ class AppTestCase(APITestCase):
         self.assertEqual(response.data['id'], self.user.pk)
 
 
+class NonFieldErrorKeyTestCase(APITestCase):
+    """
+    REST framework's key for errors which pertain to no particular field is configured to match
+    Django's, so that the API reports such an error under `__all__` regardless of which layer
+    rejected the request (see REST_FRAMEWORK['NON_FIELD_ERRORS_KEY'] in settings). Model validation
+    errors are keyed by Django, having reached the response by way of full_clean(); errors raised by
+    a serializer or field are keyed by REST framework.
+    """
+    def setUp(self):
+        super().setUp()
+        self.add_permissions('dcim.add_site', 'dcim.view_site', 'dcim.change_site')
+        self.url = reverse('dcim-api:site-list')
+
+    def test_setting_matches_django(self):
+        self.assertEqual(api_settings.NON_FIELD_ERRORS_KEY, NON_FIELD_ERRORS)
+
+    def test_serializer_error_uses_all_key(self):
+        """An error from REST framework's own machinery (here, a non-dictionary item)."""
+        response = self.client.post(self.url, ['not an object'], format='json', **self.header)
+
+        self.assertEqual(response.status_code, 400)
+        self.assertIn(NON_FIELD_ERRORS, response.data['errors'][0]['errors'])
+
+    def test_model_validation_error_uses_all_key(self):
+        """An error from Django's full_clean(), which uses this key of its own accord."""
+        site = Site.objects.create(name='Site 1', slug='site-1')
+        # A Location's name must be unique within its Site, enforced by a model constraint
+        location_url = reverse('dcim-api:location-list')
+        self.add_permissions('dcim.add_location', 'dcim.view_location')
+        data = {'name': 'Location 1', 'slug': 'location-1', 'site': site.pk}
+        self.assertEqual(self.client.post(location_url, data, format='json', **self.header).status_code, 201)
+
+        response = self.client.post(location_url, data, format='json', **self.header)
+
+        self.assertEqual(response.status_code, 400)
+        self.assertIn(NON_FIELD_ERRORS, response.data)
+
+
 class RelatedObjectCountFieldTestCase(TestCase):
     """
     RelatedObjectCountFields are populated by annotations applied to a viewset's queryset, which are only