Pārlūkot izejas kodu

Fix filtering of object-type custom fields when "is empty" is selected (#21829)

bctiemann 6 dienas atpakaļ
vecāks
revīzija
49ba0dd495

+ 12 - 5
netbox/utilities/forms/widgets/modifiers.py

@@ -48,11 +48,13 @@ class FilterModifierWidget(forms.Widget):
             Just the value string for form validation. The modifier is reconstructed
             Just the value string for form validation. The modifier is reconstructed
             during rendering from the query parameter names.
             during rendering from the query parameter names.
         """
         """
-        # Special handling for empty - check if field__empty exists
+        # Special handling for empty modifier: return None so the underlying field does not
+        # attempt to validate 'true'/'false' as a field value (e.g. a model PK). The
+        # `__empty` query parameter is consumed directly by the filterset and by
+        # `applied_filters`, so no value from the field itself is needed here.
         empty_param = f"{name}__empty"
         empty_param = f"{name}__empty"
         if empty_param in data:
         if empty_param in data:
-            # Return the boolean value for empty lookup
-            return data.get(empty_param)
+            return None
 
 
         # Try exact field name first
         # Try exact field name first
         value = self.original_widget.value_from_datadict(data, files, name)
         value = self.original_widget.value_from_datadict(data, files, name)
@@ -113,8 +115,13 @@ class FilterModifierWidget(forms.Widget):
                     # Build a minimal choice list with just the selected values
                     # Build a minimal choice list with just the selected values
                     choices = []
                     choices = []
                     if pk_values:
                     if pk_values:
-                        selected_objects = original_choices.queryset.filter(pk__in=pk_values)
-                        choices = [(obj.pk, str(obj)) for obj in selected_objects]
+                        try:
+                            selected_objects = original_choices.queryset.filter(pk__in=pk_values)
+                            choices = [(obj.pk, str(obj)) for obj in selected_objects]
+                        except (ValueError, TypeError):
+                            # pk_values may contain non-PK strings (e.g. 'true'/'false' from the
+                            # empty modifier); silently skip rendering selected choices in that case.
+                            pass
 
 
                     # Re-add the "None" option if it was selected via the null choice value
                     # Re-add the "None" option if it was selected via the null choice value
                     if settings.FILTERS_NULL_CHOICE_VALUE in values:
                     if settings.FILTERS_NULL_CHOICE_VALUE in values:

+ 29 - 0
netbox/utilities/templatetags/helpers.py

@@ -491,6 +491,35 @@ def applied_filters(context, model, form, query_params):
             'link_text': link_text,
             'link_text': link_text,
         })
         })
 
 
+    # Handle empty modifier pills separately. `FilterModifierWidget.value_from_datadict()`
+    # returns None for fields with a `field__empty` query parameter so that the underlying
+    # form field does not attempt to validate 'true'/'false' as a real field value (which
+    # would raise a ValidationError for ModelChoiceField). Because the value is None, these
+    # fields never appear in `form.changed_data`, so we build their pills directly from the
+    # query parameters here.
+    for param_name, param_value in query_params.items():
+        if not param_name.endswith('__empty'):
+            continue
+        field_name = param_name[:-len('__empty')]
+        if field_name not in form.fields or field_name == 'filter_id':
+            continue
+
+        querydict = query_params.copy()
+        querydict.pop(param_name)
+        label = form.fields[field_name].label or field_name
+
+        if param_value.lower() in ('true', '1'):
+            link_text = f'{label} {_("is empty")}'
+        else:
+            link_text = f'{label} {_("is not empty")}'
+
+        applied_filters.append({
+            'name': param_name,
+            'value': param_value,
+            'link_url': f'?{querydict.urlencode()}',
+            'link_text': link_text,
+        })
+
     save_link = None
     save_link = None
     if user.has_perm('extras.add_savedfilter') and 'filter_id' not in context['request'].GET:
     if user.has_perm('extras.add_savedfilter') and 'filter_id' not in context['request'].GET:
         object_type = ObjectType.objects.get_for_model(model).pk
         object_type = ObjectType.objects.get_for_model(model).pk

+ 72 - 2
netbox/utilities/tests/test_filter_modifiers.py

