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

Filtering reverse relations' behaviour #883

Open
Ivo-Donchev opened this issue Mar 7, 2018 · 11 comments
Open

Filtering reverse relations' behaviour #883

Ivo-Donchev opened this issue Mar 7, 2018 · 11 comments

Comments

@Ivo-Donchev
Copy link

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
Copy link

Could you provide the url with the query?

@gthieleb
Copy link

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
Copy link

mohiz commented Jun 15, 2018

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
Copy link
Owner

Should be addressed by #915

@dblenkus
Copy link

dblenkus commented Sep 1, 2018

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
Copy link
Owner

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

@carltongibson carltongibson reopened this Sep 26, 2018
@rpkilby
Copy link
Collaborator

rpkilby commented Nov 26, 2018

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

@mikhailkazagashev
Copy link

@pawelad
Copy link

pawelad commented May 13, 2020

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
Copy link
Author

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

@pawelad
Copy link

pawelad commented May 18, 2020

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants