diff --git a/docs/customization/custom-fields.md b/docs/customization/custom-fields.md index eae35f3d0b2..6c8415d364d 100644 --- a/docs/customization/custom-fields.md +++ b/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": { + "cluster": { + "value": 1, + "label": "cluster1" + }, + "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: ```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. `"cluster": 1`), not the `{value, label}` object returned on read. diff --git a/netbox/extras/api/customfields.py b/netbox/extras/api/customfields.py index 9c9cb146e15..2c36870e4ca 100644 --- a/netbox/extras/api/customfields.py +++ b/netbox/extras/api/customfields.py @@ -74,6 +74,17 @@ def to_representation(self, obj): elif value is not None and cf.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: serializer = get_serializer_for_model(cf.related_object_type.model_class()) 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 return data diff --git a/netbox/extras/models/customfields.py b/netbox/extras/models/customfields.py index 31d417564f7..74f4aa2c610 100644 --- a/netbox/extras/models/customfields.py +++ b/netbox/extras/models/customfields.py @@ -823,7 +823,11 @@ def validate(self, value): # Validate all selected choices 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( _("Invalid choice(s) ({value}) for choice set {choiceset}.").format( value=value, diff --git a/netbox/extras/tests/test_customfields.py b/netbox/extras/tests/test_customfields.py index c7c633991bf..1e6b522c222 100644 --- a/netbox/extras/tests/test_customfields.py +++ b/netbox/extras/tests/test_customfields.py @@ -908,6 +908,24 @@ def setUpTestData(cls): } 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): TYPES = { CustomFieldTypeChoices.TYPE_TEXT: 'string', @@ -981,14 +999,86 @@ def test_get_single_object_with_custom_field_data(self): 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']['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( [obj['id'] for obj in response.data['custom_fields']['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): """ Create a new site with no specified custom field values and check that it received the default values. @@ -1017,8 +1107,8 @@ def test_create_single_object_with_defaults(self): 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['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( [obj['id'] for obj in response.data['custom_fields']['multiobject_field']], @@ -1082,8 +1172,8 @@ def test_create_single_object_with_values(self): self.assertEqual(response_cf['datetime_field'], data_cf['datetime_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['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( [obj['id'] for obj in response_cf['multiobject_field']], @@ -1148,8 +1238,8 @@ def test_create_multiple_objects_with_defaults(self): 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['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( [obj['id'] for obj in response_cf['multiobject_field']], @@ -1228,8 +1318,11 @@ def test_create_multiple_objects_with_values(self): 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['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( [obj['id'] for obj in response_cf['multiobject_field']], @@ -1282,8 +1375,8 @@ def test_update_single_object_with_values(self): self.assertEqual(response_cf['datetime_field'], original_cfvs['datetime_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['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.assertListEqual( [obj['id'] for obj in response_cf['multiobject_field']],