MattBroach / DjangoRestMultipleModels

View (and mixin) for serializing multiple models or querysets in Django Rest Framework
MIT License
549 stars 67 forks source link

django-filter support #37

Open ferndot opened 6 years ago

ferndot commented 6 years ago

I really appreciate your work on this package: it is invaluable for my site.

One thing that would make it even better is support for django-filter (https://django-filter.readthedocs.io). This is a popular package that is recommended by DRF.

If I can assist with this, please let me know.

MattBroach commented 6 years ago

Hey @joshua-s -- django-filter is great, and I use it all the time in some other projects. Because drf_multiple_models calls filter_queryset on each queryset, there is some very limited support for django-filter out-of-the-box. For example, I did a little testing and this code works as is:

from django_filters import rest_framework as filters
from drf_multiple_model.views import ObjectMultipleModelAPIView, FlatMultipleModelAPIView

from .serializers import PlaySerializer, PoemSerializer
from .models import Play, Poem

class FilterObjectView(ObjectMultipleModelAPIView):
    filter_backends = (filters.DjangoFilterBackend,)
    filter_fields = ('title',)
    querylist = (
        {'queryset': Play.objects.all(), 'serializer_class': PlaySerializer},
        {'queryset': Poem.objects.all(), 'serializer_class': PoemSerializer},
    )

class FilterFlatView(FlatMultipleModelAPIView):
    filter_backends = (filters.DjangoFilterBackend,)
    filter_fields = ('title',)
    querylist = (
        {'queryset': Play.objects.all(), 'serializer_class': PlaySerializer},
        {'queryset': Poem.objects.all(), 'serializer_class': PoemSerializer},
    )

(Small caveat: the latter view doesn't work in the browseable API, but works just fine as long as you use format=json)

Of course, you're both limited to exact matches and fields that are shared by both models, but it's not nothing. However, using custom FilterSets is basically a no-go as it currently stands, because the view will error out when it tries to filter on the queryset that doesn't match the FilterSet's model Meta attribute.

I unfortunately don't have a ton of time to experiment with this right now, but I think it would be possible to implement filter_class-per-querylist-item approach. If I were going to try it, I would probably take this approach:

  1. Add an optional filter_class key to the querylist, eg:
    querylist = (
        {'queryset': Play.objects.all(), 'serializer_class': PlaySerializer, 'filter_class': PlayFilter},
        {'queryset': Poem.objects.all(), 'serializer_class': PoemSerializer, 'filter_class': PoemFilter},
    )
  2. Create a new Filter backend that inherits from DjangoFilterBackend and overwrites the get_filter_class function to load the proper filter from the querylist. See here in the django-filter code.
  3. Because we don't want to add django-filter as a general dependency to django-rest-multiple-models (many people using the library may not be using it), you'd want to make sure there is a conditional import in your new backends.py file.

If you want to take a stab at implementation, hopefully the above gives you a good starting point. If not, I probably won't be able to dig into it for at least a few weeks.

ferndot commented 6 years ago

Hey! I'm taking a stab at implementing this.

MattBroach commented 6 years ago

Great! Feel free to ping me with any questions while you're working -- happy to help in whatever way I can.

GeorgiyDemo commented 3 years ago

Hey @joshua-s & @MattBroach Is it possible to use filter_class nowadays by the same field from all the models in querylist?

MattBroach commented 3 years ago

Hey @GeorgiyDemo -- unfortunately there's been no progress on this on my end. I'm still too busy to take a stab at implementing it myself, but would happily review a PR. Otherwise you'll probably have to wait for a bit...

GeorgiyDemo commented 3 years ago

Hey @GeorgiyDemo -- unfortunately there's been no progress on this on my end. I'm still too busy to take a stab at implementing it myself, but would happily review a PR. Otherwise you'll probably have to wait for a bit...

No problem, I used get_querylist() override for my case. Thanks for a quick reply and this useful package!

ckarli commented 3 months ago

64