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

graphene-django DjangoFilterConnectionField not filtering for type ID #1078

Closed
amarkezic opened this issue May 20, 2019 · 2 comments
Closed

Comments

@amarkezic
Copy link

graphene-django uses this library to filter django model fields. Everything works fine until you try to filter a field that has graphql type ID. When exeucting query of type:

query {
  userManagerQueries {
    tenantUsers(userId: "VXNlck5vZGU6MQ==") {
	edges {
          node {
            tenantId {
              id
            }
          }
        }
      }
    }
  }

You get the all the results instead of the one you are filtering (so it like you didn't apply the filter)

After doing some digging i found out the the source of the problem is in the file
django-filters/filterset.py in the method:

    @property
    def qs(self):
        if not hasattr(self, '_qs'):
            qs = self.queryset.all()
            if self.is_bound:
                # ensure form validation before filtering
                self.errors
                qs = self.filter_queryset(qs)
            self._qs = qs
        return self._qs

Because no error is raised uppon form validation and the next method in the callstack filter_queryset (still in django-filters/filterset.py ) uses cleaned_data the error gets ignored and the filter dosen't appear in the cleaned_data dict and is not applied to the query set.

    def filter_queryset(self, queryset):
        """
        Filter the queryset with the underlying form's `cleaned_data`. You must
        call `is_valid()` or `errors` before calling this method.

        This method should be overridden if additional filtering needs to be
        applied to the queryset before it is cached.
        """

        for name, value in self.form.cleaned_data.items():
            queryset = self.filters[name].filter(queryset, value)
            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

I did a quick fix by raisging an error uppon unsuccessful form validation:

    def qs(self):
        if not hasattr(self, '_qs'):
            qs = self.queryset.all()
            if self.is_bound:
                # ensure form validation before filtering
                if len(self.errors) > 0:
                    raise Exception(self.errors)
                qs = self.filter_queryset(qs)
            self._qs = qs
        return self._qs

The question i now have is. Can this be fixed or is it intended to be this way and i have to do the form validation in some other part in my code.

@rpkilby
Copy link
Collaborator

rpkilby commented May 22, 2019

Hi @amarkezic. django-filter used to perform error handling in the .qs property, however this responsibility has since been moved to the view. The reason being that different views may want to handle errors in different ways. e.g., traditional HTML forms use will re-render the form with error messages, while APIs may want to raise an HTTP 400.

That said, docs were never updated #788, since they were supposed to undergo an overhaul that seems to have largely gone missing 😅.

@amarkezic
Copy link
Author

Ok, thank you for you response. I will close the issue now since this seems to be a graphene-django problem

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

2 participants