diff --git a/netbox/utilities/forms/widgets/modifiers.py b/netbox/utilities/forms/widgets/modifiers.py index e289adc97..1fb19e1db 100644 --- a/netbox/utilities/forms/widgets/modifiers.py +++ b/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 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" if empty_param in data: - # Return the boolean value for empty lookup - return data.get(empty_param) + return None # Try exact field name first 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 choices = [] 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 if settings.FILTERS_NULL_CHOICE_VALUE in values: diff --git a/netbox/utilities/templatetags/helpers.py b/netbox/utilities/templatetags/helpers.py index fbe2d9d12..945e442de 100644 --- a/netbox/utilities/templatetags/helpers.py +++ b/netbox/utilities/templatetags/helpers.py @@ -491,6 +491,35 @@ def applied_filters(context, model, form, query_params): '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 if user.has_perm('extras.add_savedfilter') and 'filter_id' not in context['request'].GET: object_type = ObjectType.objects.get_for_model(model).pk diff --git a/netbox/utilities/tests/test_filter_modifiers.py b/netbox/utilities/tests/test_filter_modifiers.py index 0bc6eb4c3..4bf2e88ab 100644 --- a/netbox/utilities/tests/test_filter_modifiers.py +++ b/netbox/utilities/tests/test_filter_modifiers.py @@ -6,8 +6,11 @@ from django.template import Context from django.test import RequestFactory, TestCase 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 tenancy.models import Tenant from users.models import User @@ -338,3 +341,70 @@ class EmptyLookupTest(TestCase): self.assertGreater(len(result['applied_filters']), 0) filter_pill = result['applied_filters'][0] 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())