Device Modules: REST API and Filters

[gsnider2195]

Closes #2101

What's Changed

Screenshots

TODO

• Explanation of Change(s)
• Added change log fragment(s) (for more information see the documentation)
• Attached Screenshots, Payload Example
• Unit, Integration Tests
• Documentation Updates (when adding/changing features)
• Example App Updates (when adding/changing features)
• Outline Remaining Work, Constraints from Design

[gsnider2195]

Updated query to match the query generated by the filter

[gsnider2195]

Should the change above be using rear_port_templates instead? Since a device can have rear-ports without any front-ports, but not the reverse?

I understood "has pass-through ports" to mean it has both front ports and rear ports. So if you only have rear ports, you don't have pass-through ports, right? But if you have front ports, you have to have at least one of each. The docstrings for FrontPort and RearPort are "A pass-through port on the front/rear of a Device."

[glennmatthews]

Should the change above be using rear_port_templates instead? Since a device can have rear-ports without any front-ports, but not the reverse?

I understood "has pass-through ports" to mean it has both front ports and rear ports. So if you only have rear ports, you don't have pass-through ports, right? But if you have front ports, you have to have at least one of each. The docstrings for FrontPort and RearPort are "A pass-through port on the front/rear of a Device."

Right you are. Sorry for the noise.

[glennmatthews]

Ugh we need so much to make this logic automatic.

[glennmatthews]

Should we fix up the MRO on all of these as well?

[gsnider2195]

It looks like any filterset mixins that are using get_filters() to inject filters into the filterset (StatusModelFilterSetMixin and RoleModelFilterSetMixin) require this broken MRO so that BaseFilterSet can add the extra filter lookups to these filters. We'll need to fix those mixins before we can make this change, unfortunately.

Correct MRO:
>>> [k for k in DeviceFilterSet().filters.keys() if "status" in k or "role" in k]
['role', 'status']

Old Incorrect MRO:
>>> [k for k in DeviceFilterSet().filters.keys() if "status" in k or "role" in k]
['role', 'status', 'role__n', 'status__n']

[gsnider2195]

This patch appears to fix it:

diff --git a/nautobot/core/utils/filtering.py b/nautobot/core/utils/filtering.py
index 568c34a02..9a5673738 100644
--- a/nautobot/core/utils/filtering.py
+++ b/nautobot/core/utils/filtering.py
@@ -119,6 +119,8 @@ def get_filterset_parameter_form_field(model, parameter, filterset=None):
             "to_field_name": field.extra.get("to_field_name", "id"),
             "query_params": field.extra.get("query_params", {}),
         }
+        form_attr["query_params"].setdefault("content_types", [model._meta.label_lower])
+
         # ConfigContext requires content_type set to Device and VirtualMachine
         if model == ConfigContext:
             form_attr["query_params"] = {"content_types": [Device._meta.label_lower, VirtualMachine._meta.label_lower]}
diff --git a/nautobot/extras/filters/mixins.py b/nautobot/extras/filters/mixins.py
index 56d56ba0e..6bc1fd6f0 100644
--- a/nautobot/extras/filters/mixins.py
+++ b/nautobot/extras/filters/mixins.py
@@ -303,17 +303,7 @@ class RoleModelFilterSetMixin(django_filters.FilterSet):
     Mixin to add a `role` filter field to a FilterSet.
     """
 
-    @classmethod
-    def get_filters(cls):
-        filters = super().get_filters()
-
-        if cls._meta.model is not None:
-            filters["role"] = RoleFilter(
-                field_name="role",
-                query_params={"content_types": [cls._meta.model._meta.label_lower]},
-            )
-
-        return filters
+    role = RoleFilter(field_name="role")
 
 
 class StatusFilter(NaturalKeyOrPKMultipleChoiceFilter):
@@ -332,14 +322,4 @@ class StatusModelFilterSetMixin(django_filters.FilterSet):
     Mixin to add a `status` filter field to a FilterSet.
     """
 
-    @classmethod
-    def get_filters(cls):
-        filters = super().get_filters()
-
-        if cls._meta.model is not None:
-            filters["status"] = StatusFilter(
-                field_name="status",
-                query_params={"content_types": [cls._meta.model._meta.label_lower]},
-            )
-
-        return filters
+    status = StatusFilter(field_name="status")

[glennmatthews]

That's reverting a change I made recently... IIRC the downside is that now the filters won't have the correct query_params to restrict the queryset in the UI?

As an alternative, do we just need to split the get_filters logic into a couple of separate methods so that mixin code gets called correctly before the base filterset adds the extra lookups to the filters?

[glennmatthews]

I see you added logic back into get_filterset_parameter_form_field to set the content-types, but it's a step backwards from our goal of moving the brains into FilterSet rather than having a separate API to massage the filterset fields into a more correct state.

[gsnider2195]

We might want to move the dynamic generation of filterset form fields into the filter field instead of a separate utility function. I opened #5700 to track this and I'll be reverting these changes.

[glennmatthews]

Something I noticed yesterday is that when we're using __all__ on our new FilterSets, this includes a _custom_field_data filter, which the GraphQL schema generation complains about because GraphQL filters can't start with an underscore. Not blocking for this PR in any way, but we should fix that, possibly by enhancing NautobotFilterSet to auto-exclude _custom_field_data as a filter?

[glennmatthews]

#5703.

[gsnider2195]

#5703.

Thanks!