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
This commit is contained in:
Jason Novinger 2025-11-20 05:48:22 -06:00
parent ac74d9f9be
commit 560dcb6af1
4 changed files with 27 additions and 24 deletions

View File

@ -4,7 +4,7 @@ from django.utils.translation import gettext_lazy as _
from extras.choices import * from extras.choices import *
from users.models import Owner 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 utilities.forms.mixins import FilterModifierMixin
from .mixins import CustomFieldsMixin, SavedFiltersMixin 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 selector_fields: An iterable of names of fields to display by default when rendering the form as
a selector widget a selector widget
""" """
q = forms.CharField( q = QueryField(
required=False, required=False,
label=_('Search') label=_('Search')
) )

View File

@ -17,11 +17,20 @@ __all__ = (
'JSONField', 'JSONField',
'LaxURLField', 'LaxURLField',
'MACAddressField', 'MACAddressField',
'QueryField',
'SlugField', 'SlugField',
'TagFilterField', '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): class CommentField(forms.CharField):
""" """
A textarea with support for Markdown rendering. Exists mostly just to add a standard `help_text`. A textarea with support for Markdown rendering. Exists mostly just to add a standard `help_text`.

View File

@ -4,6 +4,7 @@ from django import forms
from django.utils.translation import gettext as _ from django.utils.translation import gettext as _
from netbox.models.features import ChangeLoggingMixin from netbox.models.features import ChangeLoggingMixin
from utilities.forms.fields import QueryField
from utilities.forms.mixins import BackgroundJobMixin, FilterModifierMixin from utilities.forms.mixins import BackgroundJobMixin, FilterModifierMixin
__all__ = ( __all__ = (
@ -144,7 +145,7 @@ class FilterForm(FilterModifierMixin, forms.Form):
""" """
Base Form class for FilterSet forms. Base Form class for FilterSet forms.
""" """
q = forms.CharField( q = QueryField(
required=False, required=False,
label=_('Search') label=_('Search')
) )

View File

@ -5,7 +5,7 @@ from django import forms
from django.core.validators import MaxValueValidator, MinValueValidator from django.core.validators import MaxValueValidator, MinValueValidator
from django.utils.translation import gettext_lazy as _ 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 from utilities.forms.widgets.modifiers import MODIFIER_EMPTY_FALSE, MODIFIER_EMPTY_TRUE
__all__ = ( __all__ = (
@ -181,10 +181,7 @@ class FilterModifierMixin:
filterset = filterset_class() if filterset_class else None filterset = filterset_class() if filterset_class else None
for field_name, field in self.fields.items(): for field_name, field in self.fields.items():
if self._should_skip_field(field_name, field): lookups = self._get_lookup_choices(field)
continue
lookups = self._get_lookup_choices(field, field_name)
# Verify lookups against FilterSet if available # Verify lookups against FilterSet if available
if filterset: if filterset:
@ -196,34 +193,30 @@ class FilterModifierMixin:
lookups=lookups lookups=lookups
) )
def _should_skip_field(self, field_name, field): def _get_lookup_choices(self, field):
"""Determine if a field should be skipped for enhancement.""" """Determine the available lookup choices for a given field.
# Skip the global search field
if field_name == 'q': Returns an empty list for fields that should not be enhanced.
return True """
# Skip query/search fields
if isinstance(field, QueryField):
return []
# Skip boolean fields (no benefit from modifiers) # Skip boolean fields (no benefit from modifiers)
if isinstance(field, (forms.BooleanField, forms.NullBooleanField)): if isinstance(field, (forms.BooleanField, forms.NullBooleanField)):
return True return []
# MultipleChoiceField and TagFilterField are now supported
# (no longer skipped)
# Skip API widget fields # Skip API widget fields
if self._is_api_widget_field(field): 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 # Walk up the MRO to find a known field type
for field_class in field.__class__.__mro__: for field_class in field.__class__.__mro__:
if field_class in FORM_FIELD_LOOKUPS: if field_class in FORM_FIELD_LOOKUPS:
return FORM_FIELD_LOOKUPS[field_class] return FORM_FIELD_LOOKUPS[field_class]
# Unknown field type - return single exact option (no enhancement) # Unknown field type - return empty list (no enhancement)
return [('exact', _('Is'))] return []
def _verify_lookups_with_filterset(self, field_name, lookups, filterset): def _verify_lookups_with_filterset(self, field_name, lookups, filterset):
"""Verify which lookups are actually supported by the FilterSet.""" """Verify which lookups are actually supported by the FilterSet."""