From 560dcb6af1eed0bdf2ed746bfb6860778c1e5140 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Thu, 20 Nov 2025 05:48:22 -0600 Subject: [PATCH] Address PR feedback: Refactor and consolidate field filtering logic Consolidated field enhancement logic in FilterModifierMixin by: - Creating QueryField marker type (CharField subclass) for search fields - Updating FilterForm and NetBoxModelFilterSetForm to use QueryField for 'q' - Moving all skip logic into _get_lookup_choices() to return empty list for fields that shouldn't be enhanced - Removing separate _should_skip_field() method - Removing unused field_name parameter from _get_lookup_choices() - Replacing hardcoded field name check ('q') with type-based detection --- netbox/netbox/forms/filtersets.py | 4 +-- netbox/utilities/forms/fields/fields.py | 9 +++++++ netbox/utilities/forms/forms.py | 3 ++- netbox/utilities/forms/mixins.py | 35 ++++++++++--------------- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/netbox/netbox/forms/filtersets.py b/netbox/netbox/forms/filtersets.py index 61cc7ba9d..18c4ef548 100644 --- a/netbox/netbox/forms/filtersets.py +++ b/netbox/netbox/forms/filtersets.py @@ -4,7 +4,7 @@ from django.utils.translation import gettext_lazy as _ from extras.choices import * from users.models import Owner -from utilities.forms.fields import DynamicModelChoiceField +from utilities.forms.fields import DynamicModelChoiceField, QueryField from utilities.forms.mixins import FilterModifierMixin from .mixins import CustomFieldsMixin, SavedFiltersMixin @@ -28,7 +28,7 @@ class NetBoxModelFilterSetForm(FilterModifierMixin, CustomFieldsMixin, SavedFilt selector_fields: An iterable of names of fields to display by default when rendering the form as a selector widget """ - q = forms.CharField( + q = QueryField( required=False, label=_('Search') ) diff --git a/netbox/utilities/forms/fields/fields.py b/netbox/utilities/forms/fields/fields.py index 4e19e60a2..560ef25e9 100644 --- a/netbox/utilities/forms/fields/fields.py +++ b/netbox/utilities/forms/fields/fields.py @@ -17,11 +17,20 @@ __all__ = ( 'JSONField', 'LaxURLField', 'MACAddressField', + 'QueryField', 'SlugField', 'TagFilterField', ) +class QueryField(forms.CharField): + """ + A CharField subclass used for global search/query fields in filter forms. + This field type signals to FilterModifierMixin to skip enhancement with lookup modifiers. + """ + pass + + class CommentField(forms.CharField): """ A textarea with support for Markdown rendering. Exists mostly just to add a standard `help_text`. diff --git a/netbox/utilities/forms/forms.py b/netbox/utilities/forms/forms.py index bee050719..e7fc61b7e 100644 --- a/netbox/utilities/forms/forms.py +++ b/netbox/utilities/forms/forms.py @@ -4,6 +4,7 @@ from django import forms from django.utils.translation import gettext as _ from netbox.models.features import ChangeLoggingMixin +from utilities.forms.fields import QueryField from utilities.forms.mixins import BackgroundJobMixin, FilterModifierMixin __all__ = ( @@ -144,7 +145,7 @@ class FilterForm(FilterModifierMixin, forms.Form): """ Base Form class for FilterSet forms. """ - q = forms.CharField( + q = QueryField( required=False, label=_('Search') ) diff --git a/netbox/utilities/forms/mixins.py b/netbox/utilities/forms/mixins.py index 7a5ae7322..b6b3b7dac 100644 --- a/netbox/utilities/forms/mixins.py +++ b/netbox/utilities/forms/mixins.py @@ -5,7 +5,7 @@ from django import forms from django.core.validators import MaxValueValidator, MinValueValidator from django.utils.translation import gettext_lazy as _ -from utilities.forms.fields import ColorField, TagFilterField +from utilities.forms.fields import ColorField, QueryField, TagFilterField from utilities.forms.widgets.modifiers import MODIFIER_EMPTY_FALSE, MODIFIER_EMPTY_TRUE __all__ = ( @@ -181,10 +181,7 @@ class FilterModifierMixin: filterset = filterset_class() if filterset_class else None for field_name, field in self.fields.items(): - if self._should_skip_field(field_name, field): - continue - - lookups = self._get_lookup_choices(field, field_name) + lookups = self._get_lookup_choices(field) # Verify lookups against FilterSet if available if filterset: @@ -196,34 +193,30 @@ class FilterModifierMixin: lookups=lookups ) - def _should_skip_field(self, field_name, field): - """Determine if a field should be skipped for enhancement.""" - # Skip the global search field - if field_name == 'q': - return True + def _get_lookup_choices(self, field): + """Determine the available lookup choices for a given field. + + Returns an empty list for fields that should not be enhanced. + """ + # Skip query/search fields + if isinstance(field, QueryField): + return [] # Skip boolean fields (no benefit from modifiers) if isinstance(field, (forms.BooleanField, forms.NullBooleanField)): - return True - - # MultipleChoiceField and TagFilterField are now supported - # (no longer skipped) + return [] # Skip API widget fields if self._is_api_widget_field(field): - return True + return [] - return False - - def _get_lookup_choices(self, field, field_name=None): - """Determine the available lookup choices for a given field.""" # Walk up the MRO to find a known field type for field_class in field.__class__.__mro__: if field_class in FORM_FIELD_LOOKUPS: return FORM_FIELD_LOOKUPS[field_class] - # Unknown field type - return single exact option (no enhancement) - return [('exact', _('Is'))] + # Unknown field type - return empty list (no enhancement) + return [] def _verify_lookups_with_filterset(self, field_name, lookups, filterset): """Verify which lookups are actually supported by the FilterSet."""