Selaa lähdekoodia

Closes #20897: Expose selection custom field labels in the REST API (#22475)

Arthur Hanson 6 päivää sitten
vanhempi
commit
e1f0c18c74

+ 24 - 0
docs/customization/custom-fields.md

@@ -100,6 +100,28 @@ When retrieving an object via the REST API, all of its custom data will be inclu
     ...
     ...
 ```
 ```
 
 
+Selection and multiple selection fields are returned as objects exposing both the stored value and its human-friendly label, following the same convention used by NetBox's built-in choice fields:
+
+```json
+    "custom_fields": {
+        "site_type": {
+            "value": "datacenter",
+            "label": "Data Center"
+        },
+        "regions": [
+            {
+                "value": "us-east",
+                "label": "US East"
+            },
+            {
+                "value": "us-west",
+                "label": "US West"
+            }
+        ]
+    },
+    ...
+```
+
 To set or change these values, simply include nested JSON data. For example:
 To set or change these values, simply include nested JSON data. For example:
 
 
 ```json
 ```json
@@ -111,3 +133,5 @@ To set or change these values, simply include nested JSON data. For example:
     }
     }
 }
 }
 ```
 ```
+
+As with built-in choice fields, selection custom fields are written by passing the raw value (e.g. `"site_type": "datacenter"`), not the `{value, label}` object returned on read.

+ 11 - 0
netbox/extras/api/customfields.py

@@ -74,6 +74,17 @@ class CustomFieldsDataField(Field):
             elif value is not None and cf.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT:
             elif value is not None and cf.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT:
                 serializer = get_serializer_for_model(cf.related_object_type.model_class())
                 serializer = get_serializer_for_model(cf.related_object_type.model_class())
                 value = serializer(value, nested=True, many=True, context=self.parent.context).data
                 value = serializer(value, nested=True, many=True, context=self.parent.context).data
+            elif value is not None and cf.type == CustomFieldTypeChoices.TYPE_SELECT:
+                # Represent the selected choice as an object with its value and resolved label
+                value = {
+                    'value': value,
+                    'label': cf.get_choice_label(value),
+                }
+            elif value is not None and cf.type == CustomFieldTypeChoices.TYPE_MULTISELECT:
+                # Represent each selected choice as an object with its value and resolved label
+                value = [
+                    {'value': v, 'label': cf.get_choice_label(v)} for v in value
+                ]
             data[cf.name] = value
             data[cf.name] = value
 
 
         return data
         return data

+ 5 - 1
netbox/extras/models/customfields.py

@@ -823,7 +823,11 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
 
 
             # Validate all selected choices
             # Validate all selected choices
             elif self.type == CustomFieldTypeChoices.TYPE_MULTISELECT:
             elif self.type == CustomFieldTypeChoices.TYPE_MULTISELECT:
