Merge pull request #151 from graphql-python/fix-filter-and-resolver

Fixed FilterConnectionFields with limit in the FilterSet

Author: syrusakbary

.travis.yml

@@ -45,7 +45,7 @@ after_success:
   fi
 env:
   matrix:
-  - TEST_TYPE=build DJANGO_VERSION=1.10
+  - TEST_TYPE=build DJANGO_VERSION=1.11
 matrix:
   fast_finish: true
   include:
@@ -57,6 +57,8 @@ matrix:
     env: TEST_TYPE=build DJANGO_VERSION=1.8
   - python: '2.7'
     env: TEST_TYPE=build DJANGO_VERSION=1.9
+  - python: '2.7'
+    env: TEST_TYPE=build DJANGO_VERSION=1.10
   - python: '2.7'
     env: TEST_TYPE=lint
 deploy:

graphene_django/fields.py

@@ -46,15 +46,20 @@ def get_manager(self):
         else:
             return self.model._default_manager
 
-    @staticmethod
-    def connection_resolver(resolver, connection, default_manager, root, args, context, info):
+    @classmethod
+    def merge_querysets(cls, default_queryset, queryset):
+        return default_queryset & queryset
+
+    @classmethod
+    def connection_resolver(cls, resolver, connection, default_manager, root, args, context, info):
         iterable = resolver(root, args, context, info)
         if iterable is None:
             iterable = default_manager
         iterable = maybe_queryset(iterable)
         if isinstance(iterable, QuerySet):
             if iterable is not default_manager:
-                iterable &= maybe_queryset(default_manager)
+                default_queryset = maybe_queryset(default_manager)
+                iterable = cls.merge_querysets(default_queryset, iterable)
             _len = iterable.count()
         else:
             _len = len(iterable)

graphene_django/filter/fields.py

@@ -45,14 +45,37 @@ def filtering_args(self):
         return get_filtering_args_from_filterset(self.filterset_class, self.node_type)
 
     @staticmethod
-    def connection_resolver(resolver, connection, default_manager, filterset_class, filtering_args,
+    def merge_querysets(default_queryset, queryset):
+        # There could be the case where the default queryset (returned from the filterclass)
+        # and the resolver queryset have some limits on it.
+        # We only would be able to apply one of those, but not both
+        # at the same time.
+
+        # See related PR: https://github.com/graphql-python/graphene-django/pull/126
+
+        assert not (default_queryset.query.low_mark and queryset.query.low_mark), (
+            'Received two sliced querysets (low mark) in the connection, please slice only in one.'
+        )
+        assert not (default_queryset.query.high_mark and queryset.query.high_mark), (
+            'Received two sliced querysets (high mark) in the connection, please slice only in one.'
+        )
+        low = default_queryset.query.low_mark or queryset.query.low_mark
+        high = default_queryset.query.high_mark or queryset.query.high_mark
+        default_queryset.query.clear_limits()
+        queryset = default_queryset & queryset
+        queryset.query.set_limits(low, high)
+        return queryset
+
+    @classmethod
+    def connection_resolver(cls, resolver, connection, default_manager, filterset_class, filtering_args,
                             root, args, context, info):
         filter_kwargs = {k: v for k, v in args.items() if k in filtering_args}
         qs = filterset_class(
             data=filter_kwargs,
             queryset=default_manager.get_queryset()
         ).qs
-        return DjangoConnectionField.connection_resolver(resolver, connection, qs, root, args, context, info)
+        return super(DjangoFilterConnectionField, cls).connection_resolver(
+            resolver, connection, qs, root, args, context, info)
 
     def get_resolver(self, parent_resolver):
         return partial(self.connection_resolver, parent_resolver, self.type, self.get_manager(),

graphene_django/filter/tests/test_fields.py

@@ -14,11 +14,13 @@
 
 if DJANGO_FILTER_INSTALLED:
     import django_filters
+    from django_filters import FilterSet, NumberFilter
+
     from graphene_django.filter import (GlobalIDFilter, DjangoFilterConnectionField,
                                         GlobalIDMultipleChoiceFilter)
     from graphene_django.filter.tests.filters import ArticleFilter, PetFilter, ReporterFilter
 else:
-    pytestmark.append(pytest.mark.skipif(True, reason='django_filters not installed'))
+    pytestmark.append(pytest.mark.skipif(True, reason='django_filters not installed or not compatible'))
 
 pytestmark.append(pytest.mark.django_db)
 
@@ -365,3 +367,170 @@ class Query(ObjectType):
         all_reporters = DjangoFilterConnectionField(ReporterFilterNode)
 
     assert ReporterFilterNode._meta.fields['child_reporters'].node_type == ReporterFilterNode
+
+
+def test_should_query_filter_node_limit():
+    class ReporterFilter(FilterSet):
+        limit = NumberFilter(method='filter_limit')
+
+        def filter_limit(self, queryset, name, value):
+            return queryset[:value]
+
+        class Meta:
+            model = Reporter
+            fields = ['first_name', ]
+
+    class ReporterType(DjangoObjectType):
+
+        class Meta:
+            model = Reporter
+            interfaces = (Node, )
+
+    class ArticleType(DjangoObjectType):
+
+        class Meta:
+            model = Article
+            interfaces = (Node, )
+            filter_fields = ('lang', )
+
+    class Query(ObjectType):
+        all_reporters = DjangoFilterConnectionField(
+            ReporterType,
+            filterset_class=ReporterFilter
+        )
+
+        def resolve_all_reporters(self, args, context, info):
+            return Reporter.objects.order_by('a_choice')
+
+    Reporter.objects.create(
+        first_name='Bob',
+        last_name='Doe',
+        email='[email protected]',
+        a_choice=2
+    )
+    r = Reporter.objects.create(
+        first_name='John',
+        last_name='Doe',
+        email='[email protected]',
+        a_choice=1
+    )
+
+    Article.objects.create(
+        headline='Article Node 1',
+        pub_date=datetime.now(),
+        reporter=r,
+        editor=r,
+        lang='es'
+    )
+    Article.objects.create(
+        headline='Article Node 2',
+        pub_date=datetime.now(),
+        reporter=r,
+        editor=r,
+        lang='en'
+    )
+
+    schema = Schema(query=Query)
+    query = '''
+        query NodeFilteringQuery {
+            allReporters(limit: 1) {
+                edges {
+                    node {
+                        id
+                        firstName
+                        articles(lang: "es") {
+                            edges {
+                                node {
+                                    id
+                                    lang
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    '''
+
+    expected = {
+        'allReporters': {
+            'edges': [{
+                'node': {
+                    'id': 'UmVwb3J0ZXJUeXBlOjI=',
+                    'firstName': 'John',
+                    'articles': {
+                        'edges': [{
+                            'node': {
+                                'id': 'QXJ0aWNsZVR5cGU6MQ==',
+                                'lang': 'ES'
+                            }
+                        }]
+                    }
+                }
+            }]
+        }
+    }
+
+    result = schema.execute(query)
+    assert not result.errors
+    assert result.data == expected
+
+
+def test_should_query_filter_node_double_limit_raises():
+    class ReporterFilter(FilterSet):
+        limit = NumberFilter(method='filter_limit')
+
+        def filter_limit(self, queryset, name, value):
+            return queryset[:value]
+
+        class Meta:
+            model = Reporter
+            fields = ['first_name', ]
+
+    class ReporterType(DjangoObjectType):
+
+        class Meta:
+            model = Reporter
+            interfaces = (Node, )
+
+    class Query(ObjectType):
+        all_reporters = DjangoFilterConnectionField(
+            ReporterType,
+            filterset_class=ReporterFilter
+        )
+
+        def resolve_all_reporters(self, args, context, info):
+            return Reporter.objects.order_by('a_choice')[:2]
+
+    Reporter.objects.create(
+        first_name='Bob',
+        last_name='Doe',
+        email='[email protected]',
+        a_choice=2
+    )
+    r = Reporter.objects.create(
+        first_name='John',
+        last_name='Doe',
+        email='[email protected]',
+        a_choice=1
+    )
+
+    schema = Schema(query=Query)
+    query = '''
+        query NodeFilteringQuery {
+            allReporters(limit: 1) {
+                edges {
+                    node {
+                        id
+                        firstName
+                    }
+                }
+            }
+        }
+    '''
+
+    result = schema.execute(query)
+    assert len(result.errors) == 1
+    assert str(result.errors[0]) == (
+        'Received two sliced querysets (high mark) in the connection, please slice only in one.'
+    )

graphene_django/tests/test_query.py

@@ -42,7 +42,6 @@ class Meta:
             model = Reporter
             only_fields = ('id', )
 
-
     class Query(graphene.ObjectType):
         reporter = graphene.Field(ReporterType)
 
@@ -360,7 +359,96 @@ class Query(graphene.ObjectType):
             }]
         }
     }