@@ -6,8 +6,11 @@ from django.template import Context
 from django.test import RequestFactory, TestCase
 from django.test import RequestFactory, TestCase
 
 
 import dcim.filtersets  # noqa: F401 - Import to register Device filterset
 import dcim.filtersets  # noqa: F401 - Import to register Device filterset
-from dcim.forms.filtersets import DeviceFilterForm
-from dcim.models import Device
+from core.models import ObjectType
+from dcim.forms.filtersets import DeviceFilterForm, SiteFilterForm
+from dcim.models import Device, Manufacturer, Site
+from extras.choices import CustomFieldTypeChoices
+from extras.models import CustomField
 from netbox.filtersets import BaseFilterSet
 from netbox.filtersets import BaseFilterSet
 from tenancy.models import Tenant
 from tenancy.models import Tenant
 from users.models import User
 from users.models import User
@@ -338,3 +341,70 @@ class EmptyLookupTest(TestCase):
         self.assertGreater(len(result['applied_filters']), 0)
         self.assertGreater(len(result['applied_filters']), 0)
         filter_pill = result['applied_filters'][0]
         filter_pill = result['applied_filters'][0]
         self.assertIn('not empty', filter_pill['link_text'].lower())
         self.assertIn('not empty', filter_pill['link_text'].lower())
+
+
+class ObjectCustomFieldEmptyLookupTest(TestCase):
+    """
+    Regression test for https://github.com/netbox-community/netbox/issues/21535.
+
+    Rendering a filter form with an object-type custom field and the __empty modifier
+    must not raise a ValueError or produce a form validation error.
+    Filter pills must still appear for the empty modifier.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = User.objects.create(username='test_user_obj_cf')
+        site_type = ObjectType.objects.get_for_model(Site)
+        cf = CustomField(
+            name='test_obj_cf',
+            type=CustomFieldTypeChoices.TYPE_OBJECT,
+            related_object_type=ObjectType.objects.get_for_model(Manufacturer),
+        )
+        cf.save()
+        cf.object_types.set([site_type])
+
+    def _make_form_and_result(self, querystring):
+        query_params = QueryDict(querystring)
+        form = SiteFilterForm(query_params)
+        request = RequestFactory().get('/', query_params)
+        request.user = self.user
+        context = Context({'request': request})
+        result = applied_filters(context, Site, form, query_params)
+        return form, result
+
+    def test_render_form_with_empty_true_no_error(self):
+        """Rendering SiteFilterForm with cf__empty=true must not raise ValueError."""
+        query_params = QueryDict('cf_test_obj_cf__empty=true')
+        form = SiteFilterForm(query_params)
+        try:
+            str(form['cf_test_obj_cf'])
+        except ValueError as e:
+            self.fail(f"Rendering object-type custom field with __empty=true raised ValueError: {e}")
+
+    def test_render_form_with_empty_false_no_error(self):
+        """Rendering SiteFilterForm with cf__empty=false must not raise ValueError."""
+        query_params = QueryDict('cf_test_obj_cf__empty=false')
+        form = SiteFilterForm(query_params)
+        try:
+            str(form['cf_test_obj_cf'])
+        except ValueError as e:
+            self.fail(f"Rendering object-type custom field with __empty=false raised ValueError: {e}")
+
+    def test_no_validation_error_on_empty_true(self):
+        """The filter form must not have a validation error for the field when __empty=true."""
+        form, _ = self._make_form_and_result('cf_test_obj_cf__empty=true')
+        form.is_valid()
+        self.assertNotIn('cf_test_obj_cf', form.errors)
+
+    def test_filter_pill_appears_for_empty_true(self):
+        """A filter pill showing 'is empty' must be generated for an object-type CF with __empty=true."""
+        _, result = self._make_form_and_result('cf_test_obj_cf__empty=true')
+        self.assertGreater(len(result['applied_filters']), 0)
+        self.assertIn('empty', result['applied_filters'][0]['link_text'].lower())
+
+    def test_filter_pill_appears_for_empty_false(self):
+        """A filter pill showing 'is not empty' must be generated for an object-type CF with __empty=false."""
+        _, result = self._make_form_and_result('cf_test_obj_cf__empty=false')
+        self.assertGreater(len(result['applied_filters']), 0)
+        self.assertIn('not empty', result['applied_filters'][0]['link_text'].lower())