-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Closes #7604: Add filter modifier dropdowns for advanced lookup operators #20747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature
Are you sure you want to change the base?
Conversation
Implements dynamic filter modifier UI that allows users to select lookup operators (exact, contains, starts with, regex, negation, empty/not empty) directly in filter forms without manual URL parameter editing. Supports filters for all scalar types and strings, as well as some related object filters. Explicitly does not support filters on fields that use APIWidget. That has been broken out in to follow up work. **Backend:** - FilterModifierWidget: Wraps form widgets with lookup modifier dropdown - FilterModifierMixin: Auto-enhances filterset fields with appropriate lookups - Extended lookup support: Adds negation (n), regex, iregex, empty_true/false lookups - Field-type-aware: CharField gets text lookups, IntegerField gets comparison operators, etc. **Frontend:** - TypeScript handler syncs modifier dropdown with URL parameters - Dynamically updates form field names (serial → serial__ic) on modifier change - Flexible-width modifier dropdowns with semantic CSS classes
d1930b8 to
cebb92b
Compare
cebb92b to
5913ea8
Compare
Enable filter modifiers for single-choice ChoiceFields in addition to the existing MultipleChoiceField support. ChoiceFields can now display modifier dropdowns with "Is", "Is Not", "Is Empty", and "Is Not Empty" options when the corresponding FilterSet defines those lookups. The mixin correctly verifies lookup availability against the FilterSet, so modifiers only appear when multiple lookup options are actually supported. Currently most FilterSets only define 'exact' for single-choice fields, but this change enables future FilterSet enhancements to expose additional lookups for ChoiceFields.
jeremystretch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't quite completed a review (still need to go through the Typescript), but wanted to deliver some suggestions and clarify some pieces. Great work!
| # from .filtersets import XFilterSet | ||
| # FILTERSET_MAPPINGS[XFilterForm] = XFilterSet | ||
|
|
||
| FILTERSET_MAPPINGS = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid introducing one-off globals to be used for making associations in this manner. Instead, we can leverage the application registry to correlate FilterSets to their corresponding models; the models are already set on the filter forms. Then you could do something like this:
@register_filterset
class CircuitFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFilterSet):from netbox.registry import registry
filterset = registry['filtersets'].get(f'{app_label}.{model}')We take a similar approach to map models to their search indexes (registry['search']).
|
|
||
|
|
||
| class VirtualCircuitTerminationFilterForm(NetBoxModelFilterSetForm): | ||
| class VirtualCircuitTerminationFilterForm(FilterModifierMixin, NetBoxModelFilterSetForm): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to have NetBoxModelFilterSetForm inherit FilterModifierMixin upstream?
| """ | ||
|
|
||
| # Mapping of form field types to their supported lookups | ||
| FORM_FIELD_LOOKUPS = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest moving this to a constant outside the mixin class.
| lookups=lookups | ||
| ) | ||
|
|
||
| def _should_skip_field(self, field_name, field): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move this logic into _get_lookup_choices(), and just have it return an empty list for inapplicable fields?
| 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': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The baked-in assumption here gives me pause. Should we consider converting q on NetBoxModelFilterSetForm to a custom subclass of CharField (e.g. QueryField or something), and exclude the field based on its type instead, as we do with boolean fields below? Trying to avoid a potential conflict with a hypothetical plugin which introduces a q filter for some other purpose.
| def _is_api_widget_field(self, field): | ||
| """Check if a field uses an API-based widget.""" | ||
| # Check field class name | ||
| if 'Dynamic' in field.__class__.__name__: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to avoid making any assumptions about a field based on its name alone. Is this check needed? I would assume the attributes check below is sufficient to exclude API-backed fields.
|
|
||
| return False | ||
|
|
||
| def _get_lookup_choices(self, field, field_name=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
field_name is unused?
| def _get_lookup_choices(self, field, field_name=None): | |
| def _get_lookup_choices(self, field): |
| link_text = f'{bound_field.label} {_("is not empty")}' | ||
| else: | ||
| # Add friendly lookup label for other modifier-enhanced fields | ||
| lookup_labels = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels redundant to FORM_FIELD_LOOKUPS. Can we reuse that here?
| 'lt': _('<'), | ||
| 'lte': _('≤'), | ||
| } | ||
| if modifier != 'exact' and modifier in lookup_labels: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we simplify this a bit to check for label := lookup_labels.get(modifier)?
| """ | ||
| template_name = 'widgets/filter_modifier.html' | ||
|
|
||
| def __init__(self, original_widget, lookups, attrs=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just call this widget in the signature? Seems a bit cleaner.
| def __init__(self, original_widget, lookups, attrs=None): | |
| def __init__(self, widget, lookups, attrs=None): |
Closes: #7604
Implements dynamic filter modifier UI that allows users to select lookup operators (exact, contains, starts with, regex, negation, empty/not empty) directly in filter forms without manual URL parameter editing.
Supports filters for all scalar types and strings, as well as some related object filters. Explicitly does not support filters on fields that use
APIWidget. That has been broken out in to follow up work.How It Works
Details
Backend:
FilterModifierWidgetWraps any Django form widget with a modifier dropdown. Key method:
This allows the widget to find its value regardless of which lookup modifier is active in the URL.
Backend:
FilterModifierMixinAutomatically enhances filterset form fields based on their type:
CharField->exact,contains(ic),startswith(isw),endswith(iew),iexact(ie),negation(n),regex,iregex,emptyIntegerField->exact,gte,lte,gt,lt,negation(n),emptyDecimalField->exact,gte,lte,gt,lt,negation(n),emptyDateField->exact,gte,lte,gt,lt,negation(n),emptyChoiceField->exact,negation(n),emptyMultipleChoiceField->exact,negation(n),emptyModelChoiceField->exact,negation(n),emptyColorField->exact,negation(n),emptyTagFilterField->exact,negation(n),emptyFrontend:
filterModifiers.tsTypescript handler that:
nameattributeFilter Pills Enhancement
Modified
applied_filterstemplate tag to:serial__ic)Scope & Compatibility
What's changed:
FilterFormsubclasses now have modifier dropdowns__ic,__n, etc.)What's NOT changed:
FilterSetbehavior unchanged