Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some functionality like RangeFilter generation not correct with custom DjangoFilterBackend #1022

Closed
micahjsmith opened this issue Jul 7, 2023 · 3 comments
Labels
enhancement New feature or request fix confirmation pending issue has been fixed and confirmation from issue reporter is pending

Comments

@micahjsmith
Copy link

micahjsmith commented Jul 7, 2023

Describe the bug

Hard to exactly say what goes wrong. But at the minimum, django_filters.IsoDateTimeFromToRangeFilter is not processed correctly when a custom filter backend class is used. It should have generated dt_before and dt_after Instead it is processed as an equality filter dt.

      - name: dt
        required: false
        in: query
        description: dt
        schema:
          type: string

Note that the reason I'm using a custom filter backend class is to have support for get_filterset_class where a different filterset class might be provided for the list action compared with other actions.

To Reproduce

See example below. By replacing filter_backends from [filters.DjangoFilterBackend] to [CustomFilterBackend] the generated schema for dt query parameter filter changes.

views.py

from . import models, serializers


import django_filters
from django_filters import rest_framework as filters
from rest_framework import mixins, viewsets

class CustomFilterBackend(filters.DjangoFilterBackend):
    pass

class XyzFilter(filters.FilterSet):
    dt = django_filters.IsoDateTimeFromToRangeFilter()

    class Meta:
        model = models.XyzModel
        fields = ['dt']


class XyzViewSet(
    mixins.RetrieveModelMixin,
    mixins.ListModelMixin,
    viewsets.GenericViewSet,
):
    queryset = models.XyzModel.objects.all()
    serializer_class = serializers.XyzSerializer
    # filter_backends = [filters.DjangoFilterBackend]
    filter_backends = [CustomFilterBackend]
    filterset_class = XyzFilter

serializers.py

from . import models
from rest_framework import serializers

class XyzSerializer(serializers.ModelSerializer):

    class Meta:
        model = models.XyzModel
        fields = [
            "dt",
        ]

models.py

from django.db import models

# Create your models here.
class XyzModel(models.Model):
    dt = models.DateTimeField(auto_now_add=True)

See https://gist.github.com/micahjsmith/3ca87efc5c9fe47834f4a0e5590bbe3f Sorry to not also include settings.py and django project layout, hope this is clear as is.

Expected behavior

The generated schema should be identical whether DjangoFilterBackend or an identical subclass is used. The correct schema w/r/t the dt range filter looks like this:

      - in: query
        name: dt_after
        schema:
          type: string
          format: date-time
      - in: query
        name: dt_before
        schema:
          type: string
          format: date-time
@tfranzel
Copy link
Owner

tfranzel commented Jul 8, 2023

I was a bit puzzled for a moment there. I think everything is working as expected. The django-filter extension is not set up to match with subclasses (in contrast to some other extensions).

What you see in the schema is not coming from drf-spectcular, as we ignore the unknown FilterSet class. The schema you see is coming from the django-filter's package schema code, which is very incomplete, recently got deprecated and will be removed soon (if not already).

You need to create a extension that explicitly enables subclass matching.

from drf_spectacular.contrib.django_filters import DjangoFilterExtension

class CustomDjangoFilterExtension(DjangoFilterExtension):
    match_subclasses = True
    priority = 1

please install/setup your extension according to the docs: https://drf-spectacular.readthedocs.io/en/latest/customization.html#step-5-extensions

@micahjsmith
Copy link
Author

ah thank you for the quick response, explanation, and workaround

I guess this is confusing because django filter docs directly indicate to use a DjangoFilterBackend subclass when customizing behavior with DRF: https://django-filter.readthedocs.io/en/main/guide/rest_framework.html#overriding-filterset-creation so it's unexpected that this behavior would be unsupported by the spectacular extension. would there be a reason not enable match_subclasses by default for the DjangoFilterExtension?

@tfranzel
Copy link
Owner

I see. I was not aware this is a recommended practice. Subclass matching is only enabled where subclassing is part of how the lib works. This was mainly meant to guard against changing internals which may produce a different outcome, thus also requiring a modified extension. At least that was the intention.

I guess we can relax this restriction.

tfranzel added a commit that referenced this issue Jul 10, 2023
this is a lesser used but still recommended usage pattern
@tfranzel tfranzel added enhancement New feature or request fix confirmation pending issue has been fixed and confirmation from issue reporter is pending labels Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fix confirmation pending issue has been fixed and confirmation from issue reporter is pending
Projects
None yet
Development

No branches or pull requests

2 participants