-                if not set(value).issubset(self.choice_set.values):
+                # Require a list of valid string choices. The isinstance() check short-circuits the membership
+                # test so that non-string members (e.g. a client echoing back the {value, label} read
+                # representation) raise a ValidationError rather than an unhashable-type TypeError.
+                valid_values = set(self.choice_set.values)
+                if type(value) is not list or not all(isinstance(v, str) and v in valid_values for v in value):
                     raise ValidationError(
                     raise ValidationError(
                         _("Invalid choice(s) ({value}) for choice set {choiceset}.").format(
                         _("Invalid choice(s) ({value}) for choice set {choiceset}.").format(
                             value=value,
                             value=value,

+ 105 - 12
netbox/extras/tests/test_customfields.py

@@ -908,6 +908,24 @@ class CustomFieldAPITestCase(APITestCase):
         }
         }
         sites[1].save()
         sites[1].save()
 
 
+    # Labels for the choice set created in setUpTestData, used to build the expected
+    # API representation of selection custom fields ({'value': ..., 'label': ...}).
+    CHOICE_LABELS = {'foo': 'Foo', 'bar': 'Bar', 'baz': 'Baz'}
+
+    @classmethod
+    def _select(cls, value):
+        """Return the expected API representation of a single selection choice."""
+        if value is None:
+            return None
+        return {'value': value, 'label': cls.CHOICE_LABELS[value]}
+
+    @classmethod
+    def _multiselect(cls, values):
+        """Return the expected API representation of a multiple selection value."""
+        if values is None:
+            return None
+        return [cls._select(v) for v in values]
+
     def test_get_custom_fields(self):
     def test_get_custom_fields(self):
         TYPES = {
         TYPES = {
             CustomFieldTypeChoices.TYPE_TEXT: 'string',
             CustomFieldTypeChoices.TYPE_TEXT: 'string',
@@ -981,14 +999,86 @@ class CustomFieldAPITestCase(APITestCase):
         self.assertEqual(response.data['custom_fields']['datetime_field'], site2_cfvs['datetime_field'])
         self.assertEqual(response.data['custom_fields']['datetime_field'], site2_cfvs['datetime_field'])
         self.assertEqual(response.data['custom_fields']['url_field'], site2_cfvs['url_field'])
         self.assertEqual(response.data['custom_fields']['url_field'], site2_cfvs['url_field'])
         self.assertEqual(response.data['custom_fields']['json_field'], site2_cfvs['json_field'])
         self.assertEqual(response.data['custom_fields']['json_field'], site2_cfvs['json_field'])
-        self.assertEqual(response.data['custom_fields']['select_field'], site2_cfvs['select_field'])
-        self.assertEqual(response.data['custom_fields']['multiselect_field'], site2_cfvs['multiselect_field'])
+        self.assertEqual(response.data['custom_fields']['select_field'], self._select(site2_cfvs['select_field']))
+        self.assertEqual(
+            response.data['custom_fields']['multiselect_field'],
+            self._multiselect(site2_cfvs['multiselect_field'])
+        )
         self.assertEqual(response.data['custom_fields']['object_field']['id'], site2_cfvs['object_field'].pk)
         self.assertEqual(response.data['custom_fields']['object_field']['id'], site2_cfvs['object_field'].pk)
         self.assertEqual(
         self.assertEqual(
             [obj['id'] for obj in response.data['custom_fields']['multiobject_field']],
             [obj['id'] for obj in response.data['custom_fields']['multiobject_field']],
             [obj.pk for obj in site2_cfvs['multiobject_field']]
             [obj.pk for obj in site2_cfvs['multiobject_field']]
         )
         )
 
 
+    def test_get_object_selection_field_representation(self):
+        """
+        Selection custom fields are rendered as an object exposing both the stored value and its
+        human-friendly label on read access (see #20897).
+        """
+        site2 = Site.objects.get(name='Site 2')
+        url = reverse('dcim-api:site-detail', kwargs={'pk': site2.pk})
+        self.add_permissions('dcim.view_site')
+
+        response = self.client.get(url, **self.header)
+
+        # A single selection value is rendered as a {value, label} object
+        self.assertEqual(response.data['custom_fields']['select_field'], {
+            'value': 'bar',
+            'label': 'Bar',
+        })
+
+        # A multiple selection value is rendered as a list of {value, label} objects
+        self.assertEqual(response.data['custom_fields']['multiselect_field'], [
+            {'value': 'bar', 'label': 'Bar'},
+            {'value': 'baz', 'label': 'Baz'},
+        ])
+
+    def test_get_object_selection_field_unresolved_label(self):
+        """
+        A stored selection value with no matching choice falls back to using the raw value as its label.
+        """
+        site2 = Site.objects.get(name='Site 2')
+        site2.custom_field_data['select_field'] = 'stale'
+        site2.save()
+        url = reverse('dcim-api:site-detail', kwargs={'pk': site2.pk})
+        self.add_permissions('dcim.view_site')
+
+        response = self.client.get(url, **self.header)
+        self.assertEqual(response.data['custom_fields']['select_field'], {
+            'value': 'stale',
+            'label': 'stale',
+        })
+
+    @tag('regression')
+    def test_update_selection_field_rejects_read_format(self):
+        """
+        Selection fields are written by passing the raw value; submitting the {value, label} read
+        representation must be rejected with a clean 400, not a 500 (see #20897).
+        """
+        site2 = Site.objects.get(name='Site 2')
+        url = reverse('dcim-api:site-detail', kwargs={'pk': site2.pk})
+        self.add_permissions('dcim.change_site')
+
+        # A single selection submitted as an object is rejected
+        response = self.client.patch(
+            url, {'custom_fields': {'select_field': {'value': 'foo', 'label': 'Foo'}}}, format='json', **self.header
+        )
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+
+        # A multiple selection submitted as a list of objects is rejected (must not raise a TypeError/500)
+        response = self.client.patch(
+            url,
+            {'custom_fields': {'multiselect_field': [{'value': 'foo', 'label': 'Foo'}]}},
+            format='json',
+            **self.header
+        )
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+
+        # The stored values are unchanged
+        site2.refresh_from_db()
+        self.assertEqual(site2.custom_field_data['select_field'], 'bar')
+        self.assertEqual(site2.custom_field_data['multiselect_field'], ['bar', 'baz'])
+
     def test_create_single_object_with_defaults(self):
     def test_create_single_object_with_defaults(self):
         """
         """
         Create a new site with no specified custom field values and check that it received the default values.
         Create a new site with no specified custom field values and check that it received the default values.
@@ -1017,8 +1107,8 @@ class CustomFieldAPITestCase(APITestCase):
         self.assertEqual(response_cf['datetime_field'].isoformat(), cf_defaults['datetime_field'])
         self.assertEqual(response_cf['datetime_field'].isoformat(), cf_defaults['datetime_field'])
         self.assertEqual(response_cf['url_field'], cf_defaults['url_field'])
         self.assertEqual(response_cf['url_field'], cf_defaults['url_field'])
         self.assertEqual(response_cf['json_field'], cf_defaults['json_field'])
         self.assertEqual(response_cf['json_field'], cf_defaults['json_field'])
-        self.assertEqual(response_cf['select_field'], cf_defaults['select_field'])
-        self.assertEqual(response_cf['multiselect_field'], cf_defaults['multiselect_field'])
+        self.assertEqual(response_cf['select_field'], self._select(cf_defaults['select_field']))
+        self.assertEqual(response_cf['multiselect_field'], self._multiselect(cf_defaults['multiselect_field']))
         self.assertEqual(response_cf['object_field']['id'], cf_defaults['object_field'])
         self.assertEqual(response_cf['object_field']['id'], cf_defaults['object_field'])
         self.assertEqual(
         self.assertEqual(
             [obj['id'] for obj in response.data['custom_fields']['multiobject_field']],
             [obj['id'] for obj in response.data['custom_fields']['multiobject_field']],
@@ -1082,8 +1172,8 @@ class CustomFieldAPITestCase(APITestCase):
         self.assertEqual(response_cf['datetime_field'], data_cf['datetime_field'])
         self.assertEqual(response_cf['datetime_field'], data_cf['datetime_field'])
         self.assertEqual(response_cf['url_field'], data_cf['url_field'])
         self.assertEqual(response_cf['url_field'], data_cf['url_field'])
         self.assertEqual(response_cf['json_field'], data_cf['json_field'])
         self.assertEqual(response_cf['json_field'], data_cf['json_field'])
-        self.assertEqual(response_cf['select_field'], data_cf['select_field'])
-        self.assertEqual(response_cf['multiselect_field'], data_cf['multiselect_field'])
+        self.assertEqual(response_cf['select_field'], self._select(data_cf['select_field']))
+        self.assertEqual(response_cf['multiselect_field'], self._multiselect(data_cf['multiselect_field']))
         self.assertEqual(response_cf['object_field']['id'], data_cf['object_field'])
         self.assertEqual(response_cf['object_field']['id'], data_cf['object_field'])
         self.assertEqual(
         self.assertEqual(
             [obj['id'] for obj in response_cf['multiobject_field']],
             [obj['id'] for obj in response_cf['multiobject_field']],
@@ -1148,8 +1238,8 @@ class CustomFieldAPITestCase(APITestCase):
             self.assertEqual(response_cf['datetime_field'].isoformat(), cf_defaults['datetime_field'])
             self.assertEqual(response_cf['datetime_field'].isoformat(), cf_defaults['datetime_field'])
             self.assertEqual(response_cf['url_field'], cf_defaults['url_field'])
             self.assertEqual(response_cf['url_field'], cf_defaults['url_field'])
             self.assertEqual(response_cf['json_field'], cf_defaults['json_field'])
             self.assertEqual(response_cf['json_field'], cf_defaults['json_field'])
-            self.assertEqual(response_cf['select_field'], cf_defaults['select_field'])
-            self.assertEqual(response_cf['multiselect_field'], cf_defaults['multiselect_field'])
+            self.assertEqual(response_cf['select_field'], self._select(cf_defaults['select_field']))
+            self.assertEqual(response_cf['multiselect_field'], self._multiselect(cf_defaults['multiselect_field']))
             self.assertEqual(response_cf['object_field']['id'], cf_defaults['object_field'])
             self.assertEqual(response_cf['object_field']['id'], cf_defaults['object_field'])
             self.assertEqual(
             self.assertEqual(
                 [obj['id'] for obj in response_cf['multiobject_field']],
                 [obj['id'] for obj in response_cf['multiobject_field']],
@@ -1228,8 +1318,11 @@ class CustomFieldAPITestCase(APITestCase):
             self.assertEqual(response_cf['datetime_field'], custom_field_data['datetime_field'])
             self.assertEqual(response_cf['datetime_field'], custom_field_data['datetime_field'])
             self.assertEqual(response_cf['url_field'], custom_field_data['url_field'])
             self.assertEqual(response_cf['url_field'], custom_field_data['url_field'])
             self.assertEqual(response_cf['json_field'], custom_field_data['json_field'])
             self.assertEqual(response_cf['json_field'], custom_field_data['json_field'])
-            self.assertEqual(response_cf['select_field'], custom_field_data['select_field'])
-            self.assertEqual(response_cf['multiselect_field'], custom_field_data['multiselect_field'])
+            self.assertEqual(response_cf['select_field'], self._select(custom_field_data['select_field']))
+            self.assertEqual(
+                response_cf['multiselect_field'],
+                self._multiselect(custom_field_data['multiselect_field'])
+            )
             self.assertEqual(response_cf['object_field']['id'], custom_field_data['object_field'])
             self.assertEqual(response_cf['object_field']['id'], custom_field_data['object_field'])
             self.assertEqual(
             self.assertEqual(
                 [obj['id'] for obj in response_cf['multiobject_field']],
                 [obj['id'] for obj in response_cf['multiobject_field']],
@@ -1282,8 +1375,8 @@ class CustomFieldAPITestCase(APITestCase):
         self.assertEqual(response_cf['datetime_field'], original_cfvs['datetime_field'])
         self.assertEqual(response_cf['datetime_field'], original_cfvs['datetime_field'])
         self.assertEqual(response_cf['url_field'], original_cfvs['url_field'])
         self.assertEqual(response_cf['url_field'], original_cfvs['url_field'])
         self.assertEqual(response_cf['json_field'], original_cfvs['json_field'])
         self.assertEqual(response_cf['json_field'], original_cfvs['json_field'])
-        self.assertEqual(response_cf['select_field'], original_cfvs['select_field'])
-        self.assertEqual(response_cf['multiselect_field'], original_cfvs['multiselect_field'])
+        self.assertEqual(response_cf['select_field'], self._select(original_cfvs['select_field']))
+        self.assertEqual(response_cf['multiselect_field'], self._multiselect(original_cfvs['multiselect_field']))
         self.assertEqual(response_cf['object_field']['id'], original_cfvs['object_field'].pk)
         self.assertEqual(response_cf['object_field']['id'], original_cfvs['object_field'].pk)
         self.assertListEqual(
         self.assertListEqual(
             [obj['id'] for obj in response_cf['multiobject_field']],
             [obj['id'] for obj in response_cf['multiobject_field']],