Skip to content

Fixes #20012: Fix support for empty filter for custom fields #20072

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 21, 2025
Merged

Fixes #20012: Fix support for empty filter for custom fields #20072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8ff2bc5
Introduce JSONEmpty lookup
jeremystretch Aug 11, 2025
78b55b8
Fix inverted logic
jeremystretch Aug 11, 2025
b38a611
Use empty lookup for numeric custom fields instead of isnull
jeremystretch Aug 11, 2025
a821981
Add custom field tests for empty lookup
jeremystretch Aug 11, 2025
37abb4c
Fix multiselect and multi-object CF filters
jeremystretch Aug 18, 2025
e01125a
Revert FILTER_NUMERIC_BASED_LOOKUP_MAP change
jeremystretch Aug 18, 2025
004bb69
Override isnull lookup for custom fields
jeremystretch Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion netbox/extras/lookups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from django.db.models import CharField, Lookup
from django.db.models import CharField, JSONField, Lookup
from django.db.models.fields.json import KeyTextTransform

from .fields import CachedValueField

Expand All @@ -18,6 +19,30 @@ def as_sql(self, compiler, connection):
return f"CAST(LENGTH({sql}) AS BOOLEAN) IS TRUE", params


class JSONEmpty(Lookup):
"""
Support "empty" lookups for JSONField keys.

A key is considered empty if it is "", null, or does not exist.
"""
lookup_name = "empty"

def as_sql(self, compiler, connection):
# self.lhs.lhs is the parent expression (could be a JSONField or another KeyTransform)
# Rebuild the expression using KeyTextTransform to guarantee ->> (text)
text_expr = KeyTextTransform(self.lhs.key_name, self.lhs.lhs)
lhs_sql, lhs_params = compiler.compile(text_expr)

value = self.rhs
if value not in (True, False):
raise ValueError("The 'empty' lookup only accepts True or False.")

condition = '' if value else 'NOT '
sql = f"(NULLIF({lhs_sql}, '') IS {condition}NULL)"

return sql, lhs_params


