carltongibson / django-filter

A generic system for filtering Django QuerySets based on user selections
https://django-filter.readthedocs.io/en/main/
Other
4.42k stars 767 forks source link

Filtering reverse relations' behaviour #883

Open Ivo-Donchev opened 6 years ago

Ivo-Donchev commented 6 years ago

Hi, I hit a problem when filtering by multiple fields from a reverse relation at once. If I have the following models:

    from django.db import models

    class Country(models.Model):
        name = models.CharField(max_length=255)

    class Group(models.Model):
        title = models.CharField(max_length=255)

    class Actor(models.Model):
        name = models.CharField(max_length=255)

        def __str__(self):
            return f'{self.name} - (id={self.id})'

    class Follower(models.Model):
        full_name = models.CharField(max_length=255)
        actor = models.ForeignKey(Actor,
                                  related_name='followers',
                                  on_delete=models.CASCADE)
        country = models.ForeignKey(Country,
                                    related_name='followers',
                                    on_delete=models.CASCADE)
        group = models.ForeignKey(Group,
                                  related_name='members',
                                  on_delete=models.CASCADE)

And the following filter:

class ActorFilter(FilterSet):
    country_name = CharFilter(method='filter_country_name')
    group_title = CharFilter(method='filter_group_title')

    class Meta:
        model = Actor
        fields = ()

    def filter_country_name(self, queryset, name, value):
        return queryset.filter(followers__country__name=value)

    def filter_group_title(self, queryset, name, value):
        return queryset.filter(followers__group__title=value)

So if I filter by country_name and group_title I expected to get the actors who have followers BOTH from coutry with name=<some_value> AND from group with titile=<some_value>. But instead I got actors who either has followers from country with name=<some_value> or has followers from group with titile=<some_value>. So there was duplicated results.

Reading the django docs I found this explanation for filtering by reverse relations - https://docs.djangoproject.com/en/2.0/topics/db/queries/#lookups-that- . So I was wondering if I'm using django-filters right.

Is this the expected behaviour(having duplicated values when filtering by multiple reverse relation) or it is a bug?

ladrone-vincet commented 6 years ago

Could you provide the url with the query?

gthieleb commented 6 years ago

Same issue here:

models.py (the intermediate model containing the foreign keys):

class MiddlewareSpecification(models.Model):
    uuid = models.UUIDField(default=uuid.uuid4, unique=True, editable=False, verbose_name="UUID")
    middleware = models.ForeignKey(Middleware, to_field='name',
            verbose_name='Middleware', on_delete=models.CASCADE)
    tshirt_size = models.ForeignKey(TshirtSize, to_field='size', blank=True,
            null=True, verbose_name='T-Shirt-Size', on_delete=models.CASCADE)
    stage = models.ForeignKey(Stage, to_field='stage', blank=True, null=True,
            verbose_name='Stage', on_delete=models.CASCADE)
    service_level = models.ForeignKey(ServiceLevel, to_field='name',
            blank=True, null=True, verbose_name='Service-Level',
            on_delete=models.CASCADE)
    memory = models.IntegerField(null=True, blank=True, verbose_name="Memory")
    cores = models.IntegerField(null=True, blank=True, verbose_name="Cores")
    filesystems = models.ManyToManyField("Filesystem", blank=True, verbose_name="Filesystems")

Filtering in the shell:

Stage.objects.filter(middlewarespecification__middleware__name='TOM',
middlewarespecification__service_level__name='Silber',
middlewarespecification__tshirt_size__size='L')
<QuerySet [<Stage: T>]>

Filtering with this FilterSet (instead of methods used by OT this is using the name and distinct attr of CharFilter):

class StageFilter(FilterSet):
    """ this is the base filter
        based on the intermediate table middlewarespec
        to filter specific attributes that are related to middlewarespecification """
    middleware = filters.CharFilter(label='Middleware', 
            name='middlewarespecification__middleware__name',
            distinct=True)
    service_level = filters.CharFilter(label='Service Level', 
            name='middlewarespecification__service_level',
            distinct=True)
    tshirt_size = filters.CharFilter(label='Tshirt Size', 
            name='middlewarespecification__tshirt_size',
            distinct=True)

    class Meta:
        model = Stage
        fields = ('middleware',
                  'tshirt_size',
                  'service_level')

brings this result:

GET /api/v1/djembertest/stages/?middleware=TOM&tshirt_size=L&service_level=Silber

HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

{
    "count": 3,
    "next": null,
    "previous": null,
    "results": [
        {
            "id": 3,
            "stage": "Q",
            "__str__": "Q"
        },
        {
            "id": 1,
            "stage": "T",
            "__str__": "T"
        },
        {
            "id": 2,
            "stage": "P",
            "__str__": "P"
        }
    ]
}

^^ Please don't care about the schema

mohiz commented 6 years ago

I have the same issue. I have asked a question similar to this problem and provide some examples and code to show this problem. Also, I have created an example project that shows this problem in a basic example.

carltongibson commented 6 years ago

Should be addressed by #915

dblenkus commented 6 years ago

If i understand it right, this issue talks about THIS and THIS test and should be fixed in the referenced pull request.

But these tests are still failing on the master. Have I missed anything?

carltongibson commented 5 years ago

I’m just going to reopen this one to review.

rpkilby commented 5 years ago

Just to followup, #915 addressed #882 and not #883, which is this issue.

mikhailkazagashev commented 5 years ago

It seems like it is django queryset filters issue: https://code.djangoproject.com/ticket/27936#comment:1 https://code.djangoproject.com/ticket/27303#comment:13

pawelad commented 4 years ago

We just encountered this in our project as well. I'm wondering what's the potential solution - could we use one filter() call with all kwargs instead of one filter() call per one filter? The difference in behaviour is only in case of M2M, right?

Ivo-Donchev commented 4 years ago

@pawelad That could help, but you need a custom filter method for that :slightly_smiling_face: You may found this article useful - https://hacksoft.io/django-filter-chaining/

pawelad commented 4 years ago

@pawelad That could help, but you need a custom filter method for that 🙂 You may found this article useful - hacksoft.io/django-filter-chaining

I don't think a custom filter method is needed.

For example, this is what a colleague of mine did:

class ManyToOneFilterSet(FilterSet):
    def filter_queryset(self, queryset):
        """
        Overrides the basic methtod, so that instead of iterating over tthe queryset with multiple `.filter()`
        calls, one for each filter, it accumulates the lookup expressions and applies them all in a single
        `.filter()` call - to filter with an explicit "AND" in many to many relationships.
        """
        filter_kwargs = {}
        for name, value in self.form.cleaned_data.items():
            if value not in EMPTY_VALUES:
                lookup = "%s__%s" % (self.filters[name].field_name, self.filters[name].lookup_expr)
                filter_kwargs.update({lookup: value})

        queryset = queryset.filter(**filter_kwargs)
        assert isinstance(queryset, models.QuerySet), (
            "Expected '%s.%s' to return a QuerySet, but got a %s instead."
            % (type(self).__name__, name, type(queryset).__name__)
        )
        return queryset