bctiemann 2 недель назад
Родитель
Сommit
0984be8c04

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

@@ -135,3 +135,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.
+
+The GraphQL API's `custom_fields` field resolves selection and multiple selection values to the same `{value, label}` representation.

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

@@ -76,17 +76,8 @@ class CustomFieldsDataField(Field):
             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
-                ]
+            elif cf.type in (CustomFieldTypeChoices.TYPE_SELECT, CustomFieldTypeChoices.TYPE_MULTISELECT):
+                value = cf.resolve_selection_value(value)
             data[cf.name] = value
 
         return data

+ 7 - 3
netbox/extras/graphql/mixins.py

@@ -4,7 +4,7 @@ import strawberry
 import strawberry_django
 from strawberry.types import Info
 
-from extras.models import ImageAttachment, JournalEntry
+from extras.models import CustomField, ImageAttachment, JournalEntry
 from utilities.querysets import RestrictedPrefetch
 
 __all__ = (
@@ -50,9 +50,13 @@ class ConfigContextMixin:
 @strawberry.type
 class CustomFieldsMixin:
 
-    @strawberry_django.field
+    @strawberry_django.field(only=['custom_field_data'])
     def custom_fields(self) -> strawberry.scalars.JSON:
-        return self.custom_field_data
+        data = dict(self.custom_field_data)
+        for cf in CustomField.objects.get_for_model(type(self)):
+            if cf.name in data:
+                data[cf.name] = cf.resolve_selection_value(data[cf.name])
+        return data
 
 
 @strawberry.type

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

@@ -77,7 +77,9 @@ class CustomFieldManager(models.Manager.from_queryset(RestrictedQuerySet)):
                 return custom_fields
 
         content_type = ObjectType.objects.get_for_model(model._meta.concrete_model)
-        custom_fields = self.get_queryset().filter(object_types=content_type).select_related('related_object_type')
+        custom_fields = self.get_queryset().filter(object_types=content_type).select_related(
+            'related_object_type', 'choice_set'
+        )
 
         # Populate the request cache to avoid redundant lookups
         if cache is not None:
@@ -328,6 +330,20 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
             return self.choice_set.get_choice_color(value)
         return None
 
+    def resolve_selection_value(self, value):
+        """
+        For a Selection or Multiple selection field, wrap the value(s) with their resolved label as
+        {'value': ..., 'label': ...} (a list thereof for multi-select). Other field types pass through
+        unchanged. Shared by the REST API and GraphQL so selection labels resolve consistently (#20897).
+        """
+        if value is None:
+            return value
+        if self.type == CustomFieldTypeChoices.TYPE_SELECT:
+            return {'value': value, 'label': self.get_choice_label(value)}
+        if self.type == CustomFieldTypeChoices.TYPE_MULTISELECT:
+            return [{'value': v, 'label': self.get_choice_label(v)} for v in value]
+        return value
+
     def populate_initial_data(self, content_types):
         """
         Populate initial custom field data upon either a) the creation of a new CustomField, or

+ 84 - 0
netbox/extras/tests/test_customfields.py

@@ -3,7 +3,9 @@ import json
 from decimal import Decimal
 
 from django.core.exceptions import ValidationError
+from django.db import connection
 from django.test import override_settings, tag
+from django.test.utils import CaptureQueriesContext
 from django.urls import reverse
 from rest_framework import status
 
@@ -15,6 +17,7 @@ from extras.choices import *
 from extras.models import CustomField, CustomFieldChoiceSet
 from ipam.models import VLAN
 from netbox.choices import CSVDelimiterChoices, ImportFormatChoices
+from netbox.context import query_cache
 from netbox.tables.columns import CustomFieldColumn
 from utilities.testing import APITestCase, TestCase
 from virtualization.models import VirtualMachine
@@ -1135,6 +1138,87 @@ class CustomFieldAPITestCase(APITestCase):
             'label': 'stale',
         })
 
+    def test_graphql_selection_field_representation_matches_rest(self):
+        site2 = Site.objects.get(name='Site 2')
+        self.add_permissions('dcim.view_site')
+
+        query = f'{{ site(id: {site2.pk}) {{ custom_fields }} }}'
+        response = self.client.post(reverse('graphql'), data={'query': query}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        data = json.loads(response.content)
+        self.assertNotIn('errors', data)
+        custom_fields = data['data']['site']['custom_fields']
+
+        self.assertEqual(custom_fields['select_field'], self._select('bar'))
+        self.assertEqual(custom_fields['multiselect_field'], self._multiselect(['bar', 'baz']))
+
+    def test_graphql_selection_field_unresolved_label(self):
+        site2 = Site.objects.get(name='Site 2')
+        site2.custom_field_data['select_field'] = 'stale'
+        site2.save()
+        self.add_permissions('dcim.view_site')
+
+        query = f'{{ site(id: {site2.pk}) {{ custom_fields }} }}'
+        response = self.client.post(reverse('graphql'), data={'query': query}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        data = json.loads(response.content)
+        self.assertNotIn('errors', data)
+        self.assertEqual(data['data']['site']['custom_fields']['select_field'], {
+            'value': 'stale',
+            'label': 'stale',
+        })
+
+    def test_graphql_non_selection_fields_pass_through_unchanged(self):
+        site2 = Site.objects.get(name='Site 2')
+        self.add_permissions('dcim.view_site')
+
+        query = f'{{ site(id: {site2.pk}) {{ custom_fields }} }}'
+        response = self.client.post(reverse('graphql'), data={'query': query}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        data = json.loads(response.content)
+        self.assertNotIn('errors', data)
+        custom_fields = data['data']['site']['custom_fields']
+
+        self.assertEqual(custom_fields['text_field'], 'bar')
+        self.assertEqual(custom_fields['integer_field'], 456)
+        self.assertEqual(custom_fields['boolean_field'], True)
+
+    def test_graphql_selection_field_list_query_is_not_n_plus_one(self):
+        self.add_permissions('dcim.view_site')
+        query = '{ site_list { custom_fields } }'
+
+        Site.objects.bulk_create([Site(name=f'Site {i}', slug=f'site-{i}') for i in range(3, 8)])
+        # Prime process-level caches (e.g. ContentType) outside the measured request.
+        self.client.post(reverse('graphql'), data={'query': query}, format='json', **self.header)
+        with CaptureQueriesContext(connection) as ctx:
+            response = self.client.post(reverse('graphql'), data={'query': query}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        data = json.loads(response.content)
+        self.assertNotIn('errors', data)
+        self.assertEqual(len(data['data']['site_list']), 7)
+        baseline_query_count = len(ctx.captured_queries)
+
+        Site.objects.bulk_create([Site(name=f'Site {i}', slug=f'site-{i}') for i in range(8, 13)])
+        with CaptureQueriesContext(connection) as ctx:
+            response = self.client.post(reverse('graphql'), data={'query': query}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        data = json.loads(response.content)
+        self.assertNotIn('errors', data)
+        self.assertEqual(len(data['data']['site_list']), 12)
+
+        self.assertEqual(
+            len(ctx.captured_queries), baseline_query_count,
+            "custom_fields label resolution should not scale with the number of objects returned"
+        )
+
+    def test_get_for_model_select_related_choice_set(self):
+        query_cache.set(None)
+        custom_fields = list(CustomField.objects.get_for_model(Site))
+        with self.assertNumQueries(0):
+            resolved = {cf.name: cf.resolve_selection_value(cf.default) for cf in custom_fields}
+        self.assertEqual(resolved['select_field'], self._select('foo'))
+        self.assertEqual(resolved['multiselect_field'], self._multiselect(['foo']))
+
     @tag('regression')
     def test_update_selection_field_rejects_read_format(self):
         """