django-rest-framework
django-rest-framework copied to clipboard
Add search_fields attribute to SearchFilter
Note: Before submitting this pull request, please review our contributing guidelines.
Description
I suggest a small change in the API (backwards compatible) that will simplify some popular notation to express it as:
class CatOwnerNameSearchFilter(filters.SearchFilter):
search_param = "owner"
search_fields = ["first_name", "second_name"]
class CatNameSearchFilter(filters.SearchFilter):
search_param = "name"
search_fields = ["name"]
class CatViewSet(
viewsets.ModelViewSet
):
queryset = Cat.objects.order_by("pk")
filter_backends = [
CatOwnerNameSearchFilter,
CatNameSearchFilter,
]
I can add the tests after I get preliminary approval for such a change in the API.
for design decision I will refer this to @tomchristie
sorry for mistakenly approving. the tests are failing and we still need design decision confirmed.
Thanks for raising the issue of tests. I will take look into that next week.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Can't we handle this with existing model fields? what additional benefits this change will provide?
In order for the example to work properly without changing code it is required to format it as:
class CatOwnerNameSearchFilter(filters.SearchFilter):
search_param = "owner"
def get_search_fields(self, view, request):
return ["first_name", "second_name"]
class CatNameSearchFilter(filters.SearchFilter):
search_param = "name"
def get_search_fields(self, view, request):
return ["name"]
class CatViewSet(
viewsets.ModelViewSet
):
queryset = Cat.objects.order_by("pk")
filter_backends = [
CatOwnerNameSearchFilter,
CatNameSearchFilter,
]
I don't like that these attributes are defined in function body, because it's hard to perform dynamic introspection of them eg. generic testing if all fields are valid one.
I'm -0 on these, largely torn between this being a change which logically makes sense and this being something which should likely just be using django-filters instead. I'm not sure how much mileage we intended for the search filter class itself to get vs using the view attributes and it being used internally behind the scenes.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.