class NetHost(Lookup):
"""
Similar to ipam.lookups.NetHost, but casts the field to INET.
Expand Down Expand Up @@ -45,5 +70,6 @@ def as_sql(self, qn, connection):


CharField.register_lookup(Empty)
JSONField.register_lookup(JSONEmpty)
CachedValueField.register_lookup(NetHost)
CachedValueField.register_lookup(NetContainsOrEquals)
10 changes: 9 additions & 1 deletion netbox/extras/models/customfields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,11 +600,19 @@ def to_filter(self, lookup_expr=None):
kwargs = {
'field_name': f'custom_field_data__{self.name}'
}
# Native numeric filters will use `isnull` by default for empty lookups, but
# JSON fields require `empty` (see bug #20012).
if lookup_expr == 'isnull':
lookup_expr = 'empty'
if lookup_expr is not None:
kwargs['lookup_expr'] = lookup_expr

# 'Empty' lookup is always a boolean
if lookup_expr == 'empty':
filter_class = django_filters.BooleanFilter

# Text/URL
if self.type in (
elif self.type in (
CustomFieldTypeChoices.TYPE_TEXT,
CustomFieldTypeChoices.TYPE_LONGTEXT,
CustomFieldTypeChoices.TYPE_URL,
Expand Down
12 changes: 11 additions & 1 deletion netbox/extras/tests/test_customfields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,7 @@ def setUpTestData(cls):
'cf11': manufacturers[2].pk,
'cf12': [manufacturers[2].pk, manufacturers[3].pk],
}),
Site(name='Site 4', slug='site-4'),
])

def test_filter_integer(self):
Expand All @@ -1624,6 +1625,7 @@ def test_filter_integer(self):
self.assertEqual(self.filterset({'cf_cf1__gte': [200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__lt': [200]}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf1__lte': [200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__empty': True}, self.queryset).qs.count(), 1)

def test_filter_decimal(self):
self.assertEqual(self.filterset({'cf_cf2': [100.1, 200.2]}, self.queryset).qs.count(), 2)
Expand All @@ -1632,6 +1634,7 @@ def test_filter_decimal(self):
self.assertEqual(self.filterset({'cf_cf2__gte': [200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__lt': [200.2]}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf2__lte': [200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__empty': True}, self.queryset).qs.count(), 1)

def test_filter_boolean(self):
self.assertEqual(self.filterset({'cf_cf3': True}, self.queryset).qs.count(), 2)
Expand All @@ -1648,6 +1651,7 @@ def test_filter_text_strict(self):
self.assertEqual(self.filterset({'cf_cf4__niew': ['bar']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__ie': ['FOO']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__nie': ['FOO']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__empty': True}, self.queryset).qs.count(), 1)

def test_filter_text_loose(self):
self.assertEqual(self.filterset({'cf_cf5': ['foo']}, self.queryset).qs.count(), 2)
Expand All @@ -1659,6 +1663,7 @@ def test_filter_date(self):
self.assertEqual(self.filterset({'cf_cf6__gte': ['2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__lt': ['2016-06-27']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf6__lte': ['2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__empty': True}, self.queryset).qs.count(), 1)

def test_filter_url_strict(self):
self.assertEqual(
Expand All @@ -1674,24 +1679,28 @@ def test_filter_url_strict(self):
self.assertEqual(self.filterset({'cf_cf7__niew': ['.com']}, self.queryset).qs.count(), 0)
self.assertEqual(self.filterset({'cf_cf7__ie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__nie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf7__empty': True}, self.queryset).qs.count(), 1)

def test_filter_url_loose(self):
self.assertEqual(self.filterset({'cf_cf8': ['example.com']}, self.queryset).qs.count(), 3)

def test_filter_select(self):
self.assertEqual(self.filterset({'cf_cf9': ['A', 'B']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf9__empty': True}, self.queryset).qs.count(), 1)

def test_filter_multiselect(self):
self.assertEqual(self.filterset({'cf_cf10': ['A']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf10': ['A', 'C']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf10': ['null']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf10': ['null']}, self.queryset).qs.count(), 1) # Contains a literal null
self.assertEqual(self.filterset({'cf_cf10__empty': True}, self.queryset).qs.count(), 2)

def test_filter_object(self):
manufacturer_ids = Manufacturer.objects.values_list('id', flat=True)
self.assertEqual(
self.filterset({'cf_cf11': [manufacturer_ids[0], manufacturer_ids[1]]}, self.queryset).qs.count(),
2
)
self.assertEqual(self.filterset({'cf_cf11__empty': True}, self.queryset).qs.count(), 1)

def test_filter_multiobject(self):
manufacturer_ids = Manufacturer.objects.values_list('id', flat=True)
Expand All @@ -1703,3 +1712,4 @@ def test_filter_multiobject(self):
self.filterset({'cf_cf12': [manufacturer_ids[3]]}, self.queryset).qs.count(),
3
)
self.assertEqual(self.filterset({'cf_cf12__empty': True}, self.queryset).qs.count(), 1)
9 changes: 8 additions & 1 deletion netbox/netbox/filtersets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@
'OrganizationalModelFilterSet',
)

STANDARD_LOOKUPS = (
'exact',
'iexact',
'in',
'contains',
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally going to suggest that maybe this should be a set instead of a tuple, since it only gets instantiated once but accessed a lot later (on the order of ~2500 times within 10 list view requests). However, some quick measurements showed that with only 4 items, the tuple is actually a little faster for lookup than the set.



#
# FilterSets
Expand Down Expand Up @@ -159,7 +166,7 @@ def get_additional_lookups(cls, existing_filter_name, existing_filter):
return {}

# Skip nonstandard lookup expressions
if existing_filter.method is not None or existing_filter.lookup_expr not in ['exact', 'iexact', 'in']:
if existing_filter.method is not None or existing_filter.lookup_expr not in STANDARD_LOOKUPS:
return {}

# Choose the lookup expression map based on the filter type
Expand Down