From 71945e7bbb3d66c29d8a3cf21c81dec8a03cb97a Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 12 Sep 2024 15:41:39 -0400 Subject: [PATCH 1/9] Add EmptyStringFilter and type__empty filter on CableFilterSet --- netbox/dcim/filtersets.py | 5 ++++- netbox/dcim/tests/test_filtersets.py | 6 ++++++ netbox/utilities/filters.py | 11 +++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/netbox/dcim/filtersets.py b/netbox/dcim/filtersets.py index 6517aadb45b..183c8029943 100644 --- a/netbox/dcim/filtersets.py +++ b/netbox/dcim/filtersets.py @@ -18,7 +18,7 @@ from users.models import User from utilities.filters import ( ContentTypeFilter, MultiValueCharFilter, MultiValueMACAddressFilter, MultiValueNumberFilter, MultiValueWWNFilter, - NumericArrayFilter, TreeNodeMultipleChoiceFilter, + NumericArrayFilter, TreeNodeMultipleChoiceFilter, EmptyStringFilter, ) from virtualization.models import Cluster, ClusterGroup from vpn.models import L2VPN @@ -1983,6 +1983,9 @@ class CableFilterSet(TenancyFilterSet, NetBoxModelFilterSet): type = django_filters.MultipleChoiceFilter( choices=CableTypeChoices ) + type__empty = EmptyStringFilter( + field_name='type' + ) status = django_filters.MultipleChoiceFilter( choices=LinkStatusChoices ) diff --git a/netbox/dcim/tests/test_filtersets.py b/netbox/dcim/tests/test_filtersets.py index e2d52a60942..b93939d33ba 100644 --- a/netbox/dcim/tests/test_filtersets.py +++ b/netbox/dcim/tests/test_filtersets.py @@ -5248,6 +5248,12 @@ def test_type(self): params = {'type': [CableTypeChoices.TYPE_CAT3, CableTypeChoices.TYPE_CAT5E]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) + def test_type_empty(self): + params = {'type__empty': 'true'} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 8) + params = {'type__empty': 'false'} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6) + def test_status(self): params = {'status': [LinkStatusChoices.STATUS_CONNECTED]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 11) diff --git a/netbox/utilities/filters.py b/netbox/utilities/filters.py index 05454543e3c..95c615dcbdc 100644 --- a/netbox/utilities/filters.py +++ b/netbox/utilities/filters.py @@ -171,3 +171,14 @@ def filter(self, qs, value): f'{self.field_name}__model': model } ) + + +class EmptyStringFilter(django_filters.BooleanFilter): + def filter(self, qs, value): + if value in EMPTY_VALUES: + return qs + + exclude = self.exclude ^ (value is False) + method = qs.exclude if exclude else qs.filter + + return method(**{self.field_name: ""}) From 7195f51c6cd66a1f12c917251b25bca3a0f2b676 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Fri, 13 Sep 2024 08:52:17 -0400 Subject: [PATCH 2/9] Change to EmptyStringMultipleChoiceFilter --- netbox/dcim/choices.py | 10 ++++++++-- netbox/dcim/filtersets.py | 7 ++----- netbox/dcim/tests/test_filtersets.py | 8 +++----- netbox/utilities/filters.py | 14 ++++++-------- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/netbox/dcim/choices.py b/netbox/dcim/choices.py index 127655ba7cd..253f483e707 100644 --- a/netbox/dcim/choices.py +++ b/netbox/dcim/choices.py @@ -1470,6 +1470,7 @@ class CableTypeChoices(ChoiceSet): TYPE_AOC = 'aoc' TYPE_POWER = 'power' TYPE_USB = 'usb' + TYPE_EMPTY = 'EMPTY' CHOICES = ( ( @@ -1502,8 +1503,13 @@ class CableTypeChoices(ChoiceSet): (TYPE_AOC, 'Active Optical Cabling (AOC)'), ), ), - (TYPE_USB, _('USB')), - (TYPE_POWER, _('Power')), + ( + _('Other'), ( + (TYPE_USB, _('USB')), + (TYPE_POWER, _('Power')), + (TYPE_EMPTY, _('(unset)')), + ) + ) ) diff --git a/netbox/dcim/filtersets.py b/netbox/dcim/filtersets.py index 183c8029943..419769ce89b 100644 --- a/netbox/dcim/filtersets.py +++ b/netbox/dcim/filtersets.py @@ -18,7 +18,7 @@ from users.models import User from utilities.filters import ( ContentTypeFilter, MultiValueCharFilter, MultiValueMACAddressFilter, MultiValueNumberFilter, MultiValueWWNFilter, - NumericArrayFilter, TreeNodeMultipleChoiceFilter, EmptyStringFilter, + NumericArrayFilter, TreeNodeMultipleChoiceFilter, EmptyStringMultipleChoiceFilter, ) from virtualization.models import Cluster, ClusterGroup from vpn.models import L2VPN @@ -1980,12 +1980,9 @@ class CableFilterSet(TenancyFilterSet, NetBoxModelFilterSet): method='_unterminated', label=_('Unterminated'), ) - type = django_filters.MultipleChoiceFilter( + type = EmptyStringMultipleChoiceFilter( choices=CableTypeChoices ) - type__empty = EmptyStringFilter( - field_name='type' - ) status = django_filters.MultipleChoiceFilter( choices=LinkStatusChoices ) diff --git a/netbox/dcim/tests/test_filtersets.py b/netbox/dcim/tests/test_filtersets.py index b93939d33ba..182013867fa 100644 --- a/netbox/dcim/tests/test_filtersets.py +++ b/netbox/dcim/tests/test_filtersets.py @@ -5247,12 +5247,10 @@ def test_length_unit(self): def test_type(self): params = {'type': [CableTypeChoices.TYPE_CAT3, CableTypeChoices.TYPE_CAT5E]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) - - def test_type_empty(self): - params = {'type__empty': 'true'} + params = {'type': [CableTypeChoices.TYPE_EMPTY]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 8) - params = {'type__empty': 'false'} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 6) + params = {'type': [CableTypeChoices.TYPE_EMPTY, CableTypeChoices.TYPE_CAT3]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 10) def test_status(self): params = {'status': [LinkStatusChoices.STATUS_CONNECTED]} diff --git a/netbox/utilities/filters.py b/netbox/utilities/filters.py index 95c615dcbdc..3d8203b856a 100644 --- a/netbox/utilities/filters.py +++ b/netbox/utilities/filters.py @@ -173,12 +173,10 @@ def filter(self, qs, value): ) -class EmptyStringFilter(django_filters.BooleanFilter): - def filter(self, qs, value): - if value in EMPTY_VALUES: - return qs +class EmptyStringMultipleChoiceFilter(django_filters.MultipleChoiceFilter): + empty_value = 'EMPTY' - exclude = self.exclude ^ (value is False) - method = qs.exclude if exclude else qs.filter - - return method(**{self.field_name: ""}) + def filter(self, qs, value): + if self.empty_value in value: + value.append('') + return super().filter(qs, value) From 2ca4e971dae2246c70501ec6fb513cdc61624231 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Fri, 13 Sep 2024 09:05:27 -0400 Subject: [PATCH 3/9] Change to NullableMultipleChoiceFilter --- netbox/dcim/choices.py | 5 +++-- netbox/dcim/filtersets.py | 4 ++-- netbox/utilities/filters.py | 17 ++++++++--------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/netbox/dcim/choices.py b/netbox/dcim/choices.py index 253f483e707..974ea402412 100644 --- a/netbox/dcim/choices.py +++ b/netbox/dcim/choices.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.utils.translation import gettext_lazy as _ from utilities.choices import ChoiceSet @@ -1470,7 +1471,7 @@ class CableTypeChoices(ChoiceSet): TYPE_AOC = 'aoc' TYPE_POWER = 'power' TYPE_USB = 'usb' - TYPE_EMPTY = 'EMPTY' + TYPE_EMPTY = settings.FILTERS_NULL_CHOICE_VALUE CHOICES = ( ( @@ -1507,7 +1508,7 @@ class CableTypeChoices(ChoiceSet): _('Other'), ( (TYPE_USB, _('USB')), (TYPE_POWER, _('Power')), - (TYPE_EMPTY, _('(unset)')), + (settings.FILTERS_NULL_CHOICE_VALUE, _('(unset)')), ) ) ) diff --git a/netbox/dcim/filtersets.py b/netbox/dcim/filtersets.py index 419769ce89b..e2672f551cb 100644 --- a/netbox/dcim/filtersets.py +++ b/netbox/dcim/filtersets.py @@ -18,7 +18,7 @@ from users.models import User from utilities.filters import ( ContentTypeFilter, MultiValueCharFilter, MultiValueMACAddressFilter, MultiValueNumberFilter, MultiValueWWNFilter, - NumericArrayFilter, TreeNodeMultipleChoiceFilter, EmptyStringMultipleChoiceFilter, + NumericArrayFilter, TreeNodeMultipleChoiceFilter, NullableMultipleChoiceFilter, ) from virtualization.models import Cluster, ClusterGroup from vpn.models import L2VPN @@ -1980,7 +1980,7 @@ class CableFilterSet(TenancyFilterSet, NetBoxModelFilterSet): method='_unterminated', label=_('Unterminated'), ) - type = EmptyStringMultipleChoiceFilter( + type = NullableMultipleChoiceFilter( choices=CableTypeChoices ) status = django_filters.MultipleChoiceFilter( diff --git a/netbox/utilities/filters.py b/netbox/utilities/filters.py index 3d8203b856a..66df2e54f02 100644 --- a/netbox/utilities/filters.py +++ b/netbox/utilities/filters.py @@ -143,6 +143,14 @@ def filter(self, qs, value): return qs.distinct() if self.distinct else qs +class NullableMultipleChoiceFilter(django_filters.MultipleChoiceFilter): + + def filter(self, qs, value): + if settings.FILTERS_NULL_CHOICE_VALUE in value: + value.append('') + return super().filter(qs, value) + + class NumericArrayFilter(django_filters.NumberFilter): """ Filter based on the presence of an integer within an ArrayField. @@ -171,12 +179,3 @@ def filter(self, qs, value): f'{self.field_name}__model': model } ) - - -class EmptyStringMultipleChoiceFilter(django_filters.MultipleChoiceFilter): - empty_value = 'EMPTY' - - def filter(self, qs, value): - if self.empty_value in value: - value.append('') - return super().filter(qs, value) From 070fafe3892d2dd60004449dd970086bb4b94f2d Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Fri, 13 Sep 2024 09:13:58 -0400 Subject: [PATCH 4/9] Add docstring for NullableMultipleChoiceFilter --- netbox/utilities/filters.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/netbox/utilities/filters.py b/netbox/utilities/filters.py index 66df2e54f02..f51fa23ff6d 100644 --- a/netbox/utilities/filters.py +++ b/netbox/utilities/filters.py @@ -18,6 +18,7 @@ 'MultiValueTimeFilter', 'MultiValueWWNFilter', 'NullableCharFieldFilter', + 'NullableMultipleChoiceFilter', 'NumericArrayFilter', 'TreeNodeMultipleChoiceFilter', ) @@ -144,7 +145,9 @@ def filter(self, qs, value): class NullableMultipleChoiceFilter(django_filters.MultipleChoiceFilter): - + """ + Similar to NullableCharFieldFilter, but allows multiple values including the special NULL string. + """ def filter(self, qs, value): if settings.FILTERS_NULL_CHOICE_VALUE in value: value.append('') From 6d70c17d7861eaf32fe6bdc0bee830c76bffe328 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Wed, 18 Sep 2024 16:14:04 -0400 Subject: [PATCH 5/9] Add add_empty_filtering_choice util for empty-value filtering --- netbox/dcim/choices.py | 2 -- netbox/dcim/forms/filtersets.py | 4 ++-- netbox/utilities/forms/utils.py | 10 ++++++++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/netbox/dcim/choices.py b/netbox/dcim/choices.py index 53c3bced957..c688891548d 100644 --- a/netbox/dcim/choices.py +++ b/netbox/dcim/choices.py @@ -1473,7 +1473,6 @@ class CableTypeChoices(ChoiceSet): TYPE_AOC = 'aoc' TYPE_POWER = 'power' TYPE_USB = 'usb' - TYPE_EMPTY = settings.FILTERS_NULL_CHOICE_VALUE CHOICES = ( ( @@ -1510,7 +1509,6 @@ class CableTypeChoices(ChoiceSet): _('Other'), ( (TYPE_USB, _('USB')), (TYPE_POWER, _('Power')), - (settings.FILTERS_NULL_CHOICE_VALUE, _('(unset)')), ) ) ) diff --git a/netbox/dcim/forms/filtersets.py b/netbox/dcim/forms/filtersets.py index e2b6fda07a7..64814cf8ba7 100644 --- a/netbox/dcim/forms/filtersets.py +++ b/netbox/dcim/forms/filtersets.py @@ -10,7 +10,7 @@ from netbox.forms import NetBoxModelFilterSetForm from tenancy.forms import ContactModelFilterForm, TenancyFilterForm from users.models import User -from utilities.forms import BOOLEAN_WITH_BLANK_CHOICES, FilterForm, add_blank_choice +from utilities.forms import BOOLEAN_WITH_BLANK_CHOICES, FilterForm, add_blank_choice, add_empty_filtering_choice from utilities.forms.fields import ColorField, DynamicModelMultipleChoiceField, TagFilterField from utilities.forms.rendering import FieldSet from utilities.forms.widgets import NumberWithOptions @@ -1052,7 +1052,7 @@ class CableFilterForm(TenancyFilterForm, NetBoxModelFilterSetForm): ) type = forms.MultipleChoiceField( label=_('Type'), - choices=add_blank_choice(CableTypeChoices), + choices=add_empty_filtering_choice(add_blank_choice(CableTypeChoices)), required=False ) status = forms.MultipleChoiceField( diff --git a/netbox/utilities/forms/utils.py b/netbox/utilities/forms/utils.py index 0429fe5710e..56d13b058cb 100644 --- a/netbox/utilities/forms/utils.py +++ b/netbox/utilities/forms/utils.py @@ -1,6 +1,7 @@ import re from django import forms +from django.conf import settings from django.forms.models import fields_for_model from django.utils.translation import gettext as _ @@ -10,6 +11,7 @@ __all__ = ( 'add_blank_choice', + 'add_empty_filtering_choice', 'expand_alphanumeric_pattern', 'expand_ipaddress_pattern', 'form_from_model', @@ -189,6 +191,14 @@ def add_blank_choice(choices): return ((None, '---------'),) + tuple(choices) +def add_empty_filtering_choice(choices): + """ + Add an empty (null) choice to the end of a choices list, to be used in filtering classes + such as NullableMultipleChoiceFilter to match on an empty value. + """ + return tuple(choices) + ((settings.FILTERS_NULL_CHOICE_VALUE, '(unset)'),) + + def form_from_model(model, fields): """ Return a Form class with the specified fields derived from a model. This is useful when we need a form to be used From 4675b0cbfb8cd6cb30f0ee509b59aba9719a0ef6 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Wed, 18 Sep 2024 16:15:16 -0400 Subject: [PATCH 6/9] Remove unneeded import --- netbox/dcim/choices.py | 1 - 1 file changed, 1 deletion(-) diff --git a/netbox/dcim/choices.py b/netbox/dcim/choices.py index c688891548d..78271a9c27c 100644 --- a/netbox/dcim/choices.py +++ b/netbox/dcim/choices.py @@ -1,4 +1,3 @@ -from django.conf import settings from django.utils.translation import gettext_lazy as _ from utilities.choices import ChoiceSet From d1af212b0d0eaafb1df563c70245458ed8cba9b2 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Wed, 18 Sep 2024 16:58:46 -0400 Subject: [PATCH 7/9] Fix tests --- netbox/dcim/tests/test_filtersets.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/netbox/dcim/tests/test_filtersets.py b/netbox/dcim/tests/test_filtersets.py index 182013867fa..723a63a0976 100644 --- a/netbox/dcim/tests/test_filtersets.py +++ b/netbox/dcim/tests/test_filtersets.py @@ -1,4 +1,5 @@ from django.test import TestCase +from django.conf import settings from circuits.models import Circuit, CircuitTermination, CircuitType, Provider from dcim.choices import * @@ -5247,9 +5248,9 @@ def test_length_unit(self): def test_type(self): params = {'type': [CableTypeChoices.TYPE_CAT3, CableTypeChoices.TYPE_CAT5E]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) - params = {'type': [CableTypeChoices.TYPE_EMPTY]} + params = {'type': [settings.FILTERS_NULL_CHOICE_VALUE]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 8) - params = {'type': [CableTypeChoices.TYPE_EMPTY, CableTypeChoices.TYPE_CAT3]} + params = {'type': [settings.FILTERS_NULL_CHOICE_VALUE, CableTypeChoices.TYPE_CAT3]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 10) def test_status(self): From e462e29c30b69ba081f06de7913caab23a01342a Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 19 Sep 2024 20:18:55 -0400 Subject: [PATCH 8/9] Guard against superfluous "choices" param in filter class instantiation --- netbox/netbox/filtersets.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/netbox/netbox/filtersets.py b/netbox/netbox/filtersets.py index ac43fe57f44..807a7ea03f5 100644 --- a/netbox/netbox/filtersets.py +++ b/netbox/netbox/filtersets.py @@ -133,7 +133,7 @@ def _get_filter_lookup_dict(existing_filter): django_filters.ModelChoiceFilter, django_filters.ModelMultipleChoiceFilter, TagFilter - )) or existing_filter.extra.get('choices'): + )): # These filter types support only negation return FILTER_NEGATION_LOOKUP_MAP @@ -172,6 +172,7 @@ def get_additional_lookups(cls, existing_filter_name, existing_filter): # Create new filters for each lookup expression in the map for lookup_name, lookup_expr in lookup_map.items(): new_filter_name = f'{existing_filter_name}__{lookup_name}' + existing_filter_extra = deepcopy(existing_filter.extra) try: if existing_filter_name in cls.declared_filters: @@ -179,14 +180,18 @@ def get_additional_lookups(cls, existing_filter_name, existing_filter): # create the new filter with the same type because there is no guarantee the defined type # is the same as the default type for the field resolve_field(field, lookup_expr) # Will raise FieldLookupError if the lookup is invalid - filter_cls = django_filters.BooleanFilter if lookup_expr == 'empty' else type(existing_filter) + if lookup_expr == 'empty': + existing_filter_extra.pop('choices', None) + filter_cls = django_filters.BooleanFilter + else: + filter_cls = type(existing_filter) new_filter = filter_cls( field_name=field_name, lookup_expr=lookup_expr, label=existing_filter.label, exclude=existing_filter.exclude, distinct=existing_filter.distinct, - **existing_filter.extra + **existing_filter_extra ) elif hasattr(existing_filter, 'custom_field'): # Filter is for a custom field From 3a2046b63c94c2575c13b1738da13e953a7f4a61 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 19 Sep 2024 20:42:34 -0400 Subject: [PATCH 9/9] Also remove 'null_value' from extra (maybe will need to remove more) --- netbox/netbox/filtersets.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/netbox/netbox/filtersets.py b/netbox/netbox/filtersets.py index 807a7ea03f5..e3bd332983c 100644 --- a/netbox/netbox/filtersets.py +++ b/netbox/netbox/filtersets.py @@ -180,11 +180,9 @@ def get_additional_lookups(cls, existing_filter_name, existing_filter): # create the new filter with the same type because there is no guarantee the defined type # is the same as the default type for the field resolve_field(field, lookup_expr) # Will raise FieldLookupError if the lookup is invalid - if lookup_expr == 'empty': - existing_filter_extra.pop('choices', None) - filter_cls = django_filters.BooleanFilter - else: - filter_cls = type(existing_filter) + for field_to_remove in ('choices', 'null_value'): + existing_filter_extra.pop(field_to_remove, None) + filter_cls = django_filters.BooleanFilter if lookup_expr == 'empty' else type(existing_filter) new_filter = filter_cls( field_name=field_name, lookup_expr=lookup_expr,