[next] Add isnull Filter on Nullable Fields via Filter Field Factory (#…

…4635)

* Added the ability to automatically apply `isnull` filters when model field is nullable

* check field first with hasattr

* Account for oddness with RoleFilter

* Update test to account for a boolean in schema

* Update test to account for a boolean in schema take 2

* Fix issues with RoleFilter, align StatusFilter and RoleFilter code paths.

Co-authored-by: Jan Snasel <jan.snasel@networktocode.com>

* Update nautobot/core/filters.py

Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>

---------

Co-authored-by: itdependsnetworks <ken@celenza.org>
Co-authored-by: Jan Snasel <jan.snasel@networktocode.com>
Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>

changes/1905.added

@@ -0,0 +1,2 @@
+Added the ability to automatically apply `isnull` filters when model field is nullable.
+Enhanced `status` filters to support filtering by ID (UUID) as an alternative to filtering by `name`.

nautobot/circuits/tests/test_filters.py

@@ -91,6 +91,7 @@ class CircuitTestCase(FilterTestCases.FilterTestCase, FilterTestCases.TenancyFil
         ["provider_network", "circuit_terminations__provider_network__name"],
         ["circuit_type", "circuit_type__id"],
         ["circuit_type", "circuit_type__name"],
+        ["status", "status__id"],
         ["status", "status__name"],
         ["circuit_termination_a"],
         ["circuit_termination_z"],

nautobot/core/filters.py

@@ -630,15 +630,23 @@ def _generate_lookup_expression_filters(cls, filter_name, filter_field):
         if field is None:
             return magic_filters
 
+        # If the field allows null values, add an `isnull`` check
+        if getattr(field, "null", None):
+            # Use this method vs extend as the `lookup_map` variable is generally one of
+            # the constants which we do not want to update
+            lookup_map = dict(lookup_map, **{"isnull": "isnull"})
+
         # Create new filters for each lookup expression in the map
         for lookup_name, lookup_expr in lookup_map.items():
             new_filter_name = f"{filter_name}__{lookup_name}"
 
             try:
-                if filter_name in cls.declared_filters:
-                    # The filter field has been explicity defined on the filterset class so we must manually
+                if filter_name in cls.declared_filters and lookup_expr not in {"isnull"}:
+                    # The filter field has been explicitly defined on the filterset class so we must manually
                     # 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
+                    # is the same as the default type for the field. This does not apply if the filter
+                    # should retain the original lookup_expr type, such as `isnull` using a boolean field on a
+                    # char or date object.
                     resolve_field(field, lookup_expr)  # Will raise FieldLookupError if the lookup is invalid
                     new_filter = type(filter_field)(
                         field_name=field_name,
@@ -649,7 +657,7 @@ def _generate_lookup_expression_filters(cls, filter_name, filter_field):
                         **filter_field.extra,
                     )
                 else:
-                    # The filter field is listed in Meta.fields so we can safely rely on default behaviour
+                    # The filter field is listed in Meta.fields so we can safely rely on default behavior
                     # Will raise FieldLookupError if the lookup is invalid
                     new_filter = cls.filter_for_field(field, field_name, lookup_expr)
             except django_filters.exceptions.FieldLookupError:

nautobot/core/models/fields.py

@@ -1,7 +1,6 @@
 import json
 import re
 
-from django.contrib.contenttypes.models import ContentType
 from django.core import exceptions
 from django.core.validators import RegexValidator, MaxLengthValidator
 from django.db import models
@@ -187,7 +186,18 @@ class ForeignKeyLimitedByContentTypes(ForeignKeyWithAutoRelatedName):
     """
 
     def get_limit_choices_to(self):
-        return {"content_types": ContentType.objects.get_for_model(self.model)}
+        """
+        Limit this field to only objects which are assigned to this model's content-type.
+
+        Note that this is implemented via specifying `content_types__app_label=` and `content_types__model=`
+        rather than via the more obvious `content_types=ContentType.objects.get_for_model(self.model)`
+        because the latter approach would involve a database query, and in some cases
+        (most notably FilterSet definition) this function is called **before** database migrations can be run.
+        """
+        return {
+            "content_types__app_label": self.model._meta.app_label,
+            "content_types__model": self.model._meta.model_name,
+        }
 
     def formfield(self, **kwargs):
         """Return a prepped formfield for use in model forms."""

nautobot/core/tests/test_filters.py

@@ -1,3 +1,5 @@
+import datetime
+
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.contenttypes.models import ContentType
@@ -9,6 +11,8 @@
 import django_filters
 from tree_queries.models import TreeNodeForeignKey
 
+from nautobot.circuits.filters import CircuitFilterSet
+from nautobot.circuits.models import Circuit, CircuitType, Provider
 from nautobot.core import filters, testing
 from nautobot.core.models import fields as core_fields
 from nautobot.core.utils import lookup
@@ -1450,6 +1454,129 @@ class InvalidLocationFilterSet(dcim_filters.LocationFilterSet):  # pylint: disab
                 q = filters.SearchFilter(filter_predicates={"asn": ["icontains"]})
 
 
+class LookupIsNullTest(TestCase, testing.NautobotTestCaseMixin):
+    """
+    Validate isnull is properly applied and filtering.
+    """
+
+    platform_queryset = dcim_models.Platform.objects.all()
+    platform_filterset = dcim_filters.PlatformFilterSet
+    circuit_queryset = Circuit.objects.all()
+    circuit_filterset = CircuitFilterSet
+    device_queryset = dcim_models.Device.objects.all()
+    device_filterset = dcim_filters.DeviceFilterSet
+
+    @classmethod
+    def setUpTestData(cls):
+        location_type = dcim_models.LocationType.objects.create(name="Location Type 1")
+        location_status = extras_models.Status.objects.get_for_model(dcim_models.Location).first()
+        location = dcim_models.Location.objects.create(
+            name="Location 1",
+            location_type=location_type,
+            status=location_status,
+        )
+        manufacturer = dcim_models.Manufacturer.objects.create(name="Manufacturer 1")
+        dcim_models.Platform.objects.create(name="Platform 1")
+        platform = dcim_models.Platform.objects.create(name="Platform 2", manufacturer=manufacturer)
+
+        device_type = dcim_models.DeviceType.objects.create(
+            manufacturer=manufacturer,
+            model="Model 1",
+            is_full_depth=True,
+        )
+        device_role = extras_models.Role.objects.create(name="Active Test")
+        device_role.content_types.add(ContentType.objects.get_for_model(dcim_models.Device))
+
+        device_status = extras_models.Status.objects.get_for_model(dcim_models.Device).first()
+
+        dcim_models.Device.objects.create(
+            name="Device 1",
+            device_type=device_type,
+            role=device_role,
+            platform=platform,
+            location=location,
+            status=device_status,
+        )
+        dcim_models.Device.objects.create(
+            device_type=device_type,
+            role=device_role,
+            platform=platform,
+            location=location,
+            status=device_status,
+        )
+
+        provider = Provider.objects.create(name="provider 1", asn=1)
+        circuit_type = CircuitType.objects.create(name="Circuit Type 1")
+        circuit_status = extras_models.Status.objects.get_for_model(dcim_models.Location).first()
+
+        Circuit.objects.create(
+            cid="Circuit 1",
+            provider=provider,
+            install_date=datetime.date(2020, 1, 1),
+            commit_rate=1000,
+            circuit_type=circuit_type,
+            status=circuit_status,
+        )
+        Circuit.objects.create(
+            cid="Circuit 2",
+            provider=provider,
+            circuit_type=circuit_type,
+            status=circuit_status,
+        )
+
+    def test_isnull_on_fk(self):
+        """Test that the `isnull` filter is applied for True and False queries on foreign key fields."""
+        params = {"manufacturer__isnull": True}
+        self.assertQuerysetEqualAndNotEmpty(
+            dcim_filters.PlatformFilterSet(params, self.platform_queryset).qs,
+            dcim_models.Platform.objects.filter(manufacturer__isnull=True),
+        )
+        params = {"manufacturer__isnull": False}
+        self.assertQuerysetEqualAndNotEmpty(
+            dcim_filters.PlatformFilterSet(params, self.platform_queryset).qs,
+            dcim_models.Platform.objects.filter(manufacturer__isnull=False),
+        )
+
+    def test_isnull_on_integer(self):
+        """Test that the `isnull` filter is applied for True and False queries on integer fields."""
+        params = {"commit_rate__isnull": True}
+        self.assertQuerysetEqualAndNotEmpty(
+            CircuitFilterSet(params, self.circuit_queryset).qs,
+            Circuit.objects.filter(commit_rate__isnull=True),
+        )
+        params = {"commit_rate__isnull": False}
+        self.assertQuerysetEqualAndNotEmpty(
+            CircuitFilterSet(params, self.circuit_queryset).qs,
+            Circuit.objects.filter(commit_rate__isnull=False),
+        )
+
+    def test_isnull_on_date(self):
+        """Test that the `isnull` filter is applied for True and False queries on datetime fields."""
+        params = {"install_date__isnull": True}
+        self.assertQuerysetEqualAndNotEmpty(
+            CircuitFilterSet(params, self.circuit_queryset).qs,
+            Circuit.objects.filter(install_date__isnull=True),
+        )
+        params = {"install_date__isnull": False}
+        self.assertQuerysetEqualAndNotEmpty(
+            CircuitFilterSet(params, self.circuit_queryset).qs,
+            Circuit.objects.filter(install_date__isnull=False),
+        )
+
+    def test_isnull_on_char(self):
+        """Test that the `isnull` filter is applied for True and False queries on char fields."""
+        params = {"name__isnull": True}
+        self.assertQuerysetEqualAndNotEmpty(
+            dcim_filters.DeviceFilterSet(params, self.device_queryset).qs,
+            dcim_models.Device.objects.filter(name__isnull=True),
+        )
+        params = {"name__isnull": False}
+        self.assertQuerysetEqualAndNotEmpty(
+            dcim_filters.DeviceFilterSet(params, self.device_queryset).qs,
+            dcim_models.Device.objects.filter(name__isnull=False),
+        )
+
+
 class FilterTypeTest(TestCase):
     client_class = testing.NautobotTestClient
 

nautobot/core/tests/test_forms.py

@@ -684,7 +684,7 @@ def test_dynamic_filter_form(self):
                     ("racks", "Rack (name or ID)"),
                     ("rack_groups", "Rack groups (name or ID)"),
                     ("shipping_address", "Shipping address"),
-                    ("status", "Status"),
+                    ("status", "Status (name or ID)"),
                     ("vlans", "Tagged VLANs (VID or ID)"),
                     ("tags", "Tags"),
                     ("tenant_id", 'Tenant (ID) (deprecated, use "tenant" filter instead)'),

nautobot/core/tests/test_openapi.py

@@ -41,6 +41,9 @@ def test_filter_datetime_type(self):
         query_params = self.schema["paths"]["/dcim/devices/"]["get"]["parameters"]
         at_least_one_test = False
         for query_param_info in query_params:
+            if query_param_info["name"].endswith("_isnull"):
+                # The broad catch below does not apply to isnull, which will return a boolean.
+                continue
             if query_param_info["name"].startswith("created") or query_param_info["name"].startswith("last_updated"):
                 self.assertEqual("array", query_param_info["schema"]["type"])
                 self.assertEqual("string", query_param_info["schema"]["items"]["type"])
@@ -57,6 +60,9 @@ def test_filter_integer_type(self):
         query_params = self.schema["paths"]["/dcim/devices/"]["get"]["parameters"]
         at_least_one_test = False
         for query_param_info in query_params:
+            if query_param_info["name"].endswith("_isnull"):
+                # The broad catch below does not apply to isnull, which will return a boolean.
+                continue
             if query_param_info["name"].startswith("device_redundancy_group_priority"):
                 self.assertEqual("array", query_param_info["schema"]["type"])
                 self.assertEqual("integer", query_param_info["schema"]["items"]["type"])

nautobot/dcim/tests/test_filters.py

@@ -651,6 +651,7 @@ class LocationFilterSetTestCase(FilterTestCases.NameOnlyFilterTestCase, FilterTe
         ("racks", "racks__id"),
         ("racks", "racks__name"),
         ("shipping_address",),
+        ("status", "status__id"),
         ("status", "status__name"),
         ("time_zone",),
         ("vlan_groups", "vlan_groups__id"),
@@ -765,7 +766,9 @@ class RackTestCase(FilterTestCases.FilterTestCase, FilterTestCases.TenancyFilter
         ("rack_group", "rack_group__name"),
         ("rack_reservations", "rack_reservations__id"),
         ("role", "role__name"),
+        ("role", "role__id"),
         ("serial",),
+        ("status", "status__id"),
         ("status", "status__name"),
         ("type",),
         ("u_height",),
@@ -1331,6 +1334,7 @@ class DeviceTestCase(FilterTestCases.FilterTestCase, FilterTestCases.TenancyFilt
         ("role", "role__name"),
         ("secrets_group", "secrets_group__id"),
         ("secrets_group", "secrets_group__name"),
+        ("status", "status__id"),
         ("status", "status__name"),
         ("vc_position",),
         ("vc_priority",),
@@ -1790,6 +1794,7 @@ class InterfaceTestCase(FilterTestCases.FilterTestCase):
         ("name",),
         ("parent_interface", "parent_interface__id"),
         ("parent_interface", "parent_interface__name"),
+        ("status", "status__id"),
         ("status", "status__name"),
         ("type",),
         ("tagged_vlans", "tagged_vlans__id"),
@@ -2545,6 +2550,7 @@ class CableTestCase(FilterTestCases.FilterTestCase):
         ("color",),
         ("label",),
         ("length",),
+        ("status", "status__id"),
         ("status", "status__name"),
         ("termination_a_id",),
         ("termination_b_id",),
@@ -2835,6 +2841,7 @@ class PowerFeedTestCase(FilterTestCases.FilterTestCase):
         ("power_panel", "power_panel__name"),
         ("rack", "rack__id"),
         ("rack", "rack__name"),
+        ("status", "status__id"),
         ("status", "status__name"),
         ("voltage",),
     ]

nautobot/docs/user-guide/platform-functionality/rest-api/filtering.md

@@ -73,45 +73,62 @@ Custom fields can be mixed with built-in fields to further narrow results. When
 
 ## Lookup Expressions
 
-Certain model fields (including, in Nautobot 1.4.0 and later, custom fields of type `text`, `url`, `select`, `integer`, and `date`) also support filtering using additional lookup expressions. This allows
-for negation and other context-specific filtering.
+Certain model fields (including, in Nautobot 1.4.0 and later, custom fields of type `text`, `url`, `select`, `integer`, and `date`) also support filtering using additional lookup expressions. This allows for negation and other context-specific filtering.
 
 These lookup expressions can be applied by adding a suffix to the desired field's name, e.g. `mac_address__n`. In this case, the filter expression is for negation and it is separated by two underscores. Below are the lookup expressions that are supported across different field types.
 
-### Numeric Fields
+### Nullable Fields
 
-Numeric-based fields (ASN, VLAN ID, etc.) support these lookup expressions:
++++ 2.1.0
 
-- `n` - not equal to (negation)
-- `lt` - less than
-- `lte` - less than or equal
-- `gt` - greater than
-- `gte` - greater than or equal
+Filters corresponding to nullable model fields (that is, `null=True`) of any type support the lookup expression:
+
+- `__isnull` - field has a null value (_not_ blank -- a string-based (char) field with value `""` will match `__isnull=False`, _not_ `__isnull=True`)
+
+### Numeric and Date/Time Fields
+
+Numeric-based fields (ASN, VLAN ID, etc.) as well as date/time fields (`created`, `last_updated`, etc.) support these lookup expressions:
+
+- `__n` - not equal to (negation)
+- `__lt` - less than
+- `__lte` - less than or equal
+- `__gt` - greater than
+- `__gte` - greater than or equal
+
++++ 2.1.0
+    - `__isnull` - as above, only if this field is nullable (most numeric fields in Nautobot are not)
 
 ### String Fields
 
 String-based (char) fields (Name, Address, etc.) support these lookup expressions:
 
-- `n` - not equal to (negation)
-- `ic` - case-insensitive contains
-- `nic` - negated case-insensitive contains
-- `isw` - case-insensitive starts-with
-- `nisw` - negated case-insensitive starts-with
-- `iew` - case-insensitive ends-with
-- `niew` - negated case-insensitive ends-with
-- `ie` - case-insensitive exact match
-- `nie` - negated case-insensitive exact match
+- `__n` - not equal to (negation)
+- `__ic` - case-insensitive contains
+- `__nic` - negated case-insensitive contains
+- `__isw` - case-insensitive starts-with
+- `__nisw` - negated case-insensitive starts-with
+- `__iew` - case-insensitive ends-with
+- `__niew` - negated case-insensitive ends-with
+- `__ie` - case-insensitive exact match
+- `__nie` - negated case-insensitive exact match
 
 +++ 1.3.0
-    - `re` - case-sensitive regular expression match
-    - `nre` - negated case-sensitive regular expression match
-    - `ire` - case-insensitive regular expression match
-    - `nire` - negated case-insensitive regular expression match
+    - `__re` - case-sensitive regular expression match
+    - `__nre` - negated case-sensitive regular expression match
+    - `__ire` - case-insensitive regular expression match
+    - `__nire` - negated case-insensitive regular expression match
+
++++ 2.1.0
+    - `__isnull` - as above, only if this field is nullable (most string fields in Nautobot are not)
 
 ### Foreign Keys & Other Fields
 
-Certain other fields, namely foreign key relationships support just the negation
-expression: `n`.
+Certain other fields, namely foreign key relationships support the lookup expression:
+
+- `__n` - not equal to (negation)
+
++++ 2.1.0
+    - `__isnull` - as above, only if this field is nullable
 
 ### Network and Host Fields