From 6df76ae870c424b144646423c8d67aef1fdd3b69 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 3 Oct 2024 07:29:05 -0400 Subject: [PATCH 1/7] Only remove extraneous attributes from extra if changing to a BooleanField --- netbox/netbox/filtersets.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/netbox/netbox/filtersets.py b/netbox/netbox/filtersets.py index e3bd332983c..41a22859dea 100644 --- a/netbox/netbox/filtersets.py +++ b/netbox/netbox/filtersets.py @@ -180,9 +180,11 @@ 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 - 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) + filter_cls = type(existing_filter) + if lookup_expr == 'empty': + filter_cls = django_filters.BooleanFilter + for field_to_remove in ('choices', 'null_value'): + existing_filter_extra.pop(field_to_remove, None) new_filter = filter_cls( field_name=field_name, lookup_expr=lookup_expr, From 0904ca7f31c834d47f3a683c10a69ab277d5229d Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 3 Oct 2024 08:34:10 -0400 Subject: [PATCH 2/7] Add tests for MultipleChoiceField icontains and negation --- netbox/utilities/tests/test_filters.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/netbox/utilities/tests/test_filters.py b/netbox/utilities/tests/test_filters.py index dd619456530..2fbe80d6aa5 100644 --- a/netbox/utilities/tests/test_filters.py +++ b/netbox/utilities/tests/test_filters.py @@ -408,9 +408,9 @@ def setUpTestData(cls): region.save() sites = ( - Site(name='Site 1', slug='abc-site-1', region=regions[0]), - Site(name='Site 2', slug='def-site-2', region=regions[1]), - Site(name='Site 3', slug='ghi-site-3', region=regions[2]), + Site(name='Site 1', slug='abc-site-1', region=regions[0], status='active'), + Site(name='Site 2', slug='def-site-2', region=regions[1], status='active'), + Site(name='Site 3', slug='ghi-site-3', region=regions[2], status='planned'), ) Site.objects.bulk_create(sites) @@ -450,6 +450,14 @@ def test_site_slug_icontains(self): params = {'slug__ic': ['-1']} self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 1) + def test_site_status_icontains(self): + params = {'status__ic': [SiteStatusChoices.STATUS_ACTIVE]} + self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 2) + + def test_site_status_icontains_negation(self): + params = {'status__nic': [SiteStatusChoices.STATUS_ACTIVE]} + self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 1) + def test_site_slug_icontains_negation(self): params = {'slug__nic': ['-1']} self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 2) From 537e214d67ffd562f1964a8b5de61bde90a6a183 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 3 Oct 2024 08:36:00 -0400 Subject: [PATCH 3/7] Use enum in test consistently --- netbox/utilities/tests/test_filters.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/netbox/utilities/tests/test_filters.py b/netbox/utilities/tests/test_filters.py index 2fbe80d6aa5..7a50079a786 100644 --- a/netbox/utilities/tests/test_filters.py +++ b/netbox/utilities/tests/test_filters.py @@ -408,9 +408,9 @@ def setUpTestData(cls): region.save() sites = ( - Site(name='Site 1', slug='abc-site-1', region=regions[0], status='active'), - Site(name='Site 2', slug='def-site-2', region=regions[1], status='active'), - Site(name='Site 3', slug='ghi-site-3', region=regions[2], status='planned'), + Site(name='Site 1', slug='abc-site-1', region=regions[0], status=SiteStatusChoices.STATUS_ACTIVE), + Site(name='Site 2', slug='def-site-2', region=regions[1], status=SiteStatusChoices.STATUS_ACTIVE), + Site(name='Site 3', slug='ghi-site-3', region=regions[2], status=SiteStatusChoices.STATUS_PLANNED), ) Site.objects.bulk_create(sites) From 56c724990bbad511dafe104d929a68a39816d912 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 3 Oct 2024 09:14:58 -0400 Subject: [PATCH 4/7] Reorganize tests --- netbox/utilities/tests/test_filters.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/netbox/utilities/tests/test_filters.py b/netbox/utilities/tests/test_filters.py index 7a50079a786..f62e2441d34 100644 --- a/netbox/utilities/tests/test_filters.py +++ b/netbox/utilities/tests/test_filters.py @@ -446,10 +446,6 @@ def test_site_name_negation(self): params = {'name__n': ['Site 1']} self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 2) - def test_site_slug_icontains(self): - params = {'slug__ic': ['-1']} - self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 1) - def test_site_status_icontains(self): params = {'status__ic': [SiteStatusChoices.STATUS_ACTIVE]} self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 2) @@ -458,6 +454,10 @@ def test_site_status_icontains_negation(self): params = {'status__nic': [SiteStatusChoices.STATUS_ACTIVE]} self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 1) + def test_site_slug_icontains(self): + params = {'slug__ic': ['-1']} + self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 1) + def test_site_slug_icontains_negation(self): params = {'slug__nic': ['-1']} self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 2) From 7988fb470ea45ae79406d5cc673a38b46eb488be Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 3 Oct 2024 09:50:00 -0400 Subject: [PATCH 5/7] Add __empty test to base filter lookup tests --- netbox/utilities/tests/test_filters.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/netbox/utilities/tests/test_filters.py b/netbox/utilities/tests/test_filters.py index f62e2441d34..f16836c98b0 100644 --- a/netbox/utilities/tests/test_filters.py +++ b/netbox/utilities/tests/test_filters.py @@ -7,7 +7,7 @@ from dcim.choices import * from dcim.fields import MACAddressField -from dcim.filtersets import DeviceFilterSet, SiteFilterSet +from dcim.filtersets import DeviceFilterSet, SiteFilterSet, InterfaceFilterSet from dcim.models import ( Device, DeviceRole, DeviceType, Interface, Manufacturer, Platform, Rack, Region, Site ) @@ -16,6 +16,7 @@ from ipam.filtersets import ASNFilterSet from ipam.models import RIR, ASN from netbox.filtersets import BaseFilterSet +from wireless.choices import WirelessRoleChoices from utilities.filters import ( MultiValueCharFilter, MultiValueDateFilter, MultiValueDateTimeFilter, MultiValueMACAddressFilter, MultiValueNumberFilter, MultiValueTimeFilter, TreeNodeMultipleChoiceFilter, @@ -438,7 +439,7 @@ def setUpTestData(cls): Interface(device=devices[1], name='Interface 3', mac_address='00-00-00-00-00-02'), Interface(device=devices[1], name='Interface 4', mac_address='bb-00-00-00-00-02'), Interface(device=devices[2], name='Interface 5', mac_address='00-00-00-00-00-03'), - Interface(device=devices[2], name='Interface 6', mac_address='cc-00-00-00-00-03'), + Interface(device=devices[2], name='Interface 6', mac_address='cc-00-00-00-00-03', rf_role=WirelessRoleChoices.ROLE_AP), ) Interface.objects.bulk_create(interfaces) @@ -561,3 +562,9 @@ def test_device_mac_address_icontains(self): def test_device_mac_address_icontains_negation(self): params = {'mac_address__nic': ['aa:', 'bb']} self.assertEqual(DeviceFilterSet(params, Device.objects.all()).qs.count(), 1) + + def test_interface_wireless_role_empty(self): + params = {'rf_role__empty': 'true'} + self.assertEqual(InterfaceFilterSet(params, Interface.objects.all()).qs.count(), 5) + params = {'rf_role__empty': 'false'} + self.assertEqual(InterfaceFilterSet(params, Interface.objects.all()).qs.count(), 1) From 13d8936af4b780a05bd1dfd2312cb12874408fcc Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 3 Oct 2024 09:59:52 -0400 Subject: [PATCH 6/7] Fix test name --- netbox/utilities/tests/test_filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/utilities/tests/test_filters.py b/netbox/utilities/tests/test_filters.py index f16836c98b0..53e6eb98548 100644 --- a/netbox/utilities/tests/test_filters.py +++ b/netbox/utilities/tests/test_filters.py @@ -563,7 +563,7 @@ def test_device_mac_address_icontains_negation(self): params = {'mac_address__nic': ['aa:', 'bb']} self.assertEqual(DeviceFilterSet(params, Device.objects.all()).qs.count(), 1) - def test_interface_wireless_role_empty(self): + def test_interface_rf_role_empty(self): params = {'rf_role__empty': 'true'} self.assertEqual(InterfaceFilterSet(params, Interface.objects.all()).qs.count(), 5) params = {'rf_role__empty': 'false'} From 2f19367e20fb8e7f949ac499c45c2941e287166d Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 3 Oct 2024 11:32:32 -0400 Subject: [PATCH 7/7] Change var name for clarity --- netbox/netbox/filtersets.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/netbox/netbox/filtersets.py b/netbox/netbox/filtersets.py index 41a22859dea..637a40bf114 100644 --- a/netbox/netbox/filtersets.py +++ b/netbox/netbox/filtersets.py @@ -183,8 +183,8 @@ def get_additional_lookups(cls, existing_filter_name, existing_filter): filter_cls = type(existing_filter) if lookup_expr == 'empty': filter_cls = django_filters.BooleanFilter - for field_to_remove in ('choices', 'null_value'): - existing_filter_extra.pop(field_to_remove, None) + for param_to_remove in ('choices', 'null_value'): + existing_filter_extra.pop(param_to_remove, None) new_filter = filter_cls( field_name=field_name, lookup_expr=lookup_expr,