Fixes: #17663 - Only remove extraneous attributes from extra if changing to a BooleanFilter (#17670)

* Only remove extraneous attributes from extra if changing to a BooleanField

* Add tests for MultipleChoiceField icontains and negation

* Use enum in test consistently

* Reorganize tests

* Add __empty test to base filter lookup tests

* Fix test name

* Change var name for clarity

Author: bctiemann

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 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,

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,
@@ -408,9 +409,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=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)
 
@@ -438,14 +439,22 @@ 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)
 
     def test_site_name_negation(self):
         params = {'name__n': ['Site 1']}
         self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 2)
 
+    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(self):
         params = {'slug__ic': ['-1']}
         self.assertEqual(SiteFilterSet(params, Site.objects.all()).qs.count(), 1)
@@ -553,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_rf_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)