Add a new SearchFilter for use with filtersets
#1752
Conversation
- This replaces the need to define a `search()` method on every filterset for which one desires to implement search - This replaces all instances of `q = CharFilter(method="search")` and `search()` with `SearchFilter` across all filtersets in the core.
- Also added discrete unit tests for `SearchFilter` functionality and error cases
jathanism
force-pushed
the
jathanism-new-filters
branch
from
8a2d3c7
to
223c929
Compare
| q = SearchFilter(filter_predicates={"asn": {"lookup_expr": "exact", "preprocessor": dict}}) | ||
|
|
||
| params = {"q": "1234"} | ||
| self.assertEqual(self.get_filterset_count(params, MySiteFilterSet2), Site.objects.count()) |
There was a problem hiding this comment.
I really don't like that filters "fail open" - here we have a SearchFilter that won't match the provided query string, and the return is all Sites rather than an empty list? Does this fall under #1736 or is there something specific in the way SearchFilter is implemented that could be changed here?
There was a problem hiding this comment.
It doesn't actually fail open. It's a side effect when there's only a single predicate defined and it fails to render causing the Q filter to "pass through". This has been corrected in the latest update to add id__iexact by default because now there will always at least be the id field.
We should actually discuss this in detail. I found that when a Q object has no predicates it actually evaluates to False vs. any other case with at least one predicate, evals to True. This could be a way for us to have some sanity check like:
if not query:
qs = qs.none()- Always pass through `strip=False` by default so the form field used for validation doesn't strip whitespace - Automatically skip stripping `icontains` searches (paired with `strip=False` means searching with trailing spaces can be supported - For `icontains` searches unless a preprocessor is explicitly specified, a `noop` will be used - Fixed a small bug in `BaseFilterSet.FILTER_DEFAULTS` mapping for `JSONField` where `lookup_expr` was being passed in as a list and should be a string.
* Ability to search UUID * Add test coverage for `q` field Co-authored-by: zack <zack.tobar@networktocode.com>
Closes: #DNE
What's Changed
This implements a new
SearchFilterinnautobot.utilities.filtersthat is intended to replace the need to always define aqfilter and asearchmethod together. The goal here is to reduce boilerplate AND to support reversible queries even on the search field by making the filter predicates used to construct the search query stored on the filter itself.search()method on every filterset for which one desires to implement searchq = CharFilter(method="search")andsearch()withSearchFilteracross all filtersets in the core.TODO