- 
+
+    result = schema.execute(query)
+    assert not result.errors
+    assert result.data == expected
+
+
+@pytest.mark.skipif(not DJANGO_FILTER_INSTALLED,
+                    reason="django-filter should be installed")
+def test_should_query_node_multiple_filtering():
+    class ReporterType(DjangoObjectType):
+
+        class Meta:
+            model = Reporter
+            interfaces = (Node, )
+
+    class ArticleType(DjangoObjectType):
+
+        class Meta:
+            model = Article
+            interfaces = (Node, )
+            filter_fields = ('lang', 'headline')
+
+    class Query(graphene.ObjectType):
+        all_reporters = DjangoConnectionField(ReporterType)
+
+    r = Reporter.objects.create(
+        first_name='John',
+        last_name='Doe',
+        email='[email protected]',
+        a_choice=1
+    )
+    Article.objects.create(
+        headline='Article Node 1',
+        pub_date=datetime.date.today(),
+        reporter=r,
+        editor=r,
+        lang='es'
+    )
+    Article.objects.create(
+        headline='Article Node 2',
+        pub_date=datetime.date.today(),
+        reporter=r,
+        editor=r,
+        lang='es'
+    )
+    Article.objects.create(
+        headline='Article Node 3',
+        pub_date=datetime.date.today(),
+        reporter=r,
+        editor=r,
+        lang='en'
+    )
+
+    schema = graphene.Schema(query=Query)
+    query = '''
+        query NodeFilteringQuery {
+            allReporters {
+                edges {
+                    node {
+                        id
+                        articles(lang: "es", headline: "Article Node 1") {
+                            edges {
+                                node {
+                                    id
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    '''
+
+    expected = {
+        'allReporters': {
+            'edges': [{
+                'node': {
+                    'id': 'UmVwb3J0ZXJUeXBlOjE=',
+                    'articles': {
+                        'edges': [{
+                            'node': {
+                                'id': 'QXJ0aWNsZVR5cGU6MQ=='
+                            }
+                        }]
+                    }
+                }
+            }]
+        }
+    }
+
     result = schema.execute(query)
     assert not result.errors
     assert result.data